From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9580881F1B for ; Tue, 7 Feb 2017 03:32:16 -0800 (PST) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 00C2761B84; Tue, 7 Feb 2017 11:32:17 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-81.phx2.redhat.com [10.3.116.81]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v17BWFxM004232; Tue, 7 Feb 2017 06:32:15 -0500 To: Jordan Justen References: <20170131110221.25860-1-lersek@redhat.com> <20170131110221.25860-3-lersek@redhat.com> <148646137918.13729.17315316362028292158@jljusten-ivb> From: Laszlo Ersek Cc: edk2-devel-01 Message-ID: <934805ad-1e5a-ce9f-263a-fb72b4d0628e@redhat.com> Date: Tue, 7 Feb 2017 12:32:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <148646137918.13729.17315316362028292158@jljusten-ivb> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 07 Feb 2017 11:32:17 +0000 (UTC) Subject: Re: [PATCH v3 2/2] OvmfPkg/SmmControl2Dxe: select broadcast SMI if available X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Feb 2017 11:32:16 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 02/07/17 10:56, Jordan Justen wrote: > On 2017-01-31 03:02:21, Laszlo Ersek wrote: >> When writing to IO port 0xB2 (ICH9_APM_CNT), QEMU by default injects an >> SMI only on the VCPU that is writing the port. > > I'm still annoyed that qemu didn't follow chipset hardware in handling > the b2 port. I know this is long passed any possibility of doing > anything about it, but it is also my last chance to complain. :) It was difficult to come up with a QEMU design that everyone was willing to accept (it was a tough community effort, really -- sort of walking a fine line), and then getting the QEMU patches right (as defined by QEMU maintainers) was again super difficult to me, despite the patches' small size. So yes, I can (quite selfishly :)) agree that "just doing the broadcast" would have been better. I'm much relieved that the feature seems to be converging finally, on both sides. > >> This has exposed corner >> cases and strange behavior with edk2 code, which generally expects a >> software SMI to affect all CPUs at once. We've experienced instability >> despite the fact that OVMF sets PcdCpuSmmApSyncTimeout and >> PcdCpuSmmSyncMode differently from the UefiCpuPkg defaults, such that they >> match QEMU's unicast SMIs better. (Refer to edk2 commits 9b1e378811ff and >> bb0f18b0bce6.) >> >> Using the new fw_cfg-based SMI feature negotiation in QEMU (see commits >> 50de920b372b "hw/isa/lpc_ich9: add SMI feature negotiation via fw_cfg" and >> 5ce45c7a2b15 "hw/isa/lpc_ich9: add broadcast SMI feature"), we can ask >> QEMU to broadcast SMIs. Extensive testing from earlier proves that >> broadcast SMIs are only reliable if we use the UefiCpuPkg defaults for the >> above PCDs. With those settings however, the broadcast is very reliable -- >> the most reliable configuration encountered thus far. > > Given my experience with SMI's we could see more unexpected corner > cases. It seems that SMM is all corner cases. :) Processor mode changes, multiprocessing, and security impact. What could go wrong! :) > > Series Reviewed-by: Jordan Justen \o/ Hooray! Thank you! Pushed as 7c609a144b66..a316d7ac91d3. Cheers! Laszlo > >> >> Therefore negotiate broadcast SMIs with QEMU, and if the negotiation is >> successful, dynamically revert the PCDs to the UefiCpuPkg defaults. >> >> Setting the PCDs in this module is safe: >> >> - only PiSmmCpuDxeSmm consumes them, >> >> - PiSmmCpuDxeSmm is a DXE_SMM_DRIVER, launched by the SMM_CORE >> (MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf), >> >> - the SMM_CORE is launched by the SMM IPL runtime DXE driver >> (MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf), >> >> - the SMM IPL has a DEPEX on EFI_SMM_CONTROL2_PROTOCOL, >> >> - OvmfPkg/SmmControl2Dxe produces that protocol. >> >> The end result is that PiSmmCpuDxeSmm cannot be dispatched before >> SmmControl2Dxe installs EFI_SMM_CONTROL2_PROTOCOL and returns from its >> entry point. Hence we can set the PCD's consumed by PiSmmCpuDxeSmm in >> SmmControl2Dxe. >> >> Cc: Jordan Justen >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=230 >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek >> --- >> >> Notes: >> v3: >> - replace the ICH9_APM_STS-based feature negotiation with the new >> writeable fw_cfg kind >> >> OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf | 8 + >> OvmfPkg/SmmControl2Dxe/SmiFeatures.h | 49 +++ >> OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 324 ++++++++++++++++++++ >> OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c | 21 ++ >> 4 files changed, 402 insertions(+) >> >> diff --git a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >> index 0e9f98c2871c..31c80bd4448c 100644 >> --- a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >> +++ b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf >> @@ -35,32 +35,40 @@ [Defines] >> # >> # The following information is for reference only and not required by the build tools. >> # >> # VALID_ARCHITECTURES = IA32 X64 >> # >> >> [Sources] >> + SmiFeatures.h >> + SmiFeatures.c >> SmmControl2Dxe.c >> >> [Packages] >> MdePkg/MdePkg.dec >> OvmfPkg/OvmfPkg.dec >> + UefiCpuPkg/UefiCpuPkg.dec >> >> [LibraryClasses] >> BaseLib >> DebugLib >> IoLib >> + MemoryAllocationLib >> PcdLib >> PciLib >> QemuFwCfgLib >> UefiBootServicesTableLib >> UefiDriverEntryPoint >> >> [Protocols] >> gEfiS3SaveStateProtocolGuid ## SOMETIMES_CONSUMES >> gEfiSmmControl2ProtocolGuid ## PRODUCES >> >> +[Pcd] >> + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout ## SOMETIMES_PRODUCES >> + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode ## SOMETIMES_PRODUCES >> + >> [FeaturePcd] >> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire >> >> [Depex] >> TRUE >> diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.h b/OvmfPkg/SmmControl2Dxe/SmiFeatures.h >> new file mode 100644 >> index 000000000000..9d5f1dbcb57e >> --- /dev/null >> +++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.h >> @@ -0,0 +1,49 @@ >> +/**@file >> + Negotiate SMI features with QEMU, and configure UefiCpuPkg/PiSmmCpuDxeSmm >> + accordingly. >> + >> + Copyright (C) 2016-2017, 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. >> +**/ >> + >> +#ifndef __SMI_FEATURES_H__ >> +#define __SMI_FEATURES_H__ >> + >> +#include >> + >> +/** >> + Negotiate SMI features with QEMU. >> + >> + @retval FALSE If SMI feature negotiation is not supported by QEMU. This is >> + not an error, it just means that SaveSmiFeatures() should not >> + be called. >> + >> + @retval TRUE SMI feature negotiation is supported, and it has completed >> + successfully as well. (Failure to negotiate is a fatal error >> + and the function never returns in that case.) >> +**/ >> +BOOLEAN >> +NegotiateSmiFeatures ( >> + VOID >> + ); >> + >> +/** >> + Append a boot script fragment that will re-select the previously negotiated >> + SMI features during S3 resume. >> + >> + @param[in] S3SaveState The EFI_S3_SAVE_STATE_PROTOCOL instance to append to >> + the S3 boot script with. >> +**/ >> +VOID >> +SaveSmiFeatures ( >> + IN EFI_S3_SAVE_STATE_PROTOCOL *S3SaveState >> + ); >> + >> +#endif >> diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c >> new file mode 100644 >> index 000000000000..e070969065c0 >> --- /dev/null >> +++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c >> @@ -0,0 +1,324 @@ >> +/**@file >> + Negotiate SMI features with QEMU, and configure UefiCpuPkg/PiSmmCpuDxeSmm >> + accordingly. >> + >> + Copyright (C) 2016-2017, 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. >> +**/ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include "SmiFeatures.h" >> + >> +// >> +// The following bit value stands for "broadcast SMI" in the >> +// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg files. >> +// >> +#define ICH9_LPC_SMI_F_BROADCAST BIT0 >> + >> +// >> +// Provides a scratch buffer (allocated in EfiReservedMemoryType type memory) >> +// for the S3 boot script fragment to write to and read from. The buffer >> +// captures a combined fw_cfg item selection + write command using the DMA >> +// access method. Note that we don't trust the runtime OS to preserve the >> +// contents of the buffer, the boot script will first rewrite it. >> +// >> +#pragma pack (1) >> +typedef struct { >> + FW_CFG_DMA_ACCESS Access; >> + UINT64 Features; >> +} SCRATCH_BUFFER; >> +#pragma pack () >> + >> +// >> +// These carry the selector keys of the "etc/smi/requested-features" and >> +// "etc/smi/features-ok" fw_cfg files from NegotiateSmiFeatures() to >> +// SaveSmiFeatures(). >> +// >> +STATIC FIRMWARE_CONFIG_ITEM mRequestedFeaturesItem; >> +STATIC FIRMWARE_CONFIG_ITEM mFeaturesOkItem; >> + >> +// >> +// Carries the negotiated SMI features from NegotiateSmiFeatures() to >> +// SaveSmiFeatures(). >> +// >> +STATIC UINT64 mSmiFeatures; >> + >> +/** >> + Negotiate SMI features with QEMU. >> + >> + @retval FALSE If SMI feature negotiation is not supported by QEMU. This is >> + not an error, it just means that SaveSmiFeatures() should not >> + be called. >> + >> + @retval TRUE SMI feature negotiation is supported, and it has completed >> + successfully as well. (Failure to negotiate is a fatal error >> + and the function never returns in that case.) >> +**/ >> +BOOLEAN >> +NegotiateSmiFeatures ( >> + VOID >> + ) >> +{ >> + FIRMWARE_CONFIG_ITEM SupportedFeaturesItem; >> + UINTN SupportedFeaturesSize; >> + UINTN RequestedFeaturesSize; >> + UINTN FeaturesOkSize; >> + >> + // >> + // Look up the fw_cfg files used for feature negotiation. The selector keys >> + // of "etc/smi/requested-features" and "etc/smi/features-ok" are saved >> + // statically. If the files are missing, then QEMU doesn't support SMI >> + // feature negotiation. >> + // >> + if (RETURN_ERROR (QemuFwCfgFindFile ("etc/smi/supported-features", >> + &SupportedFeaturesItem, &SupportedFeaturesSize)) || >> + RETURN_ERROR (QemuFwCfgFindFile ("etc/smi/requested-features", >> + &mRequestedFeaturesItem, &RequestedFeaturesSize)) || >> + RETURN_ERROR (QemuFwCfgFindFile ("etc/smi/features-ok", >> + &mFeaturesOkItem, &FeaturesOkSize))) { >> + DEBUG ((DEBUG_INFO, "%a: SMI feature negotiation unavailable\n", >> + __FUNCTION__)); >> + return FALSE; >> + } >> + >> + // >> + // If the files are present but their sizes disagree with us, that's a fatal >> + // error (we can't trust the behavior of SMIs either way). >> + // >> + if (SupportedFeaturesSize != sizeof mSmiFeatures || >> + RequestedFeaturesSize != sizeof mSmiFeatures || >> + FeaturesOkSize != sizeof (UINT8)) { >> + DEBUG ((DEBUG_ERROR, "%a: size mismatch in feature negotiation\n", >> + __FUNCTION__)); >> + goto FatalError; >> + } >> + >> + // >> + // Get the features supported by the host. >> + // >> + QemuFwCfgSelectItem (SupportedFeaturesItem); >> + QemuFwCfgReadBytes (sizeof mSmiFeatures, &mSmiFeatures); >> + >> + // >> + // We want broadcast SMI and nothing else. >> + // >> + mSmiFeatures &= ICH9_LPC_SMI_F_BROADCAST; >> + QemuFwCfgSelectItem (mRequestedFeaturesItem); >> + QemuFwCfgWriteBytes (sizeof mSmiFeatures, &mSmiFeatures); >> + >> + // >> + // Invoke feature validation in QEMU. If the selection is accepted, the >> + // features will be locked down. If the selection is rejected, feature >> + // negotiation remains open; however we don't know what to do in that case, >> + // so that's a fatal error. >> + // >> + QemuFwCfgSelectItem (mFeaturesOkItem); >> + if (QemuFwCfgRead8 () != 1) { >> + DEBUG ((DEBUG_ERROR, "%a: negotiation failed for feature bitmap 0x%Lx\n", >> + __FUNCTION__, mSmiFeatures)); >> + goto FatalError; >> + } >> + >> + if ((mSmiFeatures & ICH9_LPC_SMI_F_BROADCAST) == 0) { >> + // >> + // If we can't get broadcast SMIs from QEMU, that's acceptable too, >> + // although not optimal. >> + // >> + DEBUG ((DEBUG_INFO, "%a: SMI broadcast unavailable\n", __FUNCTION__)); >> + } else { >> + // >> + // Configure the traditional AP sync / SMI delivery mode for >> + // PiSmmCpuDxeSmm. Effectively, restore the UefiCpuPkg defaults, from which >> + // the original QEMU behavior (i.e., unicast SMI) used to differ. >> + // >> + if (RETURN_ERROR (PcdSet64S (PcdCpuSmmApSyncTimeout, 1000000)) || >> + RETURN_ERROR (PcdSet8S (PcdCpuSmmSyncMode, 0x00))) { >> + DEBUG ((DEBUG_ERROR, "%a: PiSmmCpuDxeSmm PCD configuration failed\n", >> + __FUNCTION__)); >> + goto FatalError; >> + } >> + DEBUG ((DEBUG_INFO, "%a: using SMI broadcast\n", __FUNCTION__)); >> + } >> + >> + // >> + // Negotiation successful (although we may not have gotten the optimal >> + // feature set). >> + // >> + return TRUE; >> + >> +FatalError: >> + ASSERT (FALSE); >> + CpuDeadLoop (); >> + // >> + // Keep the compiler happy. >> + // >> + return FALSE; >> +} >> + >> +/** >> + Append a boot script fragment that will re-select the previously negotiated >> + SMI features during S3 resume. >> + >> + @param[in] S3SaveState The EFI_S3_SAVE_STATE_PROTOCOL instance to append to >> + the S3 boot script with. >> +**/ >> +VOID >> +SaveSmiFeatures ( >> + IN EFI_S3_SAVE_STATE_PROTOCOL *S3SaveState >> + ) >> +{ >> + SCRATCH_BUFFER *ScratchBuffer; >> + EFI_STATUS Status; >> + UINT64 AccessAddress; >> + UINT32 ControlPollData; >> + UINT32 ControlPollMask; >> + UINT16 FeaturesOkItemAsUint16; >> + UINT8 FeaturesOkData; >> + UINT8 FeaturesOkMask; >> + >> + ScratchBuffer = AllocateReservedPool (sizeof *ScratchBuffer); >> + if (ScratchBuffer == NULL) { >> + DEBUG ((DEBUG_ERROR, "%a: scratch buffer allocation failed\n", >> + __FUNCTION__)); >> + goto FatalError; >> + } >> + >> + // >> + // Populate the scratch buffer with a select + write fw_cfg DMA command that >> + // will write the negotiated feature bitmap into >> + // "etc/smi/requested-features". >> + // >> + ScratchBuffer->Access.Control = SwapBytes32 ( >> + (UINT32)mRequestedFeaturesItem << 16 | >> + FW_CFG_DMA_CTL_SELECT | >> + FW_CFG_DMA_CTL_WRITE >> + ); >> + ScratchBuffer->Access.Length = SwapBytes32 ( >> + (UINT32)sizeof ScratchBuffer->Features); >> + ScratchBuffer->Access.Address = SwapBytes64 ( >> + (UINTN)&ScratchBuffer->Features); >> + ScratchBuffer->Features = mSmiFeatures; >> + >> + // >> + // Copy the scratch buffer into the boot script. When replayed, this >> + // EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE will restore the current contents of the >> + // scratch buffer, in-place. >> + // >> + Status = S3SaveState->Write ( >> + S3SaveState, // This >> + EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE, // OpCode >> + EfiBootScriptWidthUint8, // Width >> + (UINT64)(UINTN)ScratchBuffer, // Address >> + sizeof *ScratchBuffer, // Count >> + (VOID*)ScratchBuffer // Buffer >> + ); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE: %r\n", >> + __FUNCTION__, __LINE__, Status)); >> + goto FatalError; >> + } >> + >> + // >> + // Append an opcode that will write the address of the scratch buffer to the >> + // fw_cfg DMA address register, which consists of two 32-bit IO ports. The >> + // second (highest address, least significant) write will start the transfer. >> + // >> + AccessAddress = SwapBytes64 ((UINTN)&ScratchBuffer->Access); >> + Status = S3SaveState->Write ( >> + S3SaveState, // This >> + EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode >> + EfiBootScriptWidthUint32, // Width >> + (UINT64)0x514, // Address >> + (UINTN)2, // Count >> + &AccessAddress // Buffer >> + ); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_IO_WRITE_OPCODE: %r\n", >> + __FUNCTION__, __LINE__, Status)); >> + goto FatalError; >> + } >> + >> + // >> + // The EFI_BOOT_SCRIPT_MEM_POLL_OPCODE will wait until the Control word reads >> + // as zero (transfer complete). As timeout we use MAX_UINT64 * 100ns, which >> + // is approximately 58494 years. >> + // >> + ControlPollData = 0; >> + ControlPollMask = MAX_UINT32; >> + Status = S3SaveState->Write ( >> + S3SaveState, // This >> + EFI_BOOT_SCRIPT_MEM_POLL_OPCODE, // OpCode >> + EfiBootScriptWidthUint32, // Width >> + (UINT64)(UINTN)&ScratchBuffer->Access.Control, // Address >> + &ControlPollData, // Data >> + &ControlPollMask, // DataMask >> + MAX_UINT64 // Delay >> + ); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_MEM_POLL_OPCODE: %r\n", >> + __FUNCTION__, __LINE__, Status)); >> + goto FatalError; >> + } >> + >> + // >> + // Select the "etc/smi/features-ok" fw_cfg file, which invokes the feature >> + // validation & lockdown. (The validation succeeded at first boot.) >> + // >> + FeaturesOkItemAsUint16 = (UINT16)mFeaturesOkItem; >> + Status = S3SaveState->Write ( >> + S3SaveState, // This >> + EFI_BOOT_SCRIPT_IO_WRITE_OPCODE, // OpCode >> + EfiBootScriptWidthUint16, // Width >> + (UINT64)0x510, // Address >> + (UINTN)1, // Count >> + &FeaturesOkItemAsUint16 // Buffer >> + ); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_IO_WRITE_OPCODE: %r\n", >> + __FUNCTION__, __LINE__, Status)); >> + goto FatalError; >> + } >> + >> + // >> + // Read the contents (one byte) of "etc/smi/features-ok". If the value is >> + // one, we're good. Otherwise, continue reading the data port: QEMU returns 0 >> + // past the end of the fw_cfg item, so this will hang the resume process, >> + // which matches our intent. >> + // >> + FeaturesOkData = 1; >> + FeaturesOkMask = MAX_UINT8; >> + Status = S3SaveState->Write ( >> + S3SaveState, // This >> + EFI_BOOT_SCRIPT_IO_POLL_OPCODE, // OpCode >> + EfiBootScriptWidthUint8, // Width >> + (UINT64)(UINTN)0x511, // Address >> + &FeaturesOkData, // Data >> + &FeaturesOkMask, // DataMask >> + MAX_UINT64 // Delay >> + ); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "%a:%d: EFI_BOOT_SCRIPT_IO_POLL_OPCODE: %r\n", >> + __FUNCTION__, __LINE__, Status)); >> + goto FatalError; >> + } >> + >> + DEBUG ((DEBUG_VERBOSE, "%a: ScratchBuffer@%p\n", __FUNCTION__, >> + (VOID *)ScratchBuffer)); >> + return; >> + >> +FatalError: >> + ASSERT (FALSE); >> + CpuDeadLoop (); >> +} >> diff --git a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c >> index 6c03e17a3a8d..f31646d73461 100644 >> --- a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c >> +++ b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c >> @@ -32,14 +32,16 @@ >> #include >> #include >> #include >> #include >> #include >> #include >> >> +#include "SmiFeatures.h" >> + >> // >> // Forward declaration. >> // >> STATIC >> VOID >> EFIAPI >> OnS3SaveStateInstalled ( >> @@ -52,14 +54,21 @@ OnS3SaveStateInstalled ( >> // only used to carry information from the entry point function to the >> // S3SaveState protocol installation callback, strictly before the runtime >> // phase. >> // >> STATIC UINTN mSmiEnable; >> >> // >> +// Captures whether SMI feature negotiation is supported. The variable is only >> +// used to carry this information from the entry point function to the >> +// S3SaveState protocol installation callback. >> +// >> +STATIC BOOLEAN mSmiFeatureNegotiation; >> + >> +// >> // Event signaled when an S3SaveState protocol interface is installed. >> // >> STATIC EFI_EVENT mS3SaveStateInstalled; >> >> /** >> Invokes SMI activation from either the preboot or runtime environment. >> >> @@ -225,14 +234,19 @@ SmmControl2DxeEntryPoint ( >> 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; >> } >> >> + // >> + // QEMU can inject SMIs in different ways, negotiate our preferences. >> + // >> + mSmiFeatureNegotiation = NegotiateSmiFeatures (); >> + >> 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. >> @@ -359,11 +373,18 @@ OnS3SaveStateInstalled ( >> DEBUG ((EFI_D_ERROR, >> "%a: EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE_OPCODE: %r\n", __FUNCTION__, >> Status)); >> ASSERT (FALSE); >> CpuDeadLoop (); >> } >> >> + // >> + // Append a boot script fragment that re-selects the negotiated SMI features. >> + // >> + if (mSmiFeatureNegotiation) { >> + SaveSmiFeatures (S3SaveState); >> + } >> + >> DEBUG ((EFI_D_VERBOSE, "%a: boot script fragment saved\n", __FUNCTION__)); >> gBS->CloseEvent (Event); >> mS3SaveStateInstalled = NULL; >> } >> -- >> 2.9.3 >>