Date   

Re: [PATCH v1] MdePkg/Cpuid.h: Define new element in CPUID Leaf(07h) data structure.

Jason Lou
 

Got it, the update will be included in the new patch(v3).

Thanks!
Jason Lou


Re: [PATCH 0/3] SD+USB perf/DMA fixes

Andrei Warkentin
 

I think Linux's behavior needs to be reconciled with the ACPI spec, which uses _DMA with ResourceConsumer, not ResourceProducer.

A


From: Jeremy Linton <jeremy.linton@...>
Sent: Thursday, April 8, 2021 12:58 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@... <ard.biesheuvel@...>; leif@... <leif@...>; pete@... <pete@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Andrei Warkentin <awarkentin@...>; Jeremy Linton <jeremy.linton@...>
Subject: [PATCH 0/3] SD+USB perf/DMA fixes
 
A large part of why the emmc & dwc2 usb
controllers haven't been working properly is
because the "bus" _DMA was incorrectly tagged
as a consumer, when it needs to be a producer.

That is why linux has been dropping the
translation value portions of _DMA().

Since the emmc2 dma (with the old B0 SoC), and the
dwc2 is expected to work, lets add matching 30 bit
IORT entries for them.

Finally, in the shuffle the high speed cap bit override
was dropped from the linux patches, and I failed
to add it back to the firmware values, this caused
the wifi perf to be lower than it should have been.

Jeremy Linton (3):
  Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode
  Platform/RaspberryPi/AcpiTables: Add further named components
  Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

 Platform/RaspberryPi/AcpiTables/Dsdt.asl  |  2 +-
 Platform/RaspberryPi/AcpiTables/Emmc.asl  |  2 +-
 Platform/RaspberryPi/AcpiTables/Iort.aslc | 44 ++++++++++++++++++++++++++++++-
 Platform/RaspberryPi/AcpiTables/Sdhc.asl  |  2 +-
 4 files changed, 46 insertions(+), 4 deletions(-)

--
2.13.7


Re: [PATCH 4/4] UefiCpuPkg/MpInitLib: Consume MicrocodeLib to remove duplicated code

Dong, Eric
 

Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, April 2, 2021 1:58 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: [PATCH 4/4] UefiCpuPkg/MpInitLib: Consume MicrocodeLib to remove duplicated code

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/Microcode.c | 484 ++++--------------
UefiCpuPkg/Library/MpInitLib/MpLib.h | 1 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
4 files changed, 96 insertions(+), 391 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 860a9750e2..d34419c2a5 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -52,6 +52,7 @@ [LibraryClasses]
SynchronizationLib

PcdLib

VmgExitLib

+ MicrocodeLib



[Protocols]

gEfiTimerArchProtocolGuid ## SOMETIMES_CONSUMES

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 297c2abcd1..105a9f84bf 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -1,70 +1,16 @@
/** @file

Implementation of loading microcode on processors.



- Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>

+ Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/



#include "MpLib.h"



-/**

- Get microcode update signature of currently loaded microcode update.

-

- @return Microcode signature.

-**/

-UINT32

-GetCurrentMicrocodeSignature (

- VOID

- )

-{

- MSR_IA32_BIOS_SIGN_ID_REGISTER BiosSignIdMsr;

-

- AsmWriteMsr64 (MSR_IA32_BIOS_SIGN_ID, 0);

- AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, NULL);

- BiosSignIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_BIOS_SIGN_ID);

- return BiosSignIdMsr.Bits.MicrocodeUpdateSignature;

-}

-

/**

Detect whether specified processor can find matching microcode patch and load it.



- Microcode Payload as the following format:

- +----------------------------------------+------------------+

- | CPU_MICROCODE_HEADER | |

- +----------------------------------------+ CheckSum Part1 |

- | Microcode Binary | |

- +----------------------------------------+------------------+

- | CPU_MICROCODE_EXTENDED_TABLE_HEADER | |

- +----------------------------------------+ CheckSum Part2 |

- | CPU_MICROCODE_EXTENDED_TABLE | |

- | ... | |

- +----------------------------------------+------------------+

-

- There may by multiple CPU_MICROCODE_EXTENDED_TABLE in this format.

- The count of CPU_MICROCODE_EXTENDED_TABLE is indicated by ExtendedSignatureCount

- of CPU_MICROCODE_EXTENDED_TABLE_HEADER structure.

-

- When we are trying to verify the CheckSum32 with extended table.

- We should use the fields of exnteded table to replace the corresponding

- fields in CPU_MICROCODE_HEADER structure, and recalculate the

- CheckSum32 with CPU_MICROCODE_HEADER + Microcode Binary. We named

- it as CheckSum Part3.

-

- The CheckSum Part2 is used to verify the CPU_MICROCODE_EXTENDED_TABLE_HEADER

- and CPU_MICROCODE_EXTENDED_TABLE parts. We should make sure CheckSum Part2

- is correct before we are going to verify each CPU_MICROCODE_EXTENDED_TABLE.

-

- Only ProcessorSignature, ProcessorFlag and CheckSum are different between

- CheckSum Part1 and CheckSum Part3. To avoid multiple computing CheckSum Part3.

- Save an in-complete CheckSum32 from CheckSum Part1 for common parts.

- When we are going to calculate CheckSum32, just should use the corresponding part

- of the ProcessorSignature, ProcessorFlag and CheckSum with in-complete CheckSum32.

-

- Notes: CheckSum32 is not a strong verification.

- It does not guarantee that the data has not been modified.

- CPU has its own mechanism to verify Microcode Binary part.

-

@param[in] CpuMpData The pointer to CPU MP Data structure.

@param[in] ProcessorNumber The handle number of the processor. The range is

from 0 to the total number of logical processors

@@ -76,26 +22,13 @@ MicrocodeDetect (
IN UINTN ProcessorNumber

)

{

- UINT32 ExtendedTableLength;

- UINT32 ExtendedTableCount;

- CPU_MICROCODE_EXTENDED_TABLE *ExtendedTable;

- CPU_MICROCODE_EXTENDED_TABLE_HEADER *ExtendedTableHeader;

- CPU_MICROCODE_HEADER *MicrocodeEntryPoint;

+ CPU_MICROCODE_HEADER *Microcode;

UINTN MicrocodeEnd;

- UINTN Index;

- UINT8 PlatformId;

- CPUID_VERSION_INFO_EAX Eax;

- CPU_AP_DATA *CpuData;

- UINT32 CurrentRevision;

+ CPU_AP_DATA *BspData;

UINT32 LatestRevision;

- UINTN TotalSize;

- UINT32 CheckSum32;

- UINT32 InCompleteCheckSum32;

- BOOLEAN CorrectMicrocode;

- VOID *MicrocodeData;

- MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;

+ CPU_MICROCODE_HEADER *LatestMicrocode;

UINT32 ThreadId;

- BOOLEAN IsBspCallIn;

+ EDKII_PEI_MICROCODE_CPU_ID MicrocodeCpuId;



if (CpuMpData->MicrocodePatchRegionSize == 0) {

//

@@ -104,9 +37,6 @@ MicrocodeDetect (
return;

}



- CurrentRevision = GetCurrentMicrocodeSignature ();

- IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ? TRUE : FALSE;

-

GetProcessorLocationByApicId (GetInitialApicId (), NULL, NULL, &ThreadId);

if (ThreadId != 0) {

//

@@ -115,156 +45,35 @@ MicrocodeDetect (
return;

}



- ExtendedTableLength = 0;

- //

- // Here data of CPUID leafs have not been collected into context buffer, so

- // GetProcessorCpuid() cannot be used here to retrieve CPUID data.

- //

- AsmCpuid (CPUID_VERSION_INFO, &Eax.Uint32, NULL, NULL, NULL);

+ GetProcessorMicrocodeCpuId (&MicrocodeCpuId);



- //

- // The index of platform information resides in bits 50:52 of MSR IA32_PLATFORM_ID

- //

- PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);

- PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;

-

-

- //

- // Check whether AP has same processor with BSP.

- // If yes, direct use microcode info saved by BSP.

- //

- if (!IsBspCallIn) {

+ if (ProcessorNumber != (UINTN) CpuMpData->BspNumber) {

//

- // Get the CPU data for BSP

+ // Direct use microcode of BSP if AP is the same as BSP.

+ // Assume BSP calls this routine() before AP.

//

- CpuData = &(CpuMpData->CpuData[CpuMpData->BspNumber]);

- if ((CpuData->ProcessorSignature == Eax.Uint32) &&

- (CpuData->PlatformId == PlatformId) &&

- (CpuData->MicrocodeEntryAddr != 0)) {

- MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *)(UINTN) CpuData->MicrocodeEntryAddr;

- MicrocodeData = (VOID *) (MicrocodeEntryPoint + 1);

- LatestRevision = MicrocodeEntryPoint->UpdateRevision;

- goto Done;

+ BspData = &(CpuMpData->CpuData[CpuMpData->BspNumber]);

+ if ((BspData->ProcessorSignature == MicrocodeCpuId.ProcessorSignature) &&

+ (BspData->PlatformId == MicrocodeCpuId.PlatformId) &&

+ (BspData->MicrocodeEntryAddr != 0)) {

+ LatestMicrocode = (CPU_MICROCODE_HEADER *)(UINTN) BspData->MicrocodeEntryAddr;

+ LatestRevision = LatestMicrocode->UpdateRevision;

+ goto LoadMicrocode;

}

}



- LatestRevision = 0;

- MicrocodeData = NULL;

- MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress + CpuMpData->MicrocodePatchRegionSize);

- MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN) CpuMpData->MicrocodePatchAddress;

+ //

+ // BSP or AP which is different from BSP runs here

+ // Use 0 as the starting revision to search for microcode because MicrocodePatchInfo HOB needs

+ // the latest microcode location even it's loaded to the processor.

+ //

+ LatestRevision = 0;

+ LatestMicrocode = NULL;

+ Microcode = (CPU_MICROCODE_HEADER *) (UINTN) CpuMpData->MicrocodePatchAddress;

+ MicrocodeEnd = (UINTN) Microcode + (UINTN) CpuMpData->MicrocodePatchRegionSize;



do {

- //

- // Check if the microcode is for the Cpu and the version is newer

- // and the update can be processed on the platform

- //

- CorrectMicrocode = FALSE;

-

- if (MicrocodeEntryPoint->DataSize == 0) {

- TotalSize = sizeof (CPU_MICROCODE_HEADER) + 2000;

- } else {

- TotalSize = sizeof (CPU_MICROCODE_HEADER) + MicrocodeEntryPoint->DataSize;

- }

-

- ///

- /// 0x0 MicrocodeBegin MicrocodeEntry MicrocodeEnd 0xffffffff

- /// |--------------|---------------|---------------|---------------|

- /// valid TotalSize

- /// TotalSize is only valid between 0 and (MicrocodeEnd - MicrocodeEntry).

- /// And it should be aligned with 4 bytes.

- /// If the TotalSize is invalid, skip 1KB to check next entry.

- ///

- if ( (UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize) ||

- ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||

- (TotalSize & 0x3) != 0

- ) {

- MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);

- continue;

- }

-

- //

- // Save an in-complete CheckSum32 from CheckSum Part1 for common parts.

- //

- InCompleteCheckSum32 = CalculateSum32 (

- (UINT32 *) MicrocodeEntryPoint,

- TotalSize

- );

- InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorSignature.Uint32;

- InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;

- InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;

-

- if (MicrocodeEntryPoint->HeaderVersion == 0x1) {

- //

- // It is the microcode header. It is not the padding data between microcode patches

- // because the padding data should not include 0x00000001 and it should be the repeated

- // byte format (like 0xXYXYXYXY....).

- //

- if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&

- MicrocodeEntryPoint->UpdateRevision > LatestRevision &&

- (MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))

- ) {

- //

- // Calculate CheckSum Part1.

- //

- CheckSum32 = InCompleteCheckSum32;

- CheckSum32 += MicrocodeEntryPoint->ProcessorSignature.Uint32;

- CheckSum32 += MicrocodeEntryPoint->ProcessorFlags;

- CheckSum32 += MicrocodeEntryPoint->Checksum;

- if (CheckSum32 == 0) {

- CorrectMicrocode = TRUE;

- }

- } else if ((MicrocodeEntryPoint->DataSize != 0) &&

- (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {

- ExtendedTableLength = MicrocodeEntryPoint->TotalSize - (MicrocodeEntryPoint->DataSize +

- sizeof (CPU_MICROCODE_HEADER));

- if (ExtendedTableLength != 0) {

- //

- // Extended Table exist, check if the CPU in support list

- //

- ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER *) ((UINT8 *) (MicrocodeEntryPoint)

- + MicrocodeEntryPoint->DataSize + sizeof (CPU_MICROCODE_HEADER));

- //

- // Calculate Extended Checksum

- //

- if ((ExtendedTableLength % 4) == 0) {

- //

- // Calculate CheckSum Part2.

- //

- CheckSum32 = CalculateSum32 ((UINT32 *) ExtendedTableHeader, ExtendedTableLength);

- if (CheckSum32 == 0) {

- //

- // Checksum correct

- //

- ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;

- ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *) (ExtendedTableHeader + 1);

- for (Index = 0; Index < ExtendedTableCount; Index ++) {

- //

- // Calculate CheckSum Part3.

- //

- CheckSum32 = InCompleteCheckSum32;

- CheckSum32 += ExtendedTable->ProcessorSignature.Uint32;

- CheckSum32 += ExtendedTable->ProcessorFlag;

- CheckSum32 += ExtendedTable->Checksum;

- if (CheckSum32 == 0) {

- //

- // Verify Header

- //

- if ((ExtendedTable->ProcessorSignature.Uint32 == Eax.Uint32) &&

- (ExtendedTable->ProcessorFlag & (1 << PlatformId)) ) {

- //

- // Find one

- //

- CorrectMicrocode = TRUE;

- break;

- }

- }

- ExtendedTable ++;

- }

- }

- }

- }

- }

- } else {

+ if (!IsValidMicrocode (Microcode, MicrocodeEnd - (UINTN) Microcode, LatestRevision, &MicrocodeCpuId, 1, TRUE)) {

//

// It is the padding data between the microcode patches for microcode patches alignment.

// Because the microcode patch is the multiple of 1-KByte, the padding data should not

@@ -272,156 +81,40 @@ MicrocodeDetect (
// alignment value should be larger than 1-KByte. We could skip SIZE_1KB padding data to

// find the next possible microcode patch header.

//

- MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);

+ Microcode = (CPU_MICROCODE_HEADER *) ((UINTN) Microcode + SIZE_1KB);

continue;

}

- //

- // Get the next patch.

- //

- if (MicrocodeEntryPoint->DataSize == 0) {

- TotalSize = 2048;

- } else {

- TotalSize = MicrocodeEntryPoint->TotalSize;

- }

+ LatestMicrocode = Microcode;

+ LatestRevision = LatestMicrocode->UpdateRevision;



- if (CorrectMicrocode) {

- LatestRevision = MicrocodeEntryPoint->UpdateRevision;

- MicrocodeData = (VOID *) ((UINTN) MicrocodeEntryPoint + sizeof (CPU_MICROCODE_HEADER));

- }

-

- MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + TotalSize);

- } while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));

+ Microcode = (CPU_MICROCODE_HEADER *) (((UINTN) Microcode) + GetMicrocodeLength (Microcode));

+ } while ((UINTN) Microcode < MicrocodeEnd);



-Done:

+LoadMicrocode:

if (LatestRevision != 0) {

//

- // Save the detected microcode patch entry address (including the

- // microcode patch header) for each processor.

+ // Save the detected microcode patch entry address (including the microcode

+ // patch header) for each processor even it's the same as the loaded one.

// It will be used when building the microcode patch cache HOB.

//

- CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr =

- (UINTN) MicrocodeData - sizeof (CPU_MICROCODE_HEADER);

+ CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr = (UINTN) LatestMicrocode;

}



- if (LatestRevision > CurrentRevision) {

+ if (LatestRevision > GetProcessorMicrocodeSignature ()) {

//

// BIOS only authenticate updates that contain a numerically larger revision

// than the currently loaded revision, where Current Signature < New Update

// Revision. A processor with no loaded update is considered to have a

// revision equal to zero.

//

- ASSERT (MicrocodeData != NULL);

- AsmWriteMsr64 (

- MSR_IA32_BIOS_UPDT_TRIG,

- (UINT64) (UINTN) MicrocodeData

- );

- }

- CpuMpData->CpuData[ProcessorNumber].MicrocodeRevision = GetCurrentMicrocodeSignature ();

-}

-

-/**

- Determine if a microcode patch matchs the specific processor signature and flag.

-

- @param[in] CpuMpData The pointer to CPU MP Data structure.

- @param[in] ProcessorSignature The processor signature field value

- supported by a microcode patch.

- @param[in] ProcessorFlags The prcessor flags field value supported by

- a microcode patch.

-

- @retval TRUE The specified microcode patch will be loaded.

- @retval FALSE The specified microcode patch will not be loaded.

-**/

