|
Re: [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()
Christoph Willing
On 10/8/21 4:01 pm, Gerd Hoffmann wrote:
Hi,Thanks for your interest Gerd,insmod /lib/modules/5.13.8/kernel/fs/jbd2/jbd2.ko I added "-serial mon:stdio" to the qemu invocation and "console=ttyS0" to the -append line to capture all boot output for both good and bad cases and saved the results in files. Rather than spam the list with those outputs, I've added them as attachments to the bz report at https://bugzilla.tianocore.org/show_bug.cgi?id=3504 When I first diff'd them, the slight difference in time stamps meant every line showed up as different - real differences were hard to see. Therefore I amended the files to replace the time stamps with "xxxx", enabling diff to show actual differences more meaningfully. I also attached there the output of a diff between the bad & good cases. To be honest I can't see anything striking to explain the difference in behaviour. To me, the main difference is that in the good case, an QEMU HARDDISK & QEMU DVD-ROM are discovered and allows the system to boot but no obvious indication why they are discovered in one case but not the other. Apart from that, there are a few differences in some memory locations being used but I have insufficient knowledge to judge whether those differences are significant. Is it possible for anyone to have a quick look through those files for anything that may indicate what is causing the defective booting please? Thanks, chris
|
|
|
|
Re: [PATCH v6 6/6] OvmfPkg/AmdSevDxe: Add support for SEV live migration.
Ashish Kalra
Hello Tom,
On Mon, Aug 09, 2021 at 09:29:29AM -0500, Tom Lendacky wrote: On 8/2/21 7:33 AM, Ashish Kalra wrote:I don't think we should do an assert here and abort booting, failureFrom: Ashish Kalra <ashish.kalra@...>DEBUG_ERROR? here will simply disable live migration support but i don't think that it is such a fatal error that we should abort booting because of that. OTOH, if there is a failure when doing page encryption status hypercalls then aborting boot makes sense as guest page encryption status tracking may be critical for multiple guest operations and not only live migration. Thanks, Ashish
|
|
|
|
Re: [PATCH v2 1/2] MdePkg/BaseLib: Fix unaligned API prototypes
Marvin Häuser <mhaeuser@...>
On 09/08/2021 23:32, Andrew Fish via groups.io wrote:
They do contain UB, inherently from the combination of their prototype and their usage.On Aug 9, 2021, at 9:15 AM, Michael D Kinney <michael.d.kinney@... <mailto:michael.d.kinney@...>> wrote:MIke, Please refer to the new BZ added for V2: https://bugzilla.tianocore.org/show_bug.cgi?id=3542 I could only speculate why UBsan does not check cast safety... To be fair, I don't know a real-world platform that has issues with this UB, but the fix is simple enough in my opinion. Actually, enabling "-Wcast-align" some day would be great either way. :) Best regards, Marvin
|
|
|
|
Re: [PATCH v2 2/2] UefiCpuPkg/BaseUefiCpuLib: Use toolchain-specific rodata section name
Marvin Häuser <mhaeuser@...>
On 10/08/2021 06:40, Andrew Fish via groups.io wrote:
The latter part is true only for Xcode-based toolchains, as far as I am aware now, and this is in fact the way it works right now.On Aug 9, 2021, at 7:43 PM, Ni, Ray <ray.ni@... <mailto:ray.ni@...>> wrote:Ray, You are correct, but the NASM section name semantically translates to an *ELF* .rodata section for usage during linking. The GNU linker scripts merge it into .text later [1], yielding no PE/COFF .rodata section as observed. Behaviour should be fixed for PE-based toolchains, preserved correct for ELF-based toolchains (but inconsistent with PE-based, as we merge .rodata into .text here), and as correct or broken for Xcode-based toolchains as it was before. If you can help with designing a solution, I'm more than happy to to submit a V2. Just I don't know Xcode, and I don't run macOS. :) It looks like we might have to modify GenFw if we want to create a .rodata section?For now, this patch un-breaks PE-based toolchains. They are broken in the way described in the BZ, but not in a security-critical fashion. Rather we have the worst of both worlds, the additional size of another aligned section, and the downgraded permission as we would observe with a merge into e.g. .text. I have no strong opinion on which route to pick, i.e. dedicated .r(o)data or merging into .text, but I would like it to be consistent in the end, especially across toolchains. Ideally, in my opinion, the platform maintainer can choose whether they want the extra protection (NX .rodata), or the extra space (.text merge). I can check again later, but ELF should be fine, really. Best regards, Marvin [1] https://github.com/tianocore/edk2/blob/d02dbb53cd78de799e6afaa237e98771fb5148db/BaseTools/Scripts/GccBase.lds#L25
|
|
|
|
Re: [PATCH v2 1/2] BaseTools: Define the read-only data section name per toolchain
Marvin Häuser <mhaeuser@...>
On 10/08/2021 06:19, Andrew Fish via groups.io wrote:
Yes, and ".rodata" is almost a synonym for "__DATA,__const", with a small exception [1]. Maybe it'd be clearer if the macro was renamed to "NASM_RODATA_SECTION_NAME", to indicate this is not just a "raw" name, but NASM gives it a semantic meaning?On Aug 9, 2021, at 2:51 AM, Marvin Häuser <mhaeuser@... <mailto:mhaeuser@...>> wrote:An EFI Mach-O file does not contain a .rodata section. A Mach-O contains a __DATA segment that is broken up into sections. For a typical EFI image there are __const, __data, __bss sections in the __DATA segment [1]. This is a part I missed, because I do not have an Xcode toolchain at hand, so thanks for investigating. However this, in my opinion, is a flaw with Mach-O/mtoc and not with my patch. It seems like the only difference between __TEXT,__const and __DATA,__const is whether the data is targeted by a relocation or not. Such a concept does not exist for PE/COFF (and I think not even for ELF, but I'm not too familiar with it), thus the logical PE/COFF section __DATA,__const should be merged into is .rdata (and .rdata may or may not be merged into .text in an earlier step, I assume transitivity). I could change the macro definition to explicitly declare __TEXT,__const, but that would still put the compiler-emitted data in the wrong section. Does Xcode provide anything remotely similar to GNU linker scripts which we can use to move the section? Please also note that .rodata is used for Xcode-based toolchains already (in fact, all toolchains, and this is the issue), I'm not regressing anything. I just expected it to work fine as-is. This patch mainly fixed PE/COFF-based toolchains, which get both .rdata from the compiler and .rodata with RX permissions from NASM, because ".rodata" only has a semantic meaning for ELF and Mach-O outputs, but not for PE/COFF. Well, this kind of is an issue. We would need to introduce an arbitrary constraint on the relocation part that holds only for Xcode-based toolchains. Does the compiler emit an error when data in __TEXT,__const is targeted by a relocation? Also see above regarding compiler-emitted __DATA,__const. Thanks for your notes and insight! Best regards, Marvin [1] "For compatibility with other Unix platforms, the following standard names are also supported: [...] .rodata = __DATA,__const data [...] If the .rodata section contains no relocations, it is instead put into the __TEXT,__const section unless this section has already been specified explicitly." https://www.nasm.us/xdoc/2.13.01/html/nasmdoc7.html
|
|
|
|
Re: [PATCH 0/3] BaseTools: fix some python DeprecationWarnings
Yuwei Chen
Hi Robinson,
toggle quoted messageShow quoted text
When we doing the internal test, the issue is found that py27 is blocked by this patch. You can reproduce the error with below steps: For Linux: [Error Reproduce steps] $ export PYTHON3_ENABLE=FALSE $ export PYTHON_COMMAND=/usr/bin/python2.7 $ build -p OvmfPkg/OvmfPkgIa32X64.dsc -a IA32 -a X64 -t GCC5 Active Platform = /home/jshi19/wksp_efi/edk2-3/OvmfPkg/OvmfPkgIa32X64.dsc ...... - Failed - Build end time: 13:00:59, Aug.10 2021 Build total time: 00:00:07 [After Reverting the patch] $ git revert 0b1b0a9674e27c858f05436ed92250f4498245cf $ build -p OvmfPkg/OvmfPkgIa32X64.dsc -a IA32 -a X64 -t GCC5 Pass For Windows: [Error Reproduce steps] set PYTHON3_ENABLE=FALSE set PYTHON_COMMAND=C:\Python27\python.exe build -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -t VS2015x86 Active Platform = c:\users\yuweiche\code\edk2\edk2\OvmfPkg\OvmfPkgIa32X64.dsc ...... - Failed - Build end time: 15:40:35, Aug.10 2021 Build total time: 00:00:07 [After Reverting the patch] $ git revert 0b1b0a9674e27c858f05436ed92250f4498245cf $ build -p OvmfPkg\OvmfPkgIa32X64.dsc -a IA32 -a X64 -t VS2015x86 - Done - Build end time: 15:35:16, Aug.10 2021 Build total time: 00:01:58 Hope you can fix this bug. 😊 @crobinso@... Hi Liming @Liming Gao, what do you think about this issue? If we need fix it before this quarter release? Thanks, Yuwei (Christine)
-----Original Message-----
|
|
|
|
[edk2-platforms:PATCH V6] Platform/Intel: Correct CPU APIC IDs
JackX Lin
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3365
BIOS cannot find correct AcpiProcId in mApicIdMap because of there is no suitable map, that causes ACPI_BIOS_ERROR. Remove mApicIdMap for determing AcpiProcId, uses normal countings instead. Signed-off-by: JackX Lin <JackX.Lin@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Jenny Huang <jenny.huang@...> Cc: Jiewen Yao <jiewen.yao@...> Cc: Ray Ni <ray.ni@...> Cc: Rangasai V Chaganty <rangasai.v.chaganty@...> Cc: Donald Kuo <Donald.Kuo@...> Cc: Chandana C Kumar <chandana.c.kumar@...> Cc: JackX Lin <JackX.Lin@...> --- Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 621 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 1 file changed, 254 insertions(+), 367 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c index 2b51c34ef2..174582c674 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c @@ -1,23 +1,21 @@ /** @file ACPI Platform Driver -Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/ #include "AcpiPlatform.h" -#define MAX_CPU_NUM (FixedPcdGet32(PcdMaxCpuThreadCount) * FixedPcdGet32(PcdMaxCpuCoreCount) * FixedPcdGet32(PcdMaxCpuSocketCount)) - #pragma pack(1) typedef struct { UINT32 AcpiProcessorId; UINT32 ApicId; UINT32 Flags; - UINT32 SwProcApicId; UINT32 SocketNum; + UINT32 Thread; } EFI_CPU_ID_ORDER_MAP; // @@ -50,7 +48,7 @@ VOID *mLocalTable[] = { &Wsmt, }; -EFI_ACPI_TABLE_PROTOCOL *mAcpiTable; +EFI_ACPI_TABLE_PROTOCOL *mAcpiTable; UINT32 mNumOfBitShift = 6; BOOLEAN mForceX2ApicId; @@ -58,138 +56,19 @@ BOOLEAN mX2ApicEnabled; EFI_MP_SERVICES_PROTOCOL *mMpService; BOOLEAN mCpuOrderSorted; -EFI_CPU_ID_ORDER_MAP mCpuApicIdOrderTable[MAX_CPU_NUM]; -UINTN mNumberOfCPUs = 0; +EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable = NULL; +UINTN mNumberOfCpus = 0; UINTN mNumberOfEnabledCPUs = 0; -// following are possible APICID Map for SKX -static const UINT32 ApicIdMapA[] = { //for SKUs have number of core > 16 - //it is 14 + 14 + 14 + 14 format - 0x00000000, 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005, 0x00000006, 0x00000007, - 0x00000008, 0x00000009, 0x0000000A, 0x0000000B, 0x0000000C, 0x0000000D, 0x00000010, 0x00000011, - 0x00000012, 0x00000013, 0x00000014, 0x00000015, 0x00000016, 0x00000017, 0x00000018, 0x00000019, - 0x0000001A, 0x0000001B, 0x0000001C, 0x0000001D, 0x00000020, 0x00000021, 0x00000022, 0x00000023, - 0x00000024, 0x00000025, 0x00000026, 0x00000027, 0x00000028, 0x00000029, 0x0000002A, 0x0000002B, - 0x0000002C, 0x0000002D, 0x00000030, 0x00000031, 0x00000032, 0x00000033, 0x00000034, 0x00000035, - 0x00000036, 0x00000037, 0x00000038, 0x00000039, 0x0000003A, 0x0000003B, 0x0000003C, 0x0000003D -}; - -static const UINT32 ApicIdMapB[] = { //for SKUs have number of cores <= 16 use 32 ID space - //it is 16+16 format - 0x00000000, 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005, 0x00000006, 0x00000007, - 0x00000008, 0x00000009, 0x0000000A, 0x0000000B, 0x0000000C, 0x0000000D, 0x0000000E, 0x0000000F, - 0x00000010, 0x00000011, 0x00000012, 0x00000013, 0x00000014, 0x00000015, 0x00000016, 0x00000017, - 0x00000018, 0x00000019, 0x0000001A, 0x0000001B, 0x0000001C, 0x0000001D, 0x0000001E, 0x0000001F, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF -}; - - -static const UINT32 ApicIdMapC[] = { //for SKUs have number of cores <= 16 use 64 ID space - //it is 16+0+16+0 format - 0x00000000, 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005, 0x00000006, 0x00000007, - 0x00000008, 0x00000009, 0x0000000A, 0x0000000B, 0x0000000C, 0x0000000D, 0x0000000E, 0x0000000F, - 0x00000020, 0x00000021, 0x00000022, 0x00000023, 0x00000024, 0x00000025, 0x00000026, 0x00000027, - 0x00000028, 0x00000029, 0x0000002A, 0x0000002B, 0x0000002C, 0x0000002D, 0x0000002E, 0x0000002F, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF -}; - -static const UINT32 ApicIdMapD[] = { //for SKUs have number of cores <= 8 use 16 ID space - //it is 16 format - 0x00000000, 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005, 0x00000006, 0x00000007, - 0x00000008, 0x00000009, 0x0000000A, 0x0000000B, 0x0000000C, 0x0000000D, 0x0000000E, 0x0000000F, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, - 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF -}; - -const UINT32 *mApicIdMap = NULL; - -/** - This function detect the APICID map and update ApicID Map pointer - - @param None - - @retval VOID - -**/ -VOID DetectApicIdMap(VOID) -{ - UINTN CoreCount; - - CoreCount = 0; - - if(mApicIdMap != NULL) { - return; //aleady initialized - } - - mApicIdMap = ApicIdMapA; // default to > 16C SKUs - - CoreCount = mNumberOfEnabledCPUs / 2; - DEBUG ((DEBUG_INFO, "CoreCount - %d\n", CoreCount)); - - //DEBUG((EFI_D_ERROR, ":: Default to use Map A @ %08X FusedCoreCount: %02d, sktlevel: %d\n",mApicIdMap, FusedCoreCount, mNumOfBitShift)); - // Dont assert for single core, single thread system. - //ASSERT (CoreCount != 0); - - if(CoreCount <= 16) { - - if(mNumOfBitShift == 4) { - mApicIdMap = ApicIdMapD; - //DEBUG((EFI_D_ERROR, ":: Use Map B @ %08X\n",mApicIdMap)); - } - - if(mNumOfBitShift == 5) { - mApicIdMap = ApicIdMapB; - //DEBUG((EFI_D_ERROR, ":: Use Map B @ %08X\n",mApicIdMap)); - } - - if(mNumOfBitShift == 6) { - mApicIdMap = ApicIdMapC; - //DEBUG((EFI_D_ERROR, ":: Use Map C @ %08X\n",mApicIdMap)); - } - - } - - return; -} - /** - This function return the CoreThreadId of ApicId from ACPI ApicId Map array + Find BSP in mCpuApicIdOrderTable. - @param ApicId + This function searches mCpuApicIdOrderTable to find the BSP ApicId, and returns a number where the BSP is. - @retval Index of ACPI ApicId Map array + @param[in] ApicId Apic ID. + @return Where the BSP is. **/ -UINT32 -GetIndexFromApicId ( - UINT32 ApicId - ) -{ - UINT32 CoreThreadId; - UINT32 i; - - ASSERT (mApicIdMap != NULL); - - CoreThreadId = ApicId & ((1 << mNumOfBitShift) - 1); - - for(i = 0; i < (FixedPcdGet32(PcdMaxCpuCoreCount) * FixedPcdGet32(PcdMaxCpuThreadCount)); i++) { - if(mApicIdMap[i] == CoreThreadId) { - break; - } - } - - ASSERT (i <= (FixedPcdGet32(PcdMaxCpuCoreCount) * FixedPcdGet32(PcdMaxCpuThreadCount))); - - return i; -} - UINT32 ApicId2SwProcApicId ( UINT32 ApicId @@ -197,7 +76,7 @@ ApicId2SwProcApicId ( { UINT32 Index; - for (Index = 0; Index < MAX_CPU_NUM; Index++) { + for (Index = 0; Index < mNumberOfCpus; Index++) { if ((mCpuApicIdOrderTable[Index].Flags == 1) && (mCpuApicIdOrderTable[Index].ApicId == ApicId)) { return Index; } @@ -207,21 +86,27 @@ ApicId2SwProcApicId ( } +/** + Print Cpu Apic ID Table + + @param[in] CpuApicIdOrderTable Data will be dumped. +**/ VOID -DebugDisplayReOrderTable( - VOID +DebugDisplayReOrderTable ( + IN EFI_CPU_ID_ORDER_MAP *CpuApicIdOrderTable ) { UINT32 Index; - DEBUG ((EFI_D_ERROR, "Index AcpiProcId ApicId Flags SwApicId Skt\n")); - for (Index=0; Index<MAX_CPU_NUM; Index++) { - DEBUG ((EFI_D_ERROR, " %02d 0x%02X 0x%02X %d 0x%02X %d\n", - Index, mCpuApicIdOrderTable[Index].AcpiProcessorId, - mCpuApicIdOrderTable[Index].ApicId, - mCpuApicIdOrderTable[Index].Flags, - mCpuApicIdOrderTable[Index].SwProcApicId, - mCpuApicIdOrderTable[Index].SocketNum)); + DEBUG ((DEBUG_INFO, "Index AcpiProcId ApicId Thread Flags Skt\n")); + for (Index = 0; Index < mNumberOfCpus; Index++) { + DEBUG ((DEBUG_INFO, " %02d 0x%02X 0x%02X %d %d %d\n", + Index, + CpuApicIdOrderTable[Index].AcpiProcessorId, + CpuApicIdOrderTable[Index].ApicId, + CpuApicIdOrderTable[Index].Thread, + CpuApicIdOrderTable[Index].Flags, + CpuApicIdOrderTable[Index].SocketNum)); } } @@ -281,131 +166,129 @@ SortCpuLocalApicInTable ( UINT32 Index; UINT32 CurrProcessor; UINT32 BspApicId; - UINT32 TempVal = 0; + EFI_CPU_ID_ORDER_MAP *TempVal; EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr; UINT32 CoreThreadMask; + EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable; Index = 0; Status = EFI_SUCCESS; + if (mCpuOrderSorted) { + return Status; + } + + TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP)); + TempVal = AllocateZeroPool (sizeof (EFI_CPU_ID_ORDER_MAP)); CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1); - if(!mCpuOrderSorted) { + for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++, Index++) { + Status = mMpService->GetProcessorInfo ( + mMpService, + CurrProcessor, + &ProcessorInfoBuffer + ); + + CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) &TempCpuApicIdOrderTable[Index]; + if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) { + CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId; + CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread; + CpuIdMapPtr->Flags = ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0); + CpuIdMapPtr->SocketNum = ProcessorInfoBuffer.Location.Package; + CpuIdMapPtr->AcpiProcessorId = (ProcessorInfoBuffer.Location.Package << mNumOfBitShift) + (UINT32)ProcessorInfoBuffer.ProcessorId; + + //update processorbitMask + if (CpuIdMapPtr->Flags == 1) { + if (mForceX2ApicId) { + CpuIdMapPtr->SocketNum &= 0x7; + CpuIdMapPtr->AcpiProcessorId &= 0xFF; //keep lower 8bit due to use Proc obj in dsdt + } + } + } else { //not enabled + CpuIdMapPtr->ApicId = (UINT32)-1; + CpuIdMapPtr->Thread = (UINT32)-1; + CpuIdMapPtr->Flags = 0; + CpuIdMapPtr->AcpiProcessorId = (UINT32)-1; + CpuIdMapPtr->SocketNum = (UINT32)-1; + } //end if PROC ENABLE + } //end for CurrentProcessor - Index = 0; + //keep for debug purpose + DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift)); + DebugDisplayReOrderTable (TempCpuApicIdOrderTable); - for (CurrProcessor = 0; CurrProcessor < mNumberOfCPUs; CurrProcessor++) { - Status = mMpService->GetProcessorInfo ( - mMpService, - CurrProcessor, - &ProcessorInfoBuffer - ); - if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) { - if(ProcessorInfoBuffer.ProcessorId & 1) { //is 2nd thread - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)&mCpuApicIdOrderTable[(Index - 1) + MAX_CPU_NUM / 2]; - } else { //is primary thread - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)&mCpuApicIdOrderTable[Index]; - Index++; - } - CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId; - CpuIdMapPtr->Flags = ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0); - CpuIdMapPtr->SocketNum = (UINT32)ProcessorInfoBuffer.Location.Package; - CpuIdMapPtr->AcpiProcessorId = (CpuIdMapPtr->SocketNum * FixedPcdGet32(PcdMaxCpuCoreCount) * FixedPcdGet32(PcdMaxCpuThreadCount)) + GetIndexFromApicId(CpuIdMapPtr->ApicId); //CpuIdMapPtr->ApicId; - CpuIdMapPtr->SwProcApicId = ((UINT32)(ProcessorInfoBuffer.Location.Package << mNumOfBitShift) + (((UINT32)ProcessorInfoBuffer.ProcessorId) & CoreThreadMask)); - if(mX2ApicEnabled) { //if X2Apic, re-order the socket # so it starts from base 0 and contiguous - //may not necessory!!!!! - } + // + // 1. Sort TempCpuApicIdOrderTable, Sort it by using AcpiProcessorId from minimum to maximum (Socket0 to SocketN). + // + for (CurrProcessor = 0; CurrProcessor < mNumberOfCpus; CurrProcessor++) { + for (Index = CurrProcessor+1; Index < mNumberOfCpus; Index++) { + if (TempCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId > TempCpuApicIdOrderTable[Index].AcpiProcessorId) { + CopyMem (&TempVal, &TempCpuApicIdOrderTable[CurrProcessor], sizeof (EFI_CPU_ID_ORDER_MAP)); + CopyMem (&TempCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP)); + CopyMem (&TempCpuApicIdOrderTable[Index], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP)); + } + } + } - //update processorbitMask - if (CpuIdMapPtr->Flags == 1) { + // + // 2. Sort Primary threads in the front of the CpuApicIdOrderTable + // + for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) { + if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread + CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP)); + CurrProcessor++; + } + } - if(mForceX2ApicId) { - CpuIdMapPtr->SocketNum &= 0x7; - CpuIdMapPtr->AcpiProcessorId &= 0xFF; //keep lower 8bit due to use Proc obj in dsdt - CpuIdMapPtr->SwProcApicId &= 0xFF; - } - } - } else { //not enabled - CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)&mCpuApicIdOrderTable[Index]; - CpuIdMapPtr->ApicId = (UINT32)-1; - CpuIdMapPtr->Flags = 0; - CpuIdMapPtr->AcpiProcessorId = (UINT32)-1; - CpuIdMapPtr->SwProcApicId = (UINT32)-1; - CpuIdMapPtr->SocketNum = (UINT32)-1; - } //end if PROC ENABLE - } //end for CurrentProcessor - - //keep for debug purpose - DEBUG(( EFI_D_ERROR, "::ACPI:: APIC ID Order Table Init. CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift)); - DebugDisplayReOrderTable(); - - //make sure 1st entry is BSP - if(mX2ApicEnabled) { - BspApicId = (UINT32)AsmReadMsr64(0x802); - } else { - BspApicId = (*(volatile UINT32 *)(UINTN)0xFEE00020) >> 24; + // + // 3. Sort second threads in the middle of the CpuApicIdOrderTable + // + for (Index = 0; Index < mNumberOfCpus; Index++) { + if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread + CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP)); + CurrProcessor++; } - DEBUG ((EFI_D_INFO, "BspApicId - 0x%x\n", BspApicId)); + } - if(mCpuApicIdOrderTable[0].ApicId != BspApicId) { - //check to see if 1st entry is BSP, if not swap it - Index = ApicId2SwProcApicId(BspApicId); + // + // 4. Sort not enabling threads in the bottom of the CpuApicIdOrderTable + // + for (Index = 0; Index < mNumberOfCpus; Index++) { + if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled + CopyMem (&mCpuApicIdOrderTable[CurrProcessor], &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP)); + CurrProcessor++; + } + } - if(MAX_CPU_NUM <= Index) { - DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n")); - return EFI_INVALID_PARAMETER; - } + //make sure 1st entry is BSP + if (mX2ApicEnabled) { + BspApicId = (UINT32)AsmReadMsr64 (0x802); + } else { + BspApicId = (*(volatile UINT32 *)(UINTN)0xFEE00020) >> 24; + } + DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId)); - TempVal = mCpuApicIdOrderTable[Index].ApicId; - mCpuApicIdOrderTable[Index].ApicId = mCpuApicIdOrderTable[0].ApicId; - mCpuApicIdOrderTable[0].ApicId = TempVal; - mCpuApicIdOrderTable[Index].Flags = mCpuApicIdOrderTable[0].Flags; - mCpuApicIdOrderTable[0].Flags = 1; - TempVal = mCpuApicIdOrderTable[Index].SwProcApicId; - mCpuApicIdOrderTable[Index].SwProcApicId = mCpuApicIdOrderTable[0].SwProcApicId; - mCpuApicIdOrderTable[0].SwProcApicId = TempVal; - //swap AcpiProcId - TempVal = mCpuApicIdOrderTable[Index].AcpiProcessorId; - mCpuApicIdOrderTable[Index].AcpiProcessorId = mCpuApicIdOrderTable[0].AcpiProcessorId; - mCpuApicIdOrderTable[0].AcpiProcessorId = TempVal; + if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) { + //check to see if 1st entry is BSP, if not swap it + Index = ApicId2SwProcApicId (BspApicId); + if (mNumberOfCpus <= Index) { + DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n")); + return EFI_INVALID_PARAMETER; } - //Make sure no holes between enabled threads - for(CurrProcessor = 0; CurrProcessor < MAX_CPU_NUM; CurrProcessor++) { - - if(mCpuApicIdOrderTable[CurrProcessor].Flags == 0) { - //make sure disabled entry has ProcId set to FFs - mCpuApicIdOrderTable[CurrProcessor].ApicId = (UINT32)-1; - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = (UINT32)-1; - mCpuApicIdOrderTable[CurrProcessor].SwProcApicId = (UINT32)-1; - - for(Index = CurrProcessor+1; Index < MAX_CPU_NUM; Index++) { - if(mCpuApicIdOrderTable[Index].Flags == 1) { - //move enabled entry up - mCpuApicIdOrderTable[CurrProcessor].Flags = 1; - mCpuApicIdOrderTable[CurrProcessor].ApicId = mCpuApicIdOrderTable[Index].ApicId; - mCpuApicIdOrderTable[CurrProcessor].AcpiProcessorId = mCpuApicIdOrderTable[Index].AcpiProcessorId; - mCpuApicIdOrderTable[CurrProcessor].SwProcApicId = mCpuApicIdOrderTable[Index].SwProcApicId; - mCpuApicIdOrderTable[CurrProcessor].SocketNum = mCpuApicIdOrderTable[Index].SocketNum; - //disable moved entry - mCpuApicIdOrderTable[Index].Flags = 0; - mCpuApicIdOrderTable[Index].ApicId = (UINT32)-1; - mCpuApicIdOrderTable[Index].AcpiProcessorId = (UINT32)-1; - mCpuApicIdOrderTable[Index].SwProcApicId = (UINT32)-1; - break; - } - } - } - } + CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP)); + CopyMem (&TempCpuApicIdOrderTable[Index], &TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP)); + CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof (EFI_CPU_ID_ORDER_MAP)); + TempCpuApicIdOrderTable[0].Flags = 1; + } - //keep for debug purpose - DEBUG ((EFI_D_ERROR, "APIC ID Order Table ReOrdered\n")); - DebugDisplayReOrderTable(); + //keep for debug purpose + DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n")); + DebugDisplayReOrderTable (mCpuApicIdOrderTable); - mCpuOrderSorted = TRUE; - } + mCpuOrderSorted = TRUE; return Status; } @@ -602,11 +485,11 @@ InitializeMadtHeader ( } Status = InitializeHeader ( - &MadtHeader->Header, - EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, - EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION, - 0 - ); + &MadtHeader->Header, + EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, + EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION, + 0 + ); if (EFI_ERROR (Status)) { return Status; } @@ -784,11 +667,11 @@ BuildAcpiTable ( // Allocate the memory needed for the table. // Status = AllocateTable ( - TableSpecificHdrLength, - Structures, - StructureCount, - &InternalTable - ); + TableSpecificHdrLength, + Structures, + StructureCount, + &InternalTable + ); if (EFI_ERROR (Status)) { return Status; } @@ -871,18 +754,22 @@ InstallMadtFromScratch ( NewMadtTable = NULL; MaxMadtStructCount = 0; - DetectApicIdMap(); + mCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof (EFI_CPU_ID_ORDER_MAP)); + if (mCpuApicIdOrderTable == NULL) { + DEBUG ((DEBUG_ERROR, "Could not allocate mCpuApicIdOrderTable structure pointer array\n")); + return EFI_OUT_OF_RESOURCES; + } // Call for Local APIC ID Reorder Status = SortCpuLocalApicInTable (); if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status)); + DEBUG ((DEBUG_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status)); goto Done; } MaxMadtStructCount = (UINT32) ( - MAX_CPU_NUM + // processor local APIC structures - MAX_CPU_NUM + // processor local x2APIC structures + mNumberOfCpus + // processor local APIC structures + mNumberOfCpus + // processor local x2APIC structures 1 + PcdGet8(PcdPcIoApicCount) + // I/O APIC structures 2 + // interrupt source override structures 1 + // local APIC NMI structures @@ -906,11 +793,11 @@ InstallMadtFromScratch ( // Status = InitializeMadtHeader (&MadtTableHeader); if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "InitializeMadtHeader failed: %r\n", Status)); + DEBUG ((DEBUG_ERROR, "InitializeMadtHeader failed: %r\n", Status)); goto Done; } - DEBUG ((EFI_D_INFO, "Number of CPUs detected = %d \n", mNumberOfCPUs)); + DEBUG ((DEBUG_INFO, "Number of CPUs detected = %d \n", mNumberOfCpus)); // // Build Processor Local APIC Structures and Processor Local X2APIC Structures @@ -923,7 +810,7 @@ InstallMadtFromScratch ( ProcLocalX2ApicStruct.Reserved[0] = 0; ProcLocalX2ApicStruct.Reserved[1] = 0; - for (Index = 0; Index < MAX_CPU_NUM; Index++) { + for (Index = 0; Index < mNumberOfCpus; Index++) { // // If x2APIC mode is not enabled, and if it is possible to express the // APIC ID as a UINT8, use a processor local APIC structure. Otherwise, @@ -936,10 +823,10 @@ InstallMadtFromScratch ( ASSERT (MadtStructsIndex < MaxMadtStructCount); Status = CopyStructure ( - &MadtTableHeader.Header, - (STRUCTURE_HEADER *) &ProcLocalApicStruct, - &MadtStructs[MadtStructsIndex++] - ); + &MadtTableHeader.Header, + (STRUCTURE_HEADER *) &ProcLocalApicStruct, + &MadtStructs[MadtStructsIndex++] + ); } else if (mCpuApicIdOrderTable[Index].ApicId != 0xFFFFFFFF) { ProcLocalX2ApicStruct.Flags = (UINT8) mCpuApicIdOrderTable[Index].Flags; ProcLocalX2ApicStruct.X2ApicId = mCpuApicIdOrderTable[Index].ApicId; @@ -947,13 +834,13 @@ InstallMadtFromScratch ( ASSERT (MadtStructsIndex < MaxMadtStructCount); Status = CopyStructure ( - &MadtTableHeader.Header, - (STRUCTURE_HEADER *) &ProcLocalX2ApicStruct, - &MadtStructs[MadtStructsIndex++] - ); + &MadtTableHeader.Header, + (STRUCTURE_HEADER *) &ProcLocalX2ApicStruct, + &MadtStructs[MadtStructsIndex++] + ); } if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "CopyMadtStructure (local APIC/x2APIC) failed: %r\n", Status)); + DEBUG ((DEBUG_ERROR, "CopyMadtStructure (local APIC/x2APIC) failed: %r\n", Status)); goto Done; } } @@ -965,44 +852,44 @@ InstallMadtFromScratch ( IoApicStruct.Length = sizeof (EFI_ACPI_4_0_IO_APIC_STRUCTURE); IoApicStruct.Reserved = 0; - PcIoApicEnable = PcdGet32(PcdPcIoApicEnable); + PcIoApicEnable = PcdGet32 (PcdPcIoApicEnable); - if (FixedPcdGet32(PcdMaxCpuSocketCount) <= 4) { + if (FixedPcdGet32 (PcdMaxCpuSocketCount) <= 4) { IoApicStruct.IoApicId = PcdGet8(PcdIoApicId); IoApicStruct.IoApicAddress = PcdGet32(PcdIoApicAddress); IoApicStruct.GlobalSystemInterruptBase = 0; ASSERT (MadtStructsIndex < MaxMadtStructCount); Status = CopyStructure ( - &MadtTableHeader.Header, - (STRUCTURE_HEADER *) &IoApicStruct, - &MadtStructs[MadtStructsIndex++] - ); + &MadtTableHeader.Header, + (STRUCTURE_HEADER *) &IoApicStruct, + &MadtStructs[MadtStructsIndex++] + ); if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "CopyMadtStructure (I/O APIC) failed: %r\n", Status)); + DEBUG ((DEBUG_ERROR, "CopyMadtStructure (I/O APIC) failed: %r\n", Status)); goto Done; } } for (PcIoApicIndex = 0; PcIoApicIndex < PcdGet8(PcdPcIoApicCount); PcIoApicIndex++) { - PcIoApicMask = (1 << PcIoApicIndex); - if ((PcIoApicEnable & PcIoApicMask) == 0) { - continue; - } + PcIoApicMask = (1 << PcIoApicIndex); + if ((PcIoApicEnable & PcIoApicMask) == 0) { + continue; + } - IoApicStruct.IoApicId = (UINT8)(PcdGet8(PcdPcIoApicIdBase) + PcIoApicIndex); - IoApicStruct.IoApicAddress = CurrentIoApicAddress; - CurrentIoApicAddress = (CurrentIoApicAddress & 0xFFFF8000) + 0x8000; - IoApicStruct.GlobalSystemInterruptBase = (UINT32)(24 + (PcIoApicIndex * 8)); - ASSERT (MadtStructsIndex < MaxMadtStructCount); - Status = CopyStructure ( - &MadtTableHeader.Header, - (STRUCTURE_HEADER *) &IoApicStruct, - &MadtStructs[MadtStructsIndex++] - ); - if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "CopyMadtStructure (I/O APIC) failed: %r\n", Status)); - goto Done; - } + IoApicStruct.IoApicId = (UINT8)(PcdGet8(PcdPcIoApicIdBase) + PcIoApicIndex); + IoApicStruct.IoApicAddress = CurrentIoApicAddress; + CurrentIoApicAddress = (CurrentIoApicAddress & 0xFFFF8000) + 0x8000; + IoApicStruct.GlobalSystemInterruptBase = (UINT32)(24 + (PcIoApicIndex * 8)); + ASSERT (MadtStructsIndex < MaxMadtStructCount); + Status = CopyStructure ( + &MadtTableHeader.Header, + (STRUCTURE_HEADER *) &IoApicStruct, + &MadtStructs[MadtStructsIndex++] + ); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "CopyMadtStructure (I/O APIC) failed: %r\n", Status)); + goto Done; + } } // @@ -1021,12 +908,12 @@ InstallMadtFromScratch ( ASSERT (MadtStructsIndex < MaxMadtStructCount); Status = CopyStructure ( - &MadtTableHeader.Header, - (STRUCTURE_HEADER *) &IntSrcOverrideStruct, - &MadtStructs[MadtStructsIndex++] - ); + &MadtTableHeader.Header, + (STRUCTURE_HEADER *) &IntSrcOverrideStruct, + &MadtStructs[MadtStructsIndex++] + ); if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "CopyMadtStructure (IRQ2 source override) failed: %r\n", Status)); + DEBUG ((DEBUG_ERROR, "CopyMadtStructure (IRQ2 source override) failed: %r\n", Status)); goto Done; } @@ -1040,12 +927,12 @@ InstallMadtFromScratch ( ASSERT (MadtStructsIndex < MaxMadtStructCount); Status = CopyStructure ( - &MadtTableHeader.Header, - (STRUCTURE_HEADER *) &IntSrcOverrideStruct, - &MadtStructs[MadtStructsIndex++] - ); + &MadtTableHeader.Header, + (STRUCTURE_HEADER *) &IntSrcOverrideStruct, + &MadtStructs[MadtStructsIndex++] + ); if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "CopyMadtStructure (IRQ9 source override) failed: %r\n", Status)); + DEBUG ((DEBUG_ERROR, "CopyMadtStructure (IRQ9 source override) failed: %r\n", Status)); goto Done; } @@ -1060,12 +947,12 @@ InstallMadtFromScratch ( ASSERT (MadtStructsIndex < MaxMadtStructCount); Status = CopyStructure ( - &MadtTableHeader.Header, - (STRUCTURE_HEADER *) &LocalApciNmiStruct, - &MadtStructs[MadtStructsIndex++] - ); + &MadtTableHeader.Header, + (STRUCTURE_HEADER *) &LocalApciNmiStruct, + &MadtStructs[MadtStructsIndex++] + ); if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "CopyMadtStructure (APIC NMI) failed: %r\n", Status)); + DEBUG ((DEBUG_ERROR, "CopyMadtStructure (APIC NMI) failed: %r\n", Status)); goto Done; } @@ -1084,10 +971,10 @@ InstallMadtFromScratch ( ASSERT (MadtStructsIndex < MaxMadtStructCount); Status = CopyStructure ( - &MadtTableHeader.Header, - (STRUCTURE_HEADER *) &LocalX2ApicNmiStruct, - &MadtStructs[MadtStructsIndex++] - ); + &MadtTableHeader.Header, + (STRUCTURE_HEADER *) &LocalX2ApicNmiStruct, + &MadtStructs[MadtStructsIndex++] + ); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "CopyMadtStructure (x2APIC NMI) failed: %r\n", Status)); goto Done; @@ -1098,14 +985,14 @@ InstallMadtFromScratch ( // Build Madt Structure from the Madt Header and collection of pointers in MadtStructs[] // Status = BuildAcpiTable ( - (EFI_ACPI_DESCRIPTION_HEADER *) &MadtTableHeader, - sizeof (EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER), - MadtStructs, - MadtStructsIndex, - (UINT8 **)&NewMadtTable - ); + (EFI_ACPI_DESCRIPTION_HEADER *) &MadtTableHeader, + sizeof (EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER), + MadtStructs, + MadtStructsIndex, + (UINT8 **) &NewMadtTable + ); if (EFI_ERROR (Status)) { - DEBUG ((EFI_D_ERROR, "BuildAcpiTable failed: %r\n", Status)); + DEBUG ((DEBUG_ERROR, "BuildAcpiTable failed: %r\n", Status)); goto Done; } @@ -1113,11 +1000,11 @@ InstallMadtFromScratch ( // Publish Madt Structure to ACPI // Status = mAcpiTable->InstallAcpiTable ( - mAcpiTable, - NewMadtTable, - NewMadtTable->Header.Length, - &TableHandle - ); + mAcpiTable, + NewMadtTable, + NewMadtTable->Header.Length, + &TableHandle + ); Done: // @@ -1136,6 +1023,10 @@ Done: FreePool (NewMadtTable); } + if (mCpuApicIdOrderTable != NULL) { + FreePool (mCpuApicIdOrderTable); + } + return Status; } @@ -1155,8 +1046,8 @@ InstallMcfgFromScratch ( PciSegmentInfo = GetPciSegmentInfo (&SegmentCount); McfgTable = AllocateZeroPool ( - sizeof(EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER) + - sizeof(EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE) * SegmentCount + sizeof (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER) + + sizeof (EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE) * SegmentCount ); if (McfgTable == NULL) { DEBUG ((DEBUG_ERROR, "Could not allocate MCFG structure\n")); @@ -1164,11 +1055,11 @@ InstallMcfgFromScratch ( } Status = InitializeHeader ( - &McfgTable->Header, - EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE, - EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION, - 0 - ); + &McfgTable->Header, + EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE, + EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_SPACE_ACCESS_TABLE_REVISION, + 0 + ); if (EFI_ERROR (Status)) { return Status; } @@ -1192,11 +1083,11 @@ InstallMcfgFromScratch ( // Publish Madt Structure to ACPI // Status = mAcpiTable->InstallAcpiTable ( - mAcpiTable, - McfgTable, - McfgTable->Header.Length, - &TableHandle - ); + mAcpiTable, + McfgTable, + McfgTable->Header.Length, + &TableHandle + ); return Status; } @@ -1280,7 +1171,7 @@ PlatformUpdateTables ( switch (Table->Signature) { case EFI_ACPI_4_0_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE: - ASSERT(FALSE); + ASSERT (FALSE); break; case EFI_ACPI_5_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE: @@ -1324,9 +1215,9 @@ PlatformUpdateTables ( FadtHeader->XGpe1Blk.AccessSize = 0; } - DEBUG(( EFI_D_ERROR, "ACPI FADT table @ address 0x%x\n", Table )); - DEBUG(( EFI_D_ERROR, " IaPcBootArch 0x%x\n", FadtHeader->IaPcBootArch )); - DEBUG(( EFI_D_ERROR, " Flags 0x%x\n", FadtHeader->Flags )); + DEBUG ((DEBUG_INFO, "ACPI FADT table @ address 0x%x\n", Table)); + DEBUG ((DEBUG_INFO, " IaPcBootArch 0x%x\n", FadtHeader->IaPcBootArch)); + DEBUG ((DEBUG_INFO, " Flags 0x%x\n", FadtHeader->Flags)); break; case EFI_ACPI_3_0_HIGH_PRECISION_EVENT_TIMER_TABLE_SIGNATURE: @@ -1346,12 +1237,12 @@ PlatformUpdateTables ( HpetBlockId.Bits.VendorId = HpetCapabilities.Bits.VendorId; HpetTable->EventTimerBlockId = HpetBlockId.Uint32; HpetTable->MainCounterMinimumClockTickInPeriodicMode = (UINT16)HpetCapabilities.Bits.CounterClockPeriod; - DEBUG(( EFI_D_ERROR, "ACPI HPET table @ address 0x%x\n", Table )); - DEBUG(( EFI_D_ERROR, " HPET base 0x%x\n", PcdGet32 (PcdHpetBaseAddress) )); + DEBUG ((DEBUG_INFO, "ACPI HPET table @ address 0x%x\n", Table)); + DEBUG ((DEBUG_INFO, " HPET base 0x%x\n", PcdGet32 (PcdHpetBaseAddress))); break; case EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE: - ASSERT(FALSE); + ASSERT (FALSE); break; default: @@ -1403,8 +1294,8 @@ IsHardwareChange ( // pFADT->XDsdt // HWChangeSize = HandleCount + 1; - HWChange = AllocateZeroPool( sizeof(UINT32) * HWChangeSize ); - ASSERT( HWChange != NULL ); + HWChange = AllocateZeroPool (sizeof(UINT32) * HWChangeSize); + ASSERT(HWChange != NULL); if (HWChange == NULL) return; @@ -1445,14 +1336,14 @@ IsHardwareChange ( // Calculate CRC value with HWChange data. // Status = gBS->CalculateCrc32(HWChange, HWChangeSize, &CRC); - DEBUG((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status)); + DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status)); // // Set HardwareSignature value based on CRC value. // FacsPtr = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)pFADT->FirmwareCtrl; FacsPtr->HardwareSignature = CRC; - FreePool( HWChange ); + FreePool (HWChange); } VOID @@ -1475,17 +1366,16 @@ UpdateLocalTable ( if (Version != EFI_ACPI_TABLE_VERSION_NONE) { Status = mAcpiTable->InstallAcpiTable ( - mAcpiTable, - CurrentTable, - CurrentTable->Length, - &TableHandle - ); + mAcpiTable, + CurrentTable, + CurrentTable->Length, + &TableHandle + ); ASSERT_EFI_ERROR (Status); } } } - VOID EFIAPI AcpiEndOfDxeEvent ( @@ -1493,16 +1383,14 @@ AcpiEndOfDxeEvent ( VOID *ParentImageHandle ) { - if (Event != NULL) { - gBS->CloseEvent(Event); + gBS->CloseEvent (Event); } - // // Calculate Hardware Signature value based on current platform configurations // - IsHardwareChange(); + IsHardwareChange (); } /** @@ -1526,7 +1414,6 @@ InstallAcpiPlatform ( EFI_STATUS Status; EFI_EVENT EndOfDxeEvent; - Status = gBS->LocateProtocol (&gEfiMpServiceProtocolGuid, NULL, (VOID **)&mMpService); ASSERT_EFI_ERROR (Status); @@ -1550,19 +1437,19 @@ InstallAcpiPlatform ( // Determine the number of processors // mMpService->GetNumberOfProcessors ( - mMpService, - &mNumberOfCPUs, - &mNumberOfEnabledCPUs - ); - ASSERT (mNumberOfCPUs <= MAX_CPU_NUM && mNumberOfEnabledCPUs >= 1); - DEBUG ((DEBUG_INFO, "mNumberOfCPUs - %d\n", mNumberOfCPUs)); + mMpService, + &mNumberOfCpus, + &mNumberOfEnabledCPUs + ); + + DEBUG ((DEBUG_INFO, "mNumberOfCpus - %d\n", mNumberOfCpus)); DEBUG ((DEBUG_INFO, "mNumberOfEnabledCPUs - %d\n", mNumberOfEnabledCPUs)); DEBUG ((DEBUG_INFO, "mX2ApicEnabled - 0x%x\n", mX2ApicEnabled)); DEBUG ((DEBUG_INFO, "mForceX2ApicId - 0x%x\n", mForceX2ApicId)); // support up to 64 threads/socket - AsmCpuidEx(CPUID_EXTENDED_TOPOLOGY, 1, &mNumOfBitShift, NULL, NULL, NULL); + AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 1, &mNumOfBitShift, NULL, NULL, NULL); mNumOfBitShift &= 0x1F; DEBUG ((DEBUG_INFO, "mNumOfBitShift - 0x%x\n", mNumOfBitShift)); -- 2.32.0.windows.2
|
|
|
|
Re: [PATCH] SecurityPkg: Debug code to audit BIOS TPM extend operations.
Rodrigo Gonzalez del Cueto
Hi Jiewen,
The intention of such API would be to ease debugging and auditing PCR attestation along the boot; it has been a common task while debugging several issues and
TPM configurations.
a) Configurations in which BIOS is not the S-CRTM and we need to attest what has been measured to the TPM prior to any measurements performed by BIOS.
b) Verifying the values in all the active and supported PCR banks: attestation or capping of the PCRs. (See
BZ: 3515)
Such API together with the TCG event log print out it allows us to audit and debug the measured boot sequence.
Regards,
-Rodrigo
From: Yao, Jiewen <jiewen.yao@...>
Sent: Sunday, August 8, 2021 6:24 PM To: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@...>; devel@edk2.groups.io <devel@edk2.groups.io> Cc: Wang, Jian J <jian.j.wang@...> Subject: RE: [PATCH] SecurityPkg: Debug code to audit BIOS TPM extend operations. Some feedback:
1) I think it is OK to add Tpm2PcrReadForActiveBank() API. But I feel we will add too many noise to dump Tpm2PcrReadForActiveBank() in the code everytime. I am not sure why it is needed. What is the problem statement? 2) Below definition does not follow EDKII coding style. Please use 2 "space" as indent. EFI_STATUS EFIAPI Tpm2PcrReadForActiveBank ( IN TPMI_DH_PCR PcrHandle, OUT TPML_DIGEST *HashList ) > -----Original Message----- > From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@...> > Sent: Friday, July 30, 2021 6:43 AM > To: devel@edk2.groups.io > Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@...>; Yao, > Jiewen <jiewen.yao@...>; Wang, Jian J <jian.j.wang@...> > Subject: [PATCH] SecurityPkg: Debug code to audit BIOS TPM extend operations. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2858 > > Add debug functionality to examine TPM extend operations > performed by BIOS and inspect the PCR 00 value prior to > any BIOS measurements. > > Replaced usage of EFI_D_* for DEBUG_* definitions in debug > messages. > > Signed-off-by: Rodrigo Gonzalez del Cueto > <rodrigo.gonzalez.del.cueto@...> > Cc: Jiewen Yao <jiewen.yao@...> > Cc: Jian J Wang <jian.j.wang@...> > --- > SecurityPkg/Include/Library/Tpm2CommandLib.h | 28 > ++++++++++++++++++++++------ > SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c | 226 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > ++++++++----------------------- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 34 ++++++++++++++++++++------ > -------- > 3 files changed, 245 insertions(+), 43 deletions(-) > > diff --git a/SecurityPkg/Include/Library/Tpm2CommandLib.h > b/SecurityPkg/Include/Library/Tpm2CommandLib.h > index ee8eb62295..5e5c340893 100644 > --- a/SecurityPkg/Include/Library/Tpm2CommandLib.h > +++ b/SecurityPkg/Include/Library/Tpm2CommandLib.h > @@ -1,7 +1,7 @@ > /** @file > This library is used by other modules to send TPM2 command. > > -Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved. <BR> > +Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved. <BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -505,7 +505,7 @@ EFIAPI > Tpm2PcrEvent ( > IN TPMI_DH_PCR PcrHandle, > IN TPM2B_EVENT *EventData, > - OUT TPML_DIGEST_VALUES *Digests > + OUT TPML_DIGEST_VALUES *Digests > ); > > /** > @@ -522,10 +522,10 @@ Tpm2PcrEvent ( > EFI_STATUS > EFIAPI > Tpm2PcrRead ( > - IN TPML_PCR_SELECTION *PcrSelectionIn, > - OUT UINT32 *PcrUpdateCounter, > - OUT TPML_PCR_SELECTION *PcrSelectionOut, > - OUT TPML_DIGEST *PcrValues > + IN TPML_PCR_SELECTION *PcrSelectionIn, > + OUT UINT32 *PcrUpdateCounter, > + OUT TPML_PCR_SELECTION *PcrSelectionOut, > + OUT TPML_DIGEST *PcrValues > ); > > /** > @@ -1113,4 +1113,20 @@ GetDigestFromDigestList( > OUT VOID *Digest > ); > > + /** > + This function will query the TPM to determine which hashing algorithms and > + get the digests of all active and supported PCR banks of a specific PCR > register. > + > + @param[in] PcrHandle The index of the PCR register to be read. > + @param[out] HashList List of digests from PCR register being read. > + > + @retval EFI_SUCCESS The Pcr was read successfully. > + @retval EFI_DEVICE_ERROR The command was unsuccessful. > +**/ > +EFI_STATUS > +EFIAPI > +Tpm2PcrReadForActiveBank ( > + IN TPMI_DH_PCR PcrHandle, > + OUT TPML_DIGEST *HashList > + ); > #endif > diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c > b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c > index ddb15178fb..3b49192b93 100644 > --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c > +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c > @@ -1,7 +1,7 @@ > /** @file > Implement TPM2 Integrity related command. > > -Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved. <BR> > +Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved. <BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -109,7 +109,6 @@ Tpm2PcrExtend ( > Cmd.Header.commandCode = SwapBytes32(TPM_CC_PCR_Extend); > Cmd.PcrHandle = SwapBytes32(PcrHandle); > > - > // > // Add in Auth session > // > @@ -130,14 +129,26 @@ Tpm2PcrExtend ( > Buffer += sizeof(UINT16); > DigestSize = GetHashSizeFromAlgo (Digests->digests[Index].hashAlg); > if (DigestSize == 0) { > - DEBUG ((EFI_D_ERROR, "Unknown hash algorithm %d\r\n", Digests- > >digests[Index].hashAlg)); > + DEBUG ((DEBUG_ERROR, "Unknown hash algorithm %d\r\n", Digests- > >digests[Index].hashAlg)); > return EFI_DEVICE_ERROR; > } > + > CopyMem( > Buffer, > &Digests->digests[Index].digest, > DigestSize > ); > + > + DEBUG_CODE_BEGIN (); > + UINTN Index2; > + DEBUG ((DEBUG_VERBOSE, "Tpm2PcrExtend - Hash = 0x%04x, Pcr[%02d], > digest = ", Digests->digests[Index].hashAlg, (UINT8) PcrHandle)); > + > + for (Index2 = 0; Index2 < DigestSize; Index2++) { > + DEBUG ((DEBUG_VERBOSE, "%02x ", Buffer[Index2])); > + } > + DEBUG ((DEBUG_VERBOSE, "\n")); > + DEBUG_CODE_END (); > + > Buffer += DigestSize; > } > > @@ -151,7 +162,7 @@ Tpm2PcrExtend ( > } > > if (ResultBufSize > sizeof(Res)) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrExtend: Failed ExecuteCommand: Buffer > Too Small\r\n")); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrExtend: Failed ExecuteCommand: Buffer > Too Small\r\n")); > return EFI_BUFFER_TOO_SMALL; > } > > @@ -160,7 +171,7 @@ Tpm2PcrExtend ( > // > RespSize = SwapBytes32(Res.Header.paramSize); > if (RespSize > sizeof(Res)) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrExtend: Response size too large! %d\r\n", > RespSize)); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrExtend: Response size too large! %d\r\n", > RespSize)); > return EFI_BUFFER_TOO_SMALL; > } > > @@ -168,10 +179,15 @@ Tpm2PcrExtend ( > // Fail if command failed > // > if (SwapBytes32(Res.Header.responseCode) != TPM_RC_SUCCESS) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrExtend: Response Code error! 0x%08x\r\n", > SwapBytes32(Res.Header.responseCode))); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrExtend: Response Code error! > 0x%08x\r\n", SwapBytes32(Res.Header.responseCode))); > return EFI_DEVICE_ERROR; > } > > + DEBUG_CODE_BEGIN (); > + DEBUG ((DEBUG_VERBOSE, "Tpm2PcrExtend: PCR read after extend...\n")); > + Tpm2PcrReadForActiveBank (PcrHandle, NULL); > + DEBUG_CODE_END (); > + > // > // Unmarshal the response > // > @@ -246,7 +262,7 @@ Tpm2PcrEvent ( > } > > if (ResultBufSize > sizeof(Res)) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrEvent: Failed ExecuteCommand: Buffer > Too Small\r\n")); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrEvent: Failed ExecuteCommand: Buffer > Too Small\r\n")); > return EFI_BUFFER_TOO_SMALL; > } > > @@ -255,7 +271,7 @@ Tpm2PcrEvent ( > // > RespSize = SwapBytes32(Res.Header.paramSize); > if (RespSize > sizeof(Res)) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrEvent: Response size too large! %d\r\n", > RespSize)); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrEvent: Response size too large! %d\r\n", > RespSize)); > return EFI_BUFFER_TOO_SMALL; > } > > @@ -263,7 +279,7 @@ Tpm2PcrEvent ( > // Fail if command failed > // > if (SwapBytes32(Res.Header.responseCode) != TPM_RC_SUCCESS) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrEvent: Response Code error! 0x%08x\r\n", > SwapBytes32(Res.Header.responseCode))); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrEvent: Response Code error! > 0x%08x\r\n", SwapBytes32(Res.Header.responseCode))); > return EFI_DEVICE_ERROR; > } > > @@ -284,7 +300,7 @@ Tpm2PcrEvent ( > Buffer += sizeof(UINT16); > DigestSize = GetHashSizeFromAlgo (Digests->digests[Index].hashAlg); > if (DigestSize == 0) { > - DEBUG ((EFI_D_ERROR, "Unknown hash algorithm %d\r\n", Digests- > >digests[Index].hashAlg)); > + DEBUG ((DEBUG_ERROR, "Unknown hash algorithm %d\r\n", Digests- > >digests[Index].hashAlg)); > return EFI_DEVICE_ERROR; > } > CopyMem( > @@ -298,6 +314,7 @@ Tpm2PcrEvent ( > return EFI_SUCCESS; > } > > + > /** > This command returns the values of all PCR specified in pcrSelect. > > @@ -353,11 +370,11 @@ Tpm2PcrRead ( > } > > if (RecvBufferSize < sizeof (TPM2_RESPONSE_HEADER)) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrRead - RecvBufferSize Error - %x\n", > RecvBufferSize)); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrRead - RecvBufferSize Error - %x\n", > RecvBufferSize)); > return EFI_DEVICE_ERROR; > } > if (SwapBytes32(RecvBuffer.Header.responseCode) != TPM_RC_SUCCESS) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrRead - responseCode - %x\n", > SwapBytes32(RecvBuffer.Header.responseCode))); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrRead - responseCode - %x\n", > SwapBytes32(RecvBuffer.Header.responseCode))); > return EFI_NOT_FOUND; > } > > @@ -369,7 +386,7 @@ Tpm2PcrRead ( > // PcrUpdateCounter > // > if (RecvBufferSize < sizeof (TPM2_RESPONSE_HEADER) + > sizeof(RecvBuffer.PcrUpdateCounter)) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrRead - RecvBufferSize Error - %x\n", > RecvBufferSize)); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrRead - RecvBufferSize Error - %x\n", > RecvBufferSize)); > return EFI_DEVICE_ERROR; > } > *PcrUpdateCounter = SwapBytes32(RecvBuffer.PcrUpdateCounter); > @@ -378,7 +395,7 @@ Tpm2PcrRead ( > // PcrSelectionOut > // > if (RecvBufferSize < sizeof (TPM2_RESPONSE_HEADER) + > sizeof(RecvBuffer.PcrUpdateCounter) + > sizeof(RecvBuffer.PcrSelectionOut.count)) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrRead - RecvBufferSize Error - %x\n", > RecvBufferSize)); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrRead - RecvBufferSize Error - %x\n", > RecvBufferSize)); > return EFI_DEVICE_ERROR; > } > PcrSelectionOut->count = SwapBytes32(RecvBuffer.PcrSelectionOut.count); > @@ -388,7 +405,7 @@ Tpm2PcrRead ( > } > > if (RecvBufferSize < sizeof (TPM2_RESPONSE_HEADER) + > sizeof(RecvBuffer.PcrUpdateCounter) + > sizeof(RecvBuffer.PcrSelectionOut.count) + > sizeof(RecvBuffer.PcrSelectionOut.pcrSelections[0]) * PcrSelectionOut->count) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrRead - RecvBufferSize Error - %x\n", > RecvBufferSize)); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrRead - RecvBufferSize Error - %x\n", > RecvBufferSize)); > return EFI_DEVICE_ERROR; > } > for (Index = 0; Index < PcrSelectionOut->count; Index++) { > @@ -513,7 +530,7 @@ Tpm2PcrAllocate ( > } > > if (ResultBufSize > sizeof(Res)) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrAllocate: Failed ExecuteCommand: Buffer > Too Small\r\n")); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrAllocate: Failed ExecuteCommand: > Buffer Too Small\r\n")); > Status = EFI_BUFFER_TOO_SMALL; > goto Done; > } > @@ -523,7 +540,7 @@ Tpm2PcrAllocate ( > // > RespSize = SwapBytes32(Res.Header.paramSize); > if (RespSize > sizeof(Res)) { > - DEBUG ((EFI_D_ERROR, "Tpm2PcrAllocate: Response size too large! %d\r\n", > RespSize)); > + DEBUG ((DEBUG_ERROR, "Tpm2PcrAllocate: Response size too > large! %d\r\n", RespSize)); > Status = EFI_BUFFER_TOO_SMALL; > goto Done; > } > @@ -532,7 +549,7 @@ Tpm2PcrAllocate ( > // Fail if command failed > // > if (SwapBytes32(Res.Header.responseCode) != TPM_RC_SUCCESS) { > - DEBUG((EFI_D_ERROR,"Tpm2PcrAllocate: Response Code error! 0x%08x\r\n", > SwapBytes32(Res.Header.responseCode))); > + DEBUG((DEBUG_ERROR,"Tpm2PcrAllocate: Response Code error! > 0x%08x\r\n", SwapBytes32(Res.Header.responseCode))); > Status = EFI_DEVICE_ERROR; > goto Done; > } > @@ -673,17 +690,180 @@ Tpm2PcrAllocateBanks ( > &SizeNeeded, > &SizeAvailable > ); > - DEBUG ((EFI_D_INFO, "Tpm2PcrAllocateBanks call Tpm2PcrAllocate - %r\n", > Status)); > + DEBUG ((DEBUG_INFO, "Tpm2PcrAllocateBanks call Tpm2PcrAllocate - %r\n", > Status)); > if (EFI_ERROR (Status)) { > goto Done; > } > > - DEBUG ((EFI_D_INFO, "AllocationSuccess - %02x\n", AllocationSuccess)); > - DEBUG ((EFI_D_INFO, "MaxPCR - %08x\n", MaxPCR)); > - DEBUG ((EFI_D_INFO, "SizeNeeded - %08x\n", SizeNeeded)); > - DEBUG ((EFI_D_INFO, "SizeAvailable - %08x\n", SizeAvailable)); > + DEBUG ((DEBUG_INFO, "AllocationSuccess - %02x\n", AllocationSuccess)); > + DEBUG ((DEBUG_INFO, "MaxPCR - %08x\n", MaxPCR)); > + DEBUG ((DEBUG_INFO, "SizeNeeded - %08x\n", SizeNeeded)); > + DEBUG ((DEBUG_INFO, "SizeAvailable - %08x\n", SizeAvailable)); > > Done: > ZeroMem(&LocalAuthSession.hmac, sizeof(LocalAuthSession.hmac)); > return Status; > } > + > +/** > + This function will query the TPM to determine which hashing algorithms and > + get the digests of all active and supported PCR banks of a specific PCR > register. > + > + @param[in] PcrHandle The index of the PCR register to be read. > + @param[out] HashList List of digests from PCR register being read. > + > + @retval EFI_SUCCESS The Pcr was read successfully. > + @retval EFI_DEVICE_ERROR The command was unsuccessful. > +**/ > +EFI_STATUS > +EFIAPI > +Tpm2PcrReadForActiveBank ( > + IN TPMI_DH_PCR PcrHandle, > + OUT TPML_DIGEST *HashList > +) > +{ > + EFI_STATUS Status; > + TPML_PCR_SELECTION Pcrs; > + TPML_PCR_SELECTION PcrSelectionIn; > + TPML_PCR_SELECTION PcrSelectionOut; > + TPML_DIGEST PcrValues; > + UINT32 PcrUpdateCounter; > + UINT8 PcrIndex; > + UINT32 TpmHashAlgorithmBitmap; > + TPMI_ALG_HASH CurrentPcrBankHash; > + UINT32 ActivePcrBanks; > + UINT32 TcgRegistryHashAlg; > + UINTN Index; > + UINTN Index2; > + > + PcrIndex = (UINT8) PcrHandle; > + > + if ((PcrIndex < 0) || > + (PcrIndex >= IMPLEMENTATION_PCR)) { > + return EFI_INVALID_PARAMETER; > + } > + > + ZeroMem (&PcrSelectionIn, sizeof (PcrSelectionIn)); > + ZeroMem (&PcrUpdateCounter, sizeof (UINT32)); > + ZeroMem (&PcrSelectionOut, sizeof (PcrSelectionOut)); > + ZeroMem (&PcrValues, sizeof (PcrValues)); > + ZeroMem (&Pcrs, sizeof (TPML_PCR_SELECTION)); > + > + DEBUG ((DEBUG_INFO, "ReadPcr - %02d\n", PcrIndex)); > + > + // > + // Read TPM capabilities > + // > + Status = Tpm2GetCapabilityPcrs (&Pcrs); > + > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "ReadPcr: Unable to read TPM capabilities\n")); > + return EFI_DEVICE_ERROR; > + } > + > + // > + // Get Active Pcrs > + // > + Status = Tpm2GetCapabilitySupportedAndActivePcrs ( > + &TpmHashAlgorithmBitmap, > + &ActivePcrBanks > + ); > + > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "ReadPcr: Unable to read TPM capabilities and > active PCRs\n")); > + return EFI_DEVICE_ERROR; > + } > + > + // > + // Select from Active PCRs > + // > + for (Index = 0; Index < Pcrs.count; Index++) { > + CurrentPcrBankHash = Pcrs.pcrSelections[Index].hash; > + > + switch (CurrentPcrBankHash) { > + case TPM_ALG_SHA1: > + DEBUG ((DEBUG_VERBOSE, "HASH_ALG_SHA1 Present\n")); > + TcgRegistryHashAlg = HASH_ALG_SHA1; > + break; > + case TPM_ALG_SHA256: > + DEBUG ((DEBUG_VERBOSE, "HASH_ALG_SHA256 Present\n")); > + TcgRegistryHashAlg = HASH_ALG_SHA256; > + break; > + case TPM_ALG_SHA384: > + DEBUG ((DEBUG_VERBOSE, "HASH_ALG_SHA384 Present\n")); > + TcgRegistryHashAlg = HASH_ALG_SHA384; > + break; > + case TPM_ALG_SHA512: > + DEBUG ((DEBUG_VERBOSE, "HASH_ALG_SHA512 Present\n")); > + TcgRegistryHashAlg = HASH_ALG_SHA512; > + break; > + case TPM_ALG_SM3_256: > + DEBUG ((DEBUG_VERBOSE, "HASH_ALG_SM3 Present\n")); > + TcgRegistryHashAlg = HASH_ALG_SM3_256; > + break; > + default: > + // > + // Unsupported algorithm > + // > + DEBUG ((DEBUG_VERBOSE, "Unknown algorithm present\n")); > + TcgRegistryHashAlg = 0; > + break; > + } > + // > + // Skip unsupported and inactive PCR banks > + // > + if ((TcgRegistryHashAlg & ActivePcrBanks) == 0) { > + DEBUG ((DEBUG_VERBOSE, "Skipping unsupported or inactive bank: > 0x%04x\n", CurrentPcrBankHash)); > + continue; > + } > + > + // > + // Select PCR from current active bank > + // > + PcrSelectionIn.pcrSelections[PcrSelectionIn.count].hash = > Pcrs.pcrSelections[Index].hash; > + PcrSelectionIn.pcrSelections[PcrSelectionIn.count].sizeofSelect = > PCR_SELECT_MAX; > + PcrSelectionIn.pcrSelections[PcrSelectionIn.count].pcrSelect[0] = (PcrIndex < > 8) ? 1 << PcrIndex : 0; > + PcrSelectionIn.pcrSelections[PcrSelectionIn.count].pcrSelect[1] = (PcrIndex > > 7) && (PcrIndex < 16) ? 1 << (PcrIndex - 8) : 0; > + PcrSelectionIn.pcrSelections[PcrSelectionIn.count].pcrSelect[2] = (PcrIndex > > 15) ? 1 << (PcrIndex - 16) : 0; > + PcrSelectionIn.count++; > + } > + > + // > + // Read PCRs > + // > + Status = Tpm2PcrRead ( > + &PcrSelectionIn, > + &PcrUpdateCounter, > + &PcrSelectionOut, > + &PcrValues > + ); > + > + if (EFI_ERROR (Status)) { > + DEBUG((DEBUG_ERROR, "Tpm2PcrRead failed Status = %r \n", Status)); > + return EFI_DEVICE_ERROR; > + } > + > + for (Index = 0; Index < PcrValues.count; Index++) { > + DEBUG (( > + DEBUG_INFO, > + "ReadPcr - HashAlg = 0x%04x, Pcr[%02d], digest = ", > + PcrSelectionOut.pcrSelections[Index].hash, > + PcrIndex > + )); > + > + for(Index2 = 0; Index2 < PcrValues.digests[Index].size; Index2++) { > + DEBUG ((DEBUG_INFO, "%02x ", PcrValues.digests[Index].buffer[Index2])); > + } > + DEBUG ((DEBUG_INFO, "\n")); > + } > + > + if (HashList != NULL) { > + CopyMem ( > + HashList, > + &PcrValues, > + sizeof (TPML_DIGEST) > + ); > + } > + > + return EFI_SUCCESS; > +} > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > index 93a8803ff6..ea79fa0af6 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > @@ -1,7 +1,7 @@ > /** @file > Initialize TPM2 device and measure FVs before handing off control to DXE. > > -Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR> > Copyright (c) 2017, Microsoft Corporation. All rights reserved. <BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -191,7 +191,6 @@ EFI_PEI_NOTIFY_DESCRIPTOR mNotifyList[] = { > } > }; > > - > /** > Record all measured Firmware Volume Information into a Guid Hob > Guid Hob payload layout is > @@ -267,7 +266,7 @@ SyncPcrAllocationsAndPcrMask ( > UINT32 Tpm2PcrMask; > UINT32 NewTpm2PcrMask; > > - DEBUG ((EFI_D_ERROR, "SyncPcrAllocationsAndPcrMask!\n")); > + DEBUG ((DEBUG_ERROR, "SyncPcrAllocationsAndPcrMask!\n")); > > // > // Determine the current TPM support and the Platform PCR mask. > @@ -278,7 +277,7 @@ SyncPcrAllocationsAndPcrMask ( > Tpm2PcrMask = PcdGet32 (PcdTpm2HashMask); > if (Tpm2PcrMask == 0) { > // > - // if PcdTPm2HashMask is zero, use ActivePcr setting > + // if PcdTpm2HashMask is zero, use ActivePcr setting > // > PcdSet32S (PcdTpm2HashMask, TpmActivePcrBanks); > Tpm2PcrMask = TpmActivePcrBanks; > @@ -297,9 +296,9 @@ SyncPcrAllocationsAndPcrMask ( > if ((TpmActivePcrBanks & Tpm2PcrMask) != TpmActivePcrBanks) { > NewTpmActivePcrBanks = TpmActivePcrBanks & Tpm2PcrMask; > > - DEBUG ((EFI_D_INFO, "%a - Reallocating PCR banks from 0x%X to 0x%X.\n", > __FUNCTION__, TpmActivePcrBanks, NewTpmActivePcrBanks)); > + DEBUG ((DEBUG_INFO, "%a - Reallocating PCR banks from 0x%X to 0x%X.\n", > __FUNCTION__, TpmActivePcrBanks, NewTpmActivePcrBanks)); > if (NewTpmActivePcrBanks == 0) { > - DEBUG ((EFI_D_ERROR, "%a - No viable PCRs active! Please set a less > restrictive value for PcdTpm2HashMask!\n", __FUNCTION__)); > + DEBUG ((DEBUG_ERROR, "%a - No viable PCRs active! Please set a less > restrictive value for PcdTpm2HashMask!\n", __FUNCTION__)); > ASSERT (FALSE); > } else { > Status = Tpm2PcrAllocateBanks (NULL, (UINT32)TpmHashAlgorithmBitmap, > NewTpmActivePcrBanks); > @@ -307,7 +306,7 @@ SyncPcrAllocationsAndPcrMask ( > // > // We can't do much here, but we hope that this doesn't happen. > // > - DEBUG ((EFI_D_ERROR, "%a - Failed to reallocate PCRs!\n", > __FUNCTION__)); > + DEBUG ((DEBUG_ERROR, "%a - Failed to reallocate PCRs!\n", > __FUNCTION__)); > ASSERT_EFI_ERROR (Status); > } > // > @@ -324,9 +323,9 @@ SyncPcrAllocationsAndPcrMask ( > if ((Tpm2PcrMask & TpmHashAlgorithmBitmap) != Tpm2PcrMask) { > NewTpm2PcrMask = Tpm2PcrMask & TpmHashAlgorithmBitmap; > > - DEBUG ((EFI_D_INFO, "%a - Updating PcdTpm2HashMask from 0x%X to > 0x%X.\n", __FUNCTION__, Tpm2PcrMask, NewTpm2PcrMask)); > + DEBUG ((DEBUG_INFO, "%a - Updating PcdTpm2HashMask from 0x%X to > 0x%X.\n", __FUNCTION__, Tpm2PcrMask, NewTpm2PcrMask)); > if (NewTpm2PcrMask == 0) { > - DEBUG ((EFI_D_ERROR, "%a - No viable PCRs supported! Please set a less > restrictive value for PcdTpm2HashMask!\n", __FUNCTION__)); > + DEBUG ((DEBUG_ERROR, "%a - No viable PCRs supported! Please set a less > restrictive value for PcdTpm2HashMask!\n", __FUNCTION__)); > ASSERT (FALSE); > } > > @@ -365,7 +364,7 @@ LogHashEvent ( > RetStatus = EFI_SUCCESS; > for (Index = 0; Index < sizeof(mTcg2EventInfo)/sizeof(mTcg2EventInfo[0]); > Index++) { > if ((SupportedEventLogs & mTcg2EventInfo[Index].LogFormat) != 0) { > - DEBUG ((EFI_D_INFO, " LogFormat - 0x%08x\n", > mTcg2EventInfo[Index].LogFormat)); > + DEBUG ((DEBUG_INFO, " LogFormat - 0x%08x\n", > mTcg2EventInfo[Index].LogFormat)); > switch (mTcg2EventInfo[Index].LogFormat) { > case EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2: > Status = GetDigestFromDigestList (TPM_ALG_SHA1, DigestList, > &NewEventHdr->Digest); > @@ -476,7 +475,7 @@ HashLogExtendEvent ( > } > > if (Status == EFI_DEVICE_ERROR) { > - DEBUG ((EFI_D_ERROR, "HashLogExtendEvent - %r. Disable TPM.\n", Status)); > + DEBUG ((DEBUG_ERROR, "HashLogExtendEvent - %r. Disable TPM.\n", > Status)); > BuildGuidHob (&gTpmErrorHobGuid,0); > REPORT_STATUS_CODE ( > EFI_ERROR_CODE | EFI_ERROR_MINOR, > @@ -1011,7 +1010,7 @@ PeimEntryMA ( > } > > if (GetFirstGuidHob (&gTpmErrorHobGuid) != NULL) { > - DEBUG ((EFI_D_ERROR, "TPM2 error!\n")); > + DEBUG ((DEBUG_ERROR, "TPM2 error!\n")); > return EFI_DEVICE_ERROR; > } > > @@ -1075,7 +1074,7 @@ PeimEntryMA ( > for (PcrIndex = 0; PcrIndex < 8; PcrIndex++) { > Status = MeasureSeparatorEventWithError (PcrIndex); > if (EFI_ERROR (Status)) { > - DEBUG ((EFI_D_ERROR, "Separator Event with Error not Measured. > Error!\n")); > + DEBUG ((DEBUG_ERROR, "Separator Event with Error not Measured. > Error!\n")); > } > } > } > @@ -1092,6 +1091,13 @@ PeimEntryMA ( > } > } > > + DEBUG_CODE_BEGIN (); > + // > + // Peek into TPM PCR 00 before any BIOS measurement. > + // > + Tpm2PcrReadForActiveBank (00, NULL); > + DEBUG_CODE_END (); > + > // > // Only install TpmInitializedPpi on success > // > @@ -1106,7 +1112,7 @@ PeimEntryMA ( > > Done: > if (EFI_ERROR (Status)) { > - DEBUG ((EFI_D_ERROR, "TPM2 error! Build Hob\n")); > + DEBUG ((DEBUG_ERROR, "TPM2 error! Build Hob\n")); > BuildGuidHob (&gTpmErrorHobGuid,0); > REPORT_STATUS_CODE ( > EFI_ERROR_CODE | EFI_ERROR_MINOR, > -- > 2.31.1.windows.1
|
|
|
|
Re: [PATCH] Reallocate TPM Active PCRs based on platform support.
Rodrigo Gonzalez del Cueto
Hi Jiewen,
Indeed, this bug has existed for a long time in this code. What recently changed are the TPM configurations we are testing and exposed the issue; this can be reproduced when the
BIOS supported algorithms are a strict subset of the PCRs currently active in the TPM.
Now that we are using TPM configurations with support for additional PCR banks (ex. SHA384 and SM3) the bug has been exposed when compiling a BIOS without support for these PCR
banks which are active by default in the some of the TPMs.
Regards,
-Rodrigo
From: Yao, Jiewen <jiewen.yao@...>
Sent: Sunday, August 8, 2021 6:13 PM To: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@...>; devel@edk2.groups.io <devel@edk2.groups.io> Cc: Wang, Jian J <jian.j.wang@...> Subject: RE: [PATCH] Reallocate TPM Active PCRs based on platform support. Hi Rodrigo
I don’t understand the problem statement. This code has been there for long time. What is changed recently ? Thank you Yao Jiewen > -----Original Message----- > From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@...> > Sent: Thursday, August 5, 2021 7:28 AM > To: devel@edk2.groups.io > Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@...>; > Wang, Jian J <jian.j.wang@...>; Yao, Jiewen <jiewen.yao@...> > Subject: [PATCH] Reallocate TPM Active PCRs based on platform support. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3515 > > In V2: Add case to RegisterHashInterfaceLib logic > > RegisterHashInterfaceLib needs to correctly handle registering the HashLib > instance supported algorithm bitmap when PcdTpm2HashMask is set to zero. > > The current implementation of SyncPcrAllocationsAndPcrMask() triggers > PCR bank reallocation only based on the intersection between > TpmActivePcrBanks and PcdTpm2HashMask. > > When the software HashLibBaseCryptoRouter solution is used, no PCR bank > reallocation is occurring based on the supported hashing algorithms > registered by the HashLib instances. > > Need to have an additional check for the intersection between the > TpmActivePcrBanks and the PcdTcg2HashAlgorithmBitmap populated by the > HashLib instances present on the platform's BIOS. > > Signed-off-by: Rodrigo Gonzalez del Cueto > <rodrigo.gonzalez.del.cueto@...> > > Cc: Jian J Wang <jian.j.wang@...> > Cc: Jiewen Yao <jiewen.yao@...> > --- > SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c > | 6 +++++- > SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c | > 6 +++++- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 18 > +++++++++++++++++- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 + > 4 files changed, 28 insertions(+), 3 deletions(-) > > diff --git > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > index 7a0f61efbb..0821159120 100644 > --- > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > +++ > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > @@ -230,13 +230,17 @@ RegisterHashInterfaceLib ( > { > UINTN Index; > UINT32 HashMask; > + UINT32 Tpm2HashMask; > EFI_STATUS Status; > > // > // Check allow > // > HashMask = Tpm2GetHashMaskFromAlgo (&HashInterface->HashGuid); > - if ((HashMask & PcdGet32 (PcdTpm2HashMask)) == 0) { > + Tpm2HashMask = PcdGet32 (PcdTpm2HashMask); > + > + if ((Tpm2HashMask != 0) && > + ((HashMask & Tpm2HashMask) == 0)) { > return EFI_UNSUPPORTED; > } > > diff --git > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > index 42cb562f67..6ae51dbce4 100644 > --- > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > +++ > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > @@ -327,13 +327,17 @@ RegisterHashInterfaceLib ( > UINTN Index; > HASH_INTERFACE_HOB *HashInterfaceHob; > UINT32 HashMask; > + UINT32 Tpm2HashMask; > EFI_STATUS Status; > > // > // Check allow > // > HashMask = Tpm2GetHashMaskFromAlgo (&HashInterface->HashGuid); > - if ((HashMask & PcdGet32 (PcdTpm2HashMask)) == 0) { > + Tpm2HashMask = PcdGet32 (PcdTpm2HashMask); > + > + if ((Tpm2HashMask != 0) && > + ((HashMask & Tpm2HashMask) == 0)) { > return EFI_UNSUPPORTED; > } > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > index 93a8803ff6..5ad6a45cf3 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > @@ -262,6 +262,7 @@ SyncPcrAllocationsAndPcrMask ( > { > EFI_STATUS Status; > EFI_TCG2_EVENT_ALGORITHM_BITMAP TpmHashAlgorithmBitmap; > + EFI_TCG2_EVENT_ALGORITHM_BITMAP BiosHashAlgorithmBitmap; > UINT32 TpmActivePcrBanks; > UINT32 NewTpmActivePcrBanks; > UINT32 Tpm2PcrMask; > @@ -273,16 +274,27 @@ SyncPcrAllocationsAndPcrMask ( > // Determine the current TPM support and the Platform PCR mask. > // > Status = Tpm2GetCapabilitySupportedAndActivePcrs > (&TpmHashAlgorithmBitmap, &TpmActivePcrBanks); > + > ASSERT_EFI_ERROR (Status); > + > + DEBUG ((EFI_D_INFO, "Tpm2GetCapabilitySupportedAndActivePcrs - > TpmHashAlgorithmBitmap: 0x%08x\n", TpmHashAlgorithmBitmap)); > + DEBUG ((EFI_D_INFO, "Tpm2GetCapabilitySupportedAndActivePcrs - > TpmActivePcrBanks 0x%08x\n", TpmActivePcrBanks)); > > Tpm2PcrMask = PcdGet32 (PcdTpm2HashMask); > if (Tpm2PcrMask == 0) { > // > // if PcdTPm2HashMask is zero, use ActivePcr setting > // > + DEBUG ((EFI_D_VERBOSE, "Initializing PcdTpm2HashMask to > TpmActivePcrBanks 0x%08x\n", TpmActivePcrBanks)); > PcdSet32S (PcdTpm2HashMask, TpmActivePcrBanks); > + DEBUG ((EFI_D_VERBOSE, "Initializing Tpm2PcrMask to TpmActivePcrBanks > 0x%08x\n", Tpm2PcrMask)); > Tpm2PcrMask = TpmActivePcrBanks; > } > + > + BiosHashAlgorithmBitmap = PcdGet32 (PcdTcg2HashAlgorithmBitmap); > + DEBUG ((EFI_D_INFO, "PcdTcg2HashAlgorithmBitmap 0x%08x\n", > BiosHashAlgorithmBitmap)); > + DEBUG ((EFI_D_INFO, "Tpm2PcrMask 0x%08x\n", Tpm2PcrMask)); // Active > PCR banks from TPM input > + DEBUG ((EFI_D_INFO, "TpmActivePcrBanks & BiosHashAlgorithmBitmap = > 0x%08x\n", NewTpmActivePcrBanks)); > > // > // Find the intersection of Pcd support and TPM support. > @@ -294,9 +306,12 @@ SyncPcrAllocationsAndPcrMask ( > // If there are active PCR banks that are not supported by the Platform mask, > // update the TPM allocations and reboot the machine. > // > - if ((TpmActivePcrBanks & Tpm2PcrMask) != TpmActivePcrBanks) { > + if (((TpmActivePcrBanks & Tpm2PcrMask) != TpmActivePcrBanks) || > + ((TpmActivePcrBanks & BiosHashAlgorithmBitmap) != TpmActivePcrBanks)) { > NewTpmActivePcrBanks = TpmActivePcrBanks & Tpm2PcrMask; > + NewTpmActivePcrBanks &= BiosHashAlgorithmBitmap; > > + DEBUG ((EFI_D_INFO, "NewTpmActivePcrBanks 0x%08x\n", > NewTpmActivePcrBanks)); > DEBUG ((EFI_D_INFO, "%a - Reallocating PCR banks from 0x%X to 0x%X.\n", > __FUNCTION__, TpmActivePcrBanks, NewTpmActivePcrBanks)); > if (NewTpmActivePcrBanks == 0) { > DEBUG ((EFI_D_ERROR, "%a - No viable PCRs active! Please set a less > restrictive value for PcdTpm2HashMask!\n", __FUNCTION__)); > @@ -331,6 +346,7 @@ SyncPcrAllocationsAndPcrMask ( > } > > Status = PcdSet32S (PcdTpm2HashMask, NewTpm2PcrMask); > + DEBUG ((EFI_D_INFO, "Setting PcdTpm2Hash Mask to 0x%08x\n", > NewTpm2PcrMask)); > ASSERT_EFI_ERROR (Status); > } > } > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > index 06c26a2904..17ad116126 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > @@ -86,6 +86,7 @@ > ## SOMETIMES_CONSUMES > ## SOMETIMES_PRODUCES > gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask > + gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap ## > CONSUMES > > [Depex] > gEfiPeiMasterBootModePpiGuid AND > -- > 2.31.1.windows.1
|
|
|
|
Re: [PATCH v6 1/6] OvmfPkg/BaseMemEncryptLib: Detect SEV live migration feature.
Gerd Hoffmann
Hi,
Nope. When you enable hyper-v emulation features you'll go find the kvmI still really don't understand the need for the CPUID loop. KVM only ever cpuid @ 0x40000000 and the hyper-v cpuid @ 0x40000100 (or the other way around, not sure). take care, Gerd
|
|
|
|
Re: [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()
Gerd Hoffmann
Hi,
insmod /lib/modules/5.13.8/kernel/fs/jbd2/jbd2.ko From the above, I think I've done everything possible to verify theRoot cause is not clear. Grab the kernel log ('dmesg') from a good and a bad boot and comparing them should give a clue. take care, Gerd
|
|
|
|
Re: [PATCH v2 2/2] UefiCpuPkg/BaseUefiCpuLib: Use toolchain-specific rodata section name
Ray, I made a detailed response about Mach-O with Xcode/clang and I don’t think patch works. Not sure if it breaks anything, but it puts things in the .data PE/COFF section. I’m also worried it is broken for any toolchain that generates ELF and use GenFw. I don’t think the GenFw tool creates a PE/COFF .rodata section [1] so if things work they will end up in the .data section, or things might break? Some one who knows that tool better than me should take a detailed look. I’m guessing it likely does the correct thing for toolchains that generate PE/COFF directly? My vote is to not add this feature until we can prove it works properly on all the toolchains. For Xcode it may be easier to just dump this stuff in the .text section (see my other mail for more background). It looks like we might have to modify GenFw if we want to create a .rodata section? It might be possible to cheat and use this concept to force code into the text section for ELF and Mach-O, but I’m not sure if that hits the correct security bar. But the last thing we want is to claim something is in a read only section when it is in a read write section. [1] git grep CreateSectionHeader BaseTools/Source/C/GenFw/Elf32Convert.c:602: CreateSectionHeader (".text", mTextOffset, mDataOffset - mTextOffset, BaseTools/Source/C/GenFw/Elf32Convert.c:612: CreateSectionHeader (".data", mDataOffset, mHiiRsrcOffset - mDataOffset, BaseTools/Source/C/GenFw/Elf32Convert.c:622: CreateSectionHeader (".rsrc", mHiiRsrcOffset, mRelocOffset - mHiiRsrcOffset, BaseTools/Source/C/GenFw/Elf32Convert.c:1107: CreateSectionHeader (".reloc", mRelocOffset, mCoffOffset - mRelocOffset, BaseTools/Source/C/GenFw/Elf64Convert.c:929: CreateSectionHeader (".text", mTextOffset, mDataOffset - mTextOffset, BaseTools/Source/C/GenFw/Elf64Convert.c:939: CreateSectionHeader (".data", mDataOffset, mHiiRsrcOffset - mDataOffset, BaseTools/Source/C/GenFw/Elf64Convert.c:949: CreateSectionHeader (".rsrc", mHiiRsrcOffset, mRelocOffset - mHiiRsrcOffset, BaseTools/Source/C/GenFw/Elf64Convert.c:1641: CreateSectionHeader (".reloc", mRelocOffset, mCoffOffset - mRelocOffset, BaseTools/Source/C/GenFw/ElfConvert.c:125:CreateSectionHeader ( BaseTools/Source/C/GenFw/ElfConvert.h:74:CreateSectionHeader ( Thanks, Andrew Fish
|
|
|
|
[edk2-platforms] [PATCH V1] MinPlatformPkg: Cleanup PeiFspWrapperHobProcessLib dependencies
PeiFspWrapperHobProcessLib is currently set to depens on
FspWrapperPlatformLib, but it does not use any of the functions implemented by that LibraryClass. This change removes that unneeded dependency. Cc: Chasel Chiu <chasel.chiu@...> Cc: Michael Kubacki <Michael.Kubacki@...> Cc: Benjamin Doron <benjamin.doron00@...> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@...> --- .../PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf index 64f3302959..b846e7af1d 100644 --- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/PeiFspWrapperHobProcessLib/PeiFspWrapperHobProcessLib.inf @@ -1,7 +1,7 @@ ## @file # Provide FSP wrapper hob process related function. # -# Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -55,7 +55,6 @@ BaseMemoryLib HobLib DebugLib - FspWrapperPlatformLib PeiServicesLib PeiServicesTablePointerLib -- 2.27.0.windows.1
|
|
|
|
[edk2-platforms] [PATCH V1] KabylakeSiliconPkg: Update SA_MISC_PEI_PREMEM_CONFIG
Updates SA_MISC_PEI_PREMEM_CONFIG from revision 1
to revision 3. Add initialization of the policy values. Cc: Chasel Chiu <chasel.chiu@...> Cc: Michael Kubacki <Michael.Kubacki@...> Cc: Benjamin Doron <benjamin.doron00@...> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@...> --- .../KabylakeRvp3/OpenBoardPkg.dsc | 24 +++--- .../PeiSiliconPolicyUpdateLib.c | 39 +++++++++- .../PeiSiliconPolicyUpdateLib.inf | 9 ++- .../ConfigBlock/SaMiscPeiPreMemConfig.h | 77 ++++++++++++++++++- .../Library/PeiSaPolicyLib/PeiSaPolicyLib.c | 37 ++++++++- 5 files changed, 169 insertions(+), 17 deletions(-) diff --git a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc index 8523ab3f4f..f64555e391 100644 --- a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc +++ b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc @@ -182,17 +182,6 @@ # Board-specific ####################################### PlatformHookLib|$(PROJECT)/Library/BasePlatformHookLib/BasePlatformHookLib.inf -!if gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection == 1 - # - # FSP API mode - # - SiliconPolicyUpdateLib|$(PROJECT)/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf -!else - # - # FSP Dispatch mode and non-FSP build (EDK2 build) - # - SiliconPolicyUpdateLib|$(PROJECT)/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf -!endif [LibraryClasses.IA32.SEC] ####################################### @@ -200,6 +189,7 @@ ####################################### TestPointCheckLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointCheckLib/SecTestPointCheckLib.inf SecBoardInitLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/SecBoardInitLibNull/SecBoardInitLibNull.inf + SiliconPolicyUpdateLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyUpdateLibNull/SiliconPolicyUpdateLibNull.inf [LibraryClasses.common.PEIM] ####################################### @@ -222,6 +212,18 @@ ####################################### # Board Package ####################################### +!if gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection == 1 + # + # FSP API mode + # + SiliconPolicyUpdateLib|$(PROJECT)/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf +!else + # + # FSP Dispatch mode and non-FSP build (EDK2 build) + # + SiliconPolicyUpdateLib|$(PROJECT)/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf +!endif + # Thunderbolt !if gKabylakeOpenBoardPkgTokenSpaceGuid.PcdTbtEnable == TRUE PeiDTbtInitLib|$(PLATFORM_BOARD_PACKAGE)/Features/Tbt/Library/Private/PeiDTbtInitLib/PeiDTbtInitLib.inf diff --git a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.c b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.c index 5cc7c03c61..2dce9be63c 100644 --- a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.c +++ b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.c @@ -1,7 +1,7 @@ /** @file Provides silicon policy update library functions. -Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2019 - 2021, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -398,6 +398,8 @@ SiliconPolicyUpdatePreMem ( SA_MISC_PEI_PREMEM_CONFIG *MiscPeiPreMemConfig; MEMORY_CONFIG_NO_CRC *MemConfigNoCrc; VOID *Buffer; + UINTN VariableSize; + VOID *MemorySavedData; UINT8 SpdAddressTable[4]; DEBUG((DEBUG_INFO, "\nUpdating Policy in Pre-Mem\n")); @@ -417,6 +419,41 @@ SiliconPolicyUpdatePreMem ( // Pass board specific SpdAddressTable to policy // CopyMem ((VOID *) MiscPeiPreMemConfig->SpdAddressTable, (VOID *) SpdAddressTable, (sizeof (UINT8) * 4)); + + // + // Set size of SMRAM + // + MiscPeiPreMemConfig->TsegSize = PcdGet32 (PcdTsegSize); + + // + // Initialize S3 Data variable (S3DataPtr). It may be used for warm and fast boot paths. + // Note: AmberLake FSP does not implement the FSPM_ARCH_CONFIG_PPI added in FSP 2.1, hence + // the platform specific S3DataPtr must be used instead. + // + VariableSize = 0; + MemorySavedData = NULL; + Status = PeiGetVariable ( + L"MemoryConfig", + &gFspNonVolatileStorageHobGuid, + &MemorySavedData, + &VariableSize + ); + DEBUG ((DEBUG_INFO, "Get L\"MemoryConfig\" gFspNonVolatileStorageHobGuid - %r\n", Status)); + DEBUG ((DEBUG_INFO, "MemoryConfig Size - 0x%x\n", VariableSize)); + if (!EFI_ERROR (Status)) { + MiscPeiPreMemConfig->S3DataPtr = MemorySavedData; + } + + // + // In FSP Dispatch Mode these BAR values are initialized by SiliconPolicyInitPreMem() in + // KabylakeSiliconPkg/Library/PeiSiliconPolicyInitLib/PeiPolicyInitPreMem.c; this function calls + // PEI_PREMEM_SI_DEFAULT_POLICY_INIT_PPI->PeiPreMemPolicyInit() to initialize all Config Blocks + // with default policy values (including these BAR values.) PEI_PREMEM_SI_DEFAULT_POLICY_INIT_PPI + // is implemented in the FSP. Make sure the value that FSP is using matches the value we are using. + // + ASSERT (PcdGet64 (PcdMchBaseAddress) <= 0xFFFFFFFF); + ASSERT (MiscPeiPreMemConfig->MchBar == (UINT32) PcdGet64 (PcdMchBaseAddress)); + ASSERT (MiscPeiPreMemConfig->SmbusBar == PcdGet16 (PcdSmbusBaseAddress)); } MemConfigNoCrc = NULL; Status = GetConfigBlock (Policy, &gMemoryConfigNoCrcGuid, (VOID *) &MemConfigNoCrc); diff --git a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf index 97ec70f611..5c2da68bf9 100644 --- a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf +++ b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf @@ -1,7 +1,7 @@ ### @file # Component information file for silicon policy update library # -# Copyright (c) 2019 - 2020 Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2019 - 2021 Intel Corporation. All rights reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -11,7 +11,7 @@ INF_VERSION = 0x00010005 BASE_NAME = PeiSiliconPolicyUpdateLib FILE_GUID = 14F5D83D-76A5-4241-BEC5-987E70E233D5 - MODULE_TYPE = BASE + MODULE_TYPE = PEIM VERSION_STRING = 1.0 LIBRARY_CLASS = SiliconPolicyUpdateLib @@ -33,6 +33,7 @@ [Packages] MinPlatformPkg/MinPlatformPkg.dec MdePkg/MdePkg.dec + IntelFsp2Pkg/IntelFsp2Pkg.dec UefiCpuPkg/UefiCpuPkg.dec KabylakeSiliconPkg/SiPkg.dec KabylakeOpenBoardPkg/OpenBoardPkg.dec @@ -49,11 +50,15 @@ gHsioPciePreMemConfigGuid ## CONSUMES gHsioSataPreMemConfigGuid ## CONSUMES gSaMiscPeiPreMemConfigGuid ## CONSUMES + gFspNonVolatileStorageHobGuid ## CONSUMES [Pcd] gSiPkgTokenSpaceGuid.PcdPeiMinMemorySize gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase gSiPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize + gSiPkgTokenSpaceGuid.PcdMchBaseAddress + gSiPkgTokenSpaceGuid.PcdSmbusBaseAddress + gSiPkgTokenSpaceGuid.PcdTsegSize gKabylakeOpenBoardPkgTokenSpaceGuid.PcdGraphicsVbtGuid gKabylakeOpenBoardPkgTokenSpaceGuid.PcdMrcRcompResistor ## CONSUMES gKabylakeOpenBoardPkgTokenSpaceGuid.PcdMrcRcompTarget ## CONSUMES diff --git a/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/Include/ConfigBlock/SaMiscPeiPreMemConfig.h b/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/Include/ConfigBlock/SaMiscPeiPreMemConfig.h index 4aa02e3142..2ed587f425 100644 --- a/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/Include/ConfigBlock/SaMiscPeiPreMemConfig.h +++ b/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/Include/ConfigBlock/SaMiscPeiPreMemConfig.h @@ -1,7 +1,7 @@ /** @file Policy details for miscellaneous configuration in System Agent -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -14,18 +14,91 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #define SA_MC_MAX_SOCKETS 4 #endif -#define SA_MISC_PEI_PREMEM_CONFIG_REVISION 1 +#define SA_MISC_PEI_PREMEM_CONFIG_REVISION 3 /** This configuration block is to configure SA Miscellaneous variables during PEI Pre-Mem phase like programming different System Agent BARs, TsegSize, IedSize, MmioSize required etc. <b>Revision 1</b>: - Initial version. + <b>Revision 2</b>: + - Added SgDelayAfterOffMethod, SgDelayAfterLinkEnable and SgGenSpeedChangeEnable. + <b>Revision 3</b>: + - Added BdatTestType and BdatSchema. **/ typedef struct { CONFIG_BLOCK_HEADER Header; ///< Offset 0-27 Config Block Header UINT8 SpdAddressTable[SA_MC_MAX_SOCKETS];///< Offset 28 Memory DIMMs' SPD address for reading SPD data. <b>example: SpdAddressTable[0]=0xA2(C0D0), SpdAddressTable[1]=0xA0(C0D1), SpdAddressTable[2]=0xA2(C1D0), SpdAddressTable[3]=0xA0(C1D1)</b> + VOID *S3DataPtr; ///< Offset 32 Memory data save pointer for S3 resume. The memory space should be allocated and filled with proper S3 resume data on a resume path UINT32 MchBar; ///< Offset 36 Address of System Agent MCHBAR: <b>0xFED10000</b> + UINT32 DmiBar; ///< Offset 40 Address of System Agent DMIBAR: <b>0xFED18000</b> + UINT32 EpBar; ///< Offset 44 Address of System Agent EPBAR: <b>0xFED19000</b> + UINT32 SmbusBar; ///< Offset 48 Address of System Agent SMBUS BAR: <b>0xEFA0</b> + UINT32 GdxcBar; ///< Offset 52 Address of System Agent GDXCBAR: <b>0xFED84000</b> + /** + Offset 56 Size of TSEG in bytes. (Must be power of 2) + <b>0x400000</b>: 4MB for Release build (When IED enabled, it will be 8MB) + 0x1000000 : 16MB for Debug build (Regardless IED enabled or disabled) + **/ + UINT32 TsegSize; + UINT32 EdramBar; ///< Offset 60 Address of System Agent EDRAMBAR: <b>0xFED80000</b> + /** + Offset 64 + <b>(Test)</b> Size of IED region in bytes. + <b>0</b> : IED Disabled (no memory occupied) + 0x400000 : 4MB SMM memory occupied by IED (Part of TSEG) + <b>Note: Enabling IED may also enlarge TsegSize together.</b> + **/ + UINT32 IedSize; + UINT8 UserBd; ///< Offset 68 <b>0=Mobile/Mobile Halo</b>, 1=Desktop/DT Halo, 5=ULT/ULX/Mobile Halo, 7=UP Server + UINT8 SgMode; ///< Offset 69 SgMode: <b>0=Disabled</b>, 1=SG Muxed, 2=SG Muxless, 3=PEG + UINT16 SgSubSystemId; ///< Offset 70 Switchable Graphics Subsystem ID: <b>2212</b> + UINT16 SgDelayAfterPwrEn; ///< Offset 72 Dgpu Delay after Power enable using Setup option: 0=Minimal, 1000=Maximum, <b>300=300 microseconds</b> + UINT16 SgDelayAfterHoldReset; ///< Offset 74 Dgpu Delay after Hold Reset using Setup option: 0=Minimal, 1000=Maximum, <b>100=100 microseconds</b> + UINT32 SkipExtGfxScan:1; ///< <b>(Test)</b> OFfset 76:0 :1=Skip External Gfx Device Scan; <b>0=Scan for external graphics devices</b>. Set this policy to skip External Graphics card scanning if the platform uses Internal Graphics only. + UINT32 BdatEnable:1; ///< <b>(Test)</b> OFfset 76:1 :This field enables the generation of the BIOS DATA ACPI Tables: <b>0=FALSE</b>, 1=TRUE\n Please refer to the MRC documentation for more details + UINT32 TxtImplemented:1; ///< OFfset 76:2 :This field currently is used to tell MRC if it should run after TXT initializatoin completed: <b>0=Run without waiting for TXT</b>, 1=Run after TXT initialization by callback + /** + Offset 76:3 : + <b>(Test)</b> Scan External Discrete Graphics Devices for Legacy Only VGA OpROMs + + When enabled, if the primary graphics device is an external discrete graphics device, Si will scan the + graphics device for legacy only VGA OpROMs. If the primary graphics device only implements legacy VBIOS, then the + LegacyOnlyVgaOpRomDetected field in the SA_DATA_HOB will be set to 1. + + This is intended to ease the implementation of a BIOS feature to automatically enable CSM if the Primary Gfx device + only supports Legacy VBIOS (No UEFI GOP Present). Otherwise disabling CSM won't result in no video being displayed. + This is useful for platforms that implement PCIe slots that allow the end user to install an arbitrary Gfx device. + + This setting will only take effect if SkipExtGfxScan == 0. It is ignored otherwise. + + - Disabled (0x0) : Don't Scan for Legacy Only VGA OpROMs (Default) + - <b>Enabled</b> (0x1) : Scan External Gfx for Legacy Only VGA OpROM + **/ + UINT32 ScanExtGfxForLegacyOpRom:1; + UINT32 RsvdBits0 :28; ///< OFfset 76:4 :Reserved for future use + UINT8 LockPTMregs; ///< <b>(Test)</b> Offset 80 Lock PCU Thermal Management registers: 0=FALSE, <b>1=TRUE</b> + UINT8 BdatTestType; ///< Offset 81 When BdatEnable is set to TRUE, this option selects the type of data which will be populated in the BIOS Data ACPI Tables: <b>0=RMT</b>, 1=RMT Per Bit, 2=Margin 2D. + UINT8 BdatSchema; ///< Offset 82 When BdatEnable is set to TRUE, this option selects the BDAT Schema version which will be used to format BDAT Test results: 0=Schema 2, <b>1=Schema 6B</b> + UINT8 Rsvd1; ///< Offset 83 Reserved for future use + /** + Offset 84 : + Size of reserved MMIO space for PCI devices\n + <b>0=AUTO</b>, 512=512MB, 768=768MB, 1024=1024MB, 1280=1280MB, 1536=1536MB, 1792=1792MB, + 2048=2048MB, 2304=2304MB, 2560=2560MB, 2816=2816MB, 3072=3072MB\n + When AUTO mode selected, the MMIO size will be calculated by required MMIO size from PCIe devices detected. + **/ + UINT16 MmioSize; + INT16 MmioSizeAdjustment; ///< Offset 86 Increase (given positive value) or Decrease (given negative value) the Reserved MMIO size when Dynamic Tolud/AUTO mode enabled (in MBs): <b>0=no adjustment</b> + UINT64 AcpiReservedMemoryBase; ///< Offset 88 The Base address of a Reserved memory buffer allocated in previous boot for S3 resume used. Originally it is retrieved from AcpiVariableCompatibility variable. + UINT64 SystemMemoryLength; ///< Offset 96 Total system memory length from previous boot, this is required for S3 resume. Originally it is retrieved from AcpiVariableCompatibility variable. + UINT32 AcpiReservedMemorySize; ///< Offset 104 The Size of a Reserved memory buffer allocated in previous boot for S3 resume used. Originally it is retrieved from AcpiVariableCompatibility variable. + UINT32 OpRomScanTempMmioBar; ///< <b>(Test)</b> Offset 108 Temporary address to MMIO map OpROMs during VGA scanning. Used for ScanExtGfxForLegacyOpRom feature. MUST BE 16MB ALIGNED! + UINT32 OpRomScanTempMmioLimit; ///< <b>(Test)</b> Offset 112 Limit address for OpROM MMIO range. Used for ScanExtGfxForLegacyOpRom feature. (OpROMScanTempMmioLimit - OpRomScanTempMmioBar) MUST BE >= 16MB! + UINT16 SgDelayAfterOffMethod; ///< Offset 128 Dgpu Delay after off method is called using Setup option: 0=Minimal, 1000=Maximum, <b>300=300 microseconds</b> + UINT16 SgDelayAfterLinkEnable; ///< Offset 130 Delay after link enable method is called using Setup option: 0=Minimal, 1000=Maximum, <b>100=100 microseconds</b> + UINT8 SgGenSpeedChangeEnable; ///< Offset 132 Enable/Disable Gen speed changes using Setup option: 0=Disable, 1=Enable + UINT8 Rsvd3[3]; ///< Offset 133 Reserved for future use } SA_MISC_PEI_PREMEM_CONFIG; #pragma pack(pop) diff --git a/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/Library/PeiSaPolicyLib/PeiSaPolicyLib.c b/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/Library/PeiSaPolicyLib/PeiSaPolicyLib.c index eb18d993e7..5210856346 100644 --- a/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/Library/PeiSaPolicyLib/PeiSaPolicyLib.c +++ b/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/Library/PeiSaPolicyLib/PeiSaPolicyLib.c @@ -1,7 +1,7 @@ /** @file This file provides services for PEI policy default initialization -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -19,6 +19,9 @@ extern EFI_GUID gMemoryConfigNoCrcGuid; extern EFI_GUID gGraphicsPeiConfigGuid; extern EFI_GUID gVtdConfigGuid; +#define DEFAULT_OPTION_ROM_TEMP_BAR 0x80000000 +#define DEFAULT_OPTION_ROM_TEMP_MEM_LIMIT 0xC0000000 + // // Function call to Load defaults for Individial IP Blocks // @@ -33,6 +36,38 @@ LoadSaMiscPeiPreMemDefault ( DEBUG ((DEBUG_INFO, "MiscPeiPreMemConfig->Header.GuidHob.Name = %g\n", &MiscPeiPreMemConfig->Header.GuidHob.Name)); DEBUG ((DEBUG_INFO, "MiscPeiPreMemConfig->Header.GuidHob.Header.HobLength = 0x%x\n", MiscPeiPreMemConfig->Header.GuidHob.Header.HobLength)); + + // + // Policy initialization commented out here is because it's the same with default 0 and no need to re-do again. + // + MiscPeiPreMemConfig->LockPTMregs = 1; + + // + // Initialize the Platform Configuration + // + MiscPeiPreMemConfig->MchBar = (UINT32) PcdGet64 (PcdMchBaseAddress); + MiscPeiPreMemConfig->DmiBar = 0xFED18000; + MiscPeiPreMemConfig->EpBar = 0xFED19000; + MiscPeiPreMemConfig->EdramBar = 0xFED80000; + MiscPeiPreMemConfig->SmbusBar = PcdGet16 (PcdSmbusBaseAddress); + MiscPeiPreMemConfig->TsegSize = PcdGet32 (PcdTsegSize); + MiscPeiPreMemConfig->GdxcBar = 0xFED84000; + + // + // Initialize the Switchable Graphics Default Configuration + // + MiscPeiPreMemConfig->SgDelayAfterHoldReset = 100; //100ms + MiscPeiPreMemConfig->SgDelayAfterPwrEn = 300; //300ms + MiscPeiPreMemConfig->SgDelayAfterOffMethod = 0; + MiscPeiPreMemConfig->SgDelayAfterLinkEnable = 0; + MiscPeiPreMemConfig->SgGenSpeedChangeEnable = 0; + + /// + /// Initialize the DataPtr for S3 resume + /// + MiscPeiPreMemConfig->S3DataPtr = NULL; + MiscPeiPreMemConfig->OpRomScanTempMmioBar = DEFAULT_OPTION_ROM_TEMP_BAR; + MiscPeiPreMemConfig->OpRomScanTempMmioLimit = DEFAULT_OPTION_ROM_TEMP_MEM_LIMIT; } VOID -- 2.27.0.windows.1
|
|
|
|
Re: [PATCH v2 1/2] BaseTools: Define the read-only data section name per toolchain
An EFI Mach-O file does not contain a .rodata section. A Mach-O contains a __DATA segment that is broken up into sections. For a typical EFI image there are __const, __data, __bss sections in the __DATA segment [1]. The mtoc [2] tool used to convert mach-O to PE/COFF converts the entire __DATA segment (__const, __data, and __bss) into the .data section. Thus adding any kind of new data section is a no-op at best. If you want something to be read only for Xcode/clang you are better off putting it in the __TEXT section [3]. The __TEXT section is read only and for X64 can not even contain relocations. [1] otool -lh DxeCore.dll ... Load command 1 cmd LC_SEGMENT_64 cmdsize 312 segname __DATA vmaddr 0x000000000002b000 vmsize 0x0000000000147000 fileoff 180224 filesize 8192 maxprot 0x00000003 initprot 0x00000003 nsects 3 flags 0x0 Section sectname __const segname __DATA addr 0x000000000002b000 size 0x0000000000000718 offset 180224 align 2^4 (16) reloff 0 nreloc 0 flags 0x00000000 reserved1 0 reserved2 0 Section sectname __data segname __DATA addr 0x000000000002b720 size 0x00000000000014f0 offset 182048 align 2^4 (16) reloff 0 nreloc 0 flags 0x00000000 reserved1 0 reserved2 0 Section sectname __bss segname __DATA addr 0x000000000002cc10 size 0x0000000000144e11 offset 0 align 2^4 (16) reloff 0 nreloc 0 flags 0x00000001 reserved1 0 reserved2 0 … [3] otool more output… Load command 0 cmd LC_SEGMENT_64 cmdsize 392 segname __TEXT vmaddr 0x0000000000000240 vmsize 0x00000000000296c0 fileoff 1184 filesize 169664 maxprot 0x00000005 initprot 0x00000005 nsects 4 flags 0x0 Section sectname __text segname __TEXT addr 0x0000000000000240 size 0x000000000002489f offset 1184 align 2^3 (8) reloff 0 nreloc 0 flags 0x80000400 reserved1 0 reserved2 0 Section sectname __cstring segname __TEXT addr 0x0000000000024ae0 size 0x000000000000496d offset 150848 align 2^4 (16) reloff 0 nreloc 0 flags 0x00000002 reserved1 0 reserved2 0 Section sectname __ustring segname __TEXT addr 0x000000000002944e size 0x0000000000000048 offset 169646 align 2^1 (2) reloff 0 nreloc 0 flags 0x00000000 reserved1 0 reserved2 0 Section sectname __const segname __TEXT addr 0x00000000000294a0 size 0x0000000000000448 offset 169728 align 2^4 (16) reloff 0 nreloc 0 flags 0x00000000 reserved1 0 reserved2 0 Thanks, Andrew Fish
|
|
|
|
Re: [PATCH EDK2 v2 1/1] MdeModulePkg/UefiSortLib:Add UefiSortLib unit test
Wu, Hao A
Sorry Mike,
toggle quoted messageShow quoted text
Do you have advice on how to deal with ECC reporting function naming issue on the 'main' function for unit test cases? So far, I think Wenyi has tried following the same pattern in file MdeModulePkg\Universal\Variable\RuntimeDxe\RuntimeDxeUnitTest\VariableLockRequestToLockUnitTest.c: /// /// Avoid ECC error for function name that starts with lower case letter /// #define Main main /** Standard POSIX C entry point for host based unit test execution. @param[in] Argc Number of arguments @param[in] Argv Array of pointers to arguments @retval 0 Success @retval other Error **/ INT32 Main ( IN INT32 Argc, IN CHAR8 *Argv[] ) But it looks like the ECC checker in the merge test is still complaining. Best Regards, Hao Wu
-----Original Message-----
|
|
|
|
[PATCH v2 6/6] Platform/RaspberryPi: Enable NVMe boot on CM4
Jeremy Linton
The CM4 has a number of carrier boards with PCIe
slots. With the PCIe changes in place its quite possible to utilize a NVMe root device. Lets allow people to boot from it. Reviewed-by: Andrei Warkentin <awarkentin@...> Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Platform/RaspberryPi/RPi4/RPi4.dsc | 5 +++++ Platform/RaspberryPi/RPi4/RPi4.fdf | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RP= i4/RPi4.dsc index babcbb2f41..25c29a0fbf 100644 --- a/Platform/RaspberryPi/RPi4/RPi4.dsc +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc @@ -754,6 +754,11 @@ } =20 # + # NVMe boot devices + # + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf + + # # UEFI application (Shell Embedded Boot Loader) # ShellPkg/Application/Shell/Shell.inf { diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RP= i4/RPi4.fdf index 3534cd3dc3..0c782d2f35 100644 --- a/Platform/RaspberryPi/RPi4/RPi4.fdf +++ b/Platform/RaspberryPi/RPi4/RPi4.fdf @@ -283,6 +283,11 @@ READ_LOCK_STATUS =3D TRUE INF EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.inf =20 # + # NVMe boot devices + # + INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf + + # # SCSI Bus and Disk Driver # INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf --=20 2.13.7
|
|
|
|
[PATCH v2 5/6] Silicon/Broadcom/Bcm27xx: Move linkup check into the cfg accessor
Jeremy Linton
The existing code fails to create/finish configuring the
pcie subsystem if it fails to get a linkup. This is reasonable on the RPi4 because it generally won't happen, and the OS could not see the root port. Now that the OS can see the root port, its a bit odd if it only shows up when something is plugged into the first slot. Lets move the link up check into the config accessor where it will be used to restrict sending CFG TLP's out the port when nothing is plugged in. Thus avoiding a SERROR during probe. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- .../Bcm2711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c | 5 -= ---- .../Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c | 7 += ++++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm= 2711PciHostBridgeLibConstructor.c b/Silicon/Broadcom/Bcm27xx/Library/Bcm2= 711PciHostBridgeLib/Bcm2711PciHostBridgeLibConstructor.c index 8587d2d36d..4d4c584726 100644 --- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711Pci= HostBridgeLibConstructor.c +++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciHostBridgeLib/Bcm2711Pci= HostBridgeLibConstructor.c @@ -204,11 +204,6 @@ Bcm2711PciHostBridgeLibConstructor ( } while (((Data & 0x30) !=3D 0x030) && (Timeout)); DEBUG ((DEBUG_VERBOSE, "PCIe link ready (status=3D%x) Timeout=3D%d\n",= Data, Timeout)); =20 - if ((Data & 0x30) !=3D 0x30) { - DEBUG ((DEBUG_ERROR, "PCIe link not ready (status=3D%x)\n", Data)); - return EFI_DEVICE_ERROR; - } - if ((Data & 0x80) !=3D 0x80) { DEBUG ((DEBUG_ERROR, "PCIe link not in RC mode (status=3D%x)\n", Dat= a)); return EFI_UNSUPPORTED; diff --git a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSeg= mentLib.c b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegm= entLib.c index 6d15e82fa2..b627e5730b 100644 --- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib= .c +++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib= .c @@ -105,6 +105,13 @@ PciSegmentLibGetConfigBase ( return 0xFFFFFFFF; } =20 + /* Don't probe slots if the link is down */ + Data =3D MmioRead32 (PCIE_REG_BASE + PCIE_MISC_PCIE_STATUS); + if ((Data & 0x30) !=3D 0x30) { + DEBUG ((DEBUG_ERROR, "PCIe link not ready (status=3D%x)\n", Da= ta)); + return 0xFFFFFFFF; + } + MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address); mPciSegmentLastAccess =3D Address; } --=20 2.13.7
|
|
|
|
[PATCH v2 4/6] Silicon/Broadcom/Bcm27xx: Relax PCIe device restriction
Jeremy Linton
The CM4 has an actual PCIe slot, so the device filtering
need to be a little less restrictive WRT busses with more than 1 device given that switches can now appear in the topology. Since it is possible to start numbering the busses with a non-zero value, the bus restriction should be based on the secondary side of the root port. This isn't likely but its better than hard-coding the limit. Suggested-by: René Treffer <treffer+groups.io@...> Signed-off-by: Jeremy Linton <jeremy.linton@...> --- .../Library/Bcm2711PciSegmentLib/PciSegmentLib.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c index 44ce3b4b99..6d15e82fa2 100644 --- a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c +++ b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c @@ -19,6 +19,7 @@ #include <Library/PciSegmentLib.h> #include <Library/UefiLib.h> #include <IndustryStandard/Bcm2711.h> +#include <IndustryStandard/Pci30.h> typedef enum { PciCfgWidthUint8 = 0, @@ -78,6 +79,9 @@ PciSegmentLibGetConfigBase ( UINT64 Base; UINT64 Offset; UINT32 Dev; + UINT32 Bus; + UINT32 Data; + UINT32 HostPortSec; Base = PCIE_REG_BASE; Offset = Address & 0xFFF; /* Pick off the 4k register offset */ @@ -89,17 +93,20 @@ PciSegmentLibGetConfigBase ( Base += PCIE_EXT_CFG_DATA; if (mPciSegmentLastAccess != Address) { Dev = EFI_PCI_ADDR_DEV (Address); + Bus = EFI_PCI_ADDR_BUS (Address); + HostPortSec = MmioRead8 (PCIE_REG_BASE + + PCI_BRIDGE_SECONDARY_BUS_REGISTER_OFFSET); + /* - * Scan things out directly rather than translating the "bus" to a device, etc.. - * only we need to limit each bus to a single device. + * There can only be a single device on bus 1 (downstream of root). + * Subsequent busses (behind a PCIe switch) can have more. */ - if (Dev < 1) { - MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address); - mPciSegmentLastAccess = Address; - } else { - mPciSegmentLastAccess = 0; + if (Dev > 0 && (Bus <= HostPortSec)) { return 0xFFFFFFFF; } + + MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address); + mPciSegmentLastAccess = Address; } } return Base + Offset; -- 2.13.7
|
|
|
|
[PATCH v2 3/6] Platform/RaspberryPi: Add PCIe SSDT
Jeremy Linton
Since we plan on toggling between XHCI and PCI the PCI
root needs to be in its own SSDT. This is all thats needed of UEFI. The SMC conduit is provided directly to the running OS. When the OS detects this PCIe port on a machine without a MCFG it attempts to connect to the SMC conduit. The RPi definition doesn't have any power mgmt, and only provides a description of the root port. Signed-off-by: Jeremy Linton <jeremy.linton@...> --- Platform/RaspberryPi/AcpiTables/AcpiTables.inf | 3 + Platform/RaspberryPi/AcpiTables/Pci.asl | 209 +++++++++++++++= ++++++ Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 6 + 3 files changed, 218 insertions(+) create mode 100644 Platform/RaspberryPi/AcpiTables/Pci.asl diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/Ra= spberryPi/AcpiTables/AcpiTables.inf index f3e8d950c1..da2a6db85f 100644 --- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf +++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf @@ -39,6 +39,7 @@ Pptt.aslc SsdtThermal.asl Xhci.asl + Pci.asl =20 [Packages] ArmPkg/ArmPkg.dec @@ -59,6 +60,8 @@ gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase gArmTokenSpaceGuid.PcdGicDistributorBase gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr + gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioAdr + gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen gBcm27xxTokenSpaceGuid.PcdBcm27xxPciRegBase gBcm27xxTokenSpaceGuid.PcdBcmGenetRegistersAddress gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress diff --git a/Platform/RaspberryPi/AcpiTables/Pci.asl b/Platform/Raspberry= Pi/AcpiTables/Pci.asl new file mode 100644 index 0000000000..31527d87b5 --- /dev/null +++ b/Platform/RaspberryPi/AcpiTables/Pci.asl @@ -0,0 +1,209 @@ +/** @file + * + * Copyright (c) 2019 Linaro, Limited. All rights reserved. + * Copyright (c) 2021 Arm + * + * SPDX-License-Identifier: BSD-2-Clause-Patent + * + **/ + +#include <IndustryStandard/Bcm2711.h> + +#include "AcpiTables.h" + +/* + * The following can be used to remove parenthesis from + * defined macros that the compiler complains about. + */ +#define ISOLATE_ARGS(...) __VA_ARGS__ +#define REMOVE_PARENTHESES(x) ISOLATE_ARGS x + +#define SANITIZED_PCIE_CPU_MMIO_WINDOW REMOVE_PARENTHESES(PCIE_CPU_MMIO= _WINDOW) +#define SANITIZED_PCIE_MMIO_LEN REMOVE_PARENTHESES(PCIE_BRIDGE_M= MIO_LEN) +#define SANITIZED_PCIE_PCI_MMIO_BEGIN REMOVE_PARENTHESES(PCIE_TOP_OF_M= EM_WIN) + +/* + * According to UEFI boot log for the VLI device on Pi 4. + */ +#define RT_REG_LENGTH 0x1000 + +DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI4PCIE", 2) +{ + Scope (\_SB_) + { + + Device (SCB0) { + Name (_HID, "ACPI0004") + Name (_UID, 0x1) + Name (_CCA, 0x0) + + Method (_CRS, 0, Serialized) { + // Container devices with _DMA must have _CRS,=20 + // meaning SCB0 to provide all resources that + // PCI0 consumes (except interrupts). + Name (RBUF, ResourceTemplate () { + QWordMemory (ResourceProducer, + , + MinFixed, + MaxFixed, + NonCacheable, + ReadWrite, + 0x0, + SANITIZED_PCIE_CPU_MMIO_WINDOW, // MIN + SANITIZED_PCIE_CPU_MMIO_WINDOW, // MAX + 0x0, + 0x1, // LEN + , + , + MMIO + ) + }) + CreateQwordField (RBUF, MMIO._MAX, MMBE) + CreateQwordField (RBUF, MMIO._LEN, MMLE) + Add (MMBE, RT_REG_LENGTH - 1, MMBE) + Add (MMLE, RT_REG_LENGTH - 1, MMLE) + Return (RBUF) + } + + Name (_DMA, ResourceTemplate() { + // PCIe can only DMA to first 3GB with early SOC's + // But we keep the restriction on the later ones + // To avoid DMA translation problems. + QWordMemory (ResourceProducer, + , + MinFixed, + MaxFixed, + NonCacheable, + ReadWrite, + 0x0, + 0x0, // MIN + 0xbfffffff, // MAX + 0x0, // TRA + 0xc0000000, // LEN + , + , + ) + }) + + + Device(PCI0) + { + Name(_HID, EISAID("PNP0A08")) // PCI Express Root Bridge + Name(_CID, EISAID("PNP0A03")) // Compatible PCI Root Bridge + Name(_SEG, Zero) // PCI Segment Group number + Name(_BBN, Zero) // PCI Base Bus Number + Name(_CCA, 0) // Mark the PCI noncoherent + + // Root Complex 0 + Device (RP0) { + Name(_ADR, 0xF0000000) // Dev 0, Func 0 + } + + Name (_DMA, ResourceTemplate() { + QWordMemory (ResourceConsumer, + , + MinFixed, + MaxFixed, + NonCacheable, + ReadWrite, + 0x0, + 0x0, // MIN + 0xbfffffff, // MAX + 0x0, // TRA + 0xc0000000, // LEN + , + , + ) + }) + + // PCI Routing Table + Name(_PRT, Package() { + Package (4) { 0x0000FFFF, 0, zero, 175 }, + Package (4) { 0x0000FFFF, 1, zero, 176 }, + Package (4) { 0x0000FFFF, 2, zero, 177 }, + Package (4) { 0x0000FFFF, 3, zero, 178 } + }) + + // Root complex resources + Method (_CRS, 0, Serialized) { + Name (RBUF, ResourceTemplate () { + WordBusNumber ( // Bus numbers assigned to this root + ResourceProducer, + MinFixed, MaxFixed, PosDecode, + 0, // AddressGranularity + 0, // AddressMinimum - Minimum Bus Number + 255, // AddressMaximum - Maximum Bus Number + 0, // AddressTranslation - Set to 0 + 256 // RangeLength - Number of Busses + ) + + QWordMemory ( // 32-bit BAR Windows in 64-bit addr + ResourceProducer, PosDecode, + MinFixed, MaxFixed, + NonCacheable, ReadWrite, //cacheable? is that right= ? + 0x00000000, // Granularity + 0, // SANITIZED_PCIE_PCI_MMIO= _BEGIN + 1, // SANITIZED_PCIE_MMIO_LEN= + SANITIZED_PCIE_PCI_MMIO_BEGIN + SANITIZED_PCIE_CPU_MMIO_WINDOW, // SANITIZED_PCIE_PCI_MMIO= _BEGIN - SANITIZED_PCIE_CPU_MMIO_WINDOW + 2 // SANITIZED_PCIE_MMIO_LEN= + 1 + ,,,MMI1,,TypeTranslation + ) + }) // end Name(RBUF) + + // Work around ASL's inability to add in a resource definition + // or for that matter compute the min,max,len properly + CreateQwordField (RBUF, MMI1._MIN, MMIB) + CreateQwordField (RBUF, MMI1._MAX, MMIE) + CreateQwordField (RBUF, MMI1._TRA, MMIT) + CreateQwordField (RBUF, MMI1._LEN, MMIL) + Add (MMIB, SANITIZED_PCIE_PCI_MMIO_BEGIN, MMIB) + Add (SANITIZED_PCIE_MMIO_LEN, SANITIZED_PCIE_PCI_MMIO_BEGIN, M= MIE) + Subtract (MMIT, SANITIZED_PCIE_PCI_MMIO_BEGIN, MMIT) + Add (SANITIZED_PCIE_MMIO_LEN, 1 , MMIL) + + Return (RBUF) + } // end Method(_CRS) + // + // OS Control Handoff + // + Name(SUPP, Zero) // PCI _OSC Support Field value + Name(CTRL, Zero) // PCI _OSC Control Field value + + // See [1] 6.2.10, [2] 4.5 + Method(_OSC,4) { + // Note, This code is very similar to the code in the PCIe fir= mware + // specification which can be used as a reference + // Check for proper UUID + If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))= ) { + // Create DWord-adressable fields from the Capabilities Buff= er + CreateDWordField(Arg3,0,CDW1) + CreateDWordField(Arg3,4,CDW2) + CreateDWordField(Arg3,8,CDW3) + // Save Capabilities DWord2 & 3 + Store(CDW2,SUPP) + Store(CDW3,CTRL) + // Mask out Native HotPlug + And(CTRL,0x1E,CTRL) + // Always allow native PME, AER (no dependencies) + // Never allow SHPC (no SHPC controller in this system) + And(CTRL,0x1D,CTRL) + + If(LNotEqual(Arg1,One)) { // Unknown revision + Or(CDW1,0x08,CDW1) + } + + If(LNotEqual(CDW3,CTRL)) { // Capabilities bits were masked + Or(CDW1,0x10,CDW1) + } + // Update DWORD3 in the buffer + Store(CTRL,CDW3) + Return(Arg3) + } Else { + Or(CDW1,4,CDW1) // Unrecognized UUID + Return(Arg3) + } + } // End _OSC + } // PCI0 + } //end SCB0 + } //end scope sb +} //end definition block diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platfor= m/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c index 7c5786303d..4c40820858 100644 --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c @@ -821,6 +821,12 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] =3D { PcdToken(PcdXhciPci), NULL }, + { + SIGNATURE_64 ('R', 'P', 'I', '4', 'P', 'C', 'I', 'E'), + PcdToken(PcdXhciPci), + 0, + NULL + }, #endif { // DSDT SIGNATURE_64 ('R', 'P', 'I', 0, 0, 0, 0, 0), --=20 2.13.7
|
|
|