From f039e1ce7681a1deed7961eec8f6e3201275d96a Mon Sep 17 00:00:00 2001 From: janwas Date: Fri, 7 Sep 2007 20:17:57 +0000 Subject: [PATCH] fix error path for acpi/mahaf failures (attempt#2) prevent using mahaf if it's going to fail anyway module_init: add ModuleIsError (allows acpi and mahaf init routines to pass failure notification on to their second and later callers) counter: safer memory management This was SVN commit r5333. --- source/lib/module_init.cpp | 6 ++++ source/lib/module_init.h | 9 +++++ source/lib/sysdep/acpi.cpp | 7 ++-- source/lib/sysdep/win/mahaf.cpp | 2 ++ source/lib/sysdep/win/mahaf.h | 21 ++++++----- source/lib/sysdep/win/whrt/counter.cpp | 50 +++++++++++++------------- source/lib/sysdep/win/whrt/counter.h | 3 ++ source/lib/sysdep/win/whrt/hpet.cpp | 4 ++- source/lib/sysdep/win/whrt/pmt.cpp | 1 + 9 files changed, 66 insertions(+), 37 deletions(-) diff --git a/source/lib/module_init.cpp b/source/lib/module_init.cpp index 7ce5712b17..cca5aff056 100644 --- a/source/lib/module_init.cpp +++ b/source/lib/module_init.cpp @@ -66,3 +66,9 @@ void ModuleSetError(volatile ModuleInitState* pInitState) { *pInitState = MODULE_ERROR; } + + +bool ModuleIsError(volatile ModuleInitState* pInitState) +{ + return (*pInitState == MODULE_ERROR); +} diff --git a/source/lib/module_init.h b/source/lib/module_init.h index f62aad01e0..42788328e7 100644 --- a/source/lib/module_init.h +++ b/source/lib/module_init.h @@ -44,4 +44,13 @@ extern bool ModuleShouldShutdown(volatile ModuleInitState* initState); **/ extern void ModuleSetError(volatile ModuleInitState* initState); +/** + * @return whether the module is in the failure state, i.e. ModuleSetError + * was previously called on the same initState. + * + * this function is provided so that modules can report init failure to + * the second or later caller. + **/ +extern bool ModuleIsError(volatile ModuleInitState* initState); + #endif // #ifndef INCLUDED_MODULE_INIT diff --git a/source/lib/sysdep/acpi.cpp b/source/lib/sysdep/acpi.cpp index c49eb9fb3c..c6e579c9df 100644 --- a/source/lib/sysdep/acpi.cpp +++ b/source/lib/sysdep/acpi.cpp @@ -310,14 +310,15 @@ static ModuleInitState initState; bool acpi_Init() { + if(ModuleIsError(&initState)) + return false; if(!ModuleShouldInitialize(&initState)) return true; - if(!mahaf_Init()) - goto fail; - if(mahaf_IsPhysicalMappingDangerous()) goto fail; + if(!mahaf_Init()) + goto fail; if(!LatchAllTables()) goto fail; diff --git a/source/lib/sysdep/win/mahaf.cpp b/source/lib/sysdep/win/mahaf.cpp index 3daf67f89b..29d51a7359 100644 --- a/source/lib/sysdep/win/mahaf.cpp +++ b/source/lib/sysdep/win/mahaf.cpp @@ -278,6 +278,8 @@ static ModuleInitState initState; bool mahaf_Init() { + if(ModuleIsError(&initState)) + return false; if(!ModuleShouldInitialize(&initState)) return true; diff --git a/source/lib/sysdep/win/mahaf.h b/source/lib/sysdep/win/mahaf.h index 7f4ae42c9f..8e21229382 100644 --- a/source/lib/sysdep/win/mahaf.h +++ b/source/lib/sysdep/win/mahaf.h @@ -14,6 +14,18 @@ #ifndef INCLUDED_MAHAF #define INCLUDED_MAHAF +/** + * @return whether mapping physical memory is known to be dangerous + * on this platform. + * + * callable before or after mahaf_Init. + * + * note: mahaf_MapPhysicalMemory will complain if it + * is called despite this function having returned true. + **/ +extern bool mahaf_IsPhysicalMappingDangerous(); + + extern bool mahaf_Init(); extern void mahaf_Shutdown(); @@ -24,15 +36,6 @@ extern void mahaf_WritePort8 (u16 port, u8 value); extern void mahaf_WritePort16(u16 port, u16 value); extern void mahaf_WritePort32(u16 port, u32 value); -/** - * @return whether mapping physical memory is known to be dangerous - * on this platform. - * - * note: mahaf_MapPhysicalMemory will warn if it is called despite this - * function having returned true. - **/ -extern bool mahaf_IsPhysicalMappingDangerous(); - extern volatile void* mahaf_MapPhysicalMemory(uintptr_t physicalAddress, size_t numBytes); extern void mahaf_UnmapPhysicalMemory(volatile void* virtualAddress); diff --git a/source/lib/sysdep/win/whrt/counter.cpp b/source/lib/sysdep/win/whrt/counter.cpp index 6c64c8c679..4af7eed58f 100644 --- a/source/lib/sysdep/win/whrt/counter.cpp +++ b/source/lib/sysdep/win/whrt/counter.cpp @@ -12,6 +12,7 @@ #include "counter.h" #include "lib/bits.h" +#include "lib/allocators.h" #include "tsc.h" #include "hpet.h" @@ -28,14 +29,11 @@ /** * @return pointer to a newly constructed ICounter subclass of type at * the given address, or 0 iff the ID is invalid. - * @param size receives the size [bytes] of the created instance. + * @param size maximum allowable size [bytes] of the subclass instance **/ -static ICounter* ConstructCounterAt(uint id, void* address, size_t& size) +static ICounter* ConstructCounterAt(uint id, void* address, size_t size) { // rationale for placement new: see call site. -#define CREATE(impl)\ - size = sizeof(Counter##impl);\ - return new(address) Counter##impl(); #include "lib/nommgr.h" // MMGR interferes with placement new @@ -48,25 +46,30 @@ static ICounter* ConstructCounterAt(uint id, void* address, size_t& size) switch(id) { case 0: - CREATE(HPET) + debug_assert(sizeof(CounterHPET) <= size); + return new(address) CounterHPET(); case 1: - CREATE(TSC) + debug_assert(sizeof(CounterTSC) <= size); + return new(address) CounterTSC(); case 2: - CREATE(QPC) + debug_assert(sizeof(CounterQPC) <= size); + return new(address) CounterQPC(); case 3: - CREATE(PMT) + debug_assert(sizeof(CounterPMT) <= size); + return new(address) CounterPMT(); case 4: - CREATE(TGT) + debug_assert(sizeof(CounterTGT) <= size); + return new(address) CounterTGT(); default: - size = 0; return 0; } #include "lib/mmgr.h" - -#undef CREATE } + +static volatile uintptr_t isCounterAllocated; + ICounter* CreateCounter(uint id) { // we placement-new the Counter classes in a static buffer. @@ -80,16 +83,15 @@ ICounter* CreateCounter(uint id) // first use of them. // - using static_calloc isn't possible because we don't know the // size until after the alloc / placement new. - static const size_t MEM_SIZE = 200; // checked below - static u8 mem[MEM_SIZE]; - static u8* nextMem = mem; - u8* addr = (u8*)round_up((uintptr_t)nextMem, 16); - size_t size; - ICounter* counter = ConstructCounterAt(id, addr, size); + if(!CAS(&isCounterAllocated, 0, 1)) + debug_warn("static counter memory is already in use!"); - nextMem = addr+size; - debug_assert(nextMem < mem+MEM_SIZE); // had enough room? + static const size_t memSize = 200; + static u8 mem[memSize]; + u8* alignedMem = (u8*)round_up((uintptr_t)mem, 16); + const size_t bytesLeft = mem+memSize - alignedMem; + ICounter* counter = ConstructCounterAt(id, alignedMem, bytesLeft); return counter; } @@ -97,10 +99,10 @@ ICounter* CreateCounter(uint id) void DestroyCounter(ICounter*& counter) { - if(!counter) - return; - + debug_assert(counter); counter->Shutdown(); counter->~ICounter(); // must be called due to placement new counter = 0; + + isCounterAllocated = 0; } diff --git a/source/lib/sysdep/win/whrt/counter.h b/source/lib/sysdep/win/whrt/counter.h index 3c8058251f..7871066415 100644 --- a/source/lib/sysdep/win/whrt/counter.h +++ b/source/lib/sysdep/win/whrt/counter.h @@ -64,6 +64,9 @@ public: /** * @return a newly created ICounter of type or 0 iff the ID is invalid. * @param id integer ID (0..N-1) + * + * there can only be one active counter at a time; the previous one must + * have been destroyed before creating another! **/ extern ICounter* CreateCounter(uint id); diff --git a/source/lib/sysdep/win/whrt/hpet.cpp b/source/lib/sysdep/win/whrt/hpet.cpp index 4b2fdff8d7..ed7bc6d417 100644 --- a/source/lib/sysdep/win/whrt/hpet.cpp +++ b/source/lib/sysdep/win/whrt/hpet.cpp @@ -53,10 +53,12 @@ static const u64 CONFIG_ENABLE = BIT64(0); LibError CounterHPET::Activate() { + if(mahaf_IsPhysicalMappingDangerous()) + return ERR::FAIL; // NOWARN (happens on Win2k) if(!mahaf_Init()) return ERR::FAIL; // NOWARN (no Administrator privileges) if(!acpi_Init()) - return ERR::FAIL; // NOWARN (happens on Win2k; see mahaf_IsPhysicalMappingDangerous) + WARN_RETURN(ERR::FAIL); // shouldn't fail, since we've checked mahaf_IsPhysicalMappingDangerous const HpetDescriptionTable* hpet = (const HpetDescriptionTable*)acpi_GetTable("HPET"); if(!hpet) return ERR::NO_SYS; // NOWARN (HPET not reported by BIOS) diff --git a/source/lib/sysdep/win/whrt/pmt.cpp b/source/lib/sysdep/win/whrt/pmt.cpp index 16ef609176..6108d6a6e6 100644 --- a/source/lib/sysdep/win/whrt/pmt.cpp +++ b/source/lib/sysdep/win/whrt/pmt.cpp @@ -33,6 +33,7 @@ static const u32 TMR_VAL_EXT = BIT(8); LibError CounterPMT::Activate() { + // mahaf is needed for port I/O. if(!mahaf_Init()) return ERR::FAIL; // NOWARN (no Administrator privileges) if(!acpi_Init())