-BOOLEAN

-IsProcessorMatchedMicrocodePatch (

- IN CPU_MP_DATA *CpuMpData,

- IN UINT32 ProcessorSignature,

- IN UINT32 ProcessorFlags

- )

-{

- UINTN Index;

- CPU_AP_DATA *CpuData;

-

- for (Index = 0; Index < CpuMpData->CpuCount; Index++) {

- CpuData = &CpuMpData->CpuData[Index];

- if ((ProcessorSignature == CpuData->ProcessorSignature) &&

- (ProcessorFlags & (1 << CpuData->PlatformId)) != 0) {

- return TRUE;

- }

+ LoadMicrocode (LatestMicrocode);

}

-

- return FALSE;

-}

-

-/**

- Check the 'ProcessorSignature' and 'ProcessorFlags' of the microcode

- patch header with the CPUID and PlatformID of the processors within

- system to decide if it will be copied into memory.

-

- @param[in] CpuMpData The pointer to CPU MP Data structure.

- @param[in] MicrocodeEntryPoint The pointer to the microcode patch header.

-

- @retval TRUE The specified microcode patch need to be loaded.

- @retval FALSE The specified microcode patch dosen't need to be loaded.

-**/

-BOOLEAN

-IsMicrocodePatchNeedLoad (

- IN CPU_MP_DATA *CpuMpData,

- CPU_MICROCODE_HEADER *MicrocodeEntryPoint

- )

-{

- BOOLEAN NeedLoad;

- UINTN DataSize;

- UINTN TotalSize;

- CPU_MICROCODE_EXTENDED_TABLE_HEADER *ExtendedTableHeader;

- UINT32 ExtendedTableCount;

- CPU_MICROCODE_EXTENDED_TABLE *ExtendedTable;

- UINTN Index;

-

//

- // Check the 'ProcessorSignature' and 'ProcessorFlags' in microcode patch header.

+ // It's possible that the microcode fails to load. Just capture the CPU microcode revision after loading.

//

- NeedLoad = IsProcessorMatchedMicrocodePatch (

- CpuMpData,

- MicrocodeEntryPoint->ProcessorSignature.Uint32,

- MicrocodeEntryPoint->ProcessorFlags

- );

-

- //

- // If the Extended Signature Table exists, check if the processor is in the

- // support list

- //

- DataSize = MicrocodeEntryPoint->DataSize;

- TotalSize = (DataSize == 0) ? 2048 : MicrocodeEntryPoint->TotalSize;

- if ((!NeedLoad) && (DataSize != 0) &&

- (TotalSize - DataSize > sizeof (CPU_MICROCODE_HEADER) +

- sizeof (CPU_MICROCODE_EXTENDED_TABLE_HEADER))) {

- ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER *) ((UINT8 *) (MicrocodeEntryPoint)

- + DataSize + sizeof (CPU_MICROCODE_HEADER));

- ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;

- ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *) (ExtendedTableHeader + 1);

-

- for (Index = 0; Index < ExtendedTableCount; Index ++) {

- //

- // Check the 'ProcessorSignature' and 'ProcessorFlag' of the Extended

- // Signature Table entry with the CPUID and PlatformID of the processors

- // within system to decide if it will be copied into memory

- //

- NeedLoad = IsProcessorMatchedMicrocodePatch (

- CpuMpData,

- ExtendedTable->ProcessorSignature.Uint32,

- ExtendedTable->ProcessorFlag

- );

- if (NeedLoad) {

- break;

- }

- ExtendedTable ++;

- }

- }

-

- return NeedLoad;

+ CpuMpData->CpuData[ProcessorNumber].MicrocodeRevision = GetProcessorMicrocodeSignature ();

}



-

