On 11/24/15 08:06, Kinney, Michael D wrote: > Laszlo, > > I other modules where we want to halt no matter what, the following 2 > statements are used together. ASSERT() could be removed by PCD > settings, so dead loop catches that case. If ASSERT() is enabled, > then the ASSERT() behavior of BP or dead loop is controlled by PCD. > > ASSERT (FALSE); > CpuDeadLoop ();
Perfect. I'll replace all uses of _ASSERT() in the series, with the above pattern. Thank you! Laszlo > > Thanks, > > Mike > > >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >> Laszlo Ersek >> Sent: Monday, November 23, 2015 4:56 AM >> To: Kinney, Michael D <michael.d.kin...@intel.com>; edk2-de...@ml01.01.org >> Subject: Re: [edk2] [PATCH v4 09/41] OvmfPkg: implement >> EFI_SMM_CONTROL2_PROTOCOL with a DXE_RUNTIME_DRIVER >> >> On 11/21/15 07:17, Kinney, Michael D wrote: >>> Laszlo, >>> >>> Minor comments included below. >>> >>> Reviewed-by: Michael Kinney <michael.d.kin...@intel.com> >> >> Thank you. I'll pick up this tag. Answers and further questions below: >> >>> >>> Mike >>> >>>> -----Original Message----- >>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf >>>> Of Laszlo Ersek >>>> Sent: Tuesday, November 3, 2015 1:01 PM >>>> To: edk2-de...@ml01.01.org >>>> Subject: [edk2] [PATCH v4 09/41] OvmfPkg: implement >>>> EFI_SMM_CONTROL2_PROTOCOL with a DXE_RUNTIME_DRIVER >>>> >>>> The EFI_SMM_COMMUNICATION_PROTOCOL implementation that is provided by >>>> the SMM core depends on EFI_SMM_CONTROL2_PROTOCOL; see the >>>> mSmmControl2->Trigger() call in the SmmCommunicationCommunicate() >>>> mSmmControl2->function >>>> [MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c]. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >>>> --- >>>> >>>> Notes: >>>> v2: >>>> - Set (APMC_EN|GBL_SMI_EN) in SMI_EN from the boot script at S3 resume. >>>> Otherwise, because SMI_EN is cleared during warm reset, >>>> SmmControl2DxeTrigger() would fail to trigger an SMI, and variable >>>> access through the runtime services would fail. >>>> >>>> Set SMI_LOCK in GEN_PMCON_1 similarly. >>>> >>>> OvmfPkg/OvmfPkgIa32.dsc | 1 + >>>> OvmfPkg/OvmfPkgIa32X64.dsc | 1 + >>>> OvmfPkg/OvmfPkgX64.dsc | 1 + >>>> OvmfPkg/OvmfPkgIa32.fdf | 1 + >>>> OvmfPkg/OvmfPkgIa32X64.fdf | 1 + >>>> OvmfPkg/OvmfPkgX64.fdf | 1 + >>>> OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf | 65 ++++ >>>> OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c | 365 ++++++++++++++++++++ >>>> 8 files changed, 436 insertions(+) >>>> >>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index >>>> d7bc38d..c71a2f4 100644 >>>> --- a/OvmfPkg/OvmfPkgIa32.dsc >>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc >>>> @@ -675,4 +675,5 @@ [Components] >>>> >>>> !if $(SMM_REQUIRE) == TRUE >>>> OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >>>> + OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >>>> !endif >>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >>>> index e17cbe5..a1e8f0d 100644 >>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >>>> @@ -682,4 +682,5 @@ [Components.X64] >>>> >>>> !if $(SMM_REQUIRE) == TRUE >>>> OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >>>> + OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >>>> !endif >>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index >>>> a748fb3..1176629 100644 >>>> --- a/OvmfPkg/OvmfPkgX64.dsc >>>> +++ b/OvmfPkg/OvmfPkgX64.dsc >>>> @@ -680,4 +680,5 @@ [Components] >>>> >>>> !if $(SMM_REQUIRE) == TRUE >>>> OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >>>> + OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >>>> !endif >>>> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf index >>>> 285720f..43c9c30 100644 >>>> --- a/OvmfPkg/OvmfPkgIa32.fdf >>>> +++ b/OvmfPkg/OvmfPkgIa32.fdf >>>> @@ -357,6 +357,7 @@ [FV.DXEFV] >>>> >>>> !if $(SMM_REQUIRE) == TRUE >>>> INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >>>> +INF OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >>>> !endif >>>> >>>> >>>> ##################################################################### >>>> ########### diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf >>>> b/OvmfPkg/OvmfPkgIa32X64.fdf index 02e8752..9446896 100644 >>>> --- a/OvmfPkg/OvmfPkgIa32X64.fdf >>>> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf >>>> @@ -357,6 +357,7 @@ [FV.DXEFV] >>>> >>>> !if $(SMM_REQUIRE) == TRUE >>>> INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >>>> +INF OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >>>> !endif >>>> >>>> >>>> ##################################################################### >>>> ########### diff --git a/OvmfPkg/OvmfPkgX64.fdf >>>> b/OvmfPkg/OvmfPkgX64.fdf index f04c36b..b272b76 100644 >>>> --- a/OvmfPkg/OvmfPkgX64.fdf >>>> +++ b/OvmfPkg/OvmfPkgX64.fdf >>>> @@ -357,6 +357,7 @@ [FV.DXEFV] >>>> >>>> !if $(SMM_REQUIRE) == TRUE >>>> INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >>>> +INF OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >>>> !endif >>>> >>>> >>>> ##################################################################### >>>> ########### diff --git a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >>>> b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >>>> new file mode 100644 >>>> index 0000000..bc66a27 >>>> --- /dev/null >>>> +++ b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >>>> @@ -0,0 +1,65 @@ >>>> +## @file >>>> +# A DXE_RUNTIME_DRIVER providing synchronous SMI activations via the >>>> +# EFI_SMM_CONTROL2_PROTOCOL. >>>> +# >>>> +# We expect the PEI phase to have covered the following: >>>> +# - ensure that the underlying QEMU machine type be Q35 >>>> +# (responsible: OvmfPkg/SmmAccess/SmmAccessPei.inf) >>>> +# - ensure that the ACPI PM IO space be configured >>>> +# (responsible: OvmfPkg/PlatformPei/PlatformPei.inf) >>>> +# >>>> +# Our own entry point is responsible for confirming the SMI feature >>>> +and for # configuring it. >>>> +# >>>> +# Copyright (C) 2013, 2015, Red Hat, Inc. >>>> +# >>>> +# This program and the accompanying materials are licensed and made >>>> +available # under the terms and conditions of the BSD License which >>>> +accompanies this # distribution. The full text of the license may be >>>> +found at # http://opensource.org/licenses/bsd-license.php >>>> +# >>>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>>> +BASIS, WITHOUT # WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >>>> EXPRESS OR IMPLIED. >>>> +# >>>> +## >>>> + >>>> +[Defines] >>>> + INF_VERSION = 0x00010005 >>>> + BASE_NAME = SmmControl2Dxe >>>> + FILE_GUID = 1206F7CA-A475-4624-A83E-E6FC9BB38E49 >>>> + MODULE_TYPE = DXE_RUNTIME_DRIVER >>>> + VERSION_STRING = 1.0 >>>> + PI_SPECIFICATION_VERSION = 0x00010400 >>>> + ENTRY_POINT = SmmControl2DxeEntryPoint >>>> + >>>> +# >>>> +# The following information is for reference only and not required by the >>>> build tools. >>>> +# >>>> +# VALID_ARCHITECTURES = IA32 X64 >>>> +# >>>> + >>>> +[Sources] >>>> + SmmControl2Dxe.c >>>> + >>>> +[Packages] >>>> + MdePkg/MdePkg.dec >>>> + OvmfPkg/OvmfPkg.dec >>>> + >>>> +[LibraryClasses] >>>> + DebugLib >>>> + IoLib >>>> + PcdLib >>>> + PciLib >>>> + QemuFwCfgLib >>>> + UefiBootServicesTableLib >>>> + UefiDriverEntryPoint >>>> + >>>> +[Protocols] >>>> + gEfiS3SaveStateProtocolGuid # PROTOCOL SOMETIMES_CONSUMED >>> >>> Comment should be: ## SOMETIMES_CONSUMES >> >> Will fix. >> >>> >>>> + gEfiSmmControl2ProtocolGuid # PROTOCOL ALWAYS_PRODUCED >>> >>> Comment should be: ## PRODUCES >> >> Ditto, thanks. >> >>> >>>> + >>>> +[FeaturePcd] >>>> + gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire >>>> + >>>> +[Depex] >>>> + TRUE >>>> diff --git a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c >>>> b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c >>>> new file mode 100644 >>>> index 0000000..e895fd6 >>>> --- /dev/null >>>> +++ b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c >>>> @@ -0,0 +1,365 @@ >>>> +/** @file >>>> + >>>> + A DXE_RUNTIME_DRIVER providing synchronous SMI activations via the >>>> + EFI_SMM_CONTROL2_PROTOCOL. >>>> + >>>> + We expect the PEI phase to have covered the following: >>>> + - ensure that the underlying QEMU machine type be Q35 >>>> + (responsible: OvmfPkg/SmmAccess/SmmAccessPei.inf) >>>> + - ensure that the ACPI PM IO space be configured >>>> + (responsible: OvmfPkg/PlatformPei/PlatformPei.inf) >>>> + >>>> + Our own entry point is responsible for confirming the SMI feature >>>> + and for configuring it. >>>> + >>>> + Copyright (C) 2013, 2015, Red Hat, Inc.<BR> Copyright (c) 2009 - >>>> + 2010, 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 which >>>> + accompanies this distribution. The full text of the license may be >>>> + found at http://opensource.org/licenses/bsd-license.php >>>> + >>>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>>> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >>>> EXPRESS OR IMPLIED. >>>> + >>>> +**/ >>>> + >>>> +#include <IndustryStandard/Q35MchIch9.h> #include >>>> +<Library/DebugLib.h> #include <Library/IoLib.h> #include >>>> +<Library/PcdLib.h> #include <Library/PciLib.h> #include >>>> +<Library/QemuFwCfgLib.h> #include >>>> +<Library/UefiBootServicesTableLib.h> >>>> +#include <Protocol/S3SaveState.h> >>>> +#include <Protocol/SmmControl2.h> >>>> + >>>> +// >>>> +// The absolute IO port address of the SMI Control and Enable Register. >>>> +It is // only used to carry information from the entry point >>>> +function to the // S3SaveState protocol installation callback, >>>> +strictly before the runtime // phase. >>>> +// >>>> +STATIC UINTN mSmiEnable; >>>> + >>>> +// >>>> +// Event signaled when an S3SaveState protocol interface is installed. >>>> +// >>>> +STATIC EFI_EVENT mS3SaveStateInstalled; >>>> + >>>> +/** >>>> + Invokes SMI activation from either the preboot or runtime environment. >>>> + >>>> + This function generates an SMI. >>>> + >>>> + @param[in] This The EFI_SMM_CONTROL2_PROTOCOL >>>> instance. >>>> + @param[in,out] CommandPort The value written to the command >>>> port. >>>> + @param[in,out] DataPort The value written to the data port. >>>> + @param[in] Periodic Optional mechanism to engender a >>>> periodic >>>> + stream. >>>> + @param[in] ActivationInterval Optional parameter to repeat at this >>>> + period one time or, if the Periodic >>>> + Boolean is set, periodically. >>>> + >>>> + @retval EFI_SUCCESS The SMI/PMI has been engendered. >>>> + @retval EFI_DEVICE_ERROR The timing is unsupported. >>>> + @retval EFI_INVALID_PARAMETER The activation period is unsupported. >>>> + @retval EFI_INVALID_PARAMETER The last periodic activation has not been >>>> + cleared. >>>> + @retval EFI_NOT_STARTED The SMM base service has not been >>>> initialized. >>>> +**/ >>>> +STATIC >>> >>> Recommend removing use of STATIC >> >> I'd like to stick with it in OvmfPkg. >> >>> >>>> +EFI_STATUS >>>> +EFIAPI >>>> +SmmControl2DxeTrigger ( >>>> + IN CONST EFI_SMM_CONTROL2_PROTOCOL *This, >>>> + IN OUT UINT8 *CommandPort OPTIONAL, >>>> + IN OUT UINT8 *DataPort OPTIONAL, >>>> + IN BOOLEAN Periodic OPTIONAL, >>>> + IN UINTN ActivationInterval OPTIONAL >>>> + ) >>>> +{ >>>> + // >>>> + // No support for queued or periodic activation. >>>> + // >>>> + if (Periodic || ActivationInterval > 0) { >>>> + return EFI_DEVICE_ERROR; >>>> + } >>>> + >>>> + // >>>> + // The so-called "Advanced Power Management Status Port Register" >>>> +is in fact >>>> + // a generic data passing register, between the caller and the SMI >>>> + // dispatcher. The ICH9 spec calls it "scratchpad register" -- >>>> +calling it >>>> + // "status" elsewhere seems quite the misnomer. Status registers >>>> +usually >>>> + // report about hardware status, while this register is fully >>>> +governed by >>>> + // software. >>>> + // >>>> + // Write to the status register first, as this won't trigger the >>>> +SMI just >>>> + // yet. Then write to the control register. >>>> + // >>>> + IoWrite8 (ICH9_APM_STS, DataPort == NULL ? 0 : *DataPort); >>>> + IoWrite8 (ICH9_APM_CNT, CommandPort == NULL ? 0 : *CommandPort); >>>> + return EFI_SUCCESS; >>>> +} >>>> + >>>> +/** >>>> + Clears any system state that was created in response to the Trigger() >>>> call. >>>> + >>>> + This function acknowledges and causes the deassertion of the SMI >>>> + activation source. >>>> + >>>> + @param[in] This The EFI_SMM_CONTROL2_PROTOCOL instance. >>>> + @param[in] Periodic Optional parameter to repeat at this >>>> period >>>> + one time >>>> + >>>> + @retval EFI_SUCCESS The SMI/PMI has been engendered. >>>> + @retval EFI_DEVICE_ERROR The source could not be cleared. >>>> + @retval EFI_INVALID_PARAMETER The service did not support the Periodic >>>> input >>>> + argument. >>>> +**/ >>>> +STATIC >>> >>> Recommend removing use of STATIC >>> >>>> +EFI_STATUS >>>> +EFIAPI >>>> +SmmControl2DxeClear ( >>>> + IN CONST EFI_SMM_CONTROL2_PROTOCOL *This, >>>> + IN BOOLEAN Periodic OPTIONAL >>>> + ) >>>> +{ >>>> + if (Periodic) { >>>> + return EFI_INVALID_PARAMETER; >>>> + } >>>> + >>>> + // >>>> + // The PI spec v1.4 explains that Clear() is only supposed to >>>> +clear software >>>> + // status; it is not in fact responsible for deasserting the SMI. >>>> +It gives >>>> + // two reasons for this: (a) many boards clear the SMI >>>> +automatically when >>>> + // entering SMM, (b) if Clear() actually deasserted the SMI, then >>>> +it could >>>> + // incorrectly suppress an SMI that was asynchronously asserted >>>> +between the >>>> + // last return of the SMI handler and the call made to Clear(). >>>> + // >>>> + // In fact QEMU automatically deasserts CPU_INTERRUPT_SMI in: >>>> + // - x86_cpu_exec_interrupt() [target-i386/seg_helper.c], and >>>> + // - kvm_arch_pre_run() [target-i386/kvm.c]. >>>> + // >>>> + // So, nothing to do here. >>>> + // >>>> + return EFI_SUCCESS; >>>> +} >>>> + >>>> +STATIC EFI_SMM_CONTROL2_PROTOCOL mControl2 = { >>>> + &SmmControl2DxeTrigger, >>>> + &SmmControl2DxeClear, >>>> + MAX_UINTN // MinimumTriggerPeriod -- we don't support periodic >>>> +SMIs }; >>> >>> Recommend removing use of STATIC >>> Global variable should be above function implementations >> >> I can move it to the top and assign the struct in code, if you'd like me to >> do that. >> >>> >>>> + >>>> +// >>>> +// Forward declaration. >>>> +// >>>> +STATIC >>> >>> Recommend removing use of STATIC >>> >>>> +VOID >>>> +EFIAPI >>>> +OnS3SaveStateInstalled ( >>>> + IN EFI_EVENT Event, >>>> + IN VOID *Context >>>> + ); >>> >>> Forward declarations should go before function implementations >> >> I wanted to keep it close to the reference to it, just below in >> SmmControl2DxeEntryPoint(), but I will now move it near the top. >> Thanks. >> >>> >>>> + >>>> +// >>>> +// Entry point of this driver. >>>> +// >>>> +EFI_STATUS >>>> +EFIAPI >>>> +SmmControl2DxeEntryPoint ( >>>> + IN EFI_HANDLE ImageHandle, >>>> + IN EFI_SYSTEM_TABLE *SystemTable >>>> + ) >>>> +{ >>>> + UINT32 PmBase; >>>> + UINT32 SmiEnableVal; >>>> + EFI_STATUS Status; >>>> + >>>> + // >>>> + // This module should only be included if SMRAM support is required. >>>> + // >>>> + ASSERT (FeaturePcdGet (PcdSmmSmramRequire)); >>>> + >>>> + // >>>> + // Calculate the absolute IO port address of the SMI Control and >>>> + Enable // Register. (As noted at the top, the PEI phase has left >>>> + us with a working // ACPI PM IO space.) // PmBase = PciRead32 >>>> + (POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE)) & >>>> + ICH9_PMBASE_MASK; >>>> + mSmiEnable = PmBase + ICH9_PMBASE_OFS_SMI_EN; >>>> + >>>> + // >>>> + // If APMC_EN is pre-set in SMI_EN, that's QEMU's way to tell us >>>> + that SMI // support is not available. (For example due to KVM >>>> + lacking it.) Otherwise, // this bit is clear after each reset. >>>> + // >>>> + SmiEnableVal = IoRead32 (mSmiEnable); if ((SmiEnableVal & >>>> + ICH9_SMI_EN_APMC_EN) != 0) { >>>> + DEBUG ((EFI_D_ERROR, "%a: this Q35 implementation lacks SMI\n", >>>> + __FUNCTION__)); >>>> + goto FatalError; >>>> + } >>>> + >>>> + // >>>> + // Otherwise, configure the board to inject an SMI when >>>> + ICH9_APM_CNT is // written to. (See the Trigger() method above.) >>>> + // SmiEnableVal >>>> + |= ICH9_SMI_EN_APMC_EN | ICH9_SMI_EN_GBL_SMI_EN; >>>> + IoWrite32 (mSmiEnable, SmiEnableVal); >>>> + >>>> + // >>>> + // Prevent software from undoing the above (until platform reset). >>>> + // >>>> + PciOr16 (POWER_MGMT_REGISTER_Q35 (ICH9_GEN_PMCON_1), >>>> + ICH9_GEN_PMCON_1_SMI_LOCK); >>>> + >>>> + // >>>> + // If we can clear GBL_SMI_EN now, that means QEMU's SMI support >>>> + is not // appropriate. >>>> + // >>>> + IoWrite32 (mSmiEnable, SmiEnableVal & >>>> + ~(UINT32)ICH9_SMI_EN_GBL_SMI_EN); if (IoRead32 (mSmiEnable) != >>>> SmiEnableVal) { >>>> + DEBUG ((EFI_D_ERROR, "%a: failed to lock down GBL_SMI_EN\n", >>>> + __FUNCTION__)); >>>> + goto FatalError; >>>> + } >>>> + >>>> + if (QemuFwCfgS3Enabled ()) { >>>> + VOID *Registration; >>>> + >>>> + // >>>> + // On S3 resume the above register settings have to be repeated. >>>> Register a >>>> + // protocol notify callback that, when boot script saving becomes >>>> + // available, saves operations equivalent to the above to the boot >>>> script. >>>> + // >>>> + Status = gBS->CreateEvent (EVT_NOTIFY_SIGNAL, TPL_CALLBACK, >>>> + OnS3SaveStateInstalled, NULL /* Context */, >>>> + &mS3SaveStateInstalled); >>>> + if (EFI_ERROR (Status)) { >>>> + DEBUG ((EFI_D_ERROR, "%a: CreateEvent: %r\n", __FUNCTION__, >>>> Status)); >>>> + goto FatalError; >>>> + } >>>> + >>>> + Status = gBS->RegisterProtocolNotify (&gEfiS3SaveStateProtocolGuid, >>>> + mS3SaveStateInstalled, &Registration); >>>> + if (EFI_ERROR (Status)) { >>>> + DEBUG ((EFI_D_ERROR, "%a: RegisterProtocolNotify: %r\n", >>>> __FUNCTION__, >>>> + Status)); >>>> + goto ReleaseEvent; >>>> + } >>>> + >>>> + // >>>> + // Kick the event right now -- maybe the boot script is already >>>> saveable. >>>> + // >>>> + Status = gBS->SignalEvent (mS3SaveStateInstalled); >>>> + if (EFI_ERROR (Status)) { >>>> + DEBUG ((EFI_D_ERROR, "%a: SignalEvent: %r\n", __FUNCTION__, >>>> Status)); >>>> + goto ReleaseEvent; >>>> + } >>>> + } >>>> + >>>> + // >>>> + // We have no pointers to convert to virtual addresses. The handle >>>> + itself // doesn't matter, as protocol services are not accessible at >>>> runtime. >>>> + // >>>> + Status = gBS->InstallMultipleProtocolInterfaces (&ImageHandle, >>>> + &gEfiSmmControl2ProtocolGuid, &mControl2, >>>> + NULL); >>>> + if (EFI_ERROR (Status)) { >>>> + DEBUG ((EFI_D_ERROR, "%a: InstallMultipleProtocolInterfaces: %r\n", >>>> + __FUNCTION__, Status)); >>>> + goto ReleaseEvent; >>>> + } >>>> + >>>> + return EFI_SUCCESS; >>>> + >>>> +ReleaseEvent: >>>> + if (mS3SaveStateInstalled != NULL) { >>>> + gBS->CloseEvent (mS3SaveStateInstalled); >>>> + } >>>> + >>>> +FatalError: >>>> + // >>>> + // We really don't want to continue in this case. >>>> + // >>>> + // _ASSERT() is never compiled out, and it respects >>>> PcdDebugPropertyMask (ie. >>>> + // prevent further execution with CPU breakpoint vs. dead loop). >>>> + // >>>> + _ASSERT (FALSE); >>> >>> Should not use internal macros. >>> If you need a CPU break point, then use BaseLib CpuBreakpoint () If >>> you need a deadloop, then use BaseLib CpuDeadLoop () >> >> That's the thing -- I don't know (and don't want to know) what I need; >> PcdDebugPropertyMask decides about that. Whichever is >> chosen however, that should never be compiled out of the code. So _ASSERT() >> does exactly what I'd like to have. >> >> Do you recommend to open-code the following instead? >> >> if ((PcdGet8 (PcdDebugPropertyMask) & >> DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { >> CpuBreakpoint (); >> } >> if ((PcdGet8(PcdDebugPropertyMask) & >> DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { >> CpuDeadLoop (); >> } >> >> Thanks! >> Laszlo >> >>> >>>> + return EFI_UNSUPPORTED; >>>> +} >>>> + >>>> +/** >>>> + Notification callback for S3SaveState installation. >>>> + >>>> + @param[in] Event Event whose notification function is being invoked. >>>> + >>>> + @param[in] Context The pointer to the notification function's context, >>>> which >>>> + is implementation-dependent. >>>> +**/ >>>> +STATIC >>> >>> Recommend removing use of STATIC >>> >>>> +VOID >>>> +EFIAPI >>>> +OnS3SaveStateInstalled ( >>>> + IN EFI_EVENT Event, >>>> + IN VOID *Context >>>> + ) >>>> +{ >>>> + EFI_STATUS Status; >>>> + EFI_S3_SAVE_STATE_PROTOCOL *S3SaveState; >>>> + UINT32 SmiEnOrMask, SmiEnAndMask; >>>> + UINT16 GenPmCon1OrMask, GenPmCon1AndMask; >>>> + >>>> + ASSERT (Event == mS3SaveStateInstalled); >>>> + >>>> + Status = gBS->LocateProtocol (&gEfiS3SaveStateProtocolGuid, >>>> + NULL /* Registration */, (VOID **)&S3SaveState); >>>> + if (EFI_ERROR (Status)) { >>>> + return; >>>> + } >>>> + >>>> + // >>>> + // These operations were originally done, verified and explained >>>> + in the entry // point function of the driver. >>>> + // >>>> + SmiEnOrMask = ICH9_SMI_EN_APMC_EN | ICH9_SMI_EN_GBL_SMI_EN; >>>> + SmiEnAndMask = MAX_UINT32; Status = S3SaveState->Write ( >>>> + S3SaveState, >>>> + EFI_BOOT_SCRIPT_IO_READ_WRITE_OPCODE, >>>> + EfiBootScriptWidthUint32, >>>> + (UINT64)mSmiEnable, >>>> + &SmiEnOrMask, >>>> + &SmiEnAndMask >>>> + ); >>>> + if (EFI_ERROR (Status)) { >>>> + DEBUG ((EFI_D_ERROR, "%a: EFI_BOOT_SCRIPT_IO_READ_WRITE_OPCODE: %r\n", >>>> + __FUNCTION__, Status)); >>>> + _ASSERT (FALSE); >>>> + } >>>> + >>>> + GenPmCon1OrMask = ICH9_GEN_PMCON_1_SMI_LOCK; GenPmCon1AndMask = >>>> + MAX_UINT16; Status = S3SaveState->Write ( >>>> + S3SaveState, >>>> + EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE_OPCODE, >>>> + EfiBootScriptWidthUint16, >>>> + (UINT64)POWER_MGMT_REGISTER_Q35 >>>> (ICH9_GEN_PMCON_1), >>>> + &GenPmCon1OrMask, >>>> + &GenPmCon1AndMask >>>> + ); >>>> + if (EFI_ERROR (Status)) { >>>> + DEBUG ((EFI_D_ERROR, >>>> + "%a: EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE_OPCODE: %r\n", >>>> __FUNCTION__, >>>> + Status)); >>>> + _ASSERT (FALSE); >>>> + } >>>> + >>>> + DEBUG ((EFI_D_VERBOSE, "%a: boot script fragment saved\n", >>>> +__FUNCTION__)); >>>> + gBS->CloseEvent (Event); >>>> + mS3SaveStateInstalled = NULL; >>>> +} >>>> -- >>>> 1.8.3.1 >>>> >>>> >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.01.org >>>> https://lists.01.org/mailman/listinfo/edk2-devel >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
