From 0d708b31b49fefb8adf5d50c14a3cad15127ce9f Mon Sep 17 00:00:00 2001 From: janwas Date: Sat, 24 Sep 2011 19:47:13 +0000 Subject: [PATCH] avoid rare init-order bug when SRAT ACPI table is available AND APIC IDs are NOT move APIC-related code to separate file, with its own initialization to avoid circular dependency fixes #976 This was SVN commit r10315. --- source/lib/sysdep/arch/x86_x64/apic.cpp | 138 ++++++++++++++++++++ source/lib/sysdep/arch/x86_x64/apic.h | 39 ++++++ source/lib/sysdep/arch/x86_x64/topology.cpp | 100 +++----------- source/lib/sysdep/arch/x86_x64/topology.h | 1 - source/lib/sysdep/os/win/wnuma.cpp | 6 +- source/lib/sysdep/os/win/wvm.cpp | 4 +- 6 files changed, 199 insertions(+), 89 deletions(-) create mode 100644 source/lib/sysdep/arch/x86_x64/apic.cpp create mode 100644 source/lib/sysdep/arch/x86_x64/apic.h diff --git a/source/lib/sysdep/arch/x86_x64/apic.cpp b/source/lib/sysdep/arch/x86_x64/apic.cpp new file mode 100644 index 0000000000..b05e0c33b7 --- /dev/null +++ b/source/lib/sysdep/arch/x86_x64/apic.cpp @@ -0,0 +1,138 @@ +/* Copyright (c) 2011 Wildfire Games + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#include "precompiled.h" +#include "lib/sysdep/arch/x86_x64/apic.h" + +#include "lib/module_init.h" +#include "lib/sysdep/cpu.h" // ERR::CPU_FEATURE_MISSING +#include "lib/sysdep/os_cpu.h" +#include "lib/sysdep/arch/x86_x64/x86_x64.h" // x86_x64_ApicId + +static size_t numIds; +static ApicId processorApicIds[os_cpu_MaxProcessors]; +static ApicId sortedApicIds[os_cpu_MaxProcessors]; + +static Status GetAndValidateApicIds() +{ + numIds = os_cpu_NumProcessors(); + struct StoreEachProcessorsApicId + { + static void Callback(size_t processor, uintptr_t UNUSED(data)) + { + processorApicIds[processor] = x86_x64_ApicId(); + } + }; + // (can fail due to restrictions on our process affinity or lack of + // support for affinity masks in OS X.) + RETURN_STATUS_IF_ERR(os_cpu_CallByEachCPU(StoreEachProcessorsApicId::Callback, 0)); + + std::copy(processorApicIds, processorApicIds+numIds, sortedApicIds); + std::sort(sortedApicIds, sortedApicIds+numIds); + ApicId* const end = std::unique(sortedApicIds, sortedApicIds+numIds); + const size_t numUnique = end-sortedApicIds; + + // all IDs are zero - system lacks an xAPIC. + if(numUnique == 1 && sortedApicIds[0] == 0) + { + debug_printf(L"APIC: all zero\n"); + return ERR::CPU_FEATURE_MISSING; // NOWARN + } + + // not all unique - probably running in a VM whose emulation is + // imperfect or doesn't allow access to all processors. + if(numUnique != numIds) + { + debug_printf(L"APIC: not unique\n"); + return ERR::FAIL; // NOWARN + } + + return INFO::OK; +} + +static Status InitApicIds() +{ + const Status status = GetAndValidateApicIds(); + if(status < 0) // failed + { + // generate fake but legitimate APIC IDs + for(size_t processor = 0; processor < numIds; processor++) + processorApicIds[processor] = sortedApicIds[processor] = (ApicId)processor; + } + + return status; +} + +static ModuleInitState apicInitState; + + +bool AreApicIdsReliable() +{ + ModuleInit(&apicInitState, InitApicIds); + if(apicInitState < 0) + return false; + return true; +} + + +static size_t IndexFromApicId(const ApicId* apicIds, size_t apicId) +{ + ModuleInit(&apicInitState, InitApicIds); + + const ApicId* pos = std::find(apicIds, apicIds+numIds, apicId); + if(pos == apicIds+numIds) + { + DEBUG_WARN_ERR(ERR::LOGIC); + return 0; + } + + const size_t index = pos - apicIds; + return index; +} + +size_t ProcessorFromApicId(ApicId apicId) +{ + return IndexFromApicId(processorApicIds, apicId); +} + +size_t ContiguousIdFromApicId(ApicId apicId) +{ + return IndexFromApicId(sortedApicIds, apicId); +} + + +static ApicId ApicIdFromIndex(const ApicId* apicIds, size_t index) +{ + ModuleInit(&apicInitState, InitApicIds); + ASSERT(index < numIds); + return apicIds[index]; +} + +ApicId ApicIdFromProcessor(size_t processor) +{ + return ApicIdFromIndex(processorApicIds, processor); +} + +ApicId ApicIdFromContiguousId(size_t contiguousId) +{ + return ApicIdFromIndex(sortedApicIds, contiguousId); +} diff --git a/source/lib/sysdep/arch/x86_x64/apic.h b/source/lib/sysdep/arch/x86_x64/apic.h new file mode 100644 index 0000000000..f90f2c0004 --- /dev/null +++ b/source/lib/sysdep/arch/x86_x64/apic.h @@ -0,0 +1,39 @@ +/* Copyright (c) 2011 Wildfire Games + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#ifndef INCLUDED_X86_X64_APIC +#define INCLUDED_X86_X64_APIC + +typedef u8 ApicId; // not necessarily contiguous values + +// if this returns false, apicId = contiguousId = processor. +// otherwise, there are unspecified but bijective mappings between +// apicId<->contiguousId and apicId<->processor. +LIB_API bool AreApicIdsReliable(); + +LIB_API size_t ProcessorFromApicId(ApicId apicId); +LIB_API size_t ContiguousIdFromApicId(ApicId apicId); + +LIB_API ApicId ApicIdFromProcessor(size_t contiguousId); +LIB_API ApicId ApicIdFromContiguousId(size_t contiguousId); + +#endif // #ifndef INCLUDED_X86_X64_APIC diff --git a/source/lib/sysdep/arch/x86_x64/topology.cpp b/source/lib/sysdep/arch/x86_x64/topology.cpp index 5ab42e0066..ce50531940 100644 --- a/source/lib/sysdep/arch/x86_x64/topology.cpp +++ b/source/lib/sysdep/arch/x86_x64/topology.cpp @@ -36,6 +36,7 @@ #include "lib/sysdep/numa.h" #include "lib/sysdep/arch/x86_x64/x86_x64.h" #include "lib/sysdep/arch/x86_x64/cache.h" +#include "lib/sysdep/arch/x86_x64/apic.h" //----------------------------------------------------------------------------- @@ -116,48 +117,12 @@ static size_t MaxLogicalPerCache() //----------------------------------------------------------------------------- -// APIC IDs - -typedef u8 ApicId; +// CPU topology interface // APIC IDs consist of variable-length bit fields indicating the logical, // core, package and cache IDs. Vol3a says they aren't guaranteed to be // contiguous, but that also applies to the individual fields. // for example, quad-core E5630 CPUs report 4-bit core IDs 0, 1, 6, 7. - -// (IDs are indeterminate unless INFO::OK is returned) -static Status GetApicIds(ApicId* apicIds, ApicId* sortedApicIds, size_t numIds) -{ - struct StoreEachProcessorsApicId - { - static void Callback(size_t processor, uintptr_t cbData) - { - ApicId* apicIds = (ApicId*)cbData; - apicIds[processor] = x86_x64_ApicId(); - } - }; - // (can fail due to restrictions on our process affinity or lack of - // support for affinity masks in OS X.) - RETURN_STATUS_IF_ERR(os_cpu_CallByEachCPU(StoreEachProcessorsApicId::Callback, (uintptr_t)apicIds)); - - std::copy(apicIds, apicIds+numIds, sortedApicIds); - std::sort(sortedApicIds, sortedApicIds+numIds); - ApicId* const end = std::unique(sortedApicIds, sortedApicIds+numIds); - const size_t numUnique = end-sortedApicIds; - - // all IDs are zero - system lacks an xAPIC. - if(numUnique == 1 && sortedApicIds[0] == 0) - return ERR::CPU_FEATURE_MISSING; // NOWARN - - // not all unique - probably running in a VM whose emulation is - // imperfect or doesn't allow access to all processors. - if(numUnique != numIds) - return ERR::FAIL; // NOWARN - - return INFO::OK; -} - - struct ApicField // POD { size_t operator()(size_t bits) const @@ -169,15 +134,9 @@ struct ApicField // POD size_t shift; }; - -//----------------------------------------------------------------------------- -// CPU topology interface - struct CpuTopology // POD { size_t numProcessors; // total reported by OS - ApicId apicIds[os_cpu_MaxProcessors]; - ApicId sortedApicIds[os_cpu_MaxProcessors]; ApicField logical; ApicField core; @@ -211,25 +170,26 @@ static Status InitCpuTopology() cpuTopology.core.shift = logicalWidth; cpuTopology.package.shift = logicalWidth + coreWidth; - if(GetApicIds(cpuTopology.apicIds, cpuTopology.sortedApicIds, cpuTopology.numProcessors) == INFO::OK) + if(AreApicIdsReliable()) { struct NumUniqueValuesInField { - size_t operator()(const ApicId* apicIds, const ApicField& apicField) const + size_t operator()(const ApicField& apicField) const { std::bitset values; for(size_t processor = 0; processor < os_cpu_NumProcessors(); processor++) { - const size_t value = apicField(apicIds[processor]); + const ApicId apicId = ApicIdFromProcessor(processor); + const size_t value = apicField(apicId); values.set(value); } return values.count(); } }; - cpuTopology.logicalPerCore = NumUniqueValuesInField()(cpuTopology.apicIds, cpuTopology.logical); - cpuTopology.coresPerPackage = NumUniqueValuesInField()(cpuTopology.apicIds, cpuTopology.core); - cpuTopology.numPackages = NumUniqueValuesInField()(cpuTopology.apicIds, cpuTopology.package); + cpuTopology.logicalPerCore = NumUniqueValuesInField()(cpuTopology.logical); + cpuTopology.coresPerPackage = NumUniqueValuesInField()(cpuTopology.core); + cpuTopology.numPackages = NumUniqueValuesInField()(cpuTopology.package); } else // processor lacks an xAPIC, or IDs are invalid { @@ -268,9 +228,6 @@ static Status InitCpuTopology() cpuTopology.coresPerPackage = coresPerPackage; cpuTopology.numPackages = numPackages; - // generate fake but legitimate APIC IDs - for(size_t processor = 0; processor < cpuTopology.numProcessors; processor++) - cpuTopology.apicIds[processor] = cpuTopology.sortedApicIds[processor] = (ApicId)processor; return INFO::OK; } } @@ -301,44 +258,21 @@ size_t cpu_topology_LogicalPerCore() return cpuTopology.logicalPerCore; } - -static size_t IndexFromApicId(const ApicId* apicIds, size_t apicId) -{ - ModuleInit(&cpuInitState, InitCpuTopology); - - const ApicId* end = apicIds + cpuTopology.numProcessors; - const ApicId* pos = std::find(apicIds, end, apicId); - if(pos == end) - { - DEBUG_WARN_ERR(ERR::LOGIC); - return 0; - } - - const size_t index = pos - apicIds; - return index; -} - - -size_t cpu_topology_ProcessorFromApicId(size_t apicId) -{ - return IndexFromApicId(cpuTopology.apicIds, apicId); -} - size_t cpu_topology_LogicalFromApicId(size_t apicId) { - const size_t contiguousId = IndexFromApicId(cpuTopology.sortedApicIds, apicId); + const size_t contiguousId = ContiguousIdFromApicId(apicId); return contiguousId % cpuTopology.logicalPerCore; } size_t cpu_topology_CoreFromApicId(size_t apicId) { - const size_t contiguousId = IndexFromApicId(cpuTopology.sortedApicIds, apicId); + const size_t contiguousId = ContiguousIdFromApicId(apicId); return (contiguousId / cpuTopology.logicalPerCore) % cpuTopology.coresPerPackage; } size_t cpu_topology_PackageFromApicId(size_t apicId) { - const size_t contiguousId = IndexFromApicId(cpuTopology.sortedApicIds, apicId); + const size_t contiguousId = ContiguousIdFromApicId(apicId); return contiguousId / (cpuTopology.logicalPerCore * cpuTopology.coresPerPackage); } @@ -360,7 +294,7 @@ size_t cpu_topology_ApicId(size_t idxLogical, size_t idxCore, size_t idxPackage) contiguousId += idxLogical; ENSURE(contiguousId < cpuTopology.numProcessors); - return cpuTopology.sortedApicIds[contiguousId]; + return ApicIdFromContiguousId(contiguousId); } @@ -449,16 +383,16 @@ private: std::vector m_caches; }; -static void DetermineCachesProcessorMask(const ApicId* apicIds, uintptr_t* cachesProcessorMask, size_t& numCaches) +static void DetermineCachesProcessorMask(uintptr_t* cachesProcessorMask, size_t& numCaches) { CacheRelations cacheRelations; - if(apicIds) + if(AreApicIdsReliable()) { const size_t numBits = ceil_log2(MaxLogicalPerCache()); const u8 cacheIdMask = u8((0xFF << numBits) & 0xFF); for(size_t processor = 0; processor < os_cpu_NumProcessors(); processor++) { - const ApicId apicId = apicIds[processor]; + const ApicId apicId = ApicIdFromProcessor(processor); const u8 cacheId = u8(apicId & cacheIdMask); cacheRelations.Add(cacheId, processor); } @@ -511,7 +445,7 @@ static ModuleInitState cacheInitState; static Status InitCacheTopology() { ModuleInit(&cpuInitState, InitCpuTopology); - DetermineCachesProcessorMask(cpuTopology.apicIds, cacheTopology.cachesProcessorMask, cacheTopology.numCaches); + DetermineCachesProcessorMask(cacheTopology.cachesProcessorMask, cacheTopology.numCaches); DetermineProcessorsCache(cacheTopology.cachesProcessorMask, cacheTopology.numCaches, cacheTopology.processorsCache, os_cpu_NumProcessors()); return INFO::OK; } diff --git a/source/lib/sysdep/arch/x86_x64/topology.h b/source/lib/sysdep/arch/x86_x64/topology.h index 24c0179db8..05925e22aa 100644 --- a/source/lib/sysdep/arch/x86_x64/topology.h +++ b/source/lib/sysdep/arch/x86_x64/topology.h @@ -55,7 +55,6 @@ LIB_API size_t cpu_topology_CoresPerPackage(); **/ LIB_API size_t cpu_topology_LogicalPerCore(); -LIB_API size_t cpu_topology_ProcessorFromApicId(size_t apicId); LIB_API size_t cpu_topology_PackageFromApicId(size_t apicId); LIB_API size_t cpu_topology_CoreFromApicId(size_t apicId); LIB_API size_t cpu_topology_LogicalFromApicId(size_t apicId); diff --git a/source/lib/sysdep/os/win/wnuma.cpp b/source/lib/sysdep/os/win/wnuma.cpp index ae57b26dde..a9ab8d0330 100644 --- a/source/lib/sysdep/os/win/wnuma.cpp +++ b/source/lib/sysdep/os/win/wnuma.cpp @@ -36,7 +36,7 @@ #include #if ARCH_X86_X64 -#include "lib/sysdep/arch/x86_x64/topology.h" // ApicIds +#include "lib/sysdep/arch/x86_x64/apic.h" // ProcessorFromApicId #endif @@ -231,7 +231,7 @@ static ProximityDomains ExtractProximityDomainsFromSRAT(const SRAT* srat) const AffinityAPIC* affinityAPIC = DynamicCastFromHeader(header); if(affinityAPIC) { - const size_t processor = cpu_topology_ProcessorFromApicId(affinityAPIC->apicId); + const size_t processor = ProcessorFromApicId(affinityAPIC->apicId); const u32 proximityDomainNumber = affinityAPIC->ProximityDomainNumber(); ProximityDomain& proximityDomain = proximityDomains[proximityDomainNumber]; proximityDomain.processorMask |= Bit(processor); @@ -270,7 +270,7 @@ static Status InitTopology() #if ARCH_X86_X64 const SRAT* srat = (const SRAT*)acpi_GetTable("SRAT"); - if(srat) + if(srat && AreApicIdsReliable()) { const ProximityDomains proximityDomains = ExtractProximityDomainsFromSRAT(srat); PopulateNodesFromProximityDomains(proximityDomains); diff --git a/source/lib/sysdep/os/win/wvm.cpp b/source/lib/sysdep/os/win/wvm.cpp index ddbe10ca03..9c5bcb2ed9 100644 --- a/source/lib/sysdep/os/win/wvm.cpp +++ b/source/lib/sysdep/os/win/wvm.cpp @@ -38,7 +38,7 @@ #include "lib/sysdep/cpu.h" // cpu_AtomicAdd #include "lib/sysdep/numa.h" #include "lib/sysdep/arch/x86_x64/x86_x64.h" // x86_x64_ApicId -#include "lib/sysdep/arch/x86_x64/topology.h" // cpu_topology_ProcessorFromApicId +#include "lib/sysdep/arch/x86_x64/apic.h" // ProcessorFromApicId #include "lib/sysdep/os/win/wversion.h" #include "lib/sysdep/os/win/winit.h" WINIT_REGISTER_CRITICAL_INIT(wvm_Init); @@ -54,7 +54,7 @@ static WUTIL_FUNC(pVirtualAllocExNuma, LPVOID, (HANDLE, LPVOID, SIZE_T, DWORD, D static DWORD WINAPI EmulateGetCurrentProcessorNumber(VOID) { const u8 apicId = x86_x64_ApicId(); - const DWORD processor = cpu_topology_ProcessorFromApicId(apicId); + const DWORD processor = ProcessorFromApicId(apicId); ASSERT(processor < os_cpu_MaxProcessors); return processor; }