/**

Actual worker function that shadows the required microcode patches into memory.



@@ -491,14 +184,16 @@ ShadowMicrocodePatchByPcd (
IN OUT CPU_MP_DATA *CpuMpData

)

{

+ UINTN Index;

CPU_MICROCODE_HEADER *MicrocodeEntryPoint;

UINTN MicrocodeEnd;

- UINTN DataSize;

UINTN TotalSize;

MICROCODE_PATCH_INFO *PatchInfoBuffer;

UINTN MaxPatchNumber;

UINTN PatchCount;

UINTN TotalLoadSize;

+ EDKII_PEI_MICROCODE_CPU_ID *MicrocodeCpuIds;

+ BOOLEAN Valid;



//

// Initialize the microcode patch related fields in CpuMpData as the values

@@ -526,12 +221,34 @@ ShadowMicrocodePatchByPcd (
return;

}



+ MicrocodeCpuIds = AllocatePages (

+ EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * sizeof (EDKII_PEI_MICROCODE_CPU_ID))

+ );

+ if (MicrocodeCpuIds == NULL) {

+ FreePool (PatchInfoBuffer);

+ return;

+ }

+

+ for (Index = 0; Index < CpuMpData->CpuCount; Index++) {

+ MicrocodeCpuIds[Index].PlatformId = CpuMpData->CpuData[Index].PlatformId;

+ MicrocodeCpuIds[Index].ProcessorSignature = CpuMpData->CpuData[Index].ProcessorSignature;

+ }

+

//

// Process the header of each microcode patch within the region.

// The purpose is to decide which microcode patch(es) will be loaded into memory.

+ // Microcode checksum is not verified because it's slow when performing on flash.

//

do {

- if (MicrocodeEntryPoint->HeaderVersion != 0x1) {

+ Valid = IsValidMicrocode (

+ MicrocodeEntryPoint,

+ MicrocodeEnd - (UINTN) MicrocodeEntryPoint,

+ 0,

+ MicrocodeCpuIds,

+ CpuMpData->CpuCount,

+ FALSE

+ );

+ if (!Valid) {

//

// Padding data between the microcode patches, skip 1KB to check next entry.

//

@@ -539,59 +256,44 @@ ShadowMicrocodePatchByPcd (
continue;

}



- DataSize = MicrocodeEntryPoint->DataSize;

- TotalSize = (DataSize == 0) ? 2048 : MicrocodeEntryPoint->TotalSize;

- if ( (UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize) ||

- ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||

- (DataSize & 0x3) != 0 ||

- (TotalSize & (SIZE_1KB - 1)) != 0 ||

- TotalSize < DataSize

- ) {

+ PatchCount++;

+ if (PatchCount > MaxPatchNumber) {

//

- // Not a valid microcode header, skip 1KB to check next entry.

+ // Current 'PatchInfoBuffer' cannot hold the information, double the size

+ // and allocate a new buffer.

//

- MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);

- continue;

- }

-

- if (IsMicrocodePatchNeedLoad (CpuMpData, MicrocodeEntryPoint)) {

- PatchCount++;

- if (PatchCount > MaxPatchNumber) {

+ if (MaxPatchNumber > MAX_UINTN / 2 / sizeof (MICROCODE_PATCH_INFO)) {

//

- // Current 'PatchInfoBuffer' cannot hold the information, double the size

- // and allocate a new buffer.

+ // Overflow check for MaxPatchNumber

//

- if (MaxPatchNumber > MAX_UINTN / 2 / sizeof (MICROCODE_PATCH_INFO)) {

- //

- // Overflow check for MaxPatchNumber

- //

- goto OnExit;

- }

-

- PatchInfoBuffer = ReallocatePool (

- MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),

- 2 * MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),

- PatchInfoBuffer

- );

- if (PatchInfoBuffer == NULL) {

- goto OnExit;

- }

- MaxPatchNumber = MaxPatchNumber * 2;

+ goto OnExit;

}



- //

- // Store the information of this microcode patch

- //

- PatchInfoBuffer[PatchCount - 1].Address = (UINTN) MicrocodeEntryPoint;

- PatchInfoBuffer[PatchCount - 1].Size = TotalSize;

- TotalLoadSize += TotalSize;

+ PatchInfoBuffer = ReallocatePool (

+ MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),

+ 2 * MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),

+ PatchInfoBuffer

+ );

+ if (PatchInfoBuffer == NULL) {

+ goto OnExit;

+ }

+ MaxPatchNumber = MaxPatchNumber * 2;

}



+ TotalSize = GetMicrocodeLength (MicrocodeEntryPoint);

+

+ //

+ // Store the information of this microcode patch

+ //

+ PatchInfoBuffer[PatchCount - 1].Address = (UINTN) MicrocodeEntryPoint;

+ PatchInfoBuffer[PatchCount - 1].Size = TotalSize;

+ TotalLoadSize += TotalSize;

+

//

// Process the next microcode patch

//

- MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + TotalSize);

- } while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));

+ MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) ((UINTN) MicrocodeEntryPoint + TotalSize);

+ } while ((UINTN) MicrocodeEntryPoint < MicrocodeEnd);



if (PatchCount != 0) {

DEBUG ((

@@ -607,7 +309,7 @@ OnExit:
if (PatchInfoBuffer != NULL) {

FreePool (PatchInfoBuffer);

}

- return;

+ FreePages (MicrocodeCpuIds, EFI_SIZE_TO_PAGES (CpuMpData->CpuCount * sizeof (EDKII_PEI_MICROCODE_CPU_ID)));

}



/**

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 66f9eb2304..e88a5355c9 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -32,6 +32,7 @@
#include <Library/MtrrLib.h>

#include <Library/HobLib.h>

#include <Library/PcdLib.h>

+#include <Library/MicrocodeLib.h>



#include <Guid/MicrocodePatchHob.h>



diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 49b0ffe8be..36fcb96b58 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -51,6 +51,7 @@ [LibraryClasses]
PeiServicesLib

PcdLib

VmgExitLib

+ MicrocodeLib



[Pcd]

gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## CONSUMES

--
2.27.0.windows.1


Re: [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

Andrei Warkentin
 

I don't know... the ACPI spec is weird.


...lists ResourceConsumer for _DMA.

A


From: Jeremy Linton <jeremy.linton@...>
Sent: Thursday, April 8, 2021 12:58 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@... <ard.biesheuvel@...>; leif@... <leif@...>; pete@... <pete@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Andrei Warkentin <awarkentin@...>; Jeremy Linton <jeremy.linton@...>
Subject: [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer
 
Bridge devices should be marked as producers so that their
children can consume the resources. In linux if this isn't
true then the translation gets ignored and the DMA values
are incorrect. This fixes DMA on all the devices that
need a translation.

Signed-off-by: Jeremy Linton <jeremy.linton@...>
---
 Platform/RaspberryPi/AcpiTables/Dsdt.asl | 2 +-
 Platform/RaspberryPi/AcpiTables/Emmc.asl | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
index d116f965e1..32cd5fc9f9 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -205,7 +205,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
         // Only the first GB is available.

         // Bus 0xC0000000 -> CPU 0x00000000.

         //

-        QWordMemory (ResourceConsumer,

+        QWordMemory (ResourceProducer,

           ,

           MinFixed,

           MaxFixed,

diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl b/Platform/RaspberryPi/AcpiTables/Emmc.asl
index 179dd3ecdb..0fbc2a79ea 100644
--- a/Platform/RaspberryPi/AcpiTables/Emmc.asl
+++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
@@ -32,7 +32,7 @@ DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI4EMMC", 2)
       }

 

       Name (_DMA, ResourceTemplate() {

-        QWordMemory (ResourceConsumer,

+        QWordMemory (ResourceProducer,

           ,

           MinFixed,

           MaxFixed,

--
2.13.7


Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode

Andrei Warkentin
 

Reviewed-by: Andrei Warkentin <awarkentin@...>


From: Pete Batard <pete@...>
Sent: Thursday, April 8, 2021 4:48 AM
To: Jeremy Linton <jeremy.linton@...>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@... <ard.biesheuvel@...>; leif@... <leif@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Andrei Warkentin <awarkentin@...>
Subject: Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode
 
On 2021.04.08 06:58, Jeremy Linton wrote:
> The arasan caps registers are no longer being overridden by
> the brcm iproc driver, so we should be assuring that the
> "High Speed Support" bit 21 is set in the capability
> register. This significantly improves the wifi perf
> using linux.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@...>
> ---
>   Platform/RaspberryPi/AcpiTables/Sdhc.asl | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> index 0430ab7d2d..42776e33bb 100644
> --- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> @@ -52,7 +52,7 @@ Device (SDC1)
>     Name (_DSD, Package () {
>
>       ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>
>       Package () {
>
> -      Package () { "sdhci-caps", 0x0100fa81 },
>
> +      Package () { "sdhci-caps", 0x0120fa81 },
>
>       }
>
>     })
>
>  
>

Reviewed-by: Pete Batard <pete@...>


Re: [PATCH 2/3] Platform/RaspberryPi/AcpiTables: Add further named components

Andrei Warkentin
 

Reviewed-by: Andrei Warkentin <awarkentin@...>


From: Jeremy Linton <jeremy.linton@...>
Sent: Thursday, April 8, 2021 12:58 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@... <ard.biesheuvel@...>; leif@... <leif@...>; pete@... <pete@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Andrei Warkentin <awarkentin@...>; Jeremy Linton <jeremy.linton@...>
Subject: [PATCH 2/3] Platform/RaspberryPi/AcpiTables: Add further named components
 
Add some additional IORT nodes for the USB & EMMC devices, realistically
we probably only need to have a single node with the lowest AddressSizeLimit
but this is conceptually "cleaner" should anyone actually try and use these
values rather than the _DMA provided ones.

Signed-off-by: Jeremy Linton <jeremy.linton@...>
---
 Platform/RaspberryPi/AcpiTables/Iort.aslc | 44 ++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Iort.aslc b/Platform/RaspberryPi/AcpiTables/Iort.aslc
index 00720194bb..810307ae37 100644
--- a/Platform/RaspberryPi/AcpiTables/Iort.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Iort.aslc
@@ -20,6 +20,8 @@ typedef struct {
 typedef struct {

   EFI_ACPI_6_0_IO_REMAPPING_TABLE      Iort;

   RPI4_NC_NODE                         NamedCompNode;

+  RPI4_NC_NODE                         NamedCompNode2;

+  RPI4_NC_NODE                         NamedCompNode3;

 } RPI4_IO_REMAPPING_STRUCTURE;

 

 STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {

@@ -27,7 +29,7 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
     ACPI_HEADER (EFI_ACPI_6_0_IO_REMAPPING_TABLE_SIGNATURE,

                  RPI4_IO_REMAPPING_STRUCTURE,

                  EFI_ACPI_IO_REMAPPING_TABLE_REVISION),

-    1,                                              // NumNodes

+    3,                                              // NumNodes

     sizeof (EFI_ACPI_6_0_IO_REMAPPING_TABLE),       // NodeOffset

     0                                               // Reserved

   }, {

@@ -50,6 +52,46 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
     }, {

       "\\_SB_.SCB0.XHC0"                            // ObjectName

     }

+  }, {

+    // gpu/dwc usb named component node

+    {

+      {

+        EFI_ACPI_IORT_TYPE_NAMED_COMP,              // Type

+        sizeof (RPI4_NC_NODE),                      // Length

+        0x0,                                        // Revision

+        0x0,                                        // Reserved

+        0x0,                                        // NumIdMappings

+        0x0,                                        // IdReference

+      },

+      0x0,                                          // Flags

+      0x0,                                          // CacheCoherent

+      0x0,                                          // AllocationHints

+      0x0,                                          // Reserved

+      0x0,                                          // MemoryAccessFlags

+      30,                                           // AddressSizeLimit

+    }, {

+      "\\_SB_.GDV0.USB0"                            // ObjectName

+    }

+  }, {

+    // emmc2 named component node

+    {

+      {

+        EFI_ACPI_IORT_TYPE_NAMED_COMP,              // Type

+        sizeof (RPI4_NC_NODE),                      // Length

+        0x0,                                        // Revision

+        0x0,                                        // Reserved

+        0x0,                                        // NumIdMappings

+        0x0,                                        // IdReference

+      },

+      0x0,                                          // Flags

+      0x0,                                          // CacheCoherent

+      0x0,                                          // AllocationHints

+      0x0,                                          // Reserved

+      0x0,                                          // MemoryAccessFlags

+      30,                                           // AddressSizeLimit

+    }, {

+      "\\_SB_.GDV1.SDC3"                            // ObjectName

+    }

   }

 };

 

--
2.13.7


Re: [GSoC proposal] Secure Image Loader

Andrew Fish
 

At a minimum it would be nice if we had a tool that would point out the security faults with a given PE/COFF file layout.
On Apr 8, 2021, at 4:16 AM, Laszlo Ersek <lersek@redhat.com> wrote:

On 04/06/21 12:06, Marvin Häuser wrote:
Good day Nate,

Comments are inline.

Best regards,
Marvin

On 06.04.21 11:41, Nate DeSimone wrote:
Hi Marvin,

Great to meet you and welcome back! Glad you hear you are interested!
Completing a formal verification of a PE/COFF loader is certainly
impressive. Was this done with some sort of automated theorem proving?
It would seem a rather arduous task doing an inductive proof for an
algorithm like that by hand!
I would call it "semi-automated", a great deal of intermediate goals
(preconditions, postconditions, invariants, assertions, ...) were
required to show all interesting properties. But yes, the actual proof
steps are automated by common SMT solvers. It was done using the
AstraVer Toolset and ACSL, latter basically a language to express logic
statements with C-like syntax.

I completely agree with you that getting a formally verified PE/COFF
loader into mainline is undoubtably valuable and would pay security
dividends for years to come.
I'm glad to hear that. :)

Admittedly, this is an area of computer science that I don't have a
great deal of experience with. The furthest I have gone on this topic
is writing out proofs for simple algorithms on exams in my Algorithms
class in college. Regardless you have a much better idea of what the
current status is of the work that you and Vitaly have done. I guess
my only question is do you think there is sufficient work remaining to
fill the 10 week GSoC development window?
Please don't get me wrong, but I would be surprised if the UEFI
specification changes I'd like to discuss alone would be completed
within 10 weeks, let alone implementation throughout the codebase. While
I think the plain amount of code may be a bit less than say a
MinPlatform port, the changes are much deeper and require much more
caution to avoid regressions (e.g. by invalidating undocumented
assertions). This sadly is not a matter of just replacing the underlying
library implementation or "plug-in and play" at all. It furthermore
affects many parts of the stack, the core dispatchers used for all
platforms, image emulation (EBC), UEFI userland emulation (EmuPkg), and
so on. I was rather worried the scope is too broad time-wise, but it can
be narrowed/widened as you see fit really. This is one of *the* core
components used on millions of device, and many package maintainers need
to review and validate the changes, this must really be done right the
first try. :)

Certainly we can use some of that time to perform the code reviews you
mention and write up formal ECRs for the UEFI spec changes that you
believe are needed.
I believed that was part of the workload, yes, but even without it I
think there is plenty to do.

Thank you for sending the application and alerting us to the great
work you and Vitaly have done! I'll read your paper more closely and
come back with any questions I still have.
Thank you, I will gladly explain anything unclear. Just try to not give
Laszlo too many flashbacks. :)
I haven't commented yet in this thread, as I thought my stance on this
undertaking was (or should be) obvious.

I very much welcome a replacement for the PE/COFF parser (as I consider
its security issues unfixable in an incremental manner). From my reading
of Marvin's and Vitaly's paper (draft), they have my full trust, and I'm
ready to put their upcoming code to use in ArmVirtPkg and OvmfPkg with
minimal actual code review. If fixing the pervasive security problems
around this area cannot avoid spiraling out to other core code in edk2,
such as dispatchers, and even to the PI / UEFI specs, so be it.

Regarding GSoC itself: as I stated elsewhere previously, I support
edk2's participation in GSoC, while at the same time I'm not
volunteering for mentorship at all. I'm uncertain if GSoC is the best
framework for upstreaming such a large undertaking, but if it can help,
we should use it as much as possible.

Thanks
Laszlo







With Best Regards,
Nate

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin
Häuser
Sent: Sunday, April 4, 2021 4:02 PM
To: devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; Andrew Fish
<afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [edk2-devel] [GSoC proposal] Secure Image Loader

Good day everyone,

I'll keep the introduction brief because I've been around for a while
now. :) I'm
Marvin Häuser, a third-year Computer Science student from TU
Kaiserslautern,
Germany. Late last year, my colleague Vitaly from ISP RAS and me
introduced a
formally verified Image Loader for UEFI usage at ISP RAS Open[1] due
to various
defects we outlined in the corresponding paper. Thank you once again
Laszlo
for your *incredible* review work on the publication part.

I now want to make an effort to mainline it, preferably as part of
the current
Google Summer of Code event. To be clear, my internship at ISP RAS has
concluded, and while Vitaly will be available for design discussion,
he has other
priorities at the moment and the practical part will be on me. I have
previously
submitted a proposal via the GSoC website for your review.

There are many things to consider:
1. The Image Loader is a core component, and there needs to be a
significant
level of quality and security assurance.
2. Being consumed by many packages, the proposed patch set will take
a lot of
time to review and integrate.
3. During my initial exploration, I discovered defective PPIs and
protocols (e.g.
returning data with no corresponding size) originating from the UEFI
PI and
UEFI specifications. Changes need to be discussed, settled on, and
submitted to
the UEFI Forum.
4. Some UEFI APIs like the Security Architecture protocols are
inconveniently
abstract, see 5.
5. Some of the current code does not use the existing context, or
accesses it
outside of the exposed APIs. The control flow of the dispatchers may
need to be
adapted to make the context available to appropriate APIs.

But obviously there are not only unpleasant considerations:
A. The Image Loader is mostly formally verified, and only very few
changes will
be required from the last proven state. This gives a lot of trust in
its correctness
and safety.
B. All outlined defects that are of critical nature have been fixed
successfully.
C. The Image Loader has been tested with real-world code loading
real-world
OSes on thousands of machines in the past few months, including
rejecting
malformed images (configurable by PCD).
D. The new APIs will centralise everything PE, reducing code
duplication and
potentially unsafe operations.
E. Centralising and reduced parse duplication may improve overall boot
performance.
F. The code has been coverage-tested to not contain dead code.
G. The code has been fuzz-tested including sanitizers to not invoke
undefined
behaviour.
H. I already managed to identify a malformed image in OVMF with its help
(incorrectly reported section alignment of an Intel IPXE driver). A
fix will be
submitted shortly.
I. I plan to support PE section permissions, allowing for read-only data
segments when enabled.

There are likely more points for both lists, but I hope this gives a
decent
starting point for discussion. What are your thoughts on the matter?
I strongly
encourage everyone to read the section regarding defects of our
publication[2]
to better understand the motivation. The vague points above can of
course be
elaborated in due time, as you see fit.

Thank you for your time!

Best regards,
Marvin


[1] https://github.com/mhaeuser/ISPRASOpen-SecurePE
[2] https://arxiv.org/pdf/2012.05471.pdf











Re: [PATCH 1/3] MinPlatformPkg/CoreCommonLib.dsc: Consume MicrocodeLib

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Friday, April 2, 2021 2:00 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
Dong, Eric <eric.dong@intel.com>
Subject: [edk2-devel] [PATCH 1/3] MinPlatformPkg/CoreCommonLib.dsc:
Consume MicrocodeLib

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
---
Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
index cf2940cf02..61d47109a6 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
@@ -106,6 +106,7 @@
MtrrLib|UefiCpuPkg/Library/MtrrLib/MtrrLib.inf
LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
SmmCpuFeaturesLib|UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeatures
Lib.inf+ MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf #
# Platform@@ -165,4 +166,4 @@
SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.in
f-
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Variab
lePolicyHelperLib.inf\ No newline at end of file
+
+ VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/V
+ ariablePolicyHelperLib.inf--
2.27.0.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73643): https://edk2.groups.io/g/devel/message/73643
Mute This Topic: https://groups.io/mt/81796844/1777047
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.chiu@intel.com] -=-
=-=-=-=-=


Re: [PATCH v1] MdePkg/Cpuid.h: Define new element in CPUID Leaf(07h) data structure.

Ni, Ray
 

Since Reserved2 and Reserved3 are changed, please use new name Reserved4, 5, 6.

-----Original Message-----
From: Lou, Yun <yun.lou@intel.com>
Sent: Thursday, April 8, 2021 2:35 PM
To: devel@edk2.groups.io
Cc: Lou, Yun <yun.lou@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH v1] MdePkg/Cpuid.h: Define new element in CPUID
Leaf(07h) data structure.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3309

Define new element(Hybird) in
CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS
(07h) data structure.

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
---
MdePkg/Include/Register/Intel/Cpuid.h | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Include/Register/Intel/Cpuid.h
b/MdePkg/Include/Register/Intel/Cpuid.h
index 19af99b6af..e7c0fd17e7 100644
--- a/MdePkg/Include/Register/Intel/Cpuid.h
+++ b/MdePkg/Include/Register/Intel/Cpuid.h
@@ -1550,9 +1550,17 @@ typedef union {
///

UINT32 AVX512_4FMAPS:1;

///

- /// [Bit 25:4] Reserved.

+ /// [Bit 14:4] Reserved.

///

- UINT32 Reserved2:22;

+ UINT32 Reserved2:11;

+ ///

+ /// [Bit 15] Hybrid. If 1, the processor is identified as a hybrid part.

+ ///

+ UINT32 Hybrid:1;

+ ///

+ /// [Bit 25:16] Reserved.

+ ///

+ UINT32 Reserved3:10;

///

/// [Bit 26] Enumerates support for indirect branch restricted speculation

/// (IBRS) and the indirect branch pre-dictor barrier (IBPB). Processors

@@ -1581,7 +1589,7 @@ typedef union {
///

/// [Bit 30] Reserved.

///

- UINT32 Reserved3:1;

+ UINT32 Reserved4:1;

///

/// [Bit 31] Enumerates support for Speculative Store Bypass Disable
(SSBD).

/// Processors that set this bit sup-port the IA32_SPEC_CTRL MSR. They
allow

--
2.28.0.windows.1


Re: [PATCH v2] MinPlatformPkg: Add PcdMicrocodeOffsetInFv

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, April 8, 2021 9:43 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
Dong, Eric <eric.dong@intel.com>
Subject: [PATCH v2] MinPlatformPkg: Add PcdMicrocodeOffsetInFv

Add PcdMicrocodeOffsetInFv in MinPlatformPkg.dec and update
SecFspWrapperPlatformSecLib library to use the microcode location PCDs
defined in MinPlatformPkg.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
---
.../SecFspWrapperPlatformSecLib.inf | 8 ++++----
.../SecFspWrapperPlatformSecLib/SecRamInitData.c | 6 +++---
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec | 10 +++++++++-
3 files changed, 16 insertions(+), 8 deletions(-)

diff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
SecLib/SecFspWrapperPlatformSecLib.inf
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
SecLib/SecFspWrapperPlatformSecLib.inf
index 4f3fa9fa34..2e0d67eae4 100644
---
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
SecLib/SecFspWrapperPlatformSecLib.inf
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecFspWrapperPlatformSecLib.inf
@@ -1,7 +1,7 @@
## @file # Provide FSP wrapper platform sec 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 #@@ -88,9 +88,9 @@
gMinPlatformPkgTokenSpaceGuid.PcdSecSerialPortDebugEnable ##
CONSUMES [FixedPcd]-
gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress ##
CONSUMES-
gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ##
CONSUMES- gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset
## CONSUMES+ gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase
## CONSUMES+ gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize
## CONSUMES+ gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv
## CONSUMES gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress
## CONSUMES gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize
## CONSUMES gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress
## CONSUMESdiff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
SecLib/SecRamInitData.c
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
SecLib/SecRamInitData.c
index b356327b4c..355d1e6509 100644
---
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
SecLib/SecRamInitData.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecRamInitData.c
@@ -1,7 +1,7 @@
/** @file Provide TempRamInitParams data. -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 **/@@ -24,8 +24,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST
FSPT_UPD_CORE_DATA FsptUpdDataPtr = {
} }, {- ((UINT32)FixedPcdGet64 (PcdCpuMicrocodePatchAddress) +
FixedPcdGet32 (PcdFlashMicrocodeOffset)),- ((UINT32)FixedPcdGet64
(PcdCpuMicrocodePatchRegionSize) - FixedPcdGet32
(PcdFlashMicrocodeOffset)),+ FixedPcdGet32 (PcdFlashFvMicrocodeBase) +
FixedPcdGet32 (PcdMicrocodeOffsetInFv),+ FixedPcdGet32
(PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeOffsetInFv), 0,
// Set CodeRegionBase as 0, so that caching will be 4GB-(CodeRegionSize >
LLCSize ? LLCSize : CodeRegionSize) will be used. FixedPcdGet32
(PcdFlashCodeCacheSize), { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00,diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
index 2b246cf0ac..28d2b1965e 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
@@ -6,7 +6,7 @@
# INF files to generate AutoGen.c and AutoGen.h files # for the build
infrastructure. #-# Copyright (c) 2017 - 2020, Intel Corporation. All rights
reserved.<BR>+# Copyright (c) 2017 - 2021, Intel Corporation. All rights
reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent #@@ -162,10
+162,18 @@

gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress|0xFF800000|UINT
32|0x10000001
gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize|0x00800000|UINT32|0x10
000002 + ## Indicates the MMIO base address of the microcode FV in flash.
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase|0xFFE60000|UINT
32|0x30000004++ ## Indicates the size of the microcode FV in flash.
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize|0x000A0000|UINT
32|0x30000005++ ## Indicates the offset of the microcode FV relative to the
beginning of flash.
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset|0x00660000|UI
NT32|0x30000006 + ## Indicates the offset of the actual microcode content
relative to the beginning of the microcode FV.+
gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv|0x90|UINT32|0x30
000007+
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemoryBase|0x00000000|UIN
T32|0x20000004
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemorySize|0x00000000|UIN
T32|0x20000005
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemoryOffset|0x00000000|UI
NT32|0x20000006--
2.27.0.windows.1


[PATCH v2] MinPlatformPkg: Add PcdMicrocodeOffsetInFv

Ni, Ray
 

Add PcdMicrocodeOffsetInFv in MinPlatformPkg.dec and update
SecFspWrapperPlatformSecLib library to use the microcode location
PCDs defined in MinPlatformPkg.

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
---
.../SecFspWrapperPlatformSecLib.inf | 8 ++++----
.../SecFspWrapperPlatformSecLib/SecRamInitData.c | 6 +++---
Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec | 10 +++++++++-
3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapper=
PlatformSecLib/SecFspWrapperPlatformSecLib.inf b/Platform/Intel/MinPlatform=
Pkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSec=
Lib.inf
index 4f3fa9fa34..2e0d67eae4 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor=
mSecLib/SecFspWrapperPlatformSecLib.inf
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor=
mSecLib/SecFspWrapperPlatformSecLib.inf
@@ -1,7 +1,7 @@
## @file=0D
# Provide FSP wrapper platform sec related function.=0D
#=0D
-# Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>=
=0D
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>=
=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -88,9 +88,9 @@
gMinPlatformPkgTokenSpaceGuid.PcdSecSerialPortDebugEnable ## C=
ONSUMES=0D
=0D
[FixedPcd]=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchAddress ## C=
ONSUMES=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## C=
ONSUMES=0D
- gIntelFsp2WrapperTokenSpaceGuid.PcdFlashMicrocodeOffset ## C=
ONSUMES=0D
+ gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase ## C=
ONSUMES=0D
+ gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize ## C=
ONSUMES=0D
+ gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv ## C=
ONSUMES=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheAddress ## C=
ONSUMES=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFlashCodeCacheSize ## C=
ONSUMES=0D
gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress ## C=
ONSUMES=0D
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapper=
PlatformSecLib/SecRamInitData.c b/Platform/Intel/MinPlatformPkg/FspWrapper/=
Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
index b356327b4c..355d1e6509 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor=
mSecLib/SecRamInitData.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor=
mSecLib/SecRamInitData.c
@@ -1,7 +1,7 @@
/** @file=0D
Provide TempRamInitParams data.=0D
=0D
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>=0D
+Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -24,8 +24,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA Fs=
ptUpdDataPtr =3D {
}=0D
},=0D
{=0D
- ((UINT32)FixedPcdGet64 (PcdCpuMicrocodePatchAddress) + FixedPcdGet32 (=
PcdFlashMicrocodeOffset)),=0D
- ((UINT32)FixedPcdGet64 (PcdCpuMicrocodePatchRegionSize) - FixedPcdGet3=
2 (PcdFlashMicrocodeOffset)),=0D
+ FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeO=
ffsetInFv),=0D
+ FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeO=
ffsetInFv),=0D
0, // Set CodeRegionBase as 0, so that caching will be 4GB-(C=
odeRegionSize > LLCSize ? LLCSize : CodeRegionSize) will be used.=0D
FixedPcdGet32 (PcdFlashCodeCacheSize),=0D
{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,=0D
diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/In=
tel/MinPlatformPkg/MinPlatformPkg.dec
index 2b246cf0ac..28d2b1965e 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
@@ -6,7 +6,7 @@
# INF files to generate AutoGen.c and AutoGen.h files=0D
# for the build infrastructure.=0D
#=0D
-# Copyright (c) 2017 - 2020, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -162,10 +162,18 @@
gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress|0xFF800000|UINT32|=
0x10000001=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize|0x00800000|UINT32|0x10000=
002=0D
=0D
+ ## Indicates the MMIO base address of the microcode FV in flash.=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeBase|0xFFE60000|UINT32|=
0x30000004=0D
+=0D
+ ## Indicates the size of the microcode FV in flash.=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeSize|0x000A0000|UINT32|=
0x30000005=0D
+=0D
+ ## Indicates the offset of the microcode FV relative to the beginning of=
flash.=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvMicrocodeOffset|0x00660000|UINT3=
2|0x30000006=0D
=0D
+ ## Indicates the offset of the actual microcode content relative to the =
beginning of the microcode FV.=0D
+ gMinPlatformPkgTokenSpaceGuid.PcdMicrocodeOffsetInFv|0x90|UINT32|0x30000=
007=0D
+=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemoryBase|0x00000000|UINT32|=
0x20000004=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemorySize|0x00000000|UINT32|=
0x20000005=0D
gMinPlatformPkgTokenSpaceGuid.PcdFlashFvPreMemoryOffset|0x00000000|UINT3=
2|0x20000006=0D
--=20
2.27.0.windows.1


Re: [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest

Lendacky, Thomas
 

On 4/8/21 1:24 AM, Xu, Min M wrote:
On Wednesday, April 7, 2021 11:03 PM, Laszlo wrote:
On 04/07/21 02:44, James Bottomley wrote:
On Wed, 2021-04-07 at 00:21 +0000, Xu, Min M wrote:
Hi, Laszlo

For Intel TDX supported guest, all processors start in 32-bit
protected mode, while for Non-Td guest, it starts in 16-bit real
mode. To make the ResetVector work on both Td-guest and Non-Td guest,
ResetVector are updated as below:
------------------------------------------------------------------
ALIGN 16
resetVector:
;
; Reset Vector
;
; This is where the processor will begin execution
;
nop
nop
smsw ax
test al, 1
jnz EarlyBspPmEntry
jmp EarlyBspInitReal16
Well, then use the rel8 jump like the compiler would in this situation:

smsw ax
test al, 1
jz 1f
jmp EarlyBspPmEntry
1:
jmp EarlyBspInitReal16

So now both entries can be 32k away.
The problem is that we need NASM to generate such *shared* entry code that
behaves correctly when executed in either 16-bit or 32-bit mode.

The rel8 near jumps ("short jumps") are like that -- for example, the
"74 cb" opcode decodes to the same "JZ rel8" in both modes.

But the rel16 ("non-short") near jumps turn into rel32 near jumps when
decoded in 32-bit mode. For example, "E9 cw" decodes to "JMP rel16" in 16-bit
mode, but it gets parsed as "E9 cd" (= "JMP rel32") in 32-bit mode.

So the idea is to add more BITS directives, for covering the non-short near
jumps themselves:
Yes this is the root cause. TDX requires the startup mode to be 32-bit
protected mode while the legacy VM startup mode is 16-bit real mode.
Add more BITS directives can work round this. I have tried it and it works.

So my initial solution is to use *jmp rel8* because it works in both 16-bit
and 32-bit mode. But *jmp rel8* depends on the distance which should
be less than 128 bytes. If more metadata is added in the ResetVector.asm
then we have to use the BITS solution.
To me, it sounds like the BITS solution should be the approach you use
from the start.

Thanks,
Tom



; instructions up to and including the rel8 JZ decode identically ;
between BITS 16 and BITS 32 BITS 16
smsw ax
test al, 1
jz Real

; the unconditional near jumps are mode-specific BITS 32
jmp near EarlyBspPmEntry
BITS 16
Real:
jmp near EarlyBspInitReal16

; --------------------

BITS 16
EarlyBspInitReal16:
nop

BITS 32
EarlyBspPmEntry:
nop
$ nasm -f bin jz.nasmb

Decoded (executed) in 16-bit mode:

$ ndisasm -b 16 -k 7,5 -k 0x10,1 jz
00000000 0F01E0 smsw ax
00000003 A801 test al,0x1
00000005 7405 jz 0xc ; taken
00000007 skipping 0x5 bytes
0000000C E90000 jmp word 0xf
0000000F 90 nop
00000010 skipping 0x1 bytes

Decoded (executed) in 32-bit mode:

$ ndisasm -b 32 -k 0xc,4 jz
00000000 0F01E0 smsw eax
00000003 A801 test al,0x1
00000005 7405 jz 0xc ; not taken
00000007 E904000000 jmp dword 0x10
0000000C skipping 0x4 bytes
00000010 90 nop


With the garbage *not* hidden:

$ ndisasm -b 16 -s 0xc jz

00000000 0F01E0 smsw ax
00000003 A801 test al,0x1
00000005 7405 jz 0xc ; taken
00000007 E90400 jmp word 0xe ; garbage
0000000A 0000 add [bx+si],al ; garbage
0000000C E90000 jmp word 0xf
0000000F 90 nop
00000010 90 nop ; garbage

$ ndisasm -b 32 -s 0x10 jz

00000000 0F01E0 smsw eax
00000003 A801 test al,0x1
00000005 7405 jz 0xc ; not taken
00000007 E904000000 jmp dword 0x10
0000000C E9 db 0xe9 ; garbage
0000000D 0000 add [eax],al ; garbage
0000000F 90 nop ; garbage
00000010 90 nop

Thanks
Laszlo





Re: [edk2-platforms] [PATCH v3 2/3] Platform/ARM/SgiPkg: Add HMAT ACPI table for RdN1EdgeX2

Vijayenthiran Subramaniam
 

Hi Sami,

 

Agree with the suggestion. Please make the changes.

 

Regards,

Vijay

 

From: "Sami Mujawar via Groups.Io" <sami.mujawar@...>
Reply to: Sami Mujawar <Sami.Mujawar@...>
Date: Thursday, 8 April 2021 at 12:07 AM
To: Vijayenthiran Subramaniam <Vijayenthiran.Subramaniam@...>, "devel@edk2.groups.io" <devel@edk2.groups.io>
Subject: Re: [edk2-devel] [edk2-platforms] [PATCH v3 2/3] Platform/ARM/SgiPkg: Add HMAT ACPI table for RdN1EdgeX2

 

Hi Vijay,

+#define HMAT_STRUCTURE_MEMORY_SIDE_CACHE_INFO_CACHE_ATTRIBUTES_INIT( \
+ TotalCacheLevels, CacheLevel, CacheAssociativity, WritePolicy, CacheLineSize \
+ ) \
+{ \
+ TotalCacheLevels, CacheLevel, CacheAssociativity, WritePolicy, CacheLineSize \
+}

This macros is again repeated in patch 3/3. I think this could be moved to Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h.
If you agree I can make this change locally before pushing this series.

Regards,

Sami Mujawar


Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation

Pete Batard
 

On 2021.04.08 13:26, Ard Biesheuvel wrote:
On Thu, 8 Apr 2021 at 13:43, Pete Batard <pete@akeo.ie> wrote:

On 2021.04.08 12:06, Ard Biesheuvel wrote:
On Thu, 8 Apr 2021 at 11:47, Pete Batard <pete@akeo.ie> wrote:

On 2021.04.06 15:04, Mario Bălănică wrote:
The VPU clock divisor has changed in this commit:
https://github.com/raspberrypi/firmware/commit/1e5456a,
thus breaking the mini UART clock divisor calculation on the Pi 4.

Fix this by reading the core clock from the mailbox instead.

Signed-off-by: Mario Bălănică <mariobalanica02@gmail.com>
---
Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 15 +++------------
Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 4 ----
Platform/RaspberryPi/RPi4/RPi4.dsc | 6 ++++--
3 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index 5e83bbf022eb..d2f983bf0a9f 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -34,25 +34,16 @@ SerialPortGetDivisor (
UINT32 SerialBaudRate

)

{

- UINT64 BaseClockRate;

+ UINT32 BaseClockRate;

UINT32 Divisor;



- //

- // On the Raspberry Pi, the clock to use for the 16650-compatible UART

- // is the base clock divided by the 12.12 fixed point VPU clock divisor.

- //

- BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate);

-#if (RPI_MODEL == 4)

- Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF;

- if (Divisor != 0)

- BaseClockRate = (BaseClockRate << 12) / Divisor;

-#endif

+ BaseClockRate = PcdGet32 (PcdSerialClockRate);



//

// As per the BCM2xxx datasheets:

// baudrate = system_clock_freq / (8 * (divisor + 1)).

//

- Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8);

+ Divisor = BaseClockRate / (SerialBaudRate * 8);

if (Divisor != 0) {

Divisor--;

}

diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
index 58351e4fb8cc..7008aaf8aa4c 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
+++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
@@ -85,13 +85,11 @@ ASM_FUNC (ArmPlatformPeiBootAction)
adr x2, mBoardRevision

str w0, [x2]



-#if (RPI_MODEL == 3)

run .Lclkinfo_buffer



ldr w0, .Lfrequency

adr x2, _gPcd_BinaryPatch_PcdSerialClockRate

str w0, [x2]

-#endif



ret



@@ -135,7 +133,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // end tag

.set .Lrevinfo_size, . - .Lrevinfo_buffer



-#if (RPI_MODEL == 3)

.align 4

.Lclkinfo_buffer:

.long .Lclkinfo_size

@@ -148,7 +145,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // frequency

.long 0 // end tag

.set .Lclkinfo_size, . - .Lclkinfo_buffer

-#endif



//UINTN

//ArmPlatformGetPrimaryCoreMpId (

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 2c05c31118d2..ff802d8347ea 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -429,7 +429,6 @@ [PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4

- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8

gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200

@@ -465,6 +464,9 @@ [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"

gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE



+[PcdsPatchableInModule]

+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000

+

[PcdsDynamicHii.common.DEFAULT]



#

@@ -621,7 +623,7 @@ [Components.common]
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf

MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {

<LibraryClasses>

- SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf

+ SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf

}

Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf

EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
Reviewed-by: Pete Batard <pete@akeo.ie>
Tested-by: Pete Batard <pete@akeo.ie>
Thanks Pete.

Could anyone get me a version of this patch that is not whitespace
mangled? This one does not apply with 'git am'
Done.

The version I sent is the one I applied & tested after I ran it through
my unmangling script to remove the extra lines.

Not sure if it'll still not be re-mangled by the list processing though.
Yes, which is odd given that I am receiving this email directly.
In any case, I still had to perform surgery on the patch to get it to apply.
Pushed as 7fe9704893f1..d2339f3c5f9a; mind double checking if I did
not break anything?
Just re-tested with edk2-platforms latest, and everything looks good. Thanks.

/Pete

Thanks,
Ard.


Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation

Ard Biesheuvel
 

On Thu, 8 Apr 2021 at 13:43, Pete Batard <pete@akeo.ie> wrote:

On 2021.04.08 12:06, Ard Biesheuvel wrote:
On Thu, 8 Apr 2021 at 11:47, Pete Batard <pete@akeo.ie> wrote:

On 2021.04.06 15:04, Mario Bălănică wrote:
The VPU clock divisor has changed in this commit:
https://github.com/raspberrypi/firmware/commit/1e5456a,
thus breaking the mini UART clock divisor calculation on the Pi 4.

Fix this by reading the core clock from the mailbox instead.

Signed-off-by: Mario Bălănică <mariobalanica02@gmail.com>
---
Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 15 +++------------
Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 4 ----
Platform/RaspberryPi/RPi4/RPi4.dsc | 6 ++++--
3 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index 5e83bbf022eb..d2f983bf0a9f 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -34,25 +34,16 @@ SerialPortGetDivisor (
UINT32 SerialBaudRate

)

{

- UINT64 BaseClockRate;

+ UINT32 BaseClockRate;

UINT32 Divisor;



- //

- // On the Raspberry Pi, the clock to use for the 16650-compatible UART

- // is the base clock divided by the 12.12 fixed point VPU clock divisor.

- //

- BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate);

-#if (RPI_MODEL == 4)

- Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF;

- if (Divisor != 0)

- BaseClockRate = (BaseClockRate << 12) / Divisor;

-#endif

+ BaseClockRate = PcdGet32 (PcdSerialClockRate);



//

// As per the BCM2xxx datasheets:

// baudrate = system_clock_freq / (8 * (divisor + 1)).

//

- Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8);

+ Divisor = BaseClockRate / (SerialBaudRate * 8);

if (Divisor != 0) {

Divisor--;

}

diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
index 58351e4fb8cc..7008aaf8aa4c 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
+++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
@@ -85,13 +85,11 @@ ASM_FUNC (ArmPlatformPeiBootAction)
adr x2, mBoardRevision

str w0, [x2]



-#if (RPI_MODEL == 3)

run .Lclkinfo_buffer



ldr w0, .Lfrequency

adr x2, _gPcd_BinaryPatch_PcdSerialClockRate

str w0, [x2]

-#endif



ret



@@ -135,7 +133,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // end tag

.set .Lrevinfo_size, . - .Lrevinfo_buffer



-#if (RPI_MODEL == 3)

.align 4

.Lclkinfo_buffer:

.long .Lclkinfo_size

@@ -148,7 +145,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // frequency

.long 0 // end tag

.set .Lclkinfo_size, . - .Lclkinfo_buffer

-#endif



//UINTN

//ArmPlatformGetPrimaryCoreMpId (

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 2c05c31118d2..ff802d8347ea 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -429,7 +429,6 @@ [PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4

- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8

gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200

@@ -465,6 +464,9 @@ [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"

gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE



+[PcdsPatchableInModule]

+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000

+

[PcdsDynamicHii.common.DEFAULT]



#

@@ -621,7 +623,7 @@ [Components.common]
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf

MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {

<LibraryClasses>

- SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf

+ SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf

}

Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf

EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
Reviewed-by: Pete Batard <pete@akeo.ie>
Tested-by: Pete Batard <pete@akeo.ie>
Thanks Pete.

Could anyone get me a version of this patch that is not whitespace
mangled? This one does not apply with 'git am'
Done.

The version I sent is the one I applied & tested after I ran it through
my unmangling script to remove the extra lines.

Not sure if it'll still not be re-mangled by the list processing though.
Yes, which is odd given that I am receiving this email directly.

In any case, I still had to perform surgery on the patch to get it to apply.

Pushed as 7fe9704893f1..d2339f3c5f9a; mind double checking if I did
not break anything?

Thanks,
Ard.


Re: [RFC PATCH 00/19] Add AMD Secure Nested Paging (SEV-SNP) support

Brijesh Singh
 

Hi Laszlo,

On 4/8/21 4:58 AM, Laszlo Ersek wrote:
Hi Brijesh,

On 03/24/21 16:31, Brijesh Singh wrote:
BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=G2AQ%2FCks3%2BbczHJXMwqlqpWpoBJmb0pmxb1VNLw6t%2BA%3D&amp;reserved=0

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.
Can you describe this in a bit more detail, before I look at the
individual patches? Specifically, what existing logic in the PEI phase
was taken, and extended, and how?
One of the key requirement is that the guest private pages much be
validated before the access. If guest tries to access the pages before
the validation then it will result in #VC (page-not-validated)
exception. To avoid the #VC, we propose the validating the memory before
the access. We will incrementally add the support to lazy validate (i.e
validate on access).

Let me try explaining a bit, the page validation process consist of two
steps:

1. Add the pages in the RMP table -- must be done by the hypervisor
using the RMPUPDATE instruction. The guest can use VMGEXIT NAEs to ask
hypervisor to add or remove pages from the RMP table.

2. Guest issue the PVALIDATE instruction -- this sets the validate bit
in the RMP table.

Similar to SEV, the OVMF_CODE.fd is encrypted through the SNP firmware
before the launch. The SNP firmware also validates the memory page after
encrypting. This allows us to boot the initial entry code without guest
going through the validation process.

The OVMF reset vector uses few data pages (e.g page table, early Sec
stack). Access to these data pages will result in #VC. There are two
approaches we can take to validate these data pages:

1. Ask SNP firmware to pre-validate it -- SNP firmware provides an
special command that can be used to pre-validate the pages without
affecting the measurement.

2. Enhance the reset vector code to validate the pages.

For now I choose #1.

The pre-validation performed by the SNP firmware is sufficient to boot
through the SEC phase. The SEC phase later decompress the Fv to a new
memory location. Now we need the OVMF to take over the validation
procedure.  The series extends the MemEncryptSevLib to add a new helper
MemEncryptSevSnpValidateRam(). The helper is used to validate the system
RAM. See patch #12. SEC phase calls the MemEncryptSevSnpValidateRam() to
validate the output buffer used for the decompression. This was
sufficient to boot into the PEI phase, see patch #13. The PEI detects
all the available system RAM. After the memory detection is completed
the PlatformPei calls the AmdSevSnpInitialize(). The initialization
routine iterate through the HOB and calls the
MemEncryptSevSnpValidateRam() to validate all the system RAM. Is it
possible the more system ram can be detected after the PlatformPei is
completed ?

One of the important thing is we should *never* validate the pages
twice. The MemEncryptSevSnpValidateRam() uses a interval search tree to
keep the record of what has been validated. Before validating the range,
it lookup in its tree and if it finds that range is already validated
then do nothing. If it detects an overlap then it will validate only non
overlapping regions -- see patch #14.

The patch #18 extend the MemEncrypt{Set,Clear}PageEncMask() to call the
SNP page state change during the C-bit toggle.

Please let me know if you have any questions. We can hash out the design
here before you taking a closure look at the code.


If there is a particular patch whose commit message is closely related
to my question, can you point it out? Patch#15 perhaps? (Doesn't seem
like a big patch; for some reason I'd expect something more complex, but
perhaps that's only because it builds upon the many earlier patches.)

Thanks,
Laszlo

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* AP bring up using the new SEV-SNP NAE
* Lazy validation
* Interrupt security

The series is based on commit:
e542e05d4f UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber

Additional resources
---------------------
SEV-SNP whitepaper
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=uUqlVHhWQ6geGDaNHwxGMpoSpIamB%2F1vHH69h%2FEGUro%3D&amp;reserved=0

APM 2: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=R2kU42jvCDZat8kGZ5gDDz2nFXIawHfXdRW1aovhNK8%3D&amp;reserved=0 (section 15.36)

The complete source is available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2Fovmf%2Ftree%2Fsev-snp-rfc-1&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=yreFg2hr%2F82WYEjxqCmb7pXUdtrRCJRYPrPHgfrWjM8%3D&amp;reserved=0

GHCB spec v2:
The draft specification is posted on AMD-SEV-SNP mailing list:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.suse.com%2Fmailman%2Fprivate%2Famd-sev-snp%2F&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=PFV2mA7T%2Fbl2zP5j52kNdT%2FavDMRLWDEDqz6JGusEFg%3D&amp;reserved=0

Copy of the spec is also available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2FAMDSEV%2Fblob%2Fsev-snp-devel%2Fdocs%2F56421-Guest_Hypervisor_Communication_Block_Standardization.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=U3oKRe5m0NxI0xqv1gBoyh%2BEEX1LVeCWR42rvPh6XZ8%3D&amp;reserved=0

GHCB spec v1:
SEV-SNP firmware specification:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.amd.com%2Fsev%2F&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb8ee3abf81aa4e5b1e6008d8fa74dbbc%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637534728122143364%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=G%2BttkORsTJchJ1Fy1iNlD%2B%2BqiQZuwI8md5vhJjEb%2Fn4%3D&amp;reserved=0

Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>

Brijesh Singh (19):
OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest
OvmfPkg: validate the data pages used in the SEC phase
MdePkg: Expand the SEV MSR to include the SNP definition
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
MdePkg: Define the GHCB GPA structure
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg: Add a library to support registering GHCB GPA
OvmfPkg: register GHCB gpa for the SEV-SNP guest
MdePkg: Add AsmPvalidate() support
OvmfPkg: Define the Page State Change VMGEXIT structures
OvmfPkg/ResetVector: Invalidate the GHCB page
OvmfPkg/MemEncryptSevLib: Add support to validate system RAM
OvmfPkg/SecMain: Validate the data/code pages used for the PEI phase
OvmfPkg/MemEncryptSevLib: Add support to validate RAM in PEI phase
OvmfPkg/PlatformPei: Validate the system RAM when SNP is active
OvmfPkg/MemEncryptSevLib: Add support to validate > 4GB memory in PEI
phase
OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase
OvmfPkg/MemEncryptSevLib: Validate the memory during set or clear enc
attribute
OvmfPkg/MemEncryptSevLib: Skip page state change for non RAM region

MdePkg/Include/Library/BaseLib.h | 37 +++
MdePkg/Include/Register/Amd/Fam17Msr.h | 31 ++-
MdePkg/Include/Register/Amd/Ghcb.h | 39 ++-
MdePkg/Library/BaseLib/BaseLib.inf | 1 +
MdePkg/Library/BaseLib/X64/Pvalidate.nasm | 43 +++
OvmfPkg/Include/Library/GhcbRegisterLib.h | 27 ++
OvmfPkg/Include/Library/MemEncryptSevLib.h | 30 +++
.../DxeMemEncryptSevLib.inf | 7 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/SnpPageStateChange.c | 17 ++
.../PeiMemEncryptSevLib.inf | 9 +
.../PeiMemEncryptSevLibInternal.c | 47 ++++
.../SecMemEncryptSevLib.inf | 4 +
.../SecMemEncryptSevLibInternal.c | 39 +++
.../BaseMemEncryptSevLib/SnpPageStateChange.h | 37 +++
.../X64/PeiDxeSnpSetPageState.c | 63 +++++
.../X64/PeiDxeVirtualMemory.c | 151 ++++++++++-
.../X64/PeiSnpSystemRamValidate.c | 129 +++++++++
.../X64/SecSnpSystemRamValidate.c | 23 ++
.../X64/SnpPageStateChangeInternal.c | 254 ++++++++++++++++++
.../X64/SnpPageStateTrack.c | 119 ++++++++
.../X64/SnpPageStateTrack.h | 36 +++
.../X64/SnpSetPageState.h | 27 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 ++
.../Library/GhcbRegisterLib/GhcbRegisterLib.c | 97 +++++++
.../GhcbRegisterLib/GhcbRegisterLib.inf | 33 +++
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 4 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 7 +
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 45 ++++
OvmfPkg/OvmfPkg.dec | 12 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfPkgX64.fdf | 33 ++-
OvmfPkg/PlatformPei/AmdSev.c | 52 ++++
OvmfPkg/PlatformPei/PlatformPei.inf | 2 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 24 ++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 106 ++++++++
OvmfPkg/ResetVector/ResetVector.inf | 5 +
OvmfPkg/ResetVector/ResetVector.nasmb | 4 +
OvmfPkg/Sec/SecMain.c | 102 +++++++
OvmfPkg/Sec/SecMain.inf | 2 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 ++++
UefiCpuPkg/UefiCpuPkg.dec | 6 +
47 files changed, 1790 insertions(+), 19 deletions(-)
create mode 100644 MdePkg/Library/BaseLib/X64/Pvalidate.nasm
create mode 100644 OvmfPkg/Include/Library/GhcbRegisterLib.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/SnpPageStateChange.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/SnpPageStateChange.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeSnpSetPageState.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateTrack.c
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateTrack.h
create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpSetPageState.h
create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.c
create mode 100644 OvmfPkg/Library/GhcbRegisterLib/GhcbRegisterLib.inf


Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation

Pete Batard
 

On 2021.04.08 12:06, Ard Biesheuvel wrote:
On Thu, 8 Apr 2021 at 11:47, Pete Batard <pete@akeo.ie> wrote:

On 2021.04.06 15:04, Mario Bălănică wrote:
The VPU clock divisor has changed in this commit:
https://github.com/raspberrypi/firmware/commit/1e5456a,
thus breaking the mini UART clock divisor calculation on the Pi 4.

Fix this by reading the core clock from the mailbox instead.

Signed-off-by: Mario Bălănică <mariobalanica02@gmail.com>
---
Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 15 +++------------
Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 4 ----
Platform/RaspberryPi/RPi4/RPi4.dsc | 6 ++++--
3 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index 5e83bbf022eb..d2f983bf0a9f 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -34,25 +34,16 @@ SerialPortGetDivisor (
UINT32 SerialBaudRate

)

{

- UINT64 BaseClockRate;

+ UINT32 BaseClockRate;

UINT32 Divisor;



- //

- // On the Raspberry Pi, the clock to use for the 16650-compatible UART

- // is the base clock divided by the 12.12 fixed point VPU clock divisor.

- //

- BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate);

-#if (RPI_MODEL == 4)

- Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF;

- if (Divisor != 0)

- BaseClockRate = (BaseClockRate << 12) / Divisor;

-#endif

+ BaseClockRate = PcdGet32 (PcdSerialClockRate);



//

// As per the BCM2xxx datasheets:

// baudrate = system_clock_freq / (8 * (divisor + 1)).

//

- Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8);

+ Divisor = BaseClockRate / (SerialBaudRate * 8);

if (Divisor != 0) {

Divisor--;

}

diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
index 58351e4fb8cc..7008aaf8aa4c 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
+++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
@@ -85,13 +85,11 @@ ASM_FUNC (ArmPlatformPeiBootAction)
adr x2, mBoardRevision

str w0, [x2]



-#if (RPI_MODEL == 3)

run .Lclkinfo_buffer



ldr w0, .Lfrequency

adr x2, _gPcd_BinaryPatch_PcdSerialClockRate

str w0, [x2]

-#endif



ret



@@ -135,7 +133,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // end tag

.set .Lrevinfo_size, . - .Lrevinfo_buffer



-#if (RPI_MODEL == 3)

.align 4

.Lclkinfo_buffer:

.long .Lclkinfo_size

@@ -148,7 +145,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // frequency

.long 0 // end tag

.set .Lclkinfo_size, . - .Lclkinfo_buffer

-#endif



//UINTN

//ArmPlatformGetPrimaryCoreMpId (

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 2c05c31118d2..ff802d8347ea 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -429,7 +429,6 @@ [PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4

- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8

gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200

@@ -465,6 +464,9 @@ [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"

gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE



+[PcdsPatchableInModule]

+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000

+

[PcdsDynamicHii.common.DEFAULT]



#

@@ -621,7 +623,7 @@ [Components.common]
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf

MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {

<LibraryClasses>

- SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf

+ SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf

}

Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf

EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
Reviewed-by: Pete Batard <pete@akeo.ie>
Tested-by: Pete Batard <pete@akeo.ie>
Thanks Pete.
Could anyone get me a version of this patch that is not whitespace
mangled? This one does not apply with 'git am'
Done.

The version I sent is the one I applied & tested after I ran it through my unmangling script to remove the extra lines.

Not sure if it'll still not be re-mangled by the list processing though.

Regards,

/Pete


[edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation

Pete Batard
 

From: Mario B=C4=83l=C4=83nic=C4=83 <mariobalanica02@gmail.com>=0D

The VPU clock divisor has changed in this commit: https://github.com/raspbe=
rrypi/firmware/commit/1e5456a,=0D
thus breaking the mini UART clock divisor calculation on the Pi 4.=0D
=0D
Fix this by reading the core clock from the mailbox instead.=0D
=0D
Signed-off-by: Mario B=C4=83l=C4=83nic=C4=83 <mariobalanica02@gmail.com>=0D
---=0D
Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 15 =
+++------------=0D
Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 4 =
----=0D
Platform/RaspberryPi/RPi4/RPi4.dsc | 6 =
++++--=0D
3 files changed, 7 insertions(+), 18 deletions(-)=0D
=0D
diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortL=
ib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c=0D
index 5e83bbf022eb..d2f983bf0a9f 100644=0D
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c=0D
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c=0D
@@ -34,25 +34,16 @@ SerialPortGetDivisor (=0D
UINT32 SerialBaudRate=0D
)=0D
{=0D
- UINT64 BaseClockRate;=0D
+ UINT32 BaseClockRate;=0D
UINT32 Divisor;=0D
=0D
- //=0D
- // On the Raspberry Pi, the clock to use for the 16650-compatible UART=0D
- // is the base clock divided by the 12.12 fixed point VPU clock divisor.=
=0D
- //=0D
- BaseClockRate =3D (UINT64)PcdGet32 (PcdSerialClockRate);=0D
-#if (RPI_MODEL =3D=3D 4)=0D
- Divisor =3D MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) &=
0xFFFFFF;=0D
- if (Divisor !=3D 0)=0D
- BaseClockRate =3D (BaseClockRate << 12) / Divisor;=0D
-#endif=0D
+ BaseClockRate =3D PcdGet32 (PcdSerialClockRate);=0D
=0D
//=0D
// As per the BCM2xxx datasheets:=0D
// baudrate =3D system_clock_freq / (8 * (divisor + 1)).=0D
//=0D
- Divisor =3D (UINT32)BaseClockRate / (SerialBaudRate * 8);=0D
+ Divisor =3D BaseClockRate / (SerialBaudRate * 8);=0D
if (Divisor !=3D 0) {=0D
Divisor--;=0D
}=0D
diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHe=
lper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper=
.S=0D
index 58351e4fb8cc..7008aaf8aa4c 100644=0D
--- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S=
=0D
+++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S=
=0D
@@ -85,13 +85,11 @@ ASM_FUNC (ArmPlatformPeiBootAction)=0D
adr x2, mBoardRevision=0D
str w0, [x2]=0D
=0D
-#if (RPI_MODEL =3D=3D 3)=0D
run .Lclkinfo_buffer=0D
=0D
ldr w0, .Lfrequency=0D
adr x2, _gPcd_BinaryPatch_PcdSerialClockRate=0D
str w0, [x2]=0D
-#endif=0D
=0D
ret=0D
=0D
@@ -135,7 +133,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)=0D
.long 0 // end tag=0D
.set .Lrevinfo_size, . - .Lrevinfo_buffer=0D
=0D
-#if (RPI_MODEL =3D=3D 3)=0D
.align 4=0D
.Lclkinfo_buffer:=0D
.long .Lclkinfo_size=0D
@@ -148,7 +145,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)=0D
.long 0 // frequency=0D
.long 0 // end tag=0D
.set .Lclkinfo_size, . - .Lclkinfo_buffer=0D
-#endif=0D
=0D
//UINTN=0D
//ArmPlatformGetPrimaryCoreMpId (=0D
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4=
/RPi4.dsc=0D
index 2c05c31118d2..ff802d8347ea 100644=0D
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc=0D
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc=0D
@@ -429,7 +429,6 @@ [PcdsFixedAtBuild.common]=0D
gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4=0D
- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8=0D
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200=0D
@@ -465,6 +464,9 @@ [PcdsFixedAtBuild.common]=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"=0D
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE=0D
=0D
+[PcdsPatchableInModule]=0D
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000=0D
+=0D
[PcdsDynamicHii.common.DEFAULT]=0D
=0D
#=0D
@@ -621,7 +623,7 @@ [Components.common]=0D
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf=0D
MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {=0D
<LibraryClasses>=0D
- SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSer=
ialPortLib.inf=0D
+ SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSer=
ialPortDxeLib.inf=0D
}=0D
Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf=0D
EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf=0D
-- 2.29.2.windows.2=0D


Re: [GSoC proposal] Secure Image Loader

Laszlo Ersek
 

On 04/06/21 12:06, Marvin Häuser wrote:
Good day Nate,

Comments are inline.

Best regards,
Marvin

On 06.04.21 11:41, Nate DeSimone wrote:
Hi Marvin,

Great to meet you and welcome back! Glad you hear you are interested!
Completing a formal verification of a PE/COFF loader is certainly
impressive. Was this done with some sort of automated theorem proving?
It would seem a rather arduous task doing an inductive proof for an
algorithm like that by hand!
I would call it "semi-automated", a great deal of intermediate goals
(preconditions, postconditions, invariants, assertions, ...) were
required to show all interesting properties. But yes, the actual proof
steps are automated by common SMT solvers. It was done using the
AstraVer Toolset and ACSL, latter basically a language to express logic
statements with C-like syntax.

I completely agree with you that getting a formally verified PE/COFF
loader into mainline is undoubtably valuable and would pay security
dividends for years to come.
I'm glad to hear that. :)

Admittedly, this is an area of computer science that I don't have a
great deal of experience with. The furthest I have gone on this topic
is writing out proofs for simple algorithms on exams in my Algorithms
class in college. Regardless you have a much better idea of what the
current status is of the work that you and Vitaly have done. I guess
my only question is do you think there is sufficient work remaining to
fill the 10 week GSoC development window?
Please don't get me wrong, but I would be surprised if the UEFI
specification changes I'd like to discuss alone would be completed
within 10 weeks, let alone implementation throughout the codebase. While
I think the plain amount of code may be a bit less than say a
MinPlatform port, the changes are much deeper and require much more
caution to avoid regressions (e.g. by invalidating undocumented
assertions). This sadly is not a matter of just replacing the underlying
library implementation or "plug-in and play" at all. It furthermore
affects many parts of the stack, the core dispatchers used for all
platforms, image emulation (EBC), UEFI userland emulation (EmuPkg), and
so on. I was rather worried the scope is too broad time-wise, but it can
be narrowed/widened as you see fit really. This is one of *the* core
components used on millions of device, and many package maintainers need
to review and validate the changes, this must really be done right the
first try. :)

Certainly we can use some of that time to perform the code reviews you
mention and write up formal ECRs for the UEFI spec changes that you
believe are needed.
I believed that was part of the workload, yes, but even without it I
think there is plenty to do.

Thank you for sending the application and alerting us to the great
work you and Vitaly have done! I'll read your paper more closely and
come back with any questions I still have.
Thank you, I will gladly explain anything unclear. Just try to not give
Laszlo too many flashbacks. :)
I haven't commented yet in this thread, as I thought my stance on this
undertaking was (or should be) obvious.

I very much welcome a replacement for the PE/COFF parser (as I consider
its security issues unfixable in an incremental manner). From my reading
of Marvin's and Vitaly's paper (draft), they have my full trust, and I'm
ready to put their upcoming code to use in ArmVirtPkg and OvmfPkg with
minimal actual code review. If fixing the pervasive security problems
around this area cannot avoid spiraling out to other core code in edk2,
such as dispatchers, and even to the PI / UEFI specs, so be it.

Regarding GSoC itself: as I stated elsewhere previously, I support
edk2's participation in GSoC, while at the same time I'm not
volunteering for mentorship at all. I'm uncertain if GSoC is the best
framework for upstreaming such a large undertaking, but if it can help,
we should use it as much as possible.

Thanks
Laszlo







With Best Regards,
Nate

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin
Häuser
Sent: Sunday, April 4, 2021 4:02 PM
To: devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; Andrew Fish
<afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [edk2-devel] [GSoC proposal] Secure Image Loader

Good day everyone,

I'll keep the introduction brief because I've been around for a while
now. :) I'm
Marvin Häuser, a third-year Computer Science student from TU
Kaiserslautern,
Germany. Late last year, my colleague Vitaly from ISP RAS and me
introduced a
formally verified Image Loader for UEFI usage at ISP RAS Open[1] due
to various
defects we outlined in the corresponding paper. Thank you once again
Laszlo
for your *incredible* review work on the publication part.

I now want to make an effort to mainline it, preferably as part of
the current
Google Summer of Code event. To be clear, my internship at ISP RAS has
concluded, and while Vitaly will be available for design discussion,
he has other
priorities at the moment and the practical part will be on me. I have
previously
submitted a proposal via the GSoC website for your review.

There are many things to consider:
1. The Image Loader is a core component, and there needs to be a
significant
level of quality and security assurance.
2. Being consumed by many packages, the proposed patch set will take
a lot of
time to review and integrate.
3. During my initial exploration, I discovered defective PPIs and
protocols (e.g.
returning data with no corresponding size) originating from the UEFI
PI and
UEFI specifications. Changes need to be discussed, settled on, and
submitted to
the UEFI Forum.
4. Some UEFI APIs like the Security Architecture protocols are
inconveniently
abstract, see 5.
5. Some of the current code does not use the existing context, or
accesses it
outside of the exposed APIs. The control flow of the dispatchers may
need to be
adapted to make the context available to appropriate APIs.

But obviously there are not only unpleasant considerations:
A. The Image Loader is mostly formally verified, and only very few
changes will
be required from the last proven state. This gives a lot of trust in
its correctness
and safety.
B. All outlined defects that are of critical nature have been fixed
successfully.
C. The Image Loader has been tested with real-world code loading
real-world
OSes on thousands of machines in the past few months, including
rejecting
malformed images (configurable by PCD).
D. The new APIs will centralise everything PE, reducing code
duplication and
potentially unsafe operations.
E. Centralising and reduced parse duplication may improve overall boot
performance.
F. The code has been coverage-tested to not contain dead code.
G. The code has been fuzz-tested including sanitizers to not invoke
undefined
behaviour.
H. I already managed to identify a malformed image in OVMF with its help
(incorrectly reported section alignment of an Intel IPXE driver). A
fix will be
submitted shortly.
I. I plan to support PE section permissions, allowing for read-only data
segments when enabled.

There are likely more points for both lists, but I hope this gives a
decent
starting point for discussion. What are your thoughts on the matter?
I strongly
encourage everyone to read the section regarding defects of our
publication[2]
to better understand the motivation. The vague points above can of
course be
elaborated in due time, as you see fit.

Thank you for your time!

Best regards,
Marvin


[1] https://github.com/mhaeuser/ISPRASOpen-SecurePE
[2] https://arxiv.org/pdf/2012.05471.pdf







Re: [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation

Ard Biesheuvel
 

On Thu, 8 Apr 2021 at 11:47, Pete Batard <pete@akeo.ie> wrote:

On 2021.04.06 15:04, Mario Bălănică wrote:
The VPU clock divisor has changed in this commit:
https://github.com/raspberrypi/firmware/commit/1e5456a,
thus breaking the mini UART clock divisor calculation on the Pi 4.

Fix this by reading the core clock from the mailbox instead.

Signed-off-by: Mario Bălănică <mariobalanica02@gmail.com>
---
Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c | 15 +++------------
Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S | 4 ----
Platform/RaspberryPi/RPi4/RPi4.dsc | 6 ++++--
3 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index 5e83bbf022eb..d2f983bf0a9f 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -34,25 +34,16 @@ SerialPortGetDivisor (
UINT32 SerialBaudRate

)

{

- UINT64 BaseClockRate;

+ UINT32 BaseClockRate;

UINT32 Divisor;



- //

- // On the Raspberry Pi, the clock to use for the 16650-compatible UART

- // is the base clock divided by the 12.12 fixed point VPU clock divisor.

- //

- BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate);

-#if (RPI_MODEL == 4)

- Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 0xFFFFFF;

- if (Divisor != 0)

- BaseClockRate = (BaseClockRate << 12) / Divisor;

-#endif

+ BaseClockRate = PcdGet32 (PcdSerialClockRate);



//

// As per the BCM2xxx datasheets:

// baudrate = system_clock_freq / (8 * (divisor + 1)).

//

- Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8);

+ Divisor = BaseClockRate / (SerialBaudRate * 8);

if (Divisor != 0) {

Divisor--;

}

diff --git a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
index 58351e4fb8cc..7008aaf8aa4c 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
+++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
@@ -85,13 +85,11 @@ ASM_FUNC (ArmPlatformPeiBootAction)
adr x2, mBoardRevision

str w0, [x2]



-#if (RPI_MODEL == 3)

run .Lclkinfo_buffer



ldr w0, .Lfrequency

adr x2, _gPcd_BinaryPatch_PcdSerialClockRate

str w0, [x2]

-#endif



ret



@@ -135,7 +133,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // end tag

.set .Lrevinfo_size, . - .Lrevinfo_buffer



-#if (RPI_MODEL == 3)

.align 4

.Lclkinfo_buffer:

.long .Lclkinfo_size

@@ -148,7 +145,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
.long 0 // frequency

.long 0 // end tag

.set .Lclkinfo_size, . - .Lclkinfo_buffer

-#endif



//UINTN

//ArmPlatformGetPrimaryCoreMpId (

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 2c05c31118d2..ff802d8347ea 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -429,7 +429,6 @@ [PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4

- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|1000000000

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27

gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8

gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200

@@ -465,6 +464,9 @@ [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"

gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE



+[PcdsPatchableInModule]

+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000

+

[PcdsDynamicHii.common.DEFAULT]



#

@@ -621,7 +623,7 @@ [Components.common]
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf

MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {

<LibraryClasses>

- SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf

+ SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf

}

Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf

EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
Reviewed-by: Pete Batard <pete@akeo.ie>
Tested-by: Pete Batard <pete@akeo.ie>
Thanks Pete.

Could anyone get me a version of this patch that is not whitespace
mangled? This one does not apply with 'git am'

8381 - 8400 of 82167