Laszlo and Jordan, I agree. Thanks for the feedback.
The SOURCE_DEBUG_ENABLE issue is a completely separate issue and I will regenerate a patch for that issue that is not related to SMM work. It only got into the mix of changes because I was using SOURCE_DEBUG_ENABLE to debug some SMM issues. Mike >-----Original Message----- >From: Laszlo Ersek [mailto:[email protected]] >Sent: Tuesday, October 13, 2015 11:56 AM >To: Justen, Jordan L; Kinney, Michael D; [email protected] >Subject: Re: [edk2] [PATCH 2/3] OvmfPkg: Fix VS20xx and DebugAgent ASSERT() > >On 10/13/15 20:47, Jordan Justen wrote: >> On 2015-10-13 10:49:23, Michael Kinney wrote: >>> Fix build breaks when building with VS20xx. The >>> loop that fills the BaseExtractGuidedSectionLib >>> handler table with zeros must use a volatile variable >>> to prevent the optimizing compiler from adding a call >>> to a compiler intrinsic. >>> >>> Also, update initialization of SEC phase to initialze >>> the state of the Local APIC Timer hardware before the >>> DebugAgentLib is initialized that depends on the Local >>> APIC Timer hardware being in an initialized state. >>> An ASSERT() was recently added to Local APIC libs >>> to verifiy that the Local APIC is initialized before >>> use and if SOURCE_DEBUG_ENABLE is set, this ASSERT() >>> was being triggered. >> >> Sounds like 2 separate changes. > >I agree. What's more, the APIC change seems orthogonal to the SMM work, >hence I think either the blurb (= 0/3) subject should be changed (from >"SMM patch issues"), or the LAPIC timer init should not only be in a >separate patch, but even in a separate posting (i.e., outside of this >series). > >> >> Actually, for the 'Table' issue, I think you should just reply to >> Laszlo's "OvmfPkg: Sec: force reinit of BaseExtractGuidedSectionLib >> handler table" and ask him to use ZeroMem instead of the loop. > >There's a comment about that in the code. The zeroing happens (must >happen) before any library constructors are invoked. Therefore no >library functions can be called. > >Thanks! >Laszlo > >> >> -Jordan >> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Michael Kinney <[email protected]> >>> --- >>> OvmfPkg/Sec/SecMain.c | 11 ++++++++--- >>> OvmfPkg/Sec/SecMain.inf | 3 ++- >>> 2 files changed, 10 insertions(+), 4 deletions(-) >>> >>> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c >>> index 26a54d0..5ad62a5 100644 >>> --- a/OvmfPkg/Sec/SecMain.c >>> +++ b/OvmfPkg/Sec/SecMain.c >>> @@ -1,7 +1,7 @@ >>> /** @file >>> Main SEC phase code. Transitions to PEI. >>> >>> - Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.<BR> >>> + Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.<BR> >>> >>> This program and the accompanying materials >>> are licensed and made available under the terms and conditions of the >BSD License >>> @@ -28,6 +28,7 @@ >>> #include <Library/PeCoffGetEntryPointLib.h> >>> #include <Library/PeCoffExtraActionLib.h> >>> #include <Library/ExtractGuidedSectionLib.h> >>> +#include <Library/LocalApicLib.h> >>> >>> #include <Ppi/TemporaryRamSupport.h> >>> >>> @@ -717,6 +718,7 @@ SecCoreStartupWithStack ( >>> SEC_IDT_TABLE IdtTableInStack; >>> IA32_DESCRIPTOR IdtDescriptor; >>> UINT32 Index; >>> + volatile UINT8 *Table; >>> >>> // >>> // This code may be running on the S3 resume boot path, where RAM >has been >>> @@ -731,8 +733,6 @@ SecCoreStartupWithStack ( >>> // invocations are preprocessed to constants.) >>> // >>> if (FeaturePcdGet (PcdSmmSmramRequire)) { >>> - UINT8 *Table; >>> - >>> Table = (UINT8*)(UINTN)FixedPcdGet64 >(PcdGuidedExtractHandlerTableAddress); >>> >>> for (Index = 0; >>> Index < FixedPcdGet32 (PcdGuidedExtractHandlerTableSize); >>> @@ -815,6 +815,11 @@ SecCoreStartupWithStack ( >>> // >>> IoWrite8 (0x21, 0xff); >>> IoWrite8 (0xA1, 0xff); >>> + >>> + // >>> + // Initialize Local APIC Timer hardware before initializing the Debug >Agent and the debug timer is enabled >>> + // >>> + InitializeApicTimer (0, MAX_UINT32, TRUE, 5); >>> >>> // >>> // Initialize Debug Agent to support source level debug in SEC/PEI phases >before memory ready. >>> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf >>> index 2215dd9..711b595 100644 >>> --- a/OvmfPkg/Sec/SecMain.inf >>> +++ b/OvmfPkg/Sec/SecMain.inf >>> @@ -1,7 +1,7 @@ >>> ## @file >>> # SEC Driver >>> # >>> -# Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.<BR> >>> +# Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.<BR> >>> # >>> # This program and the accompanying materials >>> # are licensed and made available under the terms and conditions of the >BSD License >>> @@ -55,6 +55,7 @@ >>> PeCoffGetEntryPointLib >>> PeCoffExtraActionLib >>> ExtractGuidedSectionLib >>> + LocalApicLib >>> >>> [Ppis] >>> gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED >>> -- >>> 1.9.5.msysgit.1 >>> >>> _______________________________________________ >>> edk2-devel mailing list >>> [email protected] >>> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
