Re: [edk2-devel] Regression found with latest edk2/OVMF SECUREBOOT/SMM build
On Fri, Feb 11, 2022 at 06:31:35PM +, Aaron Young wrote: > Hello, my apologies if this has already been > discovered/discussed/addressed, but we are seeing a reproducible exception > with latest (c9b7c6e0cc7da76b74bcdd8c90cef956d5ae971c) OVMF x86_64 build with > SECURE_BOOT/SMM enabled: > > Exception: > > X64 Exception Type - 06(#UD - Invalid Opcode) CPU Apic ID - > > RIP - 000E, CS - 0038, RFLAGS - 00010046 > RAX - , RCX - 7FED2920, RDX - > RBX - 7DB93D98, RSP - 7FF26718, RBP - 7FFE1630 > RSI - 7E9EE018, RDI - > R8 - 7FED3230, R9 - 0210, R10 - 002D > R11 - 7FF26482, R12 - 7EAC2201, R13 - 7FFFD2B0 > R14 - 7FF26A88, R15 - 2000 > DS - 0020, ES - 0020, FS - 0020 > GS - 0020, SS - 0020 > CR0 - 80010033, CR2 - , CR3 - 7FF16000 > CR4 - 0668, CR8 - > DR0 - , DR1 - , DR2 - > DR3 - , DR6 - 0FF0, DR7 - 0400 > GDTR - 7FED9000 004F, LDTR - > IDTR - 7FF2 01FF, TR - 0040 > FXSAVE_STATE - 7FF26370 > Can't find image information. > > > OVMF Debug Log tail: > > Stacks - 0x7FF21000 > mSmmStackSize- 0x6000 > PcdCpuSmmStackGuard - 0x1 > mXdSupported - 0x1 > One Semaphore Size= 0x40 > Total Semaphores Size = 0x1140 > PhysicalAddressBits = 40, 5LPageTable = 0. > 5LevelPaging Needed - 0 > 1GPageTable Support - 0 > PcdCpuSmmRestrictedMemoryAccess - 1 > PhysicalAddressBits - 36 > Initialize IDT IST field for SMM Stack Guard > InstallProtocolInterface: 26EEB3DE-B689-492E-80F0-BE8BD7DA4BA7 7FFC6100 > SMM IPL registered SMM Entry Point address 7FFE5274 > SmmInstallProtocolInterface: EB346B97-975F-4A9F-8B22-F8E92BB3D569 7FFC6040 > SmmInstallProtocolInterface: 69B792EA-39CE-402D-A2A6-F721DE351DFE 7FFC6020 > CpuSmm: SpinLock Size = 0x40, PcdCpuSmmMpTokenCountPerChunk = 0x40 > SmmInstallProtocolInterface: 5D5450D7-990C-4180-A803-8E63F0608307 7FFC6200 > SmmInstallProtocolInterface: 1D202CAB-C8AB-4D5C-94F7-3CFCC0D3D335 7FFC6140 > SMM CPU Module exit from SMRAM with EFI_SUCCESS > SMM IPL closed SMRAM window > InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7DB93E18 > SmmInstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7FFE16C0 > Loading SMM driver at 0x0007FECA000 EntryPoint=0x0007FECFE6C > FvbServicesSmm.efi > > > Failure bisected to this commit: > > commit ade62c18f4742301bbef474ac10518bde5972fba > Author: Brijesh Singh via groups.io > Date: Thu Dec 9 11:27:42 2021 +0800 > > OvmfPkg/MemEncryptSevLib: add support to validate system RAM > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275 I hit the same thing preparing a 202202-rc1 for Debian. Strangely it only seems to impact our 2M images - FD_SIZE_4MB is OK: X64 Exception Type - 06(#UD - Invalid Opcode) CPU Apic ID - RIP - FF00, CS - 0038, RFLAGS - 0002 RAX - , RCX - 0FF77040, RDX - RBX - 0FFF2690, RSP - 0FFCA6B8, RBP - RSI - 0FFFB701, RDI - 0FFC R8 - 0FF771C8, R9 - 03070002, R10 - 002D R11 - 0FF78FFF, R12 - 0DE37498, R13 - 0E9EE018 R14 - 0FF79000, R15 - 0FFFC6F8 DS - 0020, ES - 0020, FS - 0020 GS - 0020, SS - 0020 CR0 - 80010033, CR2 - , CR3 - 0FFBB000 CR4 - 0668, CR8 - DR0 - , DR1 - , DR2 - DR3 - , DR6 - 0FF0, DR7 - 0400 GDTR - 0FF7E000 004F, LDTR - IDTR - 0FFC4000 01FF, TR - 0040 FXSAVE_STATE - 0FFCA310 Can't find image information. FAIL == FAIL: test_ovmf_ms_secure_boot_unsigned (__main__.BootToShellTest) -- Traceback (most recent call last): File "/home/dannf/git/edk2/debian/tests/shell.py", line 75, in run_cmd_check_secure_boot i = child.expect( File "/usr/lib/python3/dist-packages/pexpect/spawnbase.py", line 343, in expect return self.expect_list(compiled_pattern_list, pexpect.exceptions.TIMEOUT: Timeout exceeded. command: /usr/bin/qemu-system-x86_64 args: ['/usr/bin/qemu-system-x86_64', '-machine', 'q35,accel=tcg', '-no
Re: [edk2-devel] [edk2-platforms: PATCH v3] MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
Reviewed-by: Isaac Oram Minor code style nits that can be fixed before pushing: These do not need another patch for review, if a maintainer agrees. SaveMemoryConfig.c Line 95 : EFI_ERROR( put a space before ( Line 101, 118: CpuDeadLoop( put a space before ( LargeVariableWriteLib.c: Lines 506, 519, 542: EFI_ERROR( put a space before ( Regards, Isaac -Original Message- From: devel@edk2.groups.io On Behalf Of Chiu, Chasel Sent: Friday, February 11, 2022 1:02 AM To: devel@edk2.groups.io Cc: Chiu, Chasel ; Desimone, Nathaniel L ; Gao, Liming ; Dong, Eric Subject: [edk2-devel] [edk2-platforms: PATCH v3] MinPlatformPkg/SaveMemoryConfig: Variable may not be locked. From: "Chiu, Chasel" REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3829 Fixed the bug that existing variable will not be locked when it is identical with hob data by creating LockLargeVariable function, also switched to VariablePolicyProtocol for locking variables. Failing to lock variable could be security vulnerability, so the function will return EFI_ABORTED when it failed and SaveMemoryConfig driver will halt the system for developers to resolve this issue. This patch also modified SaveMemoryConfig driver to be unloaded after execution because it does not produce any service protocol. To achieve this goal the DxeRuntimeVariableWriteLib should close registered ExitBootService events in its DESTRUCTOR. Cc: Nate DeSimone Cc: Liming Gao Cc: Eric Dong Signed-off-by: Chasel Chiu ---V3:Updated LargeVariableWriteLib to return EFI_ABORTED when locking variables failed.Also SaveMemoryConfig driver will halt the system in this case for developers to fixsuch security vulnerability issue. Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c | 27 --- Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c | 115 ++- Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c | 61 + Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf | 3 ++- Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h | 25 +++-- Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf | 8 +--- 6 files changed, 209 insertions(+), 30 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c index 820585f676..54e11e20bd 100644 --- a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemo +++ ryConfig.c @@ -2,13 +2,14 @@ This is the driver that locates the MemoryConfigurationData HOB, if it exists, and saves the data to nvRAM. -Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.+Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ #include #include +#include #include #include #include @@ -18,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include #include #include +#include #include /**@@ -86,6 +88,18 @@ SaveMemoryConfigEntryPoint ( Status = GetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, &BufferSize, VariableData); if (!EFI_ERROR (Status) && (BufferSize == DataSize) && (0 == CompareMem (HobData, VariableData, DataSize))) { DataIsIdentical = TRUE;+ //+ // No need to update Variable, only lock it.+ //+ Status = LockLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid);+ if (EFI_ERROR(Status)) {+ //+// Fail to lock variable is security vulnerability and should not happen.+//+DEBUG ((DEBUG_ERROR, "LockVariable is requested but failed unexpectedly!\n"));+ ASSERT_EFI_ERROR (Status);+CpuDeadLoop();+ } } FreePool (VariableData); }@@ -96,6 +110,13 @@ SaveMemoryConfigEntryPoint ( if (!DataIsIdentical) { Status = SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, TRUE, DataSize, HobData); ASSERT_EFI_ERROR (Status);+if (Status == EFI_ABORTED) {+ //+ // Fail to lock variable is security vulnerability and should not happen.+ //+ DEBUG ((DEBUG_ERROR, "LockVariable is requested but failed unexpectedly!\n"));+ CpuDeadLoop();+} DEBUG ((DEBUG_INFO, "Saved size of FSP / MRC
[edk2-devel][edk2-platforms][PATCH V1 1/1] BeepDebugFeaturePkg: Enable FixedAtBuild PCD type
Enable PcdStatusCodeUseBeep to allow FixedAtBuild type for backwards compatibility. Cc: Sai Chaganty Cc: Nate DeSimone Cc: Liming Gao Signed-off-by: Isaac Oram --- Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.dec | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.dec b/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.dec index d90611da51..9f44f228d1 100644 --- a/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.dec +++ b/Features/Intel/Debugging/BeepDebugFeaturePkg/BeepDebugFeaturePkg.dec @@ -34,6 +34,5 @@ [PcdsFeatureFlag] gBeepDebugFeaturePkgTokenSpaceGuid.PcdBeepDebugFeatureEnable|FALSE|BOOLEAN|0x -[PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] - # Beep is a legacy feature, disabled it by default +[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] gBeepDebugFeaturePkgTokenSpaceGuid.PcdStatusCodeUseBeep|TRUE|BOOLEAN|0x0001 -- 2.27.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86629): https://edk2.groups.io/g/devel/message/86629 Mute This Topic: https://groups.io/mt/89084754/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] **NOTICE** EDK II CI using Azure Pipelines Agents Failing
Not a great update. I have opened a ticket with our system for supporting Azure DevOps Pipelines. There have been a number of projects hit by this in the past few days so I hope we can get to mitigation quickly. At the moment I don't have a clear answer why but for some reason our usage of the build services have gotten us flagged as "abusers". More digging around has indicated that pipeline builds (in general) are getting abused for crypto mining and other nefarious uses. In some cases vulnerabilities in published packages are being used to do this (point being not always the projects intent). I have noticed that some of our builds have taken nearly two hours to install NPM cspell package (generally it only takes 44 seconds). I have no idea if that is anything other than an anomaly. https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=40249&view=logs&j=4a97f94c-8e1f-5e76-68ba-50872604dd63&t=7346d0e9-c9fc-5f26-991e-d6f31d36ba1e Anyway, I'll update when i have more. Thanks Sean On 2/11/2022 8:16 AM, Michael D Kinney wrote: Hello, The EDK II CI using Azure Pipelines Agents is not working right now. First failure occurred on 2-10-2022. The issue is being worked. We will send daily status updates until the issue is resolved. We are currently in a hard freeze for the next EDK II stable tag. We will monitor status closely and adjust the release date if needed. Thank you for your patience. Mike -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86628): https://edk2.groups.io/g/devel/message/86628 Mute This Topic: https://groups.io/mt/89075001/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request protocol before setting
seems this patch got truncated during a rebase, here is the correct version. For device that does not respond to GetProtocolRequest(), the original flow is: a.1 Execute GetProtocolRequest() a.2 Timeout occurs for GetProtocolRequest() a.3 Conditionally execute UsbSetProtocolRequest() The proposed new flow is: b.1 Always execute UsbSetProtocolRequest() The timeout delay for keyboards which do not support GetProtocolRequest() that we've seen is significant (>5s, IIRC) MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 28 ++-- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c index 5a94a4dda7..ad55fa3330 100644 --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c @@ -805,7 +805,6 @@ InitUSBKeyboard ( ) { UINT16 ConfigValue; - UINT8 Protocol; EFI_STATUS Status; UINT32 TransferResult; @@ -854,21 +853,28 @@ InitUSBKeyboard ( } } - UsbGetProtocolRequest ( -UsbKeyboardDevice->UsbIo, -UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber, -&Protocol -); // // Set boot protocol for the USB Keyboard. // This driver only supports boot protocol. // - if (Protocol != BOOT_PROTOCOL) { -UsbSetProtocolRequest ( - UsbKeyboardDevice->UsbIo, - UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber, - BOOT_PROTOCOL + Status = UsbSetProtocolRequest ( + UsbKeyboardDevice->UsbIo, + UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber, + BOOT_PROTOCOL + ); + if (EFI_ERROR (Status)) { +// +// If protocol could not be set here, it means +// the keyboard interface has some errors and could +// not be initialized +// +REPORT_STATUS_CODE_WITH_DEVICE_PATH ( + EFI_ERROR_CODE | EFI_ERROR_MINOR, + (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR), + UsbKeyboardDevice->DevicePath ); + +return EFI_DEVICE_ERROR; } UsbKeyboardDevice->CtrlOn= FALSE; -- 2.32.0 On Thu, Feb 10, 2022 at 9:32 PM Wu, Hao A wrote: > > Hello, > > Inline comments below: > > > > -Original Message- > > From: Sean Rhodes > > Sent: Friday, February 11, 2022 5:27 AM > > To: devel@edk2.groups.io > > Cc: Wu, Hao A ; Matt DeVillier > > ; Patrick Rudolph > > > > Subject: [PATCH 13/18] MdeModulePkg/Usb/Keyboard.c: don't request > > protocol before setting > > > > From: Matt DeVillier > > > > No need to check the interface protocol then conditionally setting, > > just set it to BOOT_PROTOCOL and check for error. > > > > This is what Linux does for HID devices as some don't follow the USB spec. > > One example is the Aspeed BMC HID keyboard device, which adds a massive > > boot delay without this patch as it doesn't respond to 'GetProtocolRequest'. > > > > Signed-off-by: Matt DeVillier > > Signed-off-by: Patrick Rudolph > > --- > > MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c | 22 > > +- > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c > > b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c > > index 5a94a4dda7..56d249f937 100644 > > --- a/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c > > +++ b/MdeModulePkg/Bus/Usb/UsbKbDxe/KeyBoard.c > > @@ -863,12 +863,24 @@ InitUSBKeyboard ( > >// Set boot protocol for the USB Keyboard. > > > >// This driver only supports boot protocol. > > > >// > > > > - if (Protocol != BOOT_PROTOCOL) { > > > > -UsbSetProtocolRequest ( > > > > - UsbKeyboardDevice->UsbIo, > > > > - UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber, > > > > - BOOT_PROTOCOL > > > > + Status = UsbSetProtocolRequest ( > > > > + UsbKeyboardDevice->UsbIo, > > > > + UsbKeyboardDevice->InterfaceDescriptor.InterfaceNumber, > > > > + BOOT_PROTOCOL > > > > + ); > > > > + if (EFI_ERROR (Status)) { > > > > +// > > > > +// If protocol could not be set here, it means > > > > +// the keyboard interface has some errors and could > > > > +// not be initialized > > > > +// > > > > +REPORT_STATUS_CODE_WITH_DEVICE_PATH ( > > > > + EFI_ERROR_CODE | EFI_ERROR_MINOR, > > > > + (EFI_PERIPHERAL_KEYBOARD | EFI_P_EC_INTERFACE_ERROR), > > > > + UsbKeyboardDevice->DevicePath > > > >); > > > > + > > > > +return EFI_DEVICE_ERROR; > > > >} > > > Sorry for a question, I do not understand why the proposed change will > eliminate the boot delay. > For device that does not respond to GetProtocolRequest(), the origin flow is > like: > a.1 Timeout occurs for GetProtocolRequest() > a.2 Conditionally execute UsbSetProtocolRequest() > > The proposed new flow is like: > b.1 Timeout occurs for GetProtocolRequest() > b.2 Always execute UsbSetProtocolRequest() > > Could you help to elaborate on the change? Thanks in advance. > > Best Regards
[edk2-devel] [PATCH] MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow partial free
Add support for partial free of non cached buffers. If a request for less than the full size is requested new allocations for the remaining head and tail of the buffer are added to the list. The XHCI driver does this if the page size for the controller is >4KB. Signed-off-by: Jeff Brasen --- .../NonDiscoverablePciDeviceIo.c | 48 ++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c index c1c5c6267c..858d953acf 100644 --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c @@ -960,12 +960,18 @@ NonCoherentPciIoFreeBuffer ( LIST_ENTRY *Entry; EFI_STATUS Status; NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *Alloc; + NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *AllocHead; + NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *AllocTail; BOOLEAN Found; + UINTNStartPages; + UINTNEndPages; Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO (This); Found = FALSE; Alloc = NULL; + AllocHead = NULL; + AllocTail = NULL; // // Find the uncached allocation list entry associated @@ -976,9 +982,13 @@ NonCoherentPciIoFreeBuffer ( Entry = Entry->ForwardLink) { Alloc = BASE_CR (Entry, NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION, List); -if ((Alloc->HostAddress == HostAddress) && (Alloc->NumPages == Pages)) { +StartPages = 0; +if (Alloc->HostAddress < HostAddress) { + StartPages = (HostAddress - Alloc->HostAddress) / EFI_PAGE_SIZE; +} +if ((Alloc->HostAddress <= HostAddress) && (Alloc->NumPages >= (Pages + StartPages))) { // - // We are freeing the exact allocation we were given + // We are freeing at least part of what we were given // before by AllocateBuffer() // Found = TRUE; @@ -991,7 +1001,41 @@ NonCoherentPciIoFreeBuffer ( return EFI_NOT_FOUND; } + EndPages = Alloc->NumPages - (Pages + StartPages); + + if (StartPages != 0) { +AllocHead = AllocatePool (sizeof *AllocHead); +if (AllocHead == NULL) { + return EFI_OUT_OF_RESOURCES; +} + +AllocHead->HostAddress = Alloc->HostAddress; +AllocHead->NumPages = StartPages; +AllocHead->Attributes = Alloc->Attributes; + } + + if (EndPages != 0) { +AllocTail = AllocatePool (sizeof *AllocTail); +if (AllocTail == NULL) { + return EFI_OUT_OF_RESOURCES; +} + +AllocTail->HostAddress = Alloc->HostAddress + ((Pages + StartPages) * EFI_PAGE_SIZE); +AllocTail->NumPages = EndPages; +AllocTail->Attributes = Alloc->Attributes; + } + RemoveEntryList (&Alloc->List); + // + // Record this new sub allocations in the linked list, so we + // can restore the memory space attributes later + // + if (AllocHead != NULL) { +InsertHeadList (&Dev->UncachedAllocationList, &AllocHead->List); + } + if (AllocTail != NULL) { +InsertHeadList (&Dev->UncachedAllocationList, &AllocTail->List); + } Status = gDS->SetMemorySpaceAttributes ( (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress, -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86624): https://edk2.groups.io/g/devel/message/86624 Mute This Topic: https://groups.io/mt/89079887/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] Regression found with latest edk2/OVMF SECUREBOOT/SMM build
Hello, my apologies if this has already been discovered/discussed/addressed, but we are seeing a reproducible exception with latest (c9b7c6e0cc7da76b74bcdd8c90cef956d5ae971c) OVMF x86_64 build with SECURE_BOOT/SMM enabled: Exception: X64 Exception Type - 06(#UD - Invalid Opcode) CPU Apic ID - RIP - 000E, CS - 0038, RFLAGS - 00010046 RAX - , RCX - 7FED2920, RDX - RBX - 7DB93D98, RSP - 7FF26718, RBP - 7FFE1630 RSI - 7E9EE018, RDI - R8 - 7FED3230, R9 - 0210, R10 - 002D R11 - 7FF26482, R12 - 7EAC2201, R13 - 7FFFD2B0 R14 - 7FF26A88, R15 - 2000 DS - 0020, ES - 0020, FS - 0020 GS - 0020, SS - 0020 CR0 - 80010033, CR2 - , CR3 - 7FF16000 CR4 - 0668, CR8 - DR0 - , DR1 - , DR2 - DR3 - , DR6 - 0FF0, DR7 - 0400 GDTR - 7FED9000 004F, LDTR - IDTR - 7FF2 01FF, TR - 0040 FXSAVE_STATE - 7FF26370 Can't find image information. OVMF Debug Log tail: Stacks - 0x7FF21000 mSmmStackSize- 0x6000 PcdCpuSmmStackGuard - 0x1 mXdSupported - 0x1 One Semaphore Size= 0x40 Total Semaphores Size = 0x1140 PhysicalAddressBits = 40, 5LPageTable = 0. 5LevelPaging Needed - 0 1GPageTable Support - 0 PcdCpuSmmRestrictedMemoryAccess - 1 PhysicalAddressBits - 36 Initialize IDT IST field for SMM Stack Guard InstallProtocolInterface: 26EEB3DE-B689-492E-80F0-BE8BD7DA4BA7 7FFC6100 SMM IPL registered SMM Entry Point address 7FFE5274 SmmInstallProtocolInterface: EB346B97-975F-4A9F-8B22-F8E92BB3D569 7FFC6040 SmmInstallProtocolInterface: 69B792EA-39CE-402D-A2A6-F721DE351DFE 7FFC6020 CpuSmm: SpinLock Size = 0x40, PcdCpuSmmMpTokenCountPerChunk = 0x40 SmmInstallProtocolInterface: 5D5450D7-990C-4180-A803-8E63F0608307 7FFC6200 SmmInstallProtocolInterface: 1D202CAB-C8AB-4D5C-94F7-3CFCC0D3D335 7FFC6140 SMM CPU Module exit from SMRAM with EFI_SUCCESS SMM IPL closed SMRAM window InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7DB93E18 SmmInstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7FFE16C0 Loading SMM driver at 0x0007FECA000 EntryPoint=0x0007FECFE6C FvbServicesSmm.efi Failure bisected to this commit: commit ade62c18f4742301bbef474ac10518bde5972fba Author: Brijesh Singh via groups.io Date: Thu Dec 9 11:27:42 2021 +0800 OvmfPkg/MemEncryptSevLib: add support to validate system RAM BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275 Our build procedure: git clone g...@linux-git.oraclecorp.com:QEMU/edk2.git edk2 cd edk2 git submodule update --init source ./edksetup.sh make -C BaseTools build -t GCC48 -D HTTP_BOOT_ENABLE -D FD_SIZE_4MB -D SECURE_BOOT_ENABLE -D SMM_REQUIRE -D TPM2_ENABLE -a X64 -p OvmfPkg/OvmfPkgX64.dsc QEMU command: /usr/bin/qemu-system-x86_64 -name guest=Guest8 -m 8192 -smp 8,maxcpus=16 -machine q35,accel=kvm -drive file=/Src/EDK2/edk2-latest1/Build/OvmfX64/DEBUG_GCC48/FV/OVMF_CODE.fd,index=0,if=pflash,format=raw,readonly -drive file=/Src/EDK2/edk2-latest1/Build/OvmfX64/DEBUG_GCC48/FV/OVMF_VARS.fd,index=1,if=pflash,format=raw -drive file=/root/Test/Disks/Guest8.img,format=raw,if=virtio -debugcon file:ovmf_debug.log -global isa-debugcon.iobase=0x402 -monitor stdio -nodefaults -global ICH9-LPC.disable_s3=1 -serial telnet:127.0.0.1:4556,server -vnc 0.0.0.0:1 -Aaron Young aaron.yo...@oracle.com -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86623): https://edk2.groups.io/g/devel/message/86623 Mute This Topic: https://groups.io/mt/89078047/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 2/2] MdeModulePkg/FaultTolerantWrite: Don't check for block alignment
Don't check for block alignment as it will fail for devices that are memory mapped. Cc: Hao A Wu Cc: Liming Gao Signed-off-by: Sean Rhodes --- .../Universal/FaultTolerantWriteDxe/FtwMisc.c | 30 +-- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c index 661e148767..b34b8f3561 100644 --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c @@ -1119,19 +1119,7 @@ FindFvbForFtw ( FtwDevice->WorkBlockSize = BlockSize; FtwDevice->FtwWorkSpaceBase = (UINTN)(FtwDevice->WorkSpaceAddress - (FvbBaseAddress + FtwDevice->WorkBlockSize * (LbaIndex - 1))); FtwDevice->NumberOfWorkSpaceBlock = FTW_BLOCKS (FtwDevice->FtwWorkSpaceBase + FtwDevice->FtwWorkSpaceSize, FtwDevice->WorkBlockSize); - if (FtwDevice->FtwWorkSpaceSize >= FtwDevice->WorkBlockSize) { -// -// Check the alignment of work space address and length, they should be block size aligned when work space size is larger than one block size. -// -if (((FtwDevice->WorkSpaceAddress & (FtwDevice->WorkBlockSize - 1)) != 0) || -((FtwDevice->WorkSpaceLength & (FtwDevice->WorkBlockSize - 1)) != 0)) -{ - DEBUG ((DEBUG_ERROR, "Ftw: Work space address or length is not block size aligned when work space size is larger than one block size\n")); - FreePool (HandleBuffer); - ASSERT (FALSE); - return EFI_ABORTED; -} - } else if ((FtwDevice->FtwWorkSpaceBase + FtwDevice->FtwWorkSpaceSize) > FtwDevice->WorkBlockSize) { + if ((FtwDevice->FtwWorkSpaceBase + FtwDevice->FtwWorkSpaceSize) > FtwDevice->WorkBlockSize) { DEBUG ((DEBUG_ERROR, "Ftw: The work space range should not span blocks when work space size is less than one block size\n")); FreePool (HandleBuffer); ASSERT (FALSE); @@ -1170,22 +1158,6 @@ FindFvbForFtw ( return EFI_ABORTED; } - // - // Check the alignment of spare area address and length, they should be block size aligned - // - if (((FtwDevice->SpareAreaAddress & (FtwDevice->SpareBlockSize - 1)) != 0) || - ((FtwDevice->SpareAreaLength & (FtwDevice->SpareBlockSize - 1)) != 0)) - { -DEBUG ((DEBUG_ERROR, "Ftw: Spare area address or length is not block size aligned\n")); -FreePool (HandleBuffer); -// -// Report Status Code EFI_SW_EC_ABORTED. -// -REPORT_STATUS_CODE ((EFI_ERROR_CODE | EFI_ERROR_UNRECOVERED), (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_EC_ABORTED)); -ASSERT (FALSE); -CpuDeadLoop (); - } - break; } } -- 2.32.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86622): https://edk2.groups.io/g/devel/message/86622 Mute This Topic: https://groups.io/mt/89077267/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/2] UefiPayloadPkg: Add support for Firmware Volume Block Protocol
From: Patrick Rudolph This adds support for FVB in order to support a platform independent and non-volatile variable store on UefiPayloadPkg. It is required for non-volatile variable support, TPM support, Secureboot support and more. Since commit bc744f5893fc4d53275ed26dd8d968011c6a09c1 coreboot supports the SMMSTORE v2 feature. It implements a SMI handler that is able to write, read and erase pages in the boot media (SPI flash). The communication is done using a fixed communication buffer that is allocated in CBMEM. The existence of this optional feature is advertised by a coreboot table. When the SMMSTORE feature is not available the variable emulation is used by setting PcdEmuVariableNvModeEnable to TRUE. Add a library for SMMStore to be used in DXE. The DXE component provides runtime services and takes care of virtual to physical mapping the communication buffers between SMM and OS. Make use of the APRIORI DXE to initialize an empty store on the first boot and set the PCDs to sane values before the variable driver is loaded. Tests on Intel(R) Xeon(R) E-2288G CPU @ 3.70G showed that the SMI isn't triggered with a probability of 1:40 of all cases when called in a tight loop. The CPU continues running and the SMI is triggeres asynchronously a few clock cycles later. coreboot only handels synchronous APM request and does nothing on asynchronous APM triggers. As there's no livesign from SMM it's impossible to tell if the handler has run. Just wait a bit and try again to trigger a synchronous SMI. Tests confirmed that out of 5 million tries the SMI is now always handled. Tested on Linux and Windows 10 on real hardware. Currently this cannot be tested on coreboot and qemu as it doesn't support the SMMSTORE on qemu. Signed-off-by: Patrick Rudolph --- UefiPayloadPkg/BlSMMStoreDxe/BlSMMStoreDxe.c | 352 UefiPayloadPkg/BlSMMStoreDxe/BlSMMStoreDxe.h | 118 +++ .../BlSMMStoreDxe/BlSMMStoreDxe.inf | 62 ++ .../BlSMMStoreDxe/BlSMMStoreFvbDxe.c | 834 ++ UefiPayloadPkg/Include/Coreboot.h | 13 + .../Include/Guid/SMMSTOREInfoGuid.h | 27 + UefiPayloadPkg/Include/Library/BlParseLib.h | 16 + UefiPayloadPkg/Include/Library/SMMStoreLib.h | 98 ++ .../Library/CbParseLib/CbParseLib.c | 40 + .../Library/CbSMMStoreLib/CbSMMStoreLib.inf | 28 + .../Library/CbSMMStoreLib/CorebootSMMStore.c | 317 +++ .../Library/SblParseLib/SblParseLib.c | 15 + .../Library/SblSMMStoreLib/SblSMMStore.c | 100 +++ .../Library/SblSMMStoreLib/SblSMMStoreLib.inf | 28 + .../UefiPayloadEntry/UefiPayloadEntry.c | 13 + .../UefiPayloadEntry/UefiPayloadEntry.h | 1 + .../UefiPayloadEntry/UefiPayloadEntry.inf | 2 + UefiPayloadPkg/UefiPayloadPkg.dec | 9 + UefiPayloadPkg/UefiPayloadPkg.dsc | 31 +- 19 files changed, 2095 insertions(+), 9 deletions(-) create mode 100644 UefiPayloadPkg/BlSMMStoreDxe/BlSMMStoreDxe.c create mode 100644 UefiPayloadPkg/BlSMMStoreDxe/BlSMMStoreDxe.h create mode 100644 UefiPayloadPkg/BlSMMStoreDxe/BlSMMStoreDxe.inf create mode 100644 UefiPayloadPkg/BlSMMStoreDxe/BlSMMStoreFvbDxe.c create mode 100644 UefiPayloadPkg/Include/Guid/SMMSTOREInfoGuid.h create mode 100644 UefiPayloadPkg/Include/Library/SMMStoreLib.h create mode 100644 UefiPayloadPkg/Library/CbSMMStoreLib/CbSMMStoreLib.inf create mode 100644 UefiPayloadPkg/Library/CbSMMStoreLib/CorebootSMMStore.c create mode 100644 UefiPayloadPkg/Library/SblSMMStoreLib/SblSMMStore.c create mode 100644 UefiPayloadPkg/Library/SblSMMStoreLib/SblSMMStoreLib.inf diff --git a/UefiPayloadPkg/BlSMMStoreDxe/BlSMMStoreDxe.c b/UefiPayloadPkg/BlSMMStoreDxe/BlSMMStoreDxe.c new file mode 100644 index 00..ca2b0c682b --- /dev/null +++ b/UefiPayloadPkg/BlSMMStoreDxe/BlSMMStoreDxe.c @@ -0,0 +1,352 @@ +/** @file BlSMMStoreDxe.c + + Copyright (c) 2020, 9elements Agency GmbH + + SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "BlSMMStoreDxe.h" + +STATIC EFI_EVENT mSMMStoreVirtualAddrChangeEvent; + +// +// Global variable declarations +// +SMMSTORE_INSTANCE *mSMMStoreInstance; + +SMMSTORE_INSTANCE mSMMStoreInstanceTemplate = { + SMMSTORE_SIGNATURE, // Signature + NULL, // Handle ... NEED TO BE FILLED + { +0, // MediaId ... NEED TO BE FILLED +FALSE, // RemovableMedia +TRUE, // MediaPresent +FALSE, // LogicalPartition +FALSE, // ReadOnly +FALSE, // WriteCaching; +0, // BlockSize ... NEED TO BE FILLED +4, // IoAlign +0, // LastBlock ... NEED TO BE FILLED +0, // LowestAlignedLba +1, // LogicalBlocksPerPhysicalBlock + }, // Media; + + { +FvbGetAttributes, // GetAttributes +FvbSetAttributes, // SetAttributes +FvbGetPhysicalAddress, // GetPhysicalAddress +FvbGetBlockSize, // GetBlockSize +FvbRe
Re: [edk2-devel] [Patch 0/3] Update cmocka submodule to 5a4b15870efa
Tested-by: Thomas Rdyman thomas.j.ryd...@intel.com -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86620): https://edk2.groups.io/g/devel/message/86620 Mute This Topic: https://groups.io/mt/89037998/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] **NOTICE** EDK II CI using Azure Pipelines Agents Failing
Hello, The EDK II CI using Azure Pipelines Agents is not working right now. First failure occurred on 2-10-2022. The issue is being worked. We will send daily status updates until the issue is resolved. We are currently in a hard freeze for the next EDK II stable tag. We will monitor status closely and adjust the release date if needed. Thank you for your patience. Mike -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86619): https://edk2.groups.io/g/devel/message/86619 Mute This Topic: https://groups.io/mt/89075001/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU Service with rendezvous support.
Good day, On 08.02.22 06:35, Li, Zhihao wrote: From: Zhihao Li REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3815 This patch extends the SMM CPU Service protocol with new interface SmmWaitForAllProcessor(), which can be used by SMI handler to optionally wait for other APs to complete SMM rendezvous in relaxed AP mode. A new library SmmCpuRendezvousLib is provided to abstract the service into library API to simple SMI handler code. Cc: Eric Dong Cc: Ray Ni Cc: Rahul Kumar Signed-off-by: Zhihao Li --- UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c | 109 UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c | 65 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 14 ++- UefiCpuPkg/PiSmmCpuDxeSmm/SyncTimer.c | 2 +- UefiCpuPkg/Include/Library/SmmCpuRendezvousLib.h | 27 + UefiCpuPkg/Include/Protocol/SmmCpuService.h| 40 +++ UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.inf | 32 ++ UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 28 + UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +- UefiCpuPkg/UefiCpuPkg.dec | 5 +- 10 files changed, 318 insertions(+), 7 deletions(-) diff --git a/UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c b/UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c new file mode 100644 index 00..3c5cd51d0c --- /dev/null +++ b/UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c @@ -0,0 +1,109 @@ +/** @file +SMM CPU Rendezvous library header file. + +Copyright (c) 2021, Intel Corporation. All rights reserved. +SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + + +#include +#include +#include +#include +#include +#include +#include +#include + +STATIC EDKII_SMM_CPU_SERVICE_PROTOCOL *mSmmCpuService = NULL; + +/** + This routine wait for all AP processors to arrive in SMM. + + @param BlockingMode Blocking mode or non-blocking mode. + + @retval TRUE All processors checked in to SMM + @retval FALSE Some processor not checked in to SMM + +**/ +EFI_STATUS +EFIAPI +SmmWaitForAllProcessor ( + IN BOOLEAN BlockingMode + ) +{ + EFI_STATUS Status; + + if (mSmmCpuService == NULL) { +return TRUE; + } + + Status = mSmmCpuService->WaitForAllProcessor ( + mSmmCpuService, + BlockingMode + ); + return EFI_ERROR(Status) ? FALSE : TRUE; Hmm, if there is a granular error code, why make it less granular by conversion? Also the prototype says EFI_STATUS, and the docs say BOOLEAN. +} + +/** + Register status code callback function only when Report Status Code protocol + is installed. + + @param Protocol Points to the protocol's unique identifier. + @param Interface Points to the interface instance. + @param Handle The handle on which the interface was installed. + + @retval EFI_SUCCESS Notification runs successfully. + +**/ +EFI_STATUS +EFIAPI +SmmCpuServiceProtocolNotify ( + IN CONST EFI_GUID*Protocol, + IN VOID *Interface, + IN EFI_HANDLEHandle + ) +{ + EFI_STATUS Status; + + Status = gSmst->SmmLocateProtocol ( +&gEdkiiSmmCpuServiceProtocolGuid, +NULL, +(VOID **) &mSmmCpuService +); + ASSERT_EFI_ERROR (Status); + + return EFI_SUCCESS; +} + +/** + The constructor function + + @param[in] ImageHandle The firmware allocated handle for the EFI image. + @param[in] SystemTable A pointer to the EFI System Table. + + @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS. + +**/ +EFI_STATUS +EFIAPI +SmmCpuRendezvousLibConstructor ( + IN EFI_HANDLEImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +{ + EFI_STATUS Status; + VOID*Registration; + + Status = gSmst->SmmLocateProtocol (&gEdkiiSmmCpuServiceProtocolGuid, NULL, (VOID **) &mSmmCpuService); + if (EFI_ERROR (Status)) { +Status = gSmst->SmmRegisterProtocolNotify ( +&gEdkiiSmmCpuServiceProtocolGuid, +SmmCpuServiceProtocolNotify, +&Registration +); +ASSERT_EFI_ERROR (Status); This might as well fail, why ASSERT? This would be a good candidate for the upcoming panic API I guess. :) + } + return EFI_SUCCESS; +} \ No newline at end of file diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c index 5d624f8e9e..34019c24ff 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c @@ -20,6 +20,19 @@ EFI_SMM_CPU_SERVICE_PROTOCOL mSmmCpuService = { SmmRegisterExceptionHandler }; +// +// EDKII SMM CPU
Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU Service with rendezvous support.
Hey all, On 10.02.22 09:18, Siyuan, Fu wrote: Hi, Mike -Original Message- From: devel@edk2.groups.io On Behalf Of Michael D Kinney Sent: 2022年2月9日 0:31 To: devel@edk2.groups.io; Li, Zhihao ; Kinney, Michael D Cc: Dong, Eric ; Ni, Ray ; Kumar, Rahul1 Subject: Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU Service with rendezvous support. Hi Zhihao, gEfiSmmCpuServiceProtocolGuid is defined in the UefiCpuPkg and is already an EDK II specific feature protocol. Adding an Edkii names version of the protocol does not make it clear that there is a relationship between the two versions of this protocol. You have added one new service to the existing protocol. The existing protocol does not have a Revision field so we do have to create a new Protocol Name/Protocol GUID. Based on previous use cases, we have a few options: 1) If Revision field is present, add to end and increase Revision value 2) If Revision field not present a) Define an _2 or _Ex version of the protocol with new service(s) added to end of structure and implement original version of the protocol on top of the _2 version of the protocol. b) Define a new Protocol with just the new services. (e.g. gEdkiiSmmCpuRedezvousProtocolGuid) We previously discussed with Ray when deciding the protocol name and choose the edk2 prefix. @Ni, Ray Any opinion on using an _Ex version protocol name or a separate protocol? I would strongly suggest to drop the "Ex" naming scheme entirely in the future for the simple reason of incrementing further. Some protocols already reached major version 3, and it would be awkward to have something like "ExExProtocol". Best regards, Marvin The patch also changes the DEC default value of gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode from 0x00 to 0x01. Changing the default value of a PCD in a DEC file is a non backwards compatible change. This should not be done. Instead, platforms that need the different sync mode should set that PCD in their DSC file. Is a new lib class really required at this time. The reason to add a new lib class is if there are multiple consumers. There are lots of consumers but no in edk2 repo, mostly inside platform code like edk2platforms. Technically the SMI handler which require all processors in SMM mode to complete its task (either due to security consideration or hardware/silicon restriction) will need to consume this library interface to complete the rendezvous in relax AP mode. I see the lib instance uses a RegisterProtocolNotify in its constructor. Is it possible to use a Depex instead and eliminate the additional complexity of a constructor and RegisterProtocolNotify? We can't use Depex since this is an optional protocol. It's not required to those platforms which only have traditional sync mode support. Thanks, Siyuan Best regards, Mike -Original Message- From: devel@edk2.groups.io On Behalf Of Li, Zhihao Sent: Monday, February 7, 2022 9:36 PM To: devel@edk2.groups.io Cc: Dong, Eric ; Ni, Ray ; Kumar, Rahul1 Subject: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Extend SMM CPU Service with rendezvous support. From: Zhihao Li REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3815 This patch extends the SMM CPU Service protocol with new interface SmmWaitForAllProcessor(), which can be used by SMI handler to optionally wait for other APs to complete SMM rendezvous in relaxed AP mode. A new library SmmCpuRendezvousLib is provided to abstract the service into library API to simple SMI handler code. Cc: Eric Dong Cc: Ray Ni Cc: Rahul Kumar Signed-off-by: Zhihao Li --- UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c | 109 UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c | 65 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 14 ++- UefiCpuPkg/PiSmmCpuDxeSmm/SyncTimer.c | 2 +- UefiCpuPkg/Include/Library/SmmCpuRendezvousLib.h | 27 + UefiCpuPkg/Include/Protocol/SmmCpuService.h| 40 +++ UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.inf | 32 ++ UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 28 + UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 3 +- UefiCpuPkg/UefiCpuPkg.dec | 5 +- 10 files changed, 318 insertions(+), 7 deletions(-) diff --git a/UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c b/UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c new file mode 100644 index 00..3c5cd51d0c --- /dev/null +++ b/UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.c @@ -0,0 +1,109 @@ +/** @file +SMM CPU Rendezvous library header file. + +Copyright (c) 2021, Intel Corporation. All rights reserved. +SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + + +#include +#include +#include +#include +#i
[edk2-devel] [PATCH v1] CryptoPkg: Add new hash algorithm ParallelHash256HashAll in BaseCryptLib.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3596 Parallel hash function ParallelHash256HashAll, as defined in NIST's Special Publication 800-185, published December 2016. It utilizes multi-process to calculate the digest. Cc: Jiewen Yao Cc: Jian J Wang Cc: Xiaoyu Lu Cc: Guomin Jiang Cc: Siyuan Fu Signed-off-by: Zhihao Li --- CryptoPkg/Library/BaseCryptLib/Hash/CryptCShake256.c | 313 CryptoPkg/Library/BaseCryptLib/Hash/CryptParallelHash.c| 275 + CryptoPkg/Library/BaseCryptLib/Hash/CryptSha3.c| 102 +++ CryptoPkg/Library/BaseCryptLib/Hash/CryptXkcp.c| 53 CryptoPkg/Test/UnitTest/Library/BaseCryptLib/ParallelhashTests.c | 152 ++ CryptoPkg/CryptoPkg.dec| 9 +- CryptoPkg/Include/Library/BaseCryptLib.h | 29 +- CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 12 +- CryptoPkg/Library/Include/CrtLibSupport.h | 5 +- CryptoPkg/Library/Include/sha3.h | 32 ++ CryptoPkg/Library/Include/xkcp.h | 23 ++ CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h| 3 +- CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf | 7 + CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf | 6 + 14 files changed, 1016 insertions(+), 5 deletions(-) diff --git a/CryptoPkg/Library/BaseCryptLib/Hash/CryptCShake256.c b/CryptoPkg/Library/BaseCryptLib/Hash/CryptCShake256.c new file mode 100644 index 00..5efced3f46 --- /dev/null +++ b/CryptoPkg/Library/BaseCryptLib/Hash/CryptCShake256.c @@ -0,0 +1,313 @@ +/** @file + cSHAKE-256 Digest Wrapper Implementations. + +Copyright (c) 2022, Intel Corporation. All rights reserved. +SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include "InternalCryptLib.h" +#include "sha3.h" +#include "xkcp.h" + +#define CSHAKE256_SECURITY_STRENGTH256 +#define CSHAKE256_RATE_IN_BYTES136 + +const CHAR8 mZeroPadding[CSHAKE256_RATE_IN_BYTES] = {0}; + +UINTN +EFIAPI +LeftEncode ( + OUT UINT8 *Encbuf, + IN UINTN Value + ) +{ + return left_encode (Encbuf, Value); +} + +UINTN +EFIAPI +RightEncode ( + OUT UINT8 *Encbuf, + IN UINTN Value + ) +{ + return right_encode (Encbuf, Value); +} + +/** + Retrieves the size, in bytes, of the context buffer required for cSHAKE-256 hash operations. + + @return The size, in bytes, of the context buffer required for cSHAKE-256 hash operations. + +**/ +UINTN +EFIAPI +CShake256GetContextSize ( + VOID + ) +{ + return (UINTN) (sizeof (KECCAK1600_CTX)); +} + +/** + Initializes user-supplied memory pointed by CShake256Context as cSHAKE-256 hash context for + subsequent use. + + @param[out] CShake256Context Pointer to cSHAKE-256 context being initialized. + @param[in] OutputLen The desired number of output length in bytes. + @param[in] Name Pointer to the function name string. + @param[in] NameLenThe length of the function name in bytes. + @param[in] Customization Pointer to the customization string. + @param[in] CustomizationLen The length of the customization string in bytes. + + @retval TRUE cSHAKE-256 context initialization succeeded. + @retval FALSE cSHAKE-256 context initialization failed. + @retval FALSE This interface is not supported. + +**/ +BOOLEAN +EFIAPI +CShake256Init ( + OUT VOID *CShake256Context, + INUINTN OutputLen, + INCONST VOID*Name, + INUINTN NameLen, + INCONST VOID*Customization, + INUINTN CustomizationLen + ) +{ + BOOLEAN Status; + unsigned char EncBuf[sizeof(size_t)+1]; + UINTN EncLen; + UINTN AbsorbLen; + UINTN PadLen; + + // + // Check input parameters. + // + if (CShake256Context == NULL || + OutputLen == 0 || + (NameLen != 0 && Name == NULL) || + (CustomizationLen != 0 && Customization == NULL)) { +return FALSE; + } + + // + // Initialize KECCAK context with pad value and block size. + // + if (NameLen == 0 && CustomizationLen == 0) { +// +// When N and S are both empty strings, cSHAKE(X, L, N, S) is equivalent to +// SHAKE as defined in FIPS 202. +// +return (BOOLEAN) init ( + (KECCAK1600_CTX *) CShake256Context, + '\x1f', + (KECCAK1600_WIDTH - CSHAKE256_SECURITY_STRENGTH * 2) / 8, + OutputLen + ); + } + + Status = (BOOLEAN) init ( + (KECCAK1600_CTX *) CShake256Context, + '\x04', + (KECC
[edk2-devel] [edk2-platforms: PATCH v3] MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
From: "Chiu, Chasel" REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3829 Fixed the bug that existing variable will not be locked when it is identical with hob data by creating LockLargeVariable function, also switched to VariablePolicyProtocol for locking variables. Failing to lock variable could be security vulnerability, so the function will return EFI_ABORTED when it failed and SaveMemoryConfig driver will halt the system for developers to resolve this issue. This patch also modified SaveMemoryConfig driver to be unloaded after execution because it does not produce any service protocol. To achieve this goal the DxeRuntimeVariableWriteLib should close registered ExitBootService events in its DESTRUCTOR. Cc: Nate DeSimone Cc: Liming Gao Cc: Eric Dong Signed-off-by: Chasel Chiu --- V3: Updated LargeVariableWriteLib to return EFI_ABORTED when locking variables failed. Also SaveMemoryConfig driver will halt the system in this case for developers to fix such security vulnerability issue. Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c | 27 --- Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c | 115 ++- Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c | 61 + Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf | 3 ++- Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h | 25 +++-- Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf | 8 +--- 6 files changed, 209 insertions(+), 30 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c index 820585f676..54e11e20bd 100644 --- a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c @@ -2,13 +2,14 @@ This is the driver that locates the MemoryConfigurationData HOB, if it exists, and saves the data to nvRAM. -Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved. +Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ #include #include +#include #include #include #include @@ -18,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include #include #include +#include #include /** @@ -86,6 +88,18 @@ SaveMemoryConfigEntryPoint ( Status = GetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, &BufferSize, VariableData); if (!EFI_ERROR (Status) && (BufferSize == DataSize) && (0 == CompareMem (HobData, VariableData, DataSize))) { DataIsIdentical = TRUE; + // + // No need to update Variable, only lock it. + // + Status = LockLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid); + if (EFI_ERROR(Status)) { +// +// Fail to lock variable is security vulnerability and should not happen. +// +DEBUG ((DEBUG_ERROR, "LockVariable is requested but failed unexpectedly!\n")); +ASSERT_EFI_ERROR (Status); +CpuDeadLoop(); + } } FreePool (VariableData); } @@ -96,6 +110,13 @@ SaveMemoryConfigEntryPoint ( if (!DataIsIdentical) { Status = SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, TRUE, DataSize, HobData); ASSERT_EFI_ERROR (Status); +if (Status == EFI_ABORTED) { + // + // Fail to lock variable is security vulnerability and should not happen. + // + DEBUG ((DEBUG_ERROR, "LockVariable is requested but failed unexpectedly!\n")); + CpuDeadLoop(); +} DEBUG ((DEBUG_INFO, "Saved size of FSP / MRC Training Data: 0x%x\n", DataSize)); } else { DEBUG ((DEBUG_INFO, "FSP / MRC Training Data is identical to data from last boot, no need to save.\n")); @@ -106,7 +127,7 @@ SaveMemoryConfigEntryPoint ( } // - // This driver cannot be unloaded because DxeRuntimeVariableWriteLib constructor will register ExitBootServices callback. + // This driver does not produce any protocol services, so always unload it. // - return EFI_SUCCESS; + return EFI_REQUEST_UNLOAD_IMAGE; } diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/L
[edk2-devel] [PATCH 2/2] UefiPayloadPkg: Hookup SD Card timeout
Hook SD_CARD_TIMEOUT build option to SdMmcGenericTimeoutValue PCD. Cc: Guo Dong Cc: Ray Ni Cc: Maurice Ma Cc: Benjamin You Signed-off-by: Sean Rhodes --- UefiPayloadPkg/UefiPayloadPkg.dsc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc index 1ce96a51c1..d75fe26426 100644 --- a/UefiPayloadPkg/UefiPayloadPkg.dsc +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc @@ -33,6 +33,7 @@ DEFINE UNIVERSAL_PAYLOAD= FALSE DEFINE SECURITY_STUB_ENABLE = TRUE DEFINE SMM_SUPPORT = FALSE + DEFINE SD_CARD_TIMEOUT = 100 # # SBL: UEFI payload for Slim Bootloader # COREBOOT: UEFI payload for coreboot @@ -398,6 +399,7 @@ !if $(PERFORMANCE_MEASUREMENT_ENABLE) gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask | 0x1 !endif + gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|$(SD_CARD_TIMEOUT) [PcdsPatchableInModule.X64] gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister|$(RTC_INDEX_REGISTER) -- 2.32.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86613): https://edk2.groups.io/g/devel/message/86613 Mute This Topic: https://groups.io/mt/89066989/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/2] SdMmcPciDxe: Make timeout for SD card configurable
From: Matt DeVillier The default 1s timeout can delay boot splash on some hardware with no benefit. Cc: Hao A Wu Cc: Ray Ni Cc: Jian J Wang Cc: Liming Gao Signed-off-by: Matt DeVillier Signed-off-by: Sean Rhodes --- MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 2 +- MdeModulePkg/MdeModulePkg.dec | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h index 85e09cf114..c9a21e01bd 100644 --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h @@ -49,7 +49,7 @@ extern EDKII_SD_MMC_OVERRIDE *mOverride; // // Generic time out value, 1 microsecond as unit. // -#define SD_MMC_HC_GENERIC_TIMEOUT 1 * 1000 * 1000 +#define SD_MMC_HC_GENERIC_TIMEOUT ((PcdGet32 (PcdSdMmcGenericTimeoutValue) // // SD/MMC async transfer timer interval, set by experience. diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 463e889e9a..092660f7f0 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1559,6 +1559,9 @@ # @Prompt Maximum permitted FwVol section nesting depth (exclusive). gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth|0x10|UINT32|0x0030 + ## SD Card timeout + gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|100|UINT32|0x0031 + [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] ## This PCD defines the Console output row. The default value is 25 according to UEFI spec. # This PCD could be set to 0 then console output would be at max column and max row. -- 2.32.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86612): https://edk2.groups.io/g/devel/message/86612 Mute This Topic: https://groups.io/mt/89066986/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] Ps2KbdCtrller: Make wait for SUCCESS after BAT non-fatal
From: Matt DeVillier Recent model Chromebooks only return ACK, but not BAT_SUCCESS, which causes hanging and failed ps2k init. To mitigate this, make the absence of BAT_SUCCESS reply non-fatal, and reduce the no-reply timeout from 4s to 1s. Tested on google/dracia and purism/librem_14 Cc: Hao A Wu Cc: Ray Ni Signed-off-by: Matt DeVillier Signed-off-by: Sean Rhodes --- MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c | 6 +- MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c index 77dc226222..6c71355edd 100644 --- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c +++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c @@ -1733,11 +1733,7 @@ InitKeyboard ( // mWaitForValueTimeOut = KEYBOARD_BAT_TIMEOUT; -Status = KeyboardWaitForValue (ConsoleIn, KEYBOARD_8048_RETURN_8042_BAT_SUCCESS); -if (EFI_ERROR (Status)) { - KeyboardError (ConsoleIn, L"Keyboard self test failed!\n\r"); - goto Done; -} +KeyboardWaitForValue (ConsoleIn, KEYBOARD_8048_RETURN_8042_BAT_SUCCESS); mWaitForValueTimeOut = KEYBOARD_WAITFORVALUE_TIMEOUT; diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h index ca1dd9b2c2..38df3e092d 100644 --- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h +++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h @@ -157,7 +157,7 @@ InstallPs2KeyboardDriver ( #define KEYBOARD_MAX_TRY 256 // 256 #define KEYBOARD_TIMEOUT 65536 // 0.07s #define KEYBOARD_WAITFORVALUE_TIMEOUT100 // 1s -#define KEYBOARD_BAT_TIMEOUT 400 // 4s +#define KEYBOARD_BAT_TIMEOUT 100 // 1s #define KEYBOARD_TIMER_INTERVAL 20 // 0.02s #define SCANCODE_EXTENDED0 0xE0 #define SCANCODE_EXTENDED1 0xE1 -- 2.32.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86611): https://edk2.groups.io/g/devel/message/86611 Mute This Topic: https://groups.io/mt/89066601/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-