Date   

Re: [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow BGRT specification

Sean Rhodes
 

Hi Liming


Thanks

Sean

On Fri, 5 Aug 2022 at 07:00, gaoliming via groups.io <gaoliming=byosoft.com.cn@groups.io> wrote:
Sean:
  Can you give BGRT spec link? I want to check the spec description.

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sean Rhodes
> 发送时间: 2022年7月26日 16:15
> 收件人: devel@edk2.groups.io
> 抄送: Sean Rhodes <sean@...>; Zhichao Gao
> <zhichao.gao@...>; Ray Ni <ray.ni@...>; Jian J Wang
> <jian.j.wang@...>; Liming Gao <gaoliming@...>
> 主题: [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to
> follow BGRT specification
>
> Add an option to position the logo 38.2% from the top of the screen,
> which follows the BGRT specification.
>
> Cc: Zhichao Gao <zhichao.gao@...>
> Cc: Ray Ni <ray.ni@...>
> Cc: Jian J Wang <jian.j.wang@...>
> Cc: Liming Gao <gaoliming@...>
> Signed-off-by: Sean Rhodes <sean@...>
> ---
>  MdeModulePkg/Include/Protocol/PlatformLogo.h   | 3 ++-
>  MdeModulePkg/Library/BootLogoLib/BootLogoLib.c | 7 ++++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Protocol/PlatformLogo.h
> b/MdeModulePkg/Include/Protocol/PlatformLogo.h
> index 08e1dc35a4..7c9ef63c66 100644
> --- a/MdeModulePkg/Include/Protocol/PlatformLogo.h
> +++ b/MdeModulePkg/Include/Protocol/PlatformLogo.h
> @@ -29,7 +29,8 @@ typedef enum {
>    EdkiiPlatformLogoDisplayAttributeCenterBottom,
>
>    EdkiiPlatformLogoDisplayAttributeLeftBottom,
>
>    EdkiiPlatformLogoDisplayAttributeCenterLeft,
>
> -  EdkiiPlatformLogoDisplayAttributeCenter
>
> +  EdkiiPlatformLogoDisplayAttributeCenter,
>
> +  EdkiiPlatformLogoDisplayAttributeBGRTSpecification
>
>  } EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE;
>
>
>
>  /**
>
> diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> index 478ec2d40e..ac086f9c79 100644
> --- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> +++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> @@ -169,7 +169,6 @@ BootLogoEnableLogo (
>          DestX = SizeOfX - Image.Width;
>
>          DestY = 0;
>
>          break;
>
> -
>
>        case EdkiiPlatformLogoDisplayAttributeCenterLeft:
>
>          DestX = 0;
>
>          DestY = (SizeOfY - Image.Height) / 2;
>
> @@ -182,7 +181,6 @@ BootLogoEnableLogo (
>          DestX = SizeOfX - Image.Width;
>
>          DestY = (SizeOfY - Image.Height) / 2;
>
>          break;
>
> -
>
>        case EdkiiPlatformLogoDisplayAttributeLeftBottom:
>
>          DestX = 0;
>
>          DestY = SizeOfY - Image.Height;
>
> @@ -195,7 +193,10 @@ BootLogoEnableLogo (
>          DestX = SizeOfX - Image.Width;
>
>          DestY = SizeOfY - Image.Height;
>
>          break;
>
> -
>
> +      case EdkiiPlatformLogoDisplayAttributeBGRTSpecification:
>
> +        DestX = (SizeOfX - Image.Width) / 2;
>
> +        DestY = (SizeOfY * 382) / 1000 - Image.Height / 2;
>
> +        break;
>
>        default:
>
>          ASSERT (FALSE);
>
>          continue;
>
> --
> 2.34.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#91842): https://edk2.groups.io/g/devel/message/91842
> Mute This Topic: https://groups.io/mt/92623125/4905953
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@...]
> -=-=-=-=-=-=
>









Re: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default fused in MADT

Ni, Ray
 

Jack,
The patch removes all the sorting logic. It simplifies the logic a lot. Thanks for that!

ACPI spec " 5.2.12.1 MADT Processor Local APIC / SAPIC Structure Entry Order"
talked about why the BSP needs to be in the first entry of MADT and why first logical
processor of each core needs to be before second logical processor.
With the reasons, I totally agree that we don't need to sort the MADT any more
after the hyper-threading and many-core support have been enabled for quite
a long time in OS.

Some minor comments:
1. Can you check if "CoreThreadMask" can be removed?
2. Can you please rename the SortCpuLocalApicInTable? Maybe just use "CreateCpuLocalApicInTable"? There is no sorting any more😊
3. Can you please check if " mCpuOrderSorted" is needed? It's needed when the function is called multiple times.
4. If it's needed, can you please rename it to "mCpuLocalApicInTableCreated"?
5. If it's not needed, can you please think about if " mCpuApicIdOrderTable" can be changed to a local variable?

Thanks,
Ray

-----Original Message-----
From: Lin, JackX <jackx.lin@...>
Sent: Thursday, July 28, 2022 3:25 PM
To: devel@edk2.groups.io
Cc: Lin, JackX <jackx.lin@...>; Lin, JackX <jackx.lin@...>; Chiu,
Chasel <chasel.chiu@...>; Dong, Eric <eric.dong@...>; Yao,
Jiewen <jiewen.yao@...>; Ni, Ray <ray.ni@...>; Chaganty,
Rangasai V <rangasai.v.chaganty@...>; Kuo, Donald
<donald.kuo@...>; Kumar, Chandana C
<chandana.c.kumar@...>; Palakshareddy; Palakshareddy, Lavanya C
<lavanya.c.palakshareddy@...>
Subject: [edk2-platforms:PATCH] Modify processor _UID ordering by CPU
default fused in MADT

BIOS should not reordering cpu processor_uid

Signed-off-by: JackX Lin <JackX.Lin@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Dong Eric <eric.dong@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Donald Kuo <Donald.Kuo@...>
Cc: Chandana C Kumar <chandana.c.kumar@...>
Cc: Palakshareddy, Lavanya C <lavanya.c.palakshareddy@...>
Cc: JackX Lin <JackX.Lin@...>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 99 ++++---
--------------------------------------------------------------------------------------------
1 file changed, 4 insertions(+), 95 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index c7e87cbd7d..d0e8891918 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -62,33 +62,6 @@ EFI_CPU_ID_ORDER_MAP *mCpuApicIdOrderTable
= NULL;
UINTN mNumberOfCpus = 0;
UINTN mNumberOfEnabledCPUs = 0;

-
-/**
- The function is called by PerformQuickSort to compare int values.
-
- @param[in] Left The pointer to first buffer.
- @param[in] Right The pointer to second buffer.
-
- @return -1 Buffer1 is less than Buffer2.
- @return 1 Buffer1 is greater than Buffer2.
-
-**/
-INTN
-EFIAPI
-ApicIdCompareFunction (
- IN CONST VOID *Left,
- IN CONST VOID *Right
- )
-{
- UINT32 LeftApicId;
- UINT32 RightApicId;
-
- LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
- RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
-
- return (LeftApicId > RightApicId)? 1 : (-1);
-}
-
/**
Print Cpu Apic ID Table

@@ -168,21 +141,16 @@ SortCpuLocalApicInTable (
EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
UINT32 Index;
UINT32 CurrProcessor;
- UINT32 BspApicId;
- EFI_CPU_ID_ORDER_MAP TempVal;
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
UINT32 CoreThreadMask;
- EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
UINT32 Socket;

- Index = 0;
Status = EFI_SUCCESS;

if (mCpuOrderSorted) {
return Status;
}

- TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
(EFI_CPU_ID_ORDER_MAP));
CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);

for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
CurrProcessor++, Index++) {
@@ -192,7 +160,7 @@ SortCpuLocalApicInTable (
&ProcessorInfoBuffer
);

- CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
&TempCpuApicIdOrderTable[Index];
+ CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *)
&mCpuApicIdOrderTable[Index];
if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
@@ -214,74 +182,16 @@ SortCpuLocalApicInTable (
} //end if PROC ENABLE
} //end for CurrentProcessor

- //keep for debug purpose
DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
mNumOfBitShift));
- DebugDisplayReOrderTable (TempCpuApicIdOrderTable);

//
// Get Bsp Apic Id
//
- BspApicId = GetApicId ();
- DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
-
- //
- //check to see if 1st entry is BSP, if not swap it
- //
- if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
(TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
- CopyMem (&TempVal, &TempCpuApicIdOrderTable[Index], sizeof
(EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[Index],
&TempCpuApicIdOrderTable[0], sizeof (EFI_CPU_ID_ORDER_MAP));
- CopyMem (&TempCpuApicIdOrderTable[0], &TempVal, sizeof
(EFI_CPU_ID_ORDER_MAP));
- break;
- }
- }
-
- if (mNumberOfCpus <= Index) {
- DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
Bufferflow\n"));
- return EFI_INVALID_PARAMETER;
- }
- }
-
- //
- // 1. Sort TempCpuApicIdOrderTable,
- // sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
and the BSP must in the fist location of the table.
- // So, start sorting the table from the second element and total elements
are mNumberOfCpus-1.
- //
- PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1),
sizeof (EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE)
ApicIdCompareFunction);
+ DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));

- //
- // 2. Sort and map the primary threads to the front of the
CpuApicIdOrderTable
- //
- for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
&TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }

//
- // 3. Sort and map the second threads to the middle of the
CpuApicIdOrderTable
- //
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Thread) == 1) { //second thread
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
&TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- //
- // 4. Sort and map the not enabled threads to the bottom of the
CpuApicIdOrderTable
- //
- for (Index = 0; Index < mNumberOfCpus; Index++) {
- if (TempCpuApicIdOrderTable[Index].Flags == 0) { // not enabled
- CopyMem (&mCpuApicIdOrderTable[CurrProcessor],
&TempCpuApicIdOrderTable[Index], sizeof (EFI_CPU_ID_ORDER_MAP));
- CurrProcessor++;
- }
- }
-
- //
- // 5. Re-assign AcpiProcessorId for AcpiProcessorUid uses purpose.
+ // Fill in AcpiProcessorUid.
//
for (Socket = 0; Socket < FixedPcdGet32 (PcdMaxCpuSocketCount);
Socket++) {
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
CurrProcessor++) {
@@ -292,8 +202,7 @@ SortCpuLocalApicInTable (
}
}

- //keep for debug purpose
- DEBUG ((DEBUG_INFO, "APIC ID Order Table ReOrdered\n"));
+ DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init.
CoreThreadMask = %x, mNumOfBitShift = %x\n", CoreThreadMask,
mNumOfBitShift));
DebugDisplayReOrderTable (mCpuApicIdOrderTable);

mCpuOrderSorted = TRUE;
--
2.32.0.windows.2


[PATCH v2 3/3] UefiCpuPkg: Simplify the struct definition of CPU_EXCEPTION_INIT_DATA

Zhiguang Liu
 

CPU_EXCEPTION_INIT_DATA is now an internal implementation of
CpuExceptionHandlerLib. Union can be removed since Ia32 and X64 have the
same definition

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
---
.../CpuExceptionCommon.h | 108 +++++++++---------
.../CpuExceptionHandlerLib/DxeException.c | 24 ++--
.../Ia32/ArchExceptionHandler.c | 68 +++++------
.../CpuExceptionHandlerLib/PeiCpuException.c | 24 ++--
.../X64/ArchExceptionHandler.c | 64 +++++------
5 files changed, 143 insertions(+), 145 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
index 67d81d50d2..11a5624f51 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
@@ -49,61 +49,59 @@

#define CPU_TSS_GDT_SIZE (SIZE_2KB + CPU_TSS_DESC_SIZE + CPU_TSS_SIZE)

-typedef union {
- struct {
- //
- // The address of top of known good stack reserved for *ALL* exceptions
- // listed in field StackSwitchExceptions.
- //
- UINTN KnownGoodStackTop;
- //
- // The size of known good stack for *ONE* exception only.
- //
- UINTN KnownGoodStackSize;
- //
- // Buffer of exception vector list for stack switch.
- //
- UINT8 *StackSwitchExceptions;
- //
- // Number of exception vectors in StackSwitchExceptions.
- //
- UINTN StackSwitchExceptionNumber;
- //
- // Buffer of IDT table. It must be type of IA32_IDT_GATE_DESCRIPTOR.
- // Normally there's no need to change IDT table size.
- //
- VOID *IdtTable;
- //
- // Size of buffer for IdtTable.
- //
- UINTN IdtTableSize;
- //
- // Buffer of GDT table. It must be type of IA32_SEGMENT_DESCRIPTOR.
- //
- VOID *GdtTable;
- //
- // Size of buffer for GdtTable.
- //
- UINTN GdtTableSize;
- //
- // Pointer to start address of descriptor of exception task gate in the
- // GDT table. It must be type of IA32_TSS_DESCRIPTOR.
- //
- VOID *ExceptionTssDesc;
- //
- // Size of buffer for ExceptionTssDesc.
- //
- UINTN ExceptionTssDescSize;
- //
- // Buffer of task-state segment for exceptions. It must be type of
- // IA32_TASK_STATE_SEGMENT.
- //
- VOID *ExceptionTss;
- //
- // Size of buffer for ExceptionTss.
- //
- UINTN ExceptionTssSize;
- } Ia32, X64;
+typedef struct {
+ //
+ // The address of top of known good stack reserved for *ALL* exceptions
+ // listed in field StackSwitchExceptions.
+ //
+ UINTN KnownGoodStackTop;
+ //
+ // The size of known good stack for *ONE* exception only.
+ //
+ UINTN KnownGoodStackSize;
+ //
+ // Buffer of exception vector list for stack switch.
+ //
+ UINT8 *StackSwitchExceptions;
+ //
+ // Number of exception vectors in StackSwitchExceptions.
+ //
+ UINTN StackSwitchExceptionNumber;
+ //
+ // Buffer of IDT table. It must be type of IA32_IDT_GATE_DESCRIPTOR.
+ // Normally there's no need to change IDT table size.
+ //
+ VOID *IdtTable;
+ //
+ // Size of buffer for IdtTable.
+ //
+ UINTN IdtTableSize;
+ //
+ // Buffer of GDT table. It must be type of IA32_SEGMENT_DESCRIPTOR.
+ //
+ VOID *GdtTable;
+ //
+ // Size of buffer for GdtTable.
+ //
+ UINTN GdtTableSize;
+ //
+ // Pointer to start address of descriptor of exception task gate in the
+ // GDT table. It must be type of IA32_TSS_DESCRIPTOR.
+ //
+ VOID *ExceptionTssDesc;
+ //
+ // Size of buffer for ExceptionTssDesc.
+ //
+ UINTN ExceptionTssDescSize;
+ //
+ // Buffer of task-state segment for exceptions. It must be type of
+ // IA32_TASK_STATE_SEGMENT.
+ //
+ VOID *ExceptionTss;
+ //
+ // Size of buffer for ExceptionTss.
+ //
+ UINTN ExceptionTssSize;
} CPU_EXCEPTION_INIT_DATA;

//
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index 674029f8ac..d90c607bd7 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -190,18 +190,18 @@ InitializeSeparateExceptionStacks (
}

AsmReadIdtr (&Idtr);
- EssData.X64.KnownGoodStackTop = StackTop;
- EssData.X64.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
- EssData.X64.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
- EssData.X64.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
- EssData.X64.IdtTable = (VOID *)Idtr.Base;
- EssData.X64.IdtTableSize = Idtr.Limit + 1;
- EssData.X64.GdtTable = NewGdtTable;
- EssData.X64.GdtTableSize = CPU_TSS_DESC_SIZE + Gdtr.Limit + 1;
- EssData.X64.ExceptionTssDesc = NewGdtTable + Gdtr.Limit + 1;
- EssData.X64.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
- EssData.X64.ExceptionTss = NewGdtTable + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
- EssData.X64.ExceptionTssSize = CPU_TSS_SIZE;
+ EssData.KnownGoodStackTop = StackTop;
+ EssData.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
+ EssData.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
+ EssData.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
+ EssData.IdtTable = (VOID *)Idtr.Base;
+ EssData.IdtTableSize = Idtr.Limit + 1;
+ EssData.GdtTable = NewGdtTable;
+ EssData.GdtTableSize = CPU_TSS_DESC_SIZE + Gdtr.Limit + 1;
+ EssData.ExceptionTssDesc = NewGdtTable + Gdtr.Limit + 1;
+ EssData.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
+ EssData.ExceptionTss = NewGdtTable + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
+ EssData.ExceptionTssSize = CPU_TSS_SIZE;

return ArchSetupExceptionStack (&EssData);
}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
index fa62074023..194d3a499b 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
@@ -132,15 +132,15 @@ ArchSetupExceptionStack (
EXCEPTION_HANDLER_TEMPLATE_MAP TemplateMap;

if ((StackSwitchData == NULL) ||
- (StackSwitchData->Ia32.KnownGoodStackTop == 0) ||
- (StackSwitchData->Ia32.KnownGoodStackSize == 0) ||
- (StackSwitchData->Ia32.StackSwitchExceptions == NULL) ||
- (StackSwitchData->Ia32.StackSwitchExceptionNumber == 0) ||
- (StackSwitchData->Ia32.StackSwitchExceptionNumber > CPU_EXCEPTION_NUM) ||
- (StackSwitchData->Ia32.GdtTable == NULL) ||
- (StackSwitchData->Ia32.IdtTable == NULL) ||
- (StackSwitchData->Ia32.ExceptionTssDesc == NULL) ||
- (StackSwitchData->Ia32.ExceptionTss == NULL))
+ (StackSwitchData->KnownGoodStackTop == 0) ||
+ (StackSwitchData->KnownGoodStackSize == 0) ||
+ (StackSwitchData->StackSwitchExceptions == NULL) ||
+ (StackSwitchData->StackSwitchExceptionNumber == 0) ||
+ (StackSwitchData->StackSwitchExceptionNumber > CPU_EXCEPTION_NUM) ||
+ (StackSwitchData->GdtTable == NULL) ||
+ (StackSwitchData->IdtTable == NULL) ||
+ (StackSwitchData->ExceptionTssDesc == NULL) ||
+ (StackSwitchData->ExceptionTss == NULL))
{
return EFI_INVALID_PARAMETER;
}
@@ -150,16 +150,16 @@ ArchSetupExceptionStack (
// one or newly allocated, has enough space to hold descriptors for exception
// task-state segments.
//
- if (((UINTN)StackSwitchData->Ia32.GdtTable & (IA32_GDT_ALIGNMENT - 1)) != 0) {
+ if (((UINTN)StackSwitchData->GdtTable & (IA32_GDT_ALIGNMENT - 1)) != 0) {
return EFI_INVALID_PARAMETER;
}

- if ((UINTN)StackSwitchData->Ia32.ExceptionTssDesc < (UINTN)(StackSwitchData->Ia32.GdtTable)) {
+ if ((UINTN)StackSwitchData->ExceptionTssDesc < (UINTN)(StackSwitchData->GdtTable)) {
return EFI_INVALID_PARAMETER;
}

- if ((UINTN)StackSwitchData->Ia32.ExceptionTssDesc + StackSwitchData->Ia32.ExceptionTssDescSize >
- ((UINTN)(StackSwitchData->Ia32.GdtTable) + StackSwitchData->Ia32.GdtTableSize))
+ if ((UINTN)StackSwitchData->ExceptionTssDesc + StackSwitchData->ExceptionTssDescSize >
+ ((UINTN)(StackSwitchData->GdtTable) + StackSwitchData->GdtTableSize))
{
return EFI_INVALID_PARAMETER;
}
@@ -168,20 +168,20 @@ ArchSetupExceptionStack (
// We need one descriptor and one TSS for current task and every exception
// specified.
//
- if (StackSwitchData->Ia32.ExceptionTssDescSize <
- sizeof (IA32_TSS_DESCRIPTOR) * (StackSwitchData->Ia32.StackSwitchExceptionNumber + 1))
+ if (StackSwitchData->ExceptionTssDescSize <
+ sizeof (IA32_TSS_DESCRIPTOR) * (StackSwitchData->StackSwitchExceptionNumber + 1))
{
return EFI_INVALID_PARAMETER;
}

- if (StackSwitchData->Ia32.ExceptionTssSize <
- sizeof (IA32_TASK_STATE_SEGMENT) * (StackSwitchData->Ia32.StackSwitchExceptionNumber + 1))
+ if (StackSwitchData->ExceptionTssSize <
+ sizeof (IA32_TASK_STATE_SEGMENT) * (StackSwitchData->StackSwitchExceptionNumber + 1))
{
return EFI_INVALID_PARAMETER;
}

- TssDesc = StackSwitchData->Ia32.ExceptionTssDesc;
- Tss = StackSwitchData->Ia32.ExceptionTss;
+ TssDesc = StackSwitchData->ExceptionTssDesc;
+ Tss = StackSwitchData->ExceptionTss;

//
// Initialize new GDT table and/or IDT table, if any
@@ -191,20 +191,20 @@ ArchSetupExceptionStack (

GdtSize = (UINTN)TssDesc +
sizeof (IA32_TSS_DESCRIPTOR) *
- (StackSwitchData->Ia32.StackSwitchExceptionNumber + 1) -
- (UINTN)(StackSwitchData->Ia32.GdtTable);
- if ((UINTN)StackSwitchData->Ia32.GdtTable != Gdtr.Base) {
- CopyMem (StackSwitchData->Ia32.GdtTable, (VOID *)Gdtr.Base, Gdtr.Limit + 1);
- Gdtr.Base = (UINTN)StackSwitchData->Ia32.GdtTable;
+ (StackSwitchData->StackSwitchExceptionNumber + 1) -
+ (UINTN)(StackSwitchData->GdtTable);
+ if ((UINTN)StackSwitchData->GdtTable != Gdtr.Base) {
+ CopyMem (StackSwitchData->GdtTable, (VOID *)Gdtr.Base, Gdtr.Limit + 1);
+ Gdtr.Base = (UINTN)StackSwitchData->GdtTable;
Gdtr.Limit = (UINT16)GdtSize - 1;
}

- if ((UINTN)StackSwitchData->Ia32.IdtTable != Idtr.Base) {
- Idtr.Base = (UINTN)StackSwitchData->Ia32.IdtTable;
+ if ((UINTN)StackSwitchData->IdtTable != Idtr.Base) {
+ Idtr.Base = (UINTN)StackSwitchData->IdtTable;
}

- if (StackSwitchData->Ia32.IdtTableSize > 0) {
- Idtr.Limit = (UINT16)(StackSwitchData->Ia32.IdtTableSize - 1);
+ if (StackSwitchData->IdtTableSize > 0) {
+ Idtr.Limit = (UINT16)(StackSwitchData->IdtTableSize - 1);
}

//
@@ -226,10 +226,10 @@ ArchSetupExceptionStack (
// Fixup exception task descriptor and task-state segment
//
AsmGetTssTemplateMap (&TemplateMap);
- StackTop = StackSwitchData->Ia32.KnownGoodStackTop - CPU_STACK_ALIGNMENT;
+ StackTop = StackSwitchData->KnownGoodStackTop - CPU_STACK_ALIGNMENT;
StackTop = (UINTN)ALIGN_POINTER (StackTop, CPU_STACK_ALIGNMENT);
- IdtTable = StackSwitchData->Ia32.IdtTable;
- for (Index = 0; Index < StackSwitchData->Ia32.StackSwitchExceptionNumber; ++Index) {
+ IdtTable = StackSwitchData->IdtTable;
+ for (Index = 0; Index < StackSwitchData->StackSwitchExceptionNumber; ++Index) {
TssDesc += 1;
Tss += 1;

@@ -250,7 +250,7 @@ ArchSetupExceptionStack (
//
// Fixup TSS
//
- Vector = StackSwitchData->Ia32.StackSwitchExceptions[Index];
+ Vector = StackSwitchData->StackSwitchExceptions[Index];
if ((Vector >= CPU_EXCEPTION_NUM) ||
(Vector >= (Idtr.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR)))
{
@@ -270,7 +270,7 @@ ArchSetupExceptionStack (
Tss->FS = AsmReadFs ();
Tss->GS = AsmReadGs ();

- StackTop -= StackSwitchData->Ia32.KnownGoodStackSize;
+ StackTop -= StackSwitchData->KnownGoodStackSize;

//
// Update IDT to use Task Gate for given exception
@@ -290,7 +290,7 @@ ArchSetupExceptionStack (
//
// Load current task
//
- AsmWriteTr ((UINT16)((UINTN)StackSwitchData->Ia32.ExceptionTssDesc - Gdtr.Base));
+ AsmWriteTr ((UINT16)((UINTN)StackSwitchData->ExceptionTssDesc - Gdtr.Base));

//
// Publish IDT
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
index fe8d02d3e4..5952295126 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
@@ -236,18 +236,18 @@ InitializeSeparateExceptionStacks (
NewGdtTable = ALIGN_POINTER (StackTop, sizeof (IA32_TSS_DESCRIPTOR));

AsmReadIdtr (&Idtr);
- EssData.X64.KnownGoodStackTop = StackTop;
- EssData.X64.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
- EssData.X64.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
- EssData.X64.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
- EssData.X64.IdtTable = (VOID *)Idtr.Base;
- EssData.X64.IdtTableSize = Idtr.Limit + 1;
- EssData.X64.GdtTable = NewGdtTable;
- EssData.X64.GdtTableSize = CPU_TSS_DESC_SIZE + Gdtr.Limit + 1;
- EssData.X64.ExceptionTssDesc = NewGdtTable + Gdtr.Limit + 1;
- EssData.X64.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
- EssData.X64.ExceptionTss = NewGdtTable + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
- EssData.X64.ExceptionTssSize = CPU_TSS_SIZE;
+ EssData.KnownGoodStackTop = StackTop;
+ EssData.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
+ EssData.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
+ EssData.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
+ EssData.IdtTable = (VOID *)Idtr.Base;
+ EssData.IdtTableSize = Idtr.Limit + 1;
+ EssData.GdtTable = NewGdtTable;
+ EssData.GdtTableSize = CPU_TSS_DESC_SIZE + Gdtr.Limit + 1;
+ EssData.ExceptionTssDesc = NewGdtTable + Gdtr.Limit + 1;
+ EssData.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
+ EssData.ExceptionTss = NewGdtTable + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
+ EssData.ExceptionTssSize = CPU_TSS_SIZE;

return ArchSetupExceptionStack (&EssData);
}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index ff0dde4f12..c14ac66c43 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -136,15 +136,15 @@ ArchSetupExceptionStack (
UINTN GdtSize;

if ((StackSwitchData == NULL) ||
- (StackSwitchData->X64.KnownGoodStackTop == 0) ||
- (StackSwitchData->X64.KnownGoodStackSize == 0) ||
- (StackSwitchData->X64.StackSwitchExceptions == NULL) ||
- (StackSwitchData->X64.StackSwitchExceptionNumber == 0) ||
- (StackSwitchData->X64.StackSwitchExceptionNumber > CPU_EXCEPTION_NUM) ||
- (StackSwitchData->X64.GdtTable == NULL) ||
- (StackSwitchData->X64.IdtTable == NULL) ||
- (StackSwitchData->X64.ExceptionTssDesc == NULL) ||
- (StackSwitchData->X64.ExceptionTss == NULL))
+ (StackSwitchData->KnownGoodStackTop == 0) ||
+ (StackSwitchData->KnownGoodStackSize == 0) ||
+ (StackSwitchData->StackSwitchExceptions == NULL) ||
+ (StackSwitchData->StackSwitchExceptionNumber == 0) ||
+ (StackSwitchData->StackSwitchExceptionNumber > CPU_EXCEPTION_NUM) ||
+ (StackSwitchData->GdtTable == NULL) ||
+ (StackSwitchData->IdtTable == NULL) ||
+ (StackSwitchData->ExceptionTssDesc == NULL) ||
+ (StackSwitchData->ExceptionTss == NULL))
{
return EFI_INVALID_PARAMETER;
}
@@ -154,16 +154,16 @@ ArchSetupExceptionStack (
// one or newly allocated, has enough space to hold descriptors for exception
// task-state segments.
//
- if (((UINTN)StackSwitchData->X64.GdtTable & (IA32_GDT_ALIGNMENT - 1)) != 0) {
+ if (((UINTN)StackSwitchData->GdtTable & (IA32_GDT_ALIGNMENT - 1)) != 0) {
return EFI_INVALID_PARAMETER;
}

- if ((UINTN)StackSwitchData->X64.ExceptionTssDesc < (UINTN)(StackSwitchData->X64.GdtTable)) {
+ if ((UINTN)StackSwitchData->ExceptionTssDesc < (UINTN)(StackSwitchData->GdtTable)) {
return EFI_INVALID_PARAMETER;
}

- if (((UINTN)StackSwitchData->X64.ExceptionTssDesc + StackSwitchData->X64.ExceptionTssDescSize) >
- ((UINTN)(StackSwitchData->X64.GdtTable) + StackSwitchData->X64.GdtTableSize))
+ if (((UINTN)StackSwitchData->ExceptionTssDesc + StackSwitchData->ExceptionTssDescSize) >
+ ((UINTN)(StackSwitchData->GdtTable) + StackSwitchData->GdtTableSize))
{
return EFI_INVALID_PARAMETER;
}
@@ -171,20 +171,20 @@ ArchSetupExceptionStack (
//
// One task gate descriptor and one task-state segment are needed.
//
- if (StackSwitchData->X64.ExceptionTssDescSize < sizeof (IA32_TSS_DESCRIPTOR)) {
+ if (StackSwitchData->ExceptionTssDescSize < sizeof (IA32_TSS_DESCRIPTOR)) {
return EFI_INVALID_PARAMETER;
}

- if (StackSwitchData->X64.ExceptionTssSize < sizeof (IA32_TASK_STATE_SEGMENT)) {
+ if (StackSwitchData->ExceptionTssSize < sizeof (IA32_TASK_STATE_SEGMENT)) {
return EFI_INVALID_PARAMETER;
}

//
// Interrupt stack table supports only 7 vectors.
//
- TssDesc = StackSwitchData->X64.ExceptionTssDesc;
- Tss = StackSwitchData->X64.ExceptionTss;
- if (StackSwitchData->X64.StackSwitchExceptionNumber > ARRAY_SIZE (Tss->IST)) {
+ TssDesc = StackSwitchData->ExceptionTssDesc;
+ Tss = StackSwitchData->ExceptionTss;
+ if (StackSwitchData->StackSwitchExceptionNumber > ARRAY_SIZE (Tss->IST)) {
return EFI_INVALID_PARAMETER;
}

@@ -195,19 +195,19 @@ ArchSetupExceptionStack (
AsmReadGdtr (&Gdtr);

GdtSize = (UINTN)TssDesc + sizeof (IA32_TSS_DESCRIPTOR) -
- (UINTN)(StackSwitchData->X64.GdtTable);
- if ((UINTN)StackSwitchData->X64.GdtTable != Gdtr.Base) {
- CopyMem (StackSwitchData->X64.GdtTable, (VOID *)Gdtr.Base, Gdtr.Limit + 1);
- Gdtr.Base = (UINTN)StackSwitchData->X64.GdtTable;
+ (UINTN)(StackSwitchData->GdtTable);
+ if ((UINTN)StackSwitchData->GdtTable != Gdtr.Base) {
+ CopyMem (StackSwitchData->GdtTable, (VOID *)Gdtr.Base, Gdtr.Limit + 1);
+ Gdtr.Base = (UINTN)StackSwitchData->GdtTable;
Gdtr.Limit = (UINT16)GdtSize - 1;
}

- if ((UINTN)StackSwitchData->X64.IdtTable != Idtr.Base) {
- Idtr.Base = (UINTN)StackSwitchData->X64.IdtTable;
+ if ((UINTN)StackSwitchData->IdtTable != Idtr.Base) {
+ Idtr.Base = (UINTN)StackSwitchData->IdtTable;
}

- if (StackSwitchData->X64.IdtTableSize > 0) {
- Idtr.Limit = (UINT16)(StackSwitchData->X64.IdtTableSize - 1);
+ if (StackSwitchData->IdtTableSize > 0) {
+ Idtr.Limit = (UINT16)(StackSwitchData->IdtTableSize - 1);
}

//
@@ -231,20 +231,20 @@ ArchSetupExceptionStack (
// Fixup exception task descriptor and task-state segment
//
ZeroMem (Tss, sizeof (*Tss));
- StackTop = StackSwitchData->X64.KnownGoodStackTop - CPU_STACK_ALIGNMENT;
+ StackTop = StackSwitchData->KnownGoodStackTop - CPU_STACK_ALIGNMENT;
StackTop = (UINTN)ALIGN_POINTER (StackTop, CPU_STACK_ALIGNMENT);
- IdtTable = StackSwitchData->X64.IdtTable;
- for (Index = 0; Index < StackSwitchData->X64.StackSwitchExceptionNumber; ++Index) {
+ IdtTable = StackSwitchData->IdtTable;
+ for (Index = 0; Index < StackSwitchData->StackSwitchExceptionNumber; ++Index) {
//
// Fixup IST
//
Tss->IST[Index] = StackTop;
- StackTop -= StackSwitchData->X64.KnownGoodStackSize;
+ StackTop -= StackSwitchData->KnownGoodStackSize;

//
// Set the IST field to enable corresponding IST
//
- Vector = StackSwitchData->X64.StackSwitchExceptions[Index];
+ Vector = StackSwitchData->StackSwitchExceptions[Index];
if ((Vector >= CPU_EXCEPTION_NUM) ||
(Vector >= (Idtr.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR)))
{
@@ -262,7 +262,7 @@ ArchSetupExceptionStack (
//
// Load current task
//
- AsmWriteTr ((UINT16)((UINTN)StackSwitchData->X64.ExceptionTssDesc - Gdtr.Base));
+ AsmWriteTr ((UINT16)((UINTN)StackSwitchData->ExceptionTssDesc - Gdtr.Base));

//
// Publish IDT
--
2.31.1.windows.1


[PATCH v2 2/3] MdeModulePkg: Move CPU_EXCEPTION_INIT_DATA to UefiCpuPkg

Zhiguang Liu
 

Since the API InitializeSeparateExceptionStacks is simplified and does't
use the struct CPU_EXCEPTION_INIT_DATA, CPU_EXCEPTION_INIT_DATA become
a inner implementation of CpuExcetionHandlerLib. Remove it from
MdeModulePkg. Also, two fields (Revision and InitDefaultHandlers)are
useless, can be removed.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Leif Lindholm <quic_llindhol@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Liming Gao <gaoliming@...>
Cc: Jian J Wang <jian.j.wang@...>
Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
---
.../Include/Library/CpuExceptionHandlerLib.h | 67 -------------------
.../CpuExceptionCommon.h | 59 +++++++++++++++-
2 files changed, 58 insertions(+), 68 deletions(-)

diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
index 8d44ed916a..94e9b20ae1 100644
--- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
+++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
@@ -13,73 +13,6 @@
#include <Ppi/VectorHandoffInfo.h>
#include <Protocol/Cpu.h>

-#define CPU_EXCEPTION_INIT_DATA_REV 1
-
-typedef union {
- struct {
- //
- // Revision number of this structure.
- //
- UINT32 Revision;
- //
- // The address of top of known good stack reserved for *ALL* exceptions
- // listed in field StackSwitchExceptions.
- //
- UINTN KnownGoodStackTop;
- //
- // The size of known good stack for *ONE* exception only.
- //
- UINTN KnownGoodStackSize;
- //
- // Buffer of exception vector list for stack switch.
- //
- UINT8 *StackSwitchExceptions;
- //
- // Number of exception vectors in StackSwitchExceptions.
- //
- UINTN StackSwitchExceptionNumber;
- //
- // Buffer of IDT table. It must be type of IA32_IDT_GATE_DESCRIPTOR.
- // Normally there's no need to change IDT table size.
- //
- VOID *IdtTable;
- //
- // Size of buffer for IdtTable.
- //
- UINTN IdtTableSize;
- //
- // Buffer of GDT table. It must be type of IA32_SEGMENT_DESCRIPTOR.
- //
- VOID *GdtTable;
- //
- // Size of buffer for GdtTable.
- //
- UINTN GdtTableSize;
- //
- // Pointer to start address of descriptor of exception task gate in the
- // GDT table. It must be type of IA32_TSS_DESCRIPTOR.
- //
- VOID *ExceptionTssDesc;
- //
- // Size of buffer for ExceptionTssDesc.
- //
- UINTN ExceptionTssDescSize;
- //
- // Buffer of task-state segment for exceptions. It must be type of
- // IA32_TASK_STATE_SEGMENT.
- //
- VOID *ExceptionTss;
- //
- // Size of buffer for ExceptionTss.
- //
- UINTN ExceptionTssSize;
- //
- // Flag to indicate if default handlers should be initialized or not.
- //
- BOOLEAN InitDefaultHandlers;
- } Ia32, X64;
-} CPU_EXCEPTION_INIT_DATA;
-
/**
Initializes all CPU exceptions entries and provides the default exception handlers.

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
index fd42c4be0f..67d81d50d2 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
@@ -1,7 +1,7 @@
/** @file
Common header file for CPU Exception Handler Library.

- Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -49,6 +49,63 @@

#define CPU_TSS_GDT_SIZE (SIZE_2KB + CPU_TSS_DESC_SIZE + CPU_TSS_SIZE)

+typedef union {
+ struct {
+ //
+ // The address of top of known good stack reserved for *ALL* exceptions
+ // listed in field StackSwitchExceptions.
+ //
+ UINTN KnownGoodStackTop;
+ //
+ // The size of known good stack for *ONE* exception only.
+ //
+ UINTN KnownGoodStackSize;
+ //
+ // Buffer of exception vector list for stack switch.
+ //
+ UINT8 *StackSwitchExceptions;
+ //
+ // Number of exception vectors in StackSwitchExceptions.
+ //
+ UINTN StackSwitchExceptionNumber;
+ //
+ // Buffer of IDT table. It must be type of IA32_IDT_GATE_DESCRIPTOR.
+ // Normally there's no need to change IDT table size.
+ //
+ VOID *IdtTable;
+ //
+ // Size of buffer for IdtTable.
+ //
+ UINTN IdtTableSize;
+ //
+ // Buffer of GDT table. It must be type of IA32_SEGMENT_DESCRIPTOR.
+ //
+ VOID *GdtTable;
+ //
+ // Size of buffer for GdtTable.
+ //
+ UINTN GdtTableSize;
+ //
+ // Pointer to start address of descriptor of exception task gate in the
+ // GDT table. It must be type of IA32_TSS_DESCRIPTOR.
+ //
+ VOID *ExceptionTssDesc;
+ //
+ // Size of buffer for ExceptionTssDesc.
+ //
+ UINTN ExceptionTssDescSize;
+ //
+ // Buffer of task-state segment for exceptions. It must be type of
+ // IA32_TASK_STATE_SEGMENT.
+ //
+ VOID *ExceptionTss;
+ //
+ // Size of buffer for ExceptionTss.
+ //
+ UINTN ExceptionTssSize;
+ } Ia32, X64;
+} CPU_EXCEPTION_INIT_DATA;
+
//
// Record exception handler information
//
--
2.31.1.windows.1


[PATCH v2 1/3] UefiCpuPkg: Simplify InitializeSeparateExceptionStacks

Zhiguang Liu
 

Hide the Exception implementation details in CpuExcetionHandlerLib and
caller only need to provide buffer

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Leif Lindholm <quic_llindhol@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Liming Gao <gaoliming@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Reviewed-by: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Zhiguang Liu <zhiguang.liu@...>
---
.../Library/ArmExceptionLib/ArmExceptionLib.c | 15 +-
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 4 +-
.../Include/Library/CpuExceptionHandlerLib.h | 15 +-
.../CpuExceptionHandlerLibNull.c | 15 +-
UefiCpuPkg/CpuDxe/CpuMp.c | 157 +++-------------
UefiCpuPkg/CpuDxe/CpuMp.h | 10 +-
UefiCpuPkg/CpuMpPei/CpuMpPei.c | 173 +++---------------
UefiCpuPkg/CpuMpPei/CpuMpPei.h | 10 +-
.../CpuExceptionHandlerLib/DxeException.c | 112 +++++++++---
.../Ia32/ArchExceptionHandler.c | 3 +-
.../CpuExceptionHandlerLib/PeiCpuException.c | 94 +++++++++-
.../PeiCpuExceptionHandlerLib.inf | 4 +-
.../SecPeiCpuException.c | 15 +-
.../CpuExceptionHandlerLib/SmmException.c | 15 +-
.../X64/ArchExceptionHandler.c | 3 +-
15 files changed, 294 insertions(+), 351 deletions(-)

diff --git a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.c b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.c
index 2c7bc66aa7..a521c33f32 100644
--- a/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.c
+++ b/ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.c
@@ -288,20 +288,23 @@ CommonCExceptionHandler (

/**
Setup separate stacks for certain exception handlers.
+ If the input Buffer and BufferSize are both NULL, use global variable if possible.

- InitData is optional and processor arch dependent.
-
- @param[in] InitData Pointer to data optional for information about how
- to assign stacks for certain exception handlers.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.

@retval EFI_SUCCESS The stacks are assigned successfully.
@retval EFI_UNSUPPORTED This function is not supported.
-
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
**/
EFI_STATUS
EFIAPI
InitializeSeparateExceptionStacks (
- IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
)
{
return EFI_SUCCESS;
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 0a1f3d79e2..5733f0c8ec 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -1,7 +1,7 @@
/** @file
DXE Core Main Entry Point

-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -260,7 +260,7 @@ DxeMain (
// Setup Stack Guard
//
if (PcdGetBool (PcdCpuStackGuard)) {
- Status = InitializeSeparateExceptionStacks (NULL);
+ Status = InitializeSeparateExceptionStacks (NULL, NULL);
ASSERT_EFI_ERROR (Status);
}

diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
index 9a495081f7..8d44ed916a 100644
--- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
+++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
@@ -104,20 +104,23 @@ InitializeCpuExceptionHandlers (

/**
Setup separate stacks for certain exception handlers.
+ If the input Buffer and BufferSize are both NULL, use global variable if possible.

- InitData is optional and processor arch dependent.
-
- @param[in] InitData Pointer to data optional for information about how
- to assign stacks for certain exception handlers.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.

@retval EFI_SUCCESS The stacks are assigned successfully.
@retval EFI_UNSUPPORTED This function is not supported.
-
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
**/
EFI_STATUS
EFIAPI
InitializeSeparateExceptionStacks (
- IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
);

/**
diff --git a/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c b/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c
index 8aeedcb4d1..74908a379b 100644
--- a/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c
+++ b/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c
@@ -83,20 +83,23 @@ DumpCpuContext (

/**
Setup separate stacks for certain exception handlers.
+ If the input Buffer and BufferSize are both NULL, use global variable if possible.

- InitData is optional and processor arch dependent.
-
- @param[in] InitData Pointer to data optional for information about how
- to assign stacks for certain exception handlers.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.

@retval EFI_SUCCESS The stacks are assigned successfully.
@retval EFI_UNSUPPORTED This function is not supported.
-
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
**/
EFI_STATUS
EFIAPI
InitializeSeparateExceptionStacks (
- IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
)
{
return EFI_UNSUPPORTED;
diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c
index e385f585c7..d5cf03899b 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.c
+++ b/UefiCpuPkg/CpuDxe/CpuMp.c
@@ -596,24 +596,6 @@ CollectBistDataFromHob (
}
}

-/**
- Get GDT register value.
-
- This function is mainly for AP purpose because AP may have different GDT
- table than BSP.
-
- @param[in,out] Buffer The pointer to private data buffer.
-
-**/
-VOID
-EFIAPI
-GetGdtr (
- IN OUT VOID *Buffer
- )
-{
- AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer);
-}
-
/**
Initializes CPU exceptions handlers for the sake of stack switch requirement.

@@ -629,27 +611,17 @@ InitializeExceptionStackSwitchHandlers (
IN OUT VOID *Buffer
)
{
- CPU_EXCEPTION_INIT_DATA *EssData;
- IA32_DESCRIPTOR Idtr;
- EFI_STATUS Status;
+ EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData;

- EssData = Buffer;
- //
- // We don't plan to replace IDT table with a new one, but we should not assume
- // the AP's IDT is the same as BSP's IDT either.
- //
- AsmReadIdtr (&Idtr);
- EssData->Ia32.IdtTable = (VOID *)Idtr.Base;
- EssData->Ia32.IdtTableSize = Idtr.Limit + 1;
- Status = InitializeSeparateExceptionStacks (EssData);
- ASSERT_EFI_ERROR (Status);
+ SwitchStackData = (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer;
+ InitializeSeparateExceptionStacks (SwitchStackData->Buffer, SwitchStackData->BufferSize);
}

/**
Initializes MP exceptions handlers for the sake of stack switch requirement.

This function will allocate required resources required to setup stack switch
- and pass them through CPU_EXCEPTION_INIT_DATA to each logic processor.
+ and pass them through SwitchStackData to each logic processor.

**/
VOID
@@ -657,129 +629,52 @@ InitializeMpExceptionStackSwitchHandlers (
VOID
)
{
- UINTN Index;
- UINTN Bsp;
- UINTN ExceptionNumber;
- UINTN OldGdtSize;
- UINTN NewGdtSize;
- UINTN NewStackSize;
- IA32_DESCRIPTOR Gdtr;
- CPU_EXCEPTION_INIT_DATA EssData;
- UINT8 *GdtBuffer;
- UINT8 *StackTop;
-
- ExceptionNumber = FixedPcdGetSize (PcdCpuStackSwitchExceptionList);
- NewStackSize = FixedPcdGet32 (PcdCpuKnownGoodStackSize) * ExceptionNumber;
-
- StackTop = AllocateRuntimeZeroPool (NewStackSize * mNumberOfProcessors);
- ASSERT (StackTop != NULL);
- StackTop += NewStackSize * mNumberOfProcessors;
+ UINTN Index;
+ UINTN Bsp;
+ UINT8 *Buffer;
+ EXCEPTION_STACK_SWITCH_CONTEXT SwitchStackData;
+ UINTN BufferSize;

- //
- // The default exception handlers must have been initialized. Let's just skip
- // it in this method.
- //
- EssData.Ia32.Revision = CPU_EXCEPTION_INIT_DATA_REV;
- EssData.Ia32.InitDefaultHandlers = FALSE;
-
- EssData.Ia32.StackSwitchExceptions = FixedPcdGetPtr (PcdCpuStackSwitchExceptionList);
- EssData.Ia32.StackSwitchExceptionNumber = ExceptionNumber;
- EssData.Ia32.KnownGoodStackSize = FixedPcdGet32 (PcdCpuKnownGoodStackSize);
-
- //
- // Initialize Gdtr to suppress incorrect compiler/analyzer warnings.
- //
- Gdtr.Base = 0;
- Gdtr.Limit = 0;
+ SwitchStackData.BufferSize = &BufferSize;
MpInitLibWhoAmI (&Bsp);
+
for (Index = 0; Index < mNumberOfProcessors; ++Index) {
- //
- // To support stack switch, we need to re-construct GDT but not IDT.
- //
+ SwitchStackData.Buffer = NULL;
+ BufferSize = 0;
+
if (Index == Bsp) {
- GetGdtr (&Gdtr);
+ InitializeExceptionStackSwitchHandlers (&SwitchStackData);
} else {
//
- // AP might have different size of GDT from BSP.
+ // AP might need different buffer size from BSP.
//
- MpInitLibStartupThisAP (GetGdtr, Index, NULL, 0, (VOID *)&Gdtr, NULL);
+ MpInitLibStartupThisAP (InitializeExceptionStackSwitchHandlers, Index, NULL, 0, (VOID *)&SwitchStackData, NULL);
}

- //
- // X64 needs only one TSS of current task working for all exceptions
- // because of its IST feature. IA32 needs one TSS for each exception
- // in addition to current task. Since AP is not supposed to allocate
- // memory, we have to do it in BSP. To simplify the code, we allocate
- // memory for IA32 case to cover both IA32 and X64 exception stack
- // switch.
- //
- // Layout of memory to allocate for each processor:
- // --------------------------------
- // | Alignment | (just in case)
- // --------------------------------
- // | |
- // | Original GDT |
- // | |
- // --------------------------------
- // | Current task descriptor |
- // --------------------------------
- // | |
- // | Exception task descriptors | X ExceptionNumber
- // | |
- // --------------------------------
- // | Current task-state segment |
- // --------------------------------
- // | |
- // | Exception task-state segment | X ExceptionNumber
- // | |
- // --------------------------------
- //
- OldGdtSize = Gdtr.Limit + 1;
- EssData.Ia32.ExceptionTssDescSize = sizeof (IA32_TSS_DESCRIPTOR) *
- (ExceptionNumber + 1);
- EssData.Ia32.ExceptionTssSize = sizeof (IA32_TASK_STATE_SEGMENT) *
- (ExceptionNumber + 1);
- NewGdtSize = sizeof (IA32_TSS_DESCRIPTOR) +
- OldGdtSize +
- EssData.Ia32.ExceptionTssDescSize +
- EssData.Ia32.ExceptionTssSize;
-
- GdtBuffer = AllocateRuntimeZeroPool (NewGdtSize);
- ASSERT (GdtBuffer != NULL);
-
- //
- // Make sure GDT table alignment
- //
- EssData.Ia32.GdtTable = ALIGN_POINTER (GdtBuffer, sizeof (IA32_TSS_DESCRIPTOR));
- NewGdtSize -= ((UINT8 *)EssData.Ia32.GdtTable - GdtBuffer);
- EssData.Ia32.GdtTableSize = NewGdtSize;
-
- EssData.Ia32.ExceptionTssDesc = ((UINT8 *)EssData.Ia32.GdtTable + OldGdtSize);
- EssData.Ia32.ExceptionTss = ((UINT8 *)EssData.Ia32.GdtTable + OldGdtSize +
- EssData.Ia32.ExceptionTssDescSize);
-
- EssData.Ia32.KnownGoodStackTop = (UINTN)StackTop;
+ ASSERT (BufferSize != 0);
+ Buffer = AllocateRuntimeZeroPool (BufferSize);
+ ASSERT (Buffer != NULL);
+ SwitchStackData.Buffer = Buffer;
DEBUG ((
DEBUG_INFO,
- "Exception stack top[cpu%lu]: 0x%lX\n",
+ "Buffer[cpu%lu] for InitializeExceptionStackSwitchHandlers: 0x%lX with size 0x%x\n",
(UINT64)(UINTN)Index,
- (UINT64)(UINTN)StackTop
+ (UINT64)(UINTN)Buffer,
+ (UINT32)BufferSize
));

if (Index == Bsp) {
- InitializeExceptionStackSwitchHandlers (&EssData);
+ InitializeExceptionStackSwitchHandlers (&SwitchStackData);
} else {
MpInitLibStartupThisAP (
InitializeExceptionStackSwitchHandlers,
Index,
NULL,
0,
- (VOID *)&EssData,
+ (VOID *)&SwitchStackData,
NULL
);
}
-
- StackTop -= NewStackSize;
}
}

diff --git a/UefiCpuPkg/CpuDxe/CpuMp.h b/UefiCpuPkg/CpuDxe/CpuMp.h
index b461753510..c8a726d9bc 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.h
+++ b/UefiCpuPkg/CpuDxe/CpuMp.h
@@ -1,7 +1,7 @@
/** @file
CPU DXE MP support

- Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -9,6 +9,14 @@
#ifndef _CPU_MP_H_
#define _CPU_MP_H_

+//
+// Structure for InitializeSeparateExceptionStacks
+//
+typedef struct {
+ VOID *Buffer;
+ UINTN *BufferSize;
+} EXCEPTION_STACK_SWITCH_CONTEXT;
+
/**
Initialize Multi-processor support.

diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
index d4786979fa..576e6b81a2 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
@@ -1,7 +1,7 @@
/** @file
CPU PEI Module installs CPU Multiple Processor PPI.

- Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -411,24 +411,6 @@ PeiWhoAmI (
return MpInitLibWhoAmI (ProcessorNumber);
}

-/**
- Get GDT register value.
-
- This function is mainly for AP purpose because AP may have different GDT
- table than BSP.
-
- @param[in,out] Buffer The pointer to private data buffer.
-
-**/
-VOID
-EFIAPI
-GetGdtr (
- IN OUT VOID *Buffer
- )
-{
- AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer);
-}
-
/**
Initializes CPU exceptions handlers for the sake of stack switch requirement.

@@ -444,27 +426,17 @@ InitializeExceptionStackSwitchHandlers (
IN OUT VOID *Buffer
)
{
- CPU_EXCEPTION_INIT_DATA *EssData;
- IA32_DESCRIPTOR Idtr;
- EFI_STATUS Status;
+ EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData;

- EssData = Buffer;
- //
- // We don't plan to replace IDT table with a new one, but we should not assume
- // the AP's IDT is the same as BSP's IDT either.
- //
- AsmReadIdtr (&Idtr);
- EssData->Ia32.IdtTable = (VOID *)Idtr.Base;
- EssData->Ia32.IdtTableSize = Idtr.Limit + 1;
- Status = InitializeSeparateExceptionStacks (EssData);
- ASSERT_EFI_ERROR (Status);
+ SwitchStackData = (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer;
+ InitializeSeparateExceptionStacks (SwitchStackData->Buffer, SwitchStackData->BufferSize);
}

/**
Initializes MP exceptions handlers for the sake of stack switch requirement.

This function will allocate required resources required to setup stack switch
- and pass them through CPU_EXCEPTION_INIT_DATA to each logic processor.
+ and pass them through SwitchStackData to each logic processor.

**/
VOID
@@ -472,148 +444,59 @@ InitializeMpExceptionStackSwitchHandlers (
VOID
)
{
- EFI_STATUS Status;
- UINTN Index;
- UINTN Bsp;
- UINTN ExceptionNumber;
- UINTN OldGdtSize;
- UINTN NewGdtSize;
- UINTN NewStackSize;
- IA32_DESCRIPTOR Gdtr;
- CPU_EXCEPTION_INIT_DATA EssData;
- UINT8 *GdtBuffer;
- UINT8 *StackTop;
- UINTN NumberOfProcessors;
+ UINTN Index;
+ UINTN Bsp;
+ UINT8 *Buffer;
+ EXCEPTION_STACK_SWITCH_CONTEXT SwitchStackData;
+ UINTN BufferSize;
+ UINTN NumberOfProcessors;

if (!PcdGetBool (PcdCpuStackGuard)) {
return;
}

+ SwitchStackData.BufferSize = &BufferSize;
MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
MpInitLibWhoAmI (&Bsp);

- ExceptionNumber = FixedPcdGetSize (PcdCpuStackSwitchExceptionList);
- NewStackSize = FixedPcdGet32 (PcdCpuKnownGoodStackSize) * ExceptionNumber;
-
- StackTop = AllocatePages (EFI_SIZE_TO_PAGES (NewStackSize * NumberOfProcessors));
- ASSERT (StackTop != NULL);
- if (StackTop == NULL) {
- return;
- }
-
- StackTop += NewStackSize * NumberOfProcessors;
-
- //
- // The default exception handlers must have been initialized. Let's just skip
- // it in this method.
- //
- EssData.Ia32.Revision = CPU_EXCEPTION_INIT_DATA_REV;
- EssData.Ia32.InitDefaultHandlers = FALSE;
-
- EssData.Ia32.StackSwitchExceptions = FixedPcdGetPtr (PcdCpuStackSwitchExceptionList);
- EssData.Ia32.StackSwitchExceptionNumber = ExceptionNumber;
- EssData.Ia32.KnownGoodStackSize = FixedPcdGet32 (PcdCpuKnownGoodStackSize);
-
- //
- // Initialize Gdtr to suppress incorrect compiler/analyzer warnings.
- //
- Gdtr.Base = 0;
- Gdtr.Limit = 0;
for (Index = 0; Index < NumberOfProcessors; ++Index) {
- //
- // To support stack switch, we need to re-construct GDT but not IDT.
- //
+ SwitchStackData.Buffer = NULL;
+ BufferSize = 0;
+
if (Index == Bsp) {
- GetGdtr (&Gdtr);
+ InitializeExceptionStackSwitchHandlers (&SwitchStackData);
} else {
//
- // AP might have different size of GDT from BSP.
+ // AP might need different buffer size from BSP.
//
- MpInitLibStartupThisAP (GetGdtr, Index, NULL, 0, (VOID *)&Gdtr, NULL);
- }
-
- //
- // X64 needs only one TSS of current task working for all exceptions
- // because of its IST feature. IA32 needs one TSS for each exception
- // in addition to current task. Since AP is not supposed to allocate
- // memory, we have to do it in BSP. To simplify the code, we allocate
- // memory for IA32 case to cover both IA32 and X64 exception stack
- // switch.
- //
- // Layout of memory to allocate for each processor:
- // --------------------------------
- // | Alignment | (just in case)
- // --------------------------------
- // | |
- // | Original GDT |
- // | |
- // --------------------------------
- // | Current task descriptor |
- // --------------------------------
- // | |
- // | Exception task descriptors | X ExceptionNumber
- // | |
- // --------------------------------
- // | Current task-state segment |
- // --------------------------------
- // | |
- // | Exception task-state segment | X ExceptionNumber
- // | |
- // --------------------------------
- //
- OldGdtSize = Gdtr.Limit + 1;
- EssData.Ia32.ExceptionTssDescSize = sizeof (IA32_TSS_DESCRIPTOR) *
- (ExceptionNumber + 1);
- EssData.Ia32.ExceptionTssSize = sizeof (IA32_TASK_STATE_SEGMENT) *
- (ExceptionNumber + 1);
- NewGdtSize = sizeof (IA32_TSS_DESCRIPTOR) +
- OldGdtSize +
- EssData.Ia32.ExceptionTssDescSize +
- EssData.Ia32.ExceptionTssSize;
-
- Status = PeiServicesAllocatePool (
- NewGdtSize,
- (VOID **)&GdtBuffer
- );
- ASSERT (GdtBuffer != NULL);
- if (EFI_ERROR (Status)) {
- ASSERT_EFI_ERROR (Status);
- return;
+ MpInitLibStartupThisAP (InitializeExceptionStackSwitchHandlers, Index, NULL, 0, (VOID *)&SwitchStackData, NULL);
}

- //
- // Make sure GDT table alignment
- //
- EssData.Ia32.GdtTable = ALIGN_POINTER (GdtBuffer, sizeof (IA32_TSS_DESCRIPTOR));
- NewGdtSize -= ((UINT8 *)EssData.Ia32.GdtTable - GdtBuffer);
- EssData.Ia32.GdtTableSize = NewGdtSize;
-
- EssData.Ia32.ExceptionTssDesc = ((UINT8 *)EssData.Ia32.GdtTable + OldGdtSize);
- EssData.Ia32.ExceptionTss = ((UINT8 *)EssData.Ia32.GdtTable + OldGdtSize +
- EssData.Ia32.ExceptionTssDescSize);
-
- EssData.Ia32.KnownGoodStackTop = (UINTN)StackTop;
+ ASSERT (BufferSize != 0);
+ Buffer = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
+ ASSERT (Buffer != NULL);
+ ZeroMem (Buffer, EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (BufferSize)));
+ SwitchStackData.Buffer = Buffer;
DEBUG ((
DEBUG_INFO,
- "Exception stack top[cpu%lu]: 0x%lX\n",
+ "Buffer[cpu%lu] for InitializeExceptionStackSwitchHandlers: 0x%lX with size 0x%x\n",
(UINT64)(UINTN)Index,
- (UINT64)(UINTN)StackTop
+ (UINT64)(UINTN)Buffer,
+ (UINT32)BufferSize
));

if (Index == Bsp) {
- InitializeExceptionStackSwitchHandlers (&EssData);
+ InitializeExceptionStackSwitchHandlers (&SwitchStackData);
} else {
MpInitLibStartupThisAP (
InitializeExceptionStackSwitchHandlers,
Index,
NULL,
0,
- (VOID *)&EssData,
+ (VOID *)&SwitchStackData,
NULL
);
}
-
- StackTop -= NewStackSize;
}
}

diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
index 0649c48d14..754f8901b5 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
@@ -1,7 +1,7 @@
/** @file
Definitions to install Multiple Processor PPI.

- Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -31,6 +31,14 @@

extern EFI_PEI_PPI_DESCRIPTOR mPeiCpuMpPpiDesc;

+//
+// Structure for InitializeSeparateExceptionStacks
+//
+typedef struct {
+ VOID *Buffer;
+ UINTN *BufferSize;
+} EXCEPTION_STACK_SWITCH_CONTEXT;
+
/**
This service retrieves the number of logical processor in the platform
and the number of those logical processors that are enabled on this boot.
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index e62bb5e6c0..674029f8ac 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -104,48 +104,104 @@ RegisterCpuInterruptHandler (

/**
Setup separate stacks for certain exception handlers.
+ If the input Buffer and BufferSize are both NULL, use global variable if possible.

- InitData is optional and processor arch dependent.
-
- @param[in] InitData Pointer to data optional for information about how
- to assign stacks for certain exception handlers.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.

@retval EFI_SUCCESS The stacks are assigned successfully.
@retval EFI_UNSUPPORTED This function is not supported.
-
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
**/
EFI_STATUS
EFIAPI
InitializeSeparateExceptionStacks (
- IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
)
{
CPU_EXCEPTION_INIT_DATA EssData;
IA32_DESCRIPTOR Idtr;
IA32_DESCRIPTOR Gdtr;
-
- if (InitData == NULL) {
+ UINTN NeedBufferSize;
+ UINTN StackTop;
+ UINT8 *NewGdtTable;
+
+ //
+ // X64 needs only one TSS of current task working for all exceptions
+ // because of its IST feature. IA32 needs one TSS for each exception
+ // in addition to current task. To simplify the code, we report the
+ // needed memory for IA32 case to cover both IA32 and X64 exception
+ // stack switch.
+ //
+ // Layout of memory needed for each processor:
+ // --------------------------------
+ // | Alignment | (just in case)
+ // --------------------------------
+ // | |
+ // | Original GDT |
+ // | |
+ // --------------------------------
+ // | Current task descriptor |
+ // --------------------------------
+ // | |
+ // | Exception task descriptors | X ExceptionNumber
+ // | |
+ // --------------------------------
+ // | Current task-state segment |
+ // --------------------------------
+ // | |
+ // | Exception task-state segment | X ExceptionNumber
+ // | |
+ // --------------------------------
+ //
+ AsmReadGdtr (&Gdtr);
+ if ((Buffer == NULL) && (BufferSize == NULL)) {
SetMem (mNewGdt, sizeof (mNewGdt), 0);
-
- AsmReadIdtr (&Idtr);
- AsmReadGdtr (&Gdtr);
-
- EssData.X64.Revision = CPU_EXCEPTION_INIT_DATA_REV;
- EssData.X64.KnownGoodStackTop = (UINTN)mNewStack + sizeof (mNewStack);
- EssData.X64.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
- EssData.X64.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
- EssData.X64.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
- EssData.X64.IdtTable = (VOID *)Idtr.Base;
- EssData.X64.IdtTableSize = Idtr.Limit + 1;
- EssData.X64.GdtTable = mNewGdt;
- EssData.X64.GdtTableSize = sizeof (mNewGdt);
- EssData.X64.ExceptionTssDesc = mNewGdt + Gdtr.Limit + 1;
- EssData.X64.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
- EssData.X64.ExceptionTss = mNewGdt + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
- EssData.X64.ExceptionTssSize = CPU_TSS_SIZE;
-
- InitData = &EssData;
+ StackTop = (UINTN)mNewStack + sizeof (mNewStack);
+ NewGdtTable = mNewGdt;
+ } else {
+ if (BufferSize == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Total needed size includes stack size, new GDT table size, TSS size.
+ // Add another DESCRIPTOR size for alignment requiremet.
+ //
+ NeedBufferSize = CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE +
+ CPU_TSS_DESC_SIZE + Gdtr.Limit + 1 +
+ CPU_TSS_SIZE +
+ sizeof (IA32_TSS_DESCRIPTOR);
+ if (*BufferSize < NeedBufferSize) {
+ *BufferSize = NeedBufferSize;
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ if (Buffer == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ StackTop = (UINTN)Buffer + CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE;
+ NewGdtTable = ALIGN_POINTER (StackTop, sizeof (IA32_TSS_DESCRIPTOR));
}

- return ArchSetupExceptionStack (InitData);
+ AsmReadIdtr (&Idtr);
+ EssData.X64.KnownGoodStackTop = StackTop;
+ EssData.X64.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
+ EssData.X64.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
+ EssData.X64.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
+ EssData.X64.IdtTable = (VOID *)Idtr.Base;
+ EssData.X64.IdtTableSize = Idtr.Limit + 1;
+ EssData.X64.GdtTable = NewGdtTable;
+ EssData.X64.GdtTableSize = CPU_TSS_DESC_SIZE + Gdtr.Limit + 1;
+ EssData.X64.ExceptionTssDesc = NewGdtTable + Gdtr.Limit + 1;
+ EssData.X64.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
+ EssData.X64.ExceptionTss = NewGdtTable + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
+ EssData.X64.ExceptionTssSize = CPU_TSS_SIZE;
+
+ return ArchSetupExceptionStack (&EssData);
}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
index f13e8e7020..fa62074023 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
@@ -1,7 +1,7 @@
/** @file
IA32 CPU Exception Handler functons.

- Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -132,7 +132,6 @@ ArchSetupExceptionStack (
EXCEPTION_HANDLER_TEMPLATE_MAP TemplateMap;

if ((StackSwitchData == NULL) ||
- (StackSwitchData->Ia32.Revision != CPU_EXCEPTION_INIT_DATA_REV) ||
(StackSwitchData->Ia32.KnownGoodStackTop == 0) ||
(StackSwitchData->Ia32.KnownGoodStackSize == 0) ||
(StackSwitchData->Ia32.StackSwitchExceptions == NULL) ||
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
index 494c2ab433..fe8d02d3e4 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
@@ -151,25 +151,103 @@ InitializeCpuExceptionHandlers (

/**
Setup separate stacks for certain exception handlers.
+ If the input Buffer and BufferSize are both NULL, use global variable if possible.

- InitData is optional and processor arch dependent.
-
- @param[in] InitData Pointer to data optional for information about how
- to assign stacks for certain exception handlers.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.

@retval EFI_SUCCESS The stacks are assigned successfully.
@retval EFI_UNSUPPORTED This function is not supported.
-
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
**/
EFI_STATUS
EFIAPI
InitializeSeparateExceptionStacks (
- IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
)
{
- if (InitData == NULL) {
+ CPU_EXCEPTION_INIT_DATA EssData;
+ IA32_DESCRIPTOR Idtr;
+ IA32_DESCRIPTOR Gdtr;
+ UINTN NeedBufferSize;
+ UINTN StackTop;
+ UINT8 *NewGdtTable;
+
+ //
+ // X64 needs only one TSS of current task working for all exceptions
+ // because of its IST feature. IA32 needs one TSS for each exception
+ // in addition to current task. To simplify the code, we report the
+ // needed memory for IA32 case to cover both IA32 and X64 exception
+ // stack switch.
+ //
+ // Layout of memory needed for each processor:
+ // --------------------------------
+ // | Alignment | (just in case)
+ // --------------------------------
+ // | |
+ // | Original GDT |
+ // | |
+ // --------------------------------
+ // | Current task descriptor |
+ // --------------------------------
+ // | |
+ // | Exception task descriptors | X ExceptionNumber
+ // | |
+ // --------------------------------
+ // | Current task-state segment |
+ // --------------------------------
+ // | |
+ // | Exception task-state segment | X ExceptionNumber
+ // | |
+ // --------------------------------
+ //
+
+ if ((Buffer == NULL) && (BufferSize == NULL)) {
return EFI_UNSUPPORTED;
}

- return ArchSetupExceptionStack (InitData);
+ if (BufferSize == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ AsmReadGdtr (&Gdtr);
+ //
+ // Total needed size includes stack size, new GDT table size, TSS size.
+ // Add another DESCRIPTOR size for alignment requiremet.
+ //
+ NeedBufferSize = CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE +
+ CPU_TSS_DESC_SIZE + Gdtr.Limit + 1 +
+ CPU_TSS_SIZE +
+ sizeof (IA32_TSS_DESCRIPTOR);
+ if (*BufferSize < NeedBufferSize) {
+ *BufferSize = NeedBufferSize;
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ if (Buffer == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ StackTop = (UINTN)Buffer + CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE;
+ NewGdtTable = ALIGN_POINTER (StackTop, sizeof (IA32_TSS_DESCRIPTOR));
+
+ AsmReadIdtr (&Idtr);
+ EssData.X64.KnownGoodStackTop = StackTop;
+ EssData.X64.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
+ EssData.X64.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
+ EssData.X64.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
+ EssData.X64.IdtTable = (VOID *)Idtr.Base;
+ EssData.X64.IdtTableSize = Idtr.Limit + 1;
+ EssData.X64.GdtTable = NewGdtTable;
+ EssData.X64.GdtTableSize = CPU_TSS_DESC_SIZE + Gdtr.Limit + 1;
+ EssData.X64.ExceptionTssDesc = NewGdtTable + Gdtr.Limit + 1;
+ EssData.X64.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
+ EssData.X64.ExceptionTss = NewGdtTable + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
+ EssData.X64.ExceptionTssSize = CPU_TSS_SIZE;
+
+ return ArchSetupExceptionStack (&EssData);
}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
index cf5bfe4083..7c2ec3b2db 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
@@ -1,7 +1,7 @@
## @file
# CPU Exception Handler library instance for PEI module.
#
-# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2016 - 2022, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -56,6 +56,8 @@

[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard # CONSUMES
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList

[FeaturePcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONSUMES
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
index 4313cc5582..ad5e0e9ed4 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
@@ -201,20 +201,23 @@ RegisterCpuInterruptHandler (

/**
Setup separate stacks for certain exception handlers.
+ If the input Buffer and BufferSize are both NULL, use global variable if possible.

- InitData is optional and processor arch dependent.
-
- @param[in] InitData Pointer to data optional for information about how
- to assign stacks for certain exception handlers.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.

@retval EFI_SUCCESS The stacks are assigned successfully.
@retval EFI_UNSUPPORTED This function is not supported.
-
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
**/
EFI_STATUS
EFIAPI
InitializeSeparateExceptionStacks (
- IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
)
{
return EFI_UNSUPPORTED;
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c
index 1c97dab926..46a86ad2c6 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c
@@ -97,20 +97,23 @@ RegisterCpuInterruptHandler (

/**
Setup separate stacks for certain exception handlers.
+ If the input Buffer and BufferSize are both NULL, use global variable if possible.

- InitData is optional and processor arch dependent.
-
- @param[in] InitData Pointer to data optional for information about how
- to assign stacks for certain exception handlers.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.

@retval EFI_SUCCESS The stacks are assigned successfully.
@retval EFI_UNSUPPORTED This function is not supported.
-
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
**/
EFI_STATUS
EFIAPI
InitializeSeparateExceptionStacks (
- IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
)
{
return EFI_UNSUPPORTED;
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index cd7dccd481..ff0dde4f12 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -1,7 +1,7 @@
/** @file
x64 CPU Exception Handler.

- Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -136,7 +136,6 @@ ArchSetupExceptionStack (
UINTN GdtSize;

if ((StackSwitchData == NULL) ||
- (StackSwitchData->Ia32.Revision != CPU_EXCEPTION_INIT_DATA_REV) ||
(StackSwitchData->X64.KnownGoodStackTop == 0) ||
(StackSwitchData->X64.KnownGoodStackSize == 0) ||
(StackSwitchData->X64.StackSwitchExceptions == NULL) ||
--
2.31.1.windows.1


[PATCH v2 0/3] Simplify InitializeSeparateExceptionStacks

Zhiguang Liu
 

The patch set is to hide the exception implementation details,
so that caller don't need to know anything about IDT when separate stack
for it. However, this patch set changes a library API, so I have to
change multiple packages inside one patch. Otherwise, I can make sure
every single commit can build and boot fine. If anyone has good idea to
separate the first big patch, please tell me. Thanks in advance.

V2:
Add another patch to Simplify the CPU_EXCEPTION_INIT_DATA definition
Keep the memory layout picture in CpuExceptionHandlerLib.
Fix some code and comment issue accoring to Ray's comment
Code can be seen at https://github.com/tianocore/edk2/pull/3124

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Leif Lindholm <quic_llindhol@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: Liming Gao <gaoliming@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Sami Mujawar <sami.mujawar@...>
Signed-off-by: Zhiguang Liu <zhiguang.liu@...>


Zhiguang Liu (3):
UefiCpuPkg: Simplify InitializeSeparateExceptionStacks
MdeModulePkg: Move CPU_EXCEPTION_INIT_DATA to UefiCpuPkg
UefiCpuPkg: Simplify the struct definition of CPU_EXCEPTION_INIT_DATA

.../Library/ArmExceptionLib/ArmExceptionLib.c | 15 +-
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 4 +-
.../Include/Library/CpuExceptionHandlerLib.h | 82 +--------
.../CpuExceptionHandlerLibNull.c | 15 +-
UefiCpuPkg/CpuDxe/CpuMp.c | 157 +++-------------
UefiCpuPkg/CpuDxe/CpuMp.h | 10 +-
UefiCpuPkg/CpuMpPei/CpuMpPei.c | 173 +++---------------
UefiCpuPkg/CpuMpPei/CpuMpPei.h | 10 +-
.../CpuExceptionCommon.h | 57 +++++-
.../CpuExceptionHandlerLib/DxeException.c | 112 +++++++++---
.../Ia32/ArchExceptionHandler.c | 71 ++++---
.../CpuExceptionHandlerLib/PeiCpuException.c | 94 +++++++++-
.../PeiCpuExceptionHandlerLib.inf | 4 +-
.../SecPeiCpuException.c | 15 +-
.../CpuExceptionHandlerLib/SmmException.c | 15 +-
.../X64/ArchExceptionHandler.c | 67 ++++---
16 files changed, 416 insertions(+), 485 deletions(-)

--
2.31.1.windows.1


回复: [edk2-devel] [PATCH] MdePkg/UefiDevicePathLib: reback the DevicePathUtilitiesStandaloneMm

gaoliming
 

Yanbo:
If this patch is temporarily added, you can handle it in your downstream code base.

If you request to add DevicePathUtilitiesStandaloneMm for long term compatibility, this topic has been discussed in https://edk2.groups.io/g/devel/message/91799.

Thanks
Liming

-----邮件原件-----
发件人: Huang, Yanbo <yanbo.huang@...>
发送时间: 2022年8月5日 13:34
收件人: Gao, Liming <gaoliming@...>; devel@edk2.groups.io
抄送: Kinney, Michael D <michael.d.kinney@...>; Liu, Zhiguang
<zhiguang.liu@...>; Bi, Dandan <dandan.bi@...>
主题: RE: [edk2-devel] [PATCH] MdePkg/UefiDevicePathLib: reback the
DevicePathUtilitiesStandaloneMm

Hi Liming,

You mentioned patch rename the DevicePathUtilitiesStandaloneMm to
UefiDevicePathLibBase, but there are some consumer in intel platform still
use the DevicePathUtilitiesStandaloneMm, so downstream will failed in CI
because it cannot find DevicePathUtilitiesStandaloneMm. So the
DevicePathUtilitiesStandaloneMm and UefiDevicePathLibBase must exist at
the same time for a period of time. After downstream finished and platform
change to use UefiDevicePathLibBase, then
DevicePathUtilitiesStandaloneMm can be deleted.

Best Regards,
Yanbo Huang

-----Original Message-----
From: gaoliming <gaoliming@...>
Sent: Friday, August 5, 2022 11:16 AM
To: devel@edk2.groups.io; Huang, Yanbo <yanbo.huang@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Liu, Zhiguang
<zhiguang.liu@...>
Subject: 回复: [edk2-devel] [PATCH] MdePkg/UefiDevicePathLib: reback the
DevicePathUtilitiesStandaloneMm

Yanbo:
Previous change has been reviewed and merged. Please see the detail
https://edk2.groups.io/g/devel/message/91799

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Huang,
Yanbo
发送时间: 2022年8月5日 10:42
收件人: devel@edk2.groups.io
抄送: Yanbo Huang <yanbo.huang@...>; Michael D Kinney
<michael.d.kinney@...>; Liming Gao <gaoliming@...>;
Zhiguang Liu <zhiguang.liu@...>
主题: [edk2-devel] [PATCH] MdePkg/UefiDevicePathLib: reback the
DevicePathUtilitiesStandaloneMm

From: Yanbo Huang <yanbo.huang@...>

reback the DevicePathUtilitiesStandaloneMm to unblock the downstream
sync

Signed-off-by: Yanbo Huang <yanbo.huang@...>
CC: Michael D Kinney <michael.d.kinney@...>
CC: Liming Gao <gaoliming@...>
CC: Zhiguang Liu <zhiguang.liu@...>

---
.../DevicePathUtilitiesStandaloneMm.c | 39 ++++++++++
.../UefiDevicePathLibStandaloneMm.inf | 75
+++++++++++++++++++
2 files changed, 114 insertions(+)
create mode 100644
MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
create mode 100644
MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf

diff --git
a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
new file mode 100644
index 0000000000..096f835b90
--- /dev/null
+++
b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
@@ -0,0 +1,39 @@
+/** @file
+ Device Path services. The thing to remember is device paths are
+built
out
of
+ nodes. The device path is terminated by an end node that is length
+ sizeof(EFI_DEVICE_PATH_PROTOCOL). That would be why there is
sizeof(EFI_DEVICE_PATH_PROTOCOL)
+ all over this file.
+
+ The only place where multi-instance device paths are supported is
+ in environment varibles. Multi-instance device paths should never
+ be
placed
+ on a Handle.
+
+ Copyright (c) 2006 - 2018, Intel Corporation. All rights
+ reserved.<BR> Copyright (c) Microsoft Corporation.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "UefiDevicePathLib.h"
+
+/**
+ Retrieves the device path protocol from a handle.
+
+ This function returns the device path protocol from the handle
specified by
Handle.
+ If Handle is NULL or Handle does not contain a device path
+ protocol,
then
NULL
+ is returned.
+
+ @param Handle The handle from which to
retrieve the device
+ path protocol.
+
+ @return The device path protocol from the handle specified by Handle.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+DevicePathFromHandle (
+ IN EFI_HANDLE Handle
+ )
+{
+ return NULL;
+}
diff --git
a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
new file mode 100644
index 0000000000..23fedf38b7
--- /dev/null
+++
b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
@@ -0,0 +1,75 @@
+## @file
+# Instance of Device Path Library based on Memory Allocation Library.
+#
+# Device Path Library that layers on top of the Memory Allocation
Library.
+#
+# Copyright (c) 2007 - 2018, Intel Corporation. All rights
+reserved.<BR> # Copyright (c) Microsoft Corporation.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent # # ##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = UefiDevicePathLib
+ MODULE_UNI_FILE = UefiDevicePathLib.uni
+ FILE_GUID =
D8E58437-44D3-4154-B7A7-EB794923EF12
+ MODULE_TYPE = MM_STANDALONE
+ PI_SPECIFICATION_VERSION = 0x00010032
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = DevicePathLib |
MM_STANDALONE MM_CORE_STANDALONE
+
+
+#
+# VALID_ARCHITECTURES = IA32 X64 EBC
+#
+
+[Sources]
+ DevicePathUtilities.c
+ DevicePathUtilitiesStandaloneMm.c
+ DevicePathToText.c
+ DevicePathFromText.c
+ UefiDevicePathLib.c
+ UefiDevicePathLib.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ MemoryAllocationLib
+ DebugLib
+ BaseMemoryLib
+ PcdLib
+ PrintLib
+
+[Guids]
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVTUTF8Guid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVT100Guid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVT100PlusGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPcAnsiGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiUartDevicePathGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiSasDevicePathGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVirtualDiskGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVirtualCdGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPersistentVirtualDiskGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPersistentVirtualCdGuid
+
+[Protocols]
+ gEfiDevicePathProtocolGuid ##
SOMETIMES_CONSUMES
+ gEfiDebugPortProtocolGuid ## UNDEFINED
+
+[Pcd]
+ gEfiMdePkgTokenSpaceGuid.PcdMaximumDevicePathNodeCount ##
SOMETIMES_CONSUMES
--
2.31.1.windows.1





回复: [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to follow BGRT specification

gaoliming
 

Sean:
Can you give BGRT spec link? I want to check the spec description.

Thanks
Liming

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sean Rhodes
发送时间: 2022年7月26日 16:15
收件人: devel@edk2.groups.io
抄送: Sean Rhodes <sean@...>; Zhichao Gao
<zhichao.gao@...>; Ray Ni <ray.ni@...>; Jian J Wang
<jian.j.wang@...>; Liming Gao <gaoliming@...>
主题: [edk2-devel] [PATCH 1/3] MdeModulePkg/BootLogoLib: Add option to
follow BGRT specification

Add an option to position the logo 38.2% from the top of the screen,
which follows the BGRT specification.

Cc: Zhichao Gao <zhichao.gao@...>
Cc: Ray Ni <ray.ni@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Signed-off-by: Sean Rhodes <sean@...>
---
MdeModulePkg/Include/Protocol/PlatformLogo.h | 3 ++-
MdeModulePkg/Library/BootLogoLib/BootLogoLib.c | 7 ++++---
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Include/Protocol/PlatformLogo.h
b/MdeModulePkg/Include/Protocol/PlatformLogo.h
index 08e1dc35a4..7c9ef63c66 100644
--- a/MdeModulePkg/Include/Protocol/PlatformLogo.h
+++ b/MdeModulePkg/Include/Protocol/PlatformLogo.h
@@ -29,7 +29,8 @@ typedef enum {
EdkiiPlatformLogoDisplayAttributeCenterBottom,

EdkiiPlatformLogoDisplayAttributeLeftBottom,

EdkiiPlatformLogoDisplayAttributeCenterLeft,

- EdkiiPlatformLogoDisplayAttributeCenter

+ EdkiiPlatformLogoDisplayAttributeCenter,

+ EdkiiPlatformLogoDisplayAttributeBGRTSpecification

} EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE;



/**

diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
index 478ec2d40e..ac086f9c79 100644
--- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
+++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
@@ -169,7 +169,6 @@ BootLogoEnableLogo (
DestX = SizeOfX - Image.Width;

DestY = 0;

break;

-

case EdkiiPlatformLogoDisplayAttributeCenterLeft:

DestX = 0;

DestY = (SizeOfY - Image.Height) / 2;

@@ -182,7 +181,6 @@ BootLogoEnableLogo (
DestX = SizeOfX - Image.Width;

DestY = (SizeOfY - Image.Height) / 2;

break;

-

case EdkiiPlatformLogoDisplayAttributeLeftBottom:

DestX = 0;

DestY = SizeOfY - Image.Height;

@@ -195,7 +193,10 @@ BootLogoEnableLogo (
DestX = SizeOfX - Image.Width;

DestY = SizeOfY - Image.Height;

break;

-

+ case EdkiiPlatformLogoDisplayAttributeBGRTSpecification:

+ DestX = (SizeOfX - Image.Width) / 2;

+ DestY = (SizeOfY * 382) / 1000 - Image.Height / 2;

+ break;

default:

ASSERT (FALSE);

continue;

--
2.34.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91842): https://edk2.groups.io/g/devel/message/91842
Mute This Topic: https://groups.io/mt/92623125/4905953
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[gaoliming@...]
-=-=-=-=-=-=


回复: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of the Logo

gaoliming
 

Sean:
I add my comments below.

-----邮件原件-----
发件人: Sean Rhodes <sean@...>
发送时间: 2022年7月26日 16:15
收件人: devel@edk2.groups.io
抄送: Sean Rhodes <sean@...>; Zhichao Gao
<zhichao.gao@...>; Ray Ni <ray.ni@...>; Jian J Wang
<jian.j.wang@...>; Liming Gao <gaoliming@...>
主题: [PATCH 2/3] MdeModulePkg/Logo: Add a PCD to control the position of
the Logo

When set to true, the Logo is positioned according to the BGRT
specification, 38.2% from the top of the screen. When set to false,
no behaviour is changed and the logo is positioned centrally.

Cc: Zhichao Gao <zhichao.gao@...>
Cc: Ray Ni <ray.ni@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Signed-off-by: Sean Rhodes <sean@...>
---
MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf | 5 ++++-
MdeModulePkg/Logo/Logo.c | 5 +++++
MdeModulePkg/Logo/LogoDxe.inf | 4 ++++
MdeModulePkg/MdeModulePkg.dec | 6 ++++++
MdeModulePkg/MdeModulePkg.uni | 6 ++++++
5 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
index 7d50f2dfa3..14ba8a5906 100644
--- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
+++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
@@ -48,5 +48,8 @@
gEfiUserManagerProtocolGuid ## CONSUMES

gEdkiiPlatformLogoProtocolGuid ## CONSUMES



+[Pcd]

+ gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpecification ##
CONSUMES

+

[FeaturePcd]

- gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ## CONSUMES

+ gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ##
CONSUMES
The change in BootLogoLib is not required. Please check.

diff --git a/MdeModulePkg/Logo/Logo.c b/MdeModulePkg/Logo/Logo.c
index 8ab874d2da..73546b32f4 100644
--- a/MdeModulePkg/Logo/Logo.c
+++ b/MdeModulePkg/Logo/Logo.c
@@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/HiiPackageList.h>

#include <Library/UefiBootServicesTableLib.h>

#include <Library/DebugLib.h>

+#include <Library/PcdLib.h>



typedef struct {

EFI_IMAGE_ID ImageId;

@@ -69,6 +70,10 @@ GetImage (
return EFI_NOT_FOUND;

}



+ if (FixedPcdGetBool (PcdFollowBGRTSpecification)) {

+ mLogos[Current].Attribute =
EdkiiPlatformLogoDisplayAttributeBGRTSpecification;

+ }

+
Here, please use PcdGetBool().

Thanks
Liming

(*Instance)++;

*Attribute = mLogos[Current].Attribute;

*OffsetX = mLogos[Current].OffsetX;

diff --git a/MdeModulePkg/Logo/LogoDxe.inf
b/MdeModulePkg/Logo/LogoDxe.inf
index 41215d25d8..c5c8ad0bcf 100644
--- a/MdeModulePkg/Logo/LogoDxe.inf
+++ b/MdeModulePkg/Logo/LogoDxe.inf
@@ -41,6 +41,7 @@
UefiBootServicesTableLib

UefiDriverEntryPoint

DebugLib

+ PcdLib



[Protocols]

gEfiHiiDatabaseProtocolGuid ## CONSUMES

@@ -48,6 +49,9 @@
gEfiHiiPackageListProtocolGuid ## PRODUCES CONSUMES

gEdkiiPlatformLogoProtocolGuid ## PRODUCES



+[Pcd]

+ gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpecification ##
CONSUMES

+

[Depex]

gEfiHiiDatabaseProtocolGuid AND

gEfiHiiImageExProtocolGuid

diff --git a/MdeModulePkg/MdeModulePkg.dec
b/MdeModulePkg/MdeModulePkg.dec
index 2bcb9f9453..e09918387c 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -2095,6 +2095,12 @@
# @Prompt The shared bit mask when Intel Tdx is enabled.


gEfiMdeModulePkgTokenSpaceGuid.PcdTdxSharedBitMask|0x0|UINT64|0x10
000025



+ ## This PCD sets the position of the Boot Logo.

+ # TRUE - The Logo is positioned according to the BGRT specification.

+ # FALSE - The logo is positioned in the center of the screen.

+ # @ Prompt This position of the boot logo

+
gEfiMdeModulePkgTokenSpaceGuid.PcdFollowBGRTSpecification|FALSE|BOO
LEAN|0x10000026

+

[PcdsPatchableInModule]

## Specify memory size with page number for PEI code when

# Loading Module at Fixed Address feature is enabled.

diff --git a/MdeModulePkg/MdeModulePkg.uni
b/MdeModulePkg/MdeModulePkg.uni
index b070f15ff2..c6ff7bc1bd 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -1334,3 +1334,9 @@
#string
STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPcieResizableBarSupport_HELP
#language en-US "Indicates if the PCIe Resizable BAR Capability
Supported.<BR><BR>\n"


"TRUE - PCIe Resizable BAR Capability is supported.<BR>\n"


"FALSE - PCIe Resizable BAR Capability is not supported.<BR>"

+

+#string
STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFollowBGRTSpecification_PROM
PT #language en-US "The position of the Boot Logo"

+

+#string
STR_gEfiMdeModulePkgTokenSpaceGuid_PcdFollowBGRTSpecification_HELP
#language en-US "Sets the position of the Logo. When set to true, the Logo
is
positioned according to the"

+
" BGRT specification, 38.2% from the top of the screen."

+

--
2.34.1


回复: [edk2-devel] [PATCH] MdePkg:Improved Smbios Type9 table and Smbios 3.5.0 spec changes

gaoliming
 

The change is good. But the change in Smbios type 9 will impact
ShellPkg\Library\UefiShellDebug1CommandsLib\SmbiosView\PrintInfo.c. Please
also update ShellPkg.

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sainadh
Nagolu via groups.io
发送时间: 2022年7月28日 0:21
收件人: devel@edk2.groups.io
抄送: Sainadh Nagolu <sainadhn@...>; Sundaresan S
<sundaresans@...>; Vasudevan Sambandan <vasudevans@...>;
gaoliming@...
主题: [edk2-devel] [PATCH] MdePkg:Improved Smbios Type9 table and
Smbios 3.5.0 spec changes

In Type9 structure since PeerGroups has a variable
number of entries, must not define new fields in the structure.So added
an
extended structure and defined new fields added after PeerGroups. Also
done
some improvements to Smbios 3.5.0 spec changes.

Signed-off-by:
sainadh nagolu <sainadhn@...>

---
MdePkg/Include/IndustryStandard/SmBios.h | 62 +++++++++++---------
1 file changed, 35 insertions(+), 27 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/SmBios.h
b/MdePkg/Include/IndustryStandard/SmBios.h
index c7a4971f14..f62ad7fa4d 100644
--- a/MdePkg/Include/IndustryStandard/SmBios.h
+++ b/MdePkg/Include/IndustryStandard/SmBios.h
@@ -1503,6 +1503,17 @@ typedef struct {
UINT8 DataBusWidth;

UINT8 PeerGroupingCount;

MISC_SLOT_PEER_GROUP PeerGroups[1];

+ //

+ // Since PeerGroups has a variable number of entries, must not define
new

+ // fields in the structure. Remaining fields can be referenced using

+ // SMBIOS_TABLE_TYPE9_EXTENDED structure

+ //

+} SMBIOS_TABLE_TYPE9;

+

+///

+/// Extended structure for System Slots (Type 9)

+///

+typedef struct {

//

// Add for smbios 3.4

//

@@ -1513,7 +1524,7 @@ typedef struct {
// Add for smbios 3.5

//

UINT8 SlotHeight; ///< The
enumeration value from MISC_SLOT_HEIGHT.

-} SMBIOS_TABLE_TYPE9;

+} SMBIOS_TABLE_TYPE9_EXTENDED;



///

/// On Board Devices Information - Device Types.

@@ -2746,11 +2757,11 @@ typedef enum {
///

/// Firmware Inventory Firmware Characteristics (Type 45).

///

-typedef enum {

- CharacteristicsUpdatable = 0x00,

- CharacteristicsWriteProtected = 0x01,

- CharacteristicsReserved = 0x02 /// 0x02 - 0x0F are reserved

-} FIRMWARE_INVENTORY_CHARACTERISTICS;

+typedef struct {

+ UINT16 Updatable :1;

+ UINT16 WriteProtected :1;

+ UINT16 Reserved :14;

+} FIRMWARE_CHARACTERISTICS;



///

/// Firmware Inventory State Information (Type 45).

@@ -2763,7 +2774,7 @@ typedef enum {
FirmwareInventoryStateAbsent = 0x05,

FirmwareInventoryStateStandbyOffline = 0x06,

FirmwareInventoryStateStandbySpare = 0x07,

- FirmwareInventoryStateUnavailableOffline = 0x08,

+ FirmwareInventoryStateUnavailableOffline = 0x08

} FIRMWARE_INVENTORY_STATE;



///

@@ -2780,21 +2791,19 @@ typedef enum {
/// One Type 45 structure is provided for each firmware component.

///

typedef struct {

- SMBIOS_STRUCTURE Hdr;

- SMBIOS_HANDLE RefHandle;

-

- UINT8 FirmwareComponentName;

- UINT8 FirmwareVersion;

- UINT8 FirmwareVersionFormat; ///< The
enumeration value from FIRMWARE_INVENTORY_VERSION_FORMAT_TYPE

- UINT8 FirmwareId;

- UINT8 FirmwareIdFormat;

- UINT8 ReleaseDate;

- UINT8 Manufacturer;

- UINT8 LowestSupportedVersion;

- UINT64 ImageSize;

- UINT32 Characteristics;

- UINT8 State;

- UINT8 AssociatedComponentCount;

+ SMBIOS_STRUCTURE Hdr;

+ SMBIOS_TABLE_STRING FirmwareComponentName;

+ SMBIOS_TABLE_STRING FirmwareVersion;

+ UINT8 FirmwareVersionFormat; ///< The
enumeration value from FIRMWARE_INVENTORY_VERSION_FORMAT_TYPE

+ SMBIOS_TABLE_STRING FirmwareId;

+ UINT8 FirmwareIdFormat; ///< The
enumeration value from
FIRMWARE_INVENTORY_FIRMWARE_ID_FORMAT_TYPE.

+ SMBIOS_TABLE_STRING ReleaseDate;

+ SMBIOS_TABLE_STRING Manufacturer;

+ SMBIOS_TABLE_STRING LowestSupportedVersion;

+ UINT64 ImageSize;

+ FIRMWARE_CHARACTERISTICS Characteristics;

+ UINT8 State; ///< The
enumeration value from FIRMWARE_INVENTORY_STATE.

+ UINT8 AssociatedComponentCount;

///

/// zero or n-number of handles depends on AssociatedComponentCount

/// handles are of type SMBIOS_HANDLE

@@ -2820,11 +2829,10 @@ typedef enum {
/// parent structure.

///

typedef struct {

- SMBIOS_STRUCTURE Hdr;

- SMBIOS_HANDLE RefHandle;

- UINT16 StringPropertyId;

- UINT8 StringPropertyValue;

- SMBIOS_HANDLE ParentHandle;

+ SMBIOS_STRUCTURE Hdr;

+ UINT16 StringPropertyId; ///< The
enumeration value from STRING_PROPERTY_ID.

+ SMBIOS_TABLE_STRING StringPropertyValue;

+ SMBIOS_HANDLE ParentHandle;

} SMBIOS_TABLE_TYPE46;



///

--
2.36.0.windows.1
-The information contained in this message may be confidential and
proprietary to American Megatrends (AMI). This communication is intended
to
be read only by the individual or entity to whom it is addressed or by
their
designee. If the reader of this message is not the intended recipient, you
are
on notice that any distribution of this message, in any form, is strictly
prohibited. Please promptly notify the sender by reply e-mail or by
telephone
at 770-246-8600, and then delete or destroy all copies of the
transmission.




回复: [edk2-devel] [edk2] [PATCH]MdeModulePkg\scsi: Coverity scan flags multiple issues in edk2-stable202205

gaoliming
 

This change is good to me. Reviewed-by: Liming Gao
<gaoliming@...>

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 sivaparvathi
C via groups.io
发送时间: 2022年8月2日 13:00
收件人: devel@edk2.groups.io
抄送: Vasudevan Sambandan <vasudevans@...>; Sundaresan S
<sundaresans@...>; Sivaparvathi Chellaiah <sivaparvathic@...>
主题: [edk2-devel] [edk2] [PATCH]MdeModulePkg\scsi: Coverity scan flags
multiple issues in edk2-stable202205

Attached changes to resolve the coverity Issues

Signed-off-by: sivaparvathic@...
To: sivaparvathic@...

---
MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c | 3 +++
MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 10 +++++-----
2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
b/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
index 9ea69ee740..2cc61bb942 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c
@@ -531,6 +531,9 @@ SCSIBusDriverBindingStart (
// then create handle and install scsi i/o protocol.

//

Status = ScsiScanCreateDevice (This, Controller, &ScsiTargetId, Lun,
ScsiBusDev);

+ if (Status == EFI_OUT_OF_RESOURCES) {

+ goto ErrorExit;

+ }

}



return EFI_SUCCESS;

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index 98e84b4ea8..5f4ead7669 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -4247,7 +4247,7 @@ BackOff:


if ((TargetStatus == EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION)
|| (EFI_ERROR (ReturnStatus))) {

DEBUG ((DEBUG_ERROR, "ScsiDiskRead10: Check Condition
happened!\n"));

- Status = DetectMediaParsingSenseKeys (ScsiDiskDevice,
ScsiDiskDevice->SenseData, SenseDataLength / sizeof
(EFI_SCSI_SENSE_DATA), &Action);

+ DetectMediaParsingSenseKeys (ScsiDiskDevice,
ScsiDiskDevice->SenseData, SenseDataLength / sizeof
(EFI_SCSI_SENSE_DATA), &Action);

if (Action == ACTION_RETRY_COMMAND_LATER) {

*NeedRetry = TRUE;

return EFI_DEVICE_ERROR;

@@ -4371,7 +4371,7 @@ BackOff:


if ((TargetStatus == EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION)
|| (EFI_ERROR (ReturnStatus))) {

DEBUG ((DEBUG_ERROR, "ScsiDiskWrite10: Check Condition
happened!\n"));

- Status = DetectMediaParsingSenseKeys (ScsiDiskDevice,
ScsiDiskDevice->SenseData, SenseDataLength / sizeof
(EFI_SCSI_SENSE_DATA), &Action);

+ DetectMediaParsingSenseKeys (ScsiDiskDevice,
ScsiDiskDevice->SenseData, SenseDataLength / sizeof
(EFI_SCSI_SENSE_DATA), &Action);

if (Action == ACTION_RETRY_COMMAND_LATER) {

*NeedRetry = TRUE;

return EFI_DEVICE_ERROR;

@@ -4494,7 +4494,7 @@ BackOff:


if ((TargetStatus == EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION)
|| (EFI_ERROR (ReturnStatus))) {

DEBUG ((DEBUG_ERROR, "ScsiDiskRead16: Check Condition
happened!\n"));

- Status = DetectMediaParsingSenseKeys (ScsiDiskDevice,
ScsiDiskDevice->SenseData, SenseDataLength / sizeof
(EFI_SCSI_SENSE_DATA), &Action);

+ DetectMediaParsingSenseKeys (ScsiDiskDevice,
ScsiDiskDevice->SenseData, SenseDataLength / sizeof
(EFI_SCSI_SENSE_DATA), &Action);

if (Action == ACTION_RETRY_COMMAND_LATER) {

*NeedRetry = TRUE;

return EFI_DEVICE_ERROR;

@@ -4618,7 +4618,7 @@ BackOff:


if ((TargetStatus == EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION)
|| (EFI_ERROR (ReturnStatus))) {

DEBUG ((DEBUG_ERROR, "ScsiDiskWrite16: Check Condition
happened!\n"));

- Status = DetectMediaParsingSenseKeys (ScsiDiskDevice,
ScsiDiskDevice->SenseData, SenseDataLength / sizeof
(EFI_SCSI_SENSE_DATA), &Action);

+ DetectMediaParsingSenseKeys (ScsiDiskDevice,
ScsiDiskDevice->SenseData, SenseDataLength / sizeof
(EFI_SCSI_SENSE_DATA), &Action);

if (Action == ACTION_RETRY_COMMAND_LATER) {

*NeedRetry = TRUE;

return EFI_DEVICE_ERROR;

@@ -4728,7 +4728,7 @@ ScsiDiskNotify (
if (Request->TargetStatus ==
EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION) {

DEBUG ((DEBUG_ERROR, "ScsiDiskNotify: Check Condition
happened!\n"));



- Status = DetectMediaParsingSenseKeys (

+ DetectMediaParsingSenseKeys (

ScsiDiskDevice,

Request->SenseData,

Request->SenseDataLength / sizeof
(EFI_SCSI_SENSE_DATA),

--
2.31.0.windows.1
-The information contained in this message may be confidential and
proprietary to American Megatrends (AMI). This communication is intended
to
be read only by the individual or entity to whom it is addressed or by
their
designee. If the reader of this message is not the intended recipient, you
are
on notice that any distribution of this message, in any form, is strictly
prohibited. Please promptly notify the sender by reply e-mail or by
telephone
at 770-246-8600, and then delete or destroy all copies of the
transmission.




回复: [edk2-devel] [PATCH] Add support for SMBIOS Spec 3.6.0 to SmBios.h

gaoliming
 

Reviewed-by: Liming Gao <gaoliming@...>

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Samer
El-Haj-Mahmoud
发送时间: 2022年8月3日 21:11
收件人: devel@edk2.groups.io; sainadhn@...
抄送: Sundaresan S <sundaresans@...>; Vasudevan Sambandan
<vasudevans@...>; gaoliming@...; Samer
El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; nd <nd@...>
主题: Re: [edk2-devel] [PATCH] Add support for SMBIOS Spec 3.6.0 to
SmBios.h

A quick compare against the SMBIOS 3.6.0. Looks good! Thanks for adding
this.

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sainadh
Nagolu via groups.io
Sent: Wednesday, August 3, 2022 12:52 AM
To: devel@edk2.groups.io; Sainadh Nagolu <sainadhn@...>
Cc: Sundaresan S <sundaresans@...>; Vasudevan Sambandan
<vasudevans@...>; gaoliming@...
Subject: [edk2-devel] [PATCH] Add support for SMBIOS Spec 3.6.0 to
SmBios.h

Updated SmBios.h with new fields added as part of SMBIOS 3.6.0 spec
update.

Signed-off-by: Sainadh Nagolu <sainadhn@...>

CC: Vasudevan Sambandan <vasudevans@...>
CC: Sundaresan S <sundaresans@...>

---
MdePkg/Include/IndustryStandard/SmBios.h | 88
++++++++++++++++--------
1 file changed, 61 insertions(+), 27 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/SmBios.h
b/MdePkg/Include/IndustryStandard/SmBios.h
index c7a4971f14..3b296ab308 100644
--- a/MdePkg/Include/IndustryStandard/SmBios.h
+++ b/MdePkg/Include/IndustryStandard/SmBios.h
@@ -1,5 +1,5 @@
/** @file

- Industry Standard Definitions of SMBIOS Table Specification v3.5.0.

+ Industry Standard Definitions of SMBIOS Table Specification v3.6.0.



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

(C) Copyright 2015-2017 Hewlett Packard Enterprise Development
LP<BR>

@@ -722,21 +722,39 @@ typedef enum {
/// Processor Information2 - Processor Family2.

///

typedef enum {

- ProcessorFamilyARMv7 = 0x0100,

- ProcessorFamilyARMv8 = 0x0101,

- ProcessorFamilySH3 = 0x0104,

- ProcessorFamilySH4 = 0x0105,

- ProcessorFamilyARM = 0x0118,

- ProcessorFamilyStrongARM = 0x0119,

- ProcessorFamily6x86 = 0x012C,

- ProcessorFamilyMediaGX = 0x012D,

- ProcessorFamilyMII = 0x012E,

- ProcessorFamilyWinChip = 0x0140,

- ProcessorFamilyDSP = 0x015E,

- ProcessorFamilyVideoProcessor = 0x01F4,

- ProcessorFamilyRiscvRV32 = 0x0200,

- ProcessorFamilyRiscVRV64 = 0x0201,

- ProcessorFamilyRiscVRV128 = 0x0202

+ ProcessorFamilyARMv7 = 0x0100,

+ ProcessorFamilyARMv8 = 0x0101,

+ ProcessorFamilyARMv9 = 0x0102,

+ ProcessorFamilySH3 = 0x0104,

+ ProcessorFamilySH4 = 0x0105,

+ ProcessorFamilyARM = 0x0118,

+ ProcessorFamilyStrongARM = 0x0119,

+ ProcessorFamily6x86 = 0x012C,

+ ProcessorFamilyMediaGX = 0x012D,

+ ProcessorFamilyMII = 0x012E,

+ ProcessorFamilyWinChip = 0x0140,

+ ProcessorFamilyDSP = 0x015E,

+ ProcessorFamilyVideoProcessor = 0x01F4,

+ ProcessorFamilyRiscvRV32 = 0x0200,

+ ProcessorFamilyRiscVRV64 = 0x0201,

+ ProcessorFamilyRiscVRV128 = 0x0202,

+ ProcessorFamilyLoongArch = 0x0258,

+ ProcessorFamilyLoongson1 = 0x0259,

+ ProcessorFamilyLoongson2 = 0x025A,

+ ProcessorFamilyLoongson3 = 0x025B,

+ ProcessorFamilyLoongson2K = 0x025C,

+ ProcessorFamilyLoongson3A = 0x025D,

+ ProcessorFamilyLoongson3B = 0x025E,

+ ProcessorFamilyLoongson3C = 0x025F,

+ ProcessorFamilyLoongson3D = 0x0260,

+ ProcessorFamilyLoongson3E = 0x0261,

+ ProcessorFamilyDualCoreLoongson2K = 0x0262,

+ ProcessorFamilyQuadCoreLoongson3A = 0x026C,

+ ProcessorFamilyMultiCoreLoongson3A = 0x026D,

+ ProcessorFamilyQuadCoreLoongson3B = 0x026E,

+ ProcessorFamilyMultiCoreLoongson3B = 0x026F,

+ ProcessorFamilyMultiCoreLoongson3C = 0x0270,

+ ProcessorFamilyMultiCoreLoongson3D = 0x0271

} PROCESSOR_FAMILY2_DATA;



///

@@ -817,7 +835,16 @@ typedef enum {
ProcessorUpgradeSocketBGA1528 = 0x3C,

ProcessorUpgradeSocketLGA4189 = 0x3D,

ProcessorUpgradeSocketLGA1200 = 0x3E,

- ProcessorUpgradeSocketLGA4677 = 0x3F

+ ProcessorUpgradeSocketLGA4677 = 0x3F,

+ ProcessorUpgradeSocketLGA1700 = 0x40,

+ ProcessorUpgradeSocketBGA1744 = 0x41,

+ ProcessorUpgradeSocketBGA1781 = 0x42,

+ ProcessorUpgradeSocketBGA1211 = 0x43,

+ ProcessorUpgradeSocketBGA2422 = 0x44,

+ ProcessorUpgradeSocketLGA1211 = 0x45,

+ ProcessorUpgradeSocketLGA2422 = 0x46,

+ ProcessorUpgradeSocketLGA5773 = 0x47,

+ ProcessorUpgradeSocketBGA5773 = 0x48

} PROCESSOR_UPGRADE;



///

@@ -946,6 +973,10 @@ typedef struct {
UINT16 CoreCount2;

UINT16 EnabledCoreCount2;

UINT16 ThreadCount2;

+ //

+ // Add for smbios 3.6

+ //

+ UINT16 ThreadEnabled;

} SMBIOS_TABLE_TYPE4;



///

@@ -1811,7 +1842,8 @@ typedef enum {
MemoryTypeHBM = 0x20,

MemoryTypeHBM2 = 0x21,

MemoryTypeDdr5 = 0x22,

- MemoryTypeLpddr5 = 0x23

+ MemoryTypeLpddr5 = 0x23,

+ MemoryTypeHBM3 = 0x24

} MEMORY_DEVICE_TYPE;



///

@@ -2660,15 +2692,17 @@ typedef struct {
/// Processor Specific Block - Processor Architecture Type

///

typedef enum {

- ProcessorSpecificBlockArchTypeReserved = 0x00,

- ProcessorSpecificBlockArchTypeIa32 = 0x01,

- ProcessorSpecificBlockArchTypeX64 = 0x02,

- ProcessorSpecificBlockArchTypeItanium = 0x03,

- ProcessorSpecificBlockArchTypeAarch32 = 0x04,

- ProcessorSpecificBlockArchTypeAarch64 = 0x05,

- ProcessorSpecificBlockArchTypeRiscVRV32 = 0x06,

- ProcessorSpecificBlockArchTypeRiscVRV64 = 0x07,

- ProcessorSpecificBlockArchTypeRiscVRV128 = 0x08

+ ProcessorSpecificBlockArchTypeReserved = 0x00,

+ ProcessorSpecificBlockArchTypeIa32 = 0x01,

+ ProcessorSpecificBlockArchTypeX64 = 0x02,

+ ProcessorSpecificBlockArchTypeItanium = 0x03,

+ ProcessorSpecificBlockArchTypeAarch32 = 0x04,

+ ProcessorSpecificBlockArchTypeAarch64 = 0x05,

+ ProcessorSpecificBlockArchTypeRiscVRV32 = 0x06,

+ ProcessorSpecificBlockArchTypeRiscVRV64 = 0x07,

+ ProcessorSpecificBlockArchTypeRiscVRV128 = 0x08,

+ ProcessorSpecificBlockArchTypeLoongArch32 = 0x09,

+ ProcessorSpecificBlockArchTypeLoongArch64 = 0x0A

} PROCESSOR_SPECIFIC_BLOCK_ARCH_TYPE;



///

--
2.36.0.windows.1
-The information contained in this message may be confidential and
proprietary
to American Megatrends (AMI). This communication is intended to be read
only
by the individual or entity to whom it is addressed or by their
designee. If
the
reader of this message is not the intended recipient, you are on notice
that
any
distribution of this message, in any form, is strictly prohibited.
Please
promptly
notify the sender by reply e-mail or by telephone at 770-246-8600, and
then
delete or destroy all copies of the transmission.







Re: [PATCH] MdePkg/UefiDevicePathLib: reback the DevicePathUtilitiesStandaloneMm

Huang, Yanbo
 

Hi Liming,

You mentioned patch rename the DevicePathUtilitiesStandaloneMm to UefiDevicePathLibBase, but there are some consumer in intel platform still use the DevicePathUtilitiesStandaloneMm, so downstream will failed in CI because it cannot find DevicePathUtilitiesStandaloneMm. So the DevicePathUtilitiesStandaloneMm and UefiDevicePathLibBase must exist at the same time for a period of time. After downstream finished and platform change to use UefiDevicePathLibBase, then DevicePathUtilitiesStandaloneMm can be deleted.

Best Regards,
Yanbo Huang

-----Original Message-----
From: gaoliming <gaoliming@...>
Sent: Friday, August 5, 2022 11:16 AM
To: devel@edk2.groups.io; Huang, Yanbo <yanbo.huang@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Liu, Zhiguang <zhiguang.liu@...>
Subject: 回复: [edk2-devel] [PATCH] MdePkg/UefiDevicePathLib: reback the DevicePathUtilitiesStandaloneMm

Yanbo:
Previous change has been reviewed and merged. Please see the detail
https://edk2.groups.io/g/devel/message/91799

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Huang, Yanbo
发送时间: 2022年8月5日 10:42
收件人: devel@edk2.groups.io
抄送: Yanbo Huang <yanbo.huang@...>; Michael D Kinney
<michael.d.kinney@...>; Liming Gao <gaoliming@...>;
Zhiguang Liu <zhiguang.liu@...>
主题: [edk2-devel] [PATCH] MdePkg/UefiDevicePathLib: reback the
DevicePathUtilitiesStandaloneMm

From: Yanbo Huang <yanbo.huang@...>

reback the DevicePathUtilitiesStandaloneMm to unblock the downstream
sync

Signed-off-by: Yanbo Huang <yanbo.huang@...>
CC: Michael D Kinney <michael.d.kinney@...>
CC: Liming Gao <gaoliming@...>
CC: Zhiguang Liu <zhiguang.liu@...>

---
.../DevicePathUtilitiesStandaloneMm.c | 39 ++++++++++
.../UefiDevicePathLibStandaloneMm.inf | 75
+++++++++++++++++++
2 files changed, 114 insertions(+)
create mode 100644
MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
create mode 100644
MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf

diff --git
a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
new file mode 100644
index 0000000000..096f835b90
--- /dev/null
+++
b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
@@ -0,0 +1,39 @@
+/** @file
+ Device Path services. The thing to remember is device paths are
+built
out
of
+ nodes. The device path is terminated by an end node that is length
+ sizeof(EFI_DEVICE_PATH_PROTOCOL). That would be why there is
sizeof(EFI_DEVICE_PATH_PROTOCOL)
+ all over this file.
+
+ The only place where multi-instance device paths are supported is
+ in environment varibles. Multi-instance device paths should never
+ be
placed
+ on a Handle.
+
+ Copyright (c) 2006 - 2018, Intel Corporation. All rights
+ reserved.<BR> Copyright (c) Microsoft Corporation.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "UefiDevicePathLib.h"
+
+/**
+ Retrieves the device path protocol from a handle.
+
+ This function returns the device path protocol from the handle
specified by
Handle.
+ If Handle is NULL or Handle does not contain a device path
+ protocol,
then
NULL
+ is returned.
+
+ @param Handle The handle from which to
retrieve the device
+ path protocol.
+
+ @return The device path protocol from the handle specified by Handle.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+DevicePathFromHandle (
+ IN EFI_HANDLE Handle
+ )
+{
+ return NULL;
+}
diff --git
a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
new file mode 100644
index 0000000000..23fedf38b7
--- /dev/null
+++
b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
@@ -0,0 +1,75 @@
+## @file
+# Instance of Device Path Library based on Memory Allocation Library.
+#
+# Device Path Library that layers on top of the Memory Allocation
Library.
+#
+# Copyright (c) 2007 - 2018, Intel Corporation. All rights
+reserved.<BR> # Copyright (c) Microsoft Corporation.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent # # ##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = UefiDevicePathLib
+ MODULE_UNI_FILE = UefiDevicePathLib.uni
+ FILE_GUID =
D8E58437-44D3-4154-B7A7-EB794923EF12
+ MODULE_TYPE = MM_STANDALONE
+ PI_SPECIFICATION_VERSION = 0x00010032
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = DevicePathLib |
MM_STANDALONE MM_CORE_STANDALONE
+
+
+#
+# VALID_ARCHITECTURES = IA32 X64 EBC
+#
+
+[Sources]
+ DevicePathUtilities.c
+ DevicePathUtilitiesStandaloneMm.c
+ DevicePathToText.c
+ DevicePathFromText.c
+ UefiDevicePathLib.c
+ UefiDevicePathLib.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ MemoryAllocationLib
+ DebugLib
+ BaseMemoryLib
+ PcdLib
+ PrintLib
+
+[Guids]
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVTUTF8Guid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVT100Guid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVT100PlusGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPcAnsiGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiUartDevicePathGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiSasDevicePathGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVirtualDiskGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVirtualCdGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPersistentVirtualDiskGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPersistentVirtualCdGuid
+
+[Protocols]
+ gEfiDevicePathProtocolGuid ##
SOMETIMES_CONSUMES
+ gEfiDebugPortProtocolGuid ## UNDEFINED
+
+[Pcd]
+ gEfiMdePkgTokenSpaceGuid.PcdMaximumDevicePathNodeCount ##
SOMETIMES_CONSUMES
--
2.31.1.windows.1





回复: [edk2-devel] [PATCH] MdeModulePkg/DumpDynPcd: Remove unsupported format specifiers

gaoliming
 

Thanks for your update. This change is good to me. Reviewed-by: Liming Gao
<gaoliming@...>

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Konstantin
Aladyshev
发送时间: 2022年7月27日 16:45
收件人: devel@edk2.groups.io
抄送: jian.j.wang@...; gaoliming@...;
dandan.bi@...; shenglei.zhang@...; Konstantin Aladyshev
<aladyshev22@...>
主题: [edk2-devel] [PATCH] MdeModulePkg/DumpDynPcd: Remove
unsupported format specifiers

Some print statements use format specifiers like %N/%H/%E/%B that are
only supported in the shell print functions. In the ordinary 'Print'
function they are just displayed as letters N/H/E/B.
Remove these unsupported format specifiers from the 'Print' statements
to fix the issue.

Signed-off-by: Konstantin Aladyshev <aladyshev22@...>
---
.../Application/DumpDynPcd/DumpDynPcd.c | 28
+++++++++----------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
index b8571c4556..013198963e 100644
--- a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
+++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
@@ -361,7 +361,7 @@ DumpPcdInfo (
Uint8 = mPiPcd->Get8 (TokenSpace, TokenNumber);

}



- Print (L" Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value
= 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint8);

+ Print (L" Token = 0x%08x - Type = %-17s - Size = 0x%x - Value =
0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint8);

break;

case EFI_PCD_TYPE_16:

if (TokenSpace == NULL) {

@@ -370,7 +370,7 @@ DumpPcdInfo (
Uint16 = mPiPcd->Get16 (TokenSpace, TokenNumber);

}



- Print (L" Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value
= 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint16);

+ Print (L" Token = 0x%08x - Type = %-17s - Size = 0x%x - Value =
0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint16);

break;

case EFI_PCD_TYPE_32:

if (TokenSpace == NULL) {

@@ -379,7 +379,7 @@ DumpPcdInfo (
Uint32 = mPiPcd->Get32 (TokenSpace, TokenNumber);

}



- Print (L" Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value
= 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint32);

+ Print (L" Token = 0x%08x - Type = %-17s - Size = 0x%x - Value =
0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint32);

break;

case EFI_PCD_TYPE_64:

if (TokenSpace == NULL) {

@@ -388,7 +388,7 @@ DumpPcdInfo (
Uint64 = mPiPcd->Get64 (TokenSpace, TokenNumber);

}



- Print (L" Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value
= 0x%lx\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint64);

+ Print (L" Token = 0x%08x - Type = %-17s - Size = 0x%x - Value =
0x%lx\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint64);

break;

case EFI_PCD_TYPE_BOOL:

if (TokenSpace == NULL) {

@@ -397,7 +397,7 @@ DumpPcdInfo (
Boolean = mPiPcd->GetBool (TokenSpace, TokenNumber);

}



- Print (L" Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value
= %a\n", TokenNumber, RetString, PcdInfo->PcdSize, Boolean ? "TRUE" :
"FALSE");

+ Print (L" Token = 0x%08x - Type = %-17s - Size = 0x%x - Value
= %a\n", TokenNumber, RetString, PcdInfo->PcdSize, Boolean ? "TRUE" :
"FALSE");

break;

case EFI_PCD_TYPE_PTR:

if (TokenSpace == NULL) {

@@ -406,7 +406,7 @@ DumpPcdInfo (
PcdData = mPiPcd->GetPtr (TokenSpace, TokenNumber);

}



- Print (L" Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x\n",
TokenNumber, RetString, PcdInfo->PcdSize);

+ Print (L" Token = 0x%08x - Type = %-17s - Size = 0x%x\n",
TokenNumber, RetString, PcdInfo->PcdSize);

DumpHex (2, 0, PcdInfo->PcdSize, PcdData);

break;

default:

@@ -509,7 +509,7 @@ ProcessPcd (
//

// The specified PCD is not found, print error.

//

- Print (L"%EError. %NNo matching PCD found: %s.\n", InputPcdName);

+ Print (L"Error. No matching PCD found: %s.\n", InputPcdName);

return EFI_NOT_FOUND;

}



@@ -548,25 +548,25 @@ DumpDynPcdMain (


Status = gBS->LocateProtocol (&gEfiPcdProtocolGuid, NULL, (VOID
**)&mPiPcd);

if (EFI_ERROR (Status)) {

- Print (L"DumpDynPcd: %EError. %NPI PCD protocol is not present.\n");

+ Print (L"DumpDynPcd: Error. PI PCD protocol is not present.\n");

return Status;

}



Status = gBS->LocateProtocol (&gEfiGetPcdInfoProtocolGuid, NULL, (VOID
**)&mPiPcdInfo);

if (EFI_ERROR (Status)) {

- Print (L"DumpDynPcd: %EError. %NPI PCD info protocol is not
present.\n");

+ Print (L"DumpDynPcd: Error. PI PCD info protocol is not present.\n");

return Status;

}



Status = gBS->LocateProtocol (&gPcdProtocolGuid, NULL, (VOID
**)&mPcd);

if (EFI_ERROR (Status)) {

- Print (L"DumpDynPcd: %EError. %NPCD protocol is not present.\n");

+ Print (L"DumpDynPcd: Error. PCD protocol is not present.\n");

return Status;

}



Status = gBS->LocateProtocol (&gGetPcdInfoProtocolGuid, NULL, (VOID
**)&mPcdInfo);

if (EFI_ERROR (Status)) {

- Print (L"DumpDynPcd: %EError. %NPCD info protocol is not
present.\n");

+ Print (L"DumpDynPcd: Error. PCD info protocol is not present.\n");

return Status;

}



@@ -575,13 +575,13 @@ DumpDynPcdMain (
//

Status = GetArg ();

if (EFI_ERROR (Status)) {

- Print (L"DumpDynPcd: %EError. %NThe input parameters are not
recognized.\n");

+ Print (L"DumpDynPcd: Error. The input parameters are not
recognized.\n");

Status = EFI_INVALID_PARAMETER;

return Status;

}



if (Argc > 2) {

- Print (L"DumpDynPcd: %EError. %NToo many arguments specified.\n");

+ Print (L"DumpDynPcd: Error. Too many arguments specified.\n");

Status = EFI_INVALID_PARAMETER;

return Status;

}

@@ -600,7 +600,7 @@ DumpDynPcdMain (
goto Done;

} else {

if (StrStr (Argv[1], L"-") != NULL) {

- Print (L"DumpDynPcd: %EError. %NThe argument '%B%s%N' is
invalid.\n", Argv[1]);

+ Print (L"DumpDynPcd: Error. The argument '%s' is invalid.\n",
Argv[1]);

goto Done;

}

}

--
2.25.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91887): https://edk2.groups.io/g/devel/message/91887
Mute This Topic: https://groups.io/mt/92645389/4905953
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[gaoliming@...]
-=-=-=-=-=-=


回复: [edk2-devel] [PATCH] MdePkg/UefiDevicePathLib: reback the DevicePathUtilitiesStandaloneMm

gaoliming
 

Yanbo:
Previous change has been reviewed and merged. Please see the detail
https://edk2.groups.io/g/devel/message/91799

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Huang, Yanbo
发送时间: 2022年8月5日 10:42
收件人: devel@edk2.groups.io
抄送: Yanbo Huang <yanbo.huang@...>; Michael D Kinney
<michael.d.kinney@...>; Liming Gao <gaoliming@...>;
Zhiguang Liu <zhiguang.liu@...>
主题: [edk2-devel] [PATCH] MdePkg/UefiDevicePathLib: reback the
DevicePathUtilitiesStandaloneMm

From: Yanbo Huang <yanbo.huang@...>

reback the DevicePathUtilitiesStandaloneMm to unblock the downstream
sync

Signed-off-by: Yanbo Huang <yanbo.huang@...>
CC: Michael D Kinney <michael.d.kinney@...>
CC: Liming Gao <gaoliming@...>
CC: Zhiguang Liu <zhiguang.liu@...>

---
.../DevicePathUtilitiesStandaloneMm.c | 39 ++++++++++
.../UefiDevicePathLibStandaloneMm.inf | 75
+++++++++++++++++++
2 files changed, 114 insertions(+)
create mode 100644
MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
create mode 100644
MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf

diff --git
a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
new file mode 100644
index 0000000000..096f835b90
--- /dev/null
+++
b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
@@ -0,0 +1,39 @@
+/** @file
+ Device Path services. The thing to remember is device paths are built
out
of
+ nodes. The device path is terminated by an end node that is length
+ sizeof(EFI_DEVICE_PATH_PROTOCOL). That would be why there is
sizeof(EFI_DEVICE_PATH_PROTOCOL)
+ all over this file.
+
+ The only place where multi-instance device paths are supported is in
+ environment varibles. Multi-instance device paths should never be
placed
+ on a Handle.
+
+ Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) Microsoft Corporation.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "UefiDevicePathLib.h"
+
+/**
+ Retrieves the device path protocol from a handle.
+
+ This function returns the device path protocol from the handle
specified by
Handle.
+ If Handle is NULL or Handle does not contain a device path protocol,
then
NULL
+ is returned.
+
+ @param Handle The handle from which to
retrieve the device
+ path protocol.
+
+ @return The device path protocol from the handle specified by Handle.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+DevicePathFromHandle (
+ IN EFI_HANDLE Handle
+ )
+{
+ return NULL;
+}
diff --git
a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
new file mode 100644
index 0000000000..23fedf38b7
--- /dev/null
+++
b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
@@ -0,0 +1,75 @@
+## @file
+# Instance of Device Path Library based on Memory Allocation Library.
+#
+# Device Path Library that layers on top of the Memory Allocation
Library.
+#
+# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) Microsoft Corporation.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = UefiDevicePathLib
+ MODULE_UNI_FILE = UefiDevicePathLib.uni
+ FILE_GUID =
D8E58437-44D3-4154-B7A7-EB794923EF12
+ MODULE_TYPE = MM_STANDALONE
+ PI_SPECIFICATION_VERSION = 0x00010032
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = DevicePathLib |
MM_STANDALONE MM_CORE_STANDALONE
+
+
+#
+# VALID_ARCHITECTURES = IA32 X64 EBC
+#
+
+[Sources]
+ DevicePathUtilities.c
+ DevicePathUtilitiesStandaloneMm.c
+ DevicePathToText.c
+ DevicePathFromText.c
+ UefiDevicePathLib.c
+ UefiDevicePathLib.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ MemoryAllocationLib
+ DebugLib
+ BaseMemoryLib
+ PcdLib
+ PrintLib
+
+[Guids]
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVTUTF8Guid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVT100Guid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVT100PlusGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPcAnsiGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiUartDevicePathGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiSasDevicePathGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVirtualDiskGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVirtualCdGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPersistentVirtualDiskGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPersistentVirtualCdGuid
+
+[Protocols]
+ gEfiDevicePathProtocolGuid ##
SOMETIMES_CONSUMES
+ gEfiDebugPortProtocolGuid ## UNDEFINED
+
+[Pcd]
+ gEfiMdePkgTokenSpaceGuid.PcdMaximumDevicePathNodeCount ##
SOMETIMES_CONSUMES
--
2.31.1.windows.1





回复: [PATCH v1 3/5] MdeModulePkg: Fix imbalanced debug macros

gaoliming
 

Reviewed-by: Liming Gao <gaoliming@...>

-----邮件原件-----
发件人: mikuback@... <mikuback@...>
发送时间: 2022年8月3日 2:04
收件人: devel@edk2.groups.io
抄送: Dandan Bi <dandan.bi@...>; Guomin Jiang
<guomin.jiang@...>; Hao A Wu <hao.a.wu@...>; Jian J Wang
<jian.j.wang@...>; Liming Gao <gaoliming@...>; Ray Ni
<ray.ni@...>
主题: [PATCH v1 3/5] MdeModulePkg: Fix imbalanced debug macros

From: Michael Kubacki <michael.kubacki@...>

Updates debug macros in the package that have an imbalanced number
of print specifiers to arguments. These changes try to preserve
what was likely intended by the author. In cases information was
missing due to the bug, the specifier may be removed since it was
not previously accurately printing the expected value.

Cc: Dandan Bi <dandan.bi@...>
Cc: Guomin Jiang <guomin.jiang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Cc: Liming Gao <gaoliming@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
| 2 +-
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
| 8 ++++----
MdeModulePkg/Core/Dxe/Image/Image.c
| 2 +-

MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdatePr
ogressLibGraphics.c | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 53b63ab52b93..dd45167a009e 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -64,7 +64,7 @@ DumpCapabilityReg (
DEBUG ((DEBUG_INFO, " Driver Type D %a\n",
Capability->DriverTypeD ? "TRUE" : "FALSE"));
DEBUG ((DEBUG_INFO, " Driver Type 4 %a\n",
Capability->DriverType4 ? "TRUE" : "FALSE"));
if (Capability->TimerCount == 0) {
- DEBUG ((DEBUG_INFO, " Retuning TimerCnt Disabled\n", 2 *
(Capability->TimerCount - 1)));
+ DEBUG ((DEBUG_INFO, " Retuning TimerCnt Disabled\n"));
} else {
DEBUG ((DEBUG_INFO, " Retuning TimerCnt %dseconds\n", 2 *
(Capability->TimerCount - 1)));
}
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
index 5495b324b381..aed34596f469 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
@@ -941,7 +941,7 @@ UsbEnumeratePort (
// which probably is caused by short circuit. It has to wait
system
hardware
// to perform recovery.
//
- DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over
Current\n", Port));
+ DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over Current
(port %d)\n", Port));
return EFI_DEVICE_ERROR;
}

@@ -951,7 +951,7 @@ UsbEnumeratePort (
// over current. As a result, all ports are nearly power-off, so
// it's necessary to detach and enumerate all ports again.
//
- DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery
Over Current\n", Port));
+ DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery
Over Current (port %d)\n", Port));
}

if (USB_BIT_IS_SET (PortState.PortChangeStatus,
USB_PORT_STAT_C_ENABLE)) {
@@ -961,7 +961,7 @@ UsbEnumeratePort (
// on 2.0 roothub does. When over-current has influence on 1.1
device, the port
// would be disabled, so it's also necessary to detach and
enumerate
again.
//
- DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery
Over Current\n", Port));
+ DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery
Over Current (port %d)\n", Port));
}

if (USB_BIT_IS_SET (PortState.PortChangeStatus,
USB_PORT_STAT_C_CONNECTION)) {
@@ -969,7 +969,7 @@ UsbEnumeratePort (
// Case4:
// Device connected or disconnected normally.
//
- DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device
Connect/Disconnect Normally\n", Port));
+ DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device
Connect/Disconnect Normally (port %d)\n", Port));
}

//
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
b/MdeModulePkg/Core/Dxe/Image/Image.c
index 68bde5c15c52..06cc6744b8c6 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -1741,7 +1741,7 @@ CoreStartImage (
if ((Image->ExitDataSize != 0) || (Image->ExitData != NULL)) {
DEBUG ((DEBUG_LOAD, "StartImage: ExitDataSize %d, ExitData %p",
(UINT32)Image->ExitDataSize, Image->ExitData));
if (Image->ExitData != NULL) {
- DEBUG ((DEBUG_LOAD, " (%hs)", Image->ExitData));
+ DEBUG ((DEBUG_LOAD, " (%s)", Image->ExitData));
}

DEBUG ((DEBUG_LOAD, "\n"));
diff --git
a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdate
ProgressLibGraphics.c
b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdate
ProgressLibGraphics.c
index 83053464e06e..6b012fed35db 100644
---
a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdate
ProgressLibGraphics.c
+++
b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdate
ProgressLibGraphics.c
@@ -148,7 +148,7 @@ FindDim (
(VOID **)&BootLogo
);
if ((BootLogo == NULL) || (EFI_ERROR (Status))) {
- DEBUG ((DEBUG_ERROR, "Failed to locate gEdkiiBootLogo2ProtocolGuid.
No Progress bar support. \n", Status));
+ DEBUG ((DEBUG_ERROR, "Failed to locate
gEdkiiBootLogo2ProtocolGuid Status = %r. No Progress bar support. \n",
Status));
return;
}

--
2.28.0.windows.1


[PATCH] MdePkg/UefiDevicePathLib: reback the DevicePathUtilitiesStandaloneMm

Huang, Yanbo
 

From: Yanbo Huang <yanbo.huang@...>

reback the DevicePathUtilitiesStandaloneMm to unblock the downstream sync

Signed-off-by: Yanbo Huang <yanbo.huang@...>
CC: Michael D Kinney <michael.d.kinney@...>
CC: Liming Gao <gaoliming@...>
CC: Zhiguang Liu <zhiguang.liu@...>

---
.../DevicePathUtilitiesStandaloneMm.c | 39 ++++++++++
.../UefiDevicePathLibStandaloneMm.inf | 75 +++++++++++++++++++
2 files changed, 114 insertions(+)
create mode 100644 MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
create mode 100644 MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf

diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
new file mode 100644
index 0000000000..096f835b90
--- /dev/null
+++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilitiesStandaloneMm.c
@@ -0,0 +1,39 @@
+/** @file
+ Device Path services. The thing to remember is device paths are built out of
+ nodes. The device path is terminated by an end node that is length
+ sizeof(EFI_DEVICE_PATH_PROTOCOL). That would be why there is sizeof(EFI_DEVICE_PATH_PROTOCOL)
+ all over this file.
+
+ The only place where multi-instance device paths are supported is in
+ environment varibles. Multi-instance device paths should never be placed
+ on a Handle.
+
+ Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) Microsoft Corporation.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "UefiDevicePathLib.h"
+
+/**
+ Retrieves the device path protocol from a handle.
+
+ This function returns the device path protocol from the handle specified by Handle.
+ If Handle is NULL or Handle does not contain a device path protocol, then NULL
+ is returned.
+
+ @param Handle The handle from which to retrieve the device
+ path protocol.
+
+ @return The device path protocol from the handle specified by Handle.
+
+**/
+EFI_DEVICE_PATH_PROTOCOL *
+EFIAPI
+DevicePathFromHandle (
+ IN EFI_HANDLE Handle
+ )
+{
+ return NULL;
+}
diff --git a/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
new file mode 100644
index 0000000000..23fedf38b7
--- /dev/null
+++ b/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibStandaloneMm.inf
@@ -0,0 +1,75 @@
+## @file
+# Instance of Device Path Library based on Memory Allocation Library.
+#
+# Device Path Library that layers on top of the Memory Allocation Library.
+#
+# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) Microsoft Corporation.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = UefiDevicePathLib
+ MODULE_UNI_FILE = UefiDevicePathLib.uni
+ FILE_GUID = D8E58437-44D3-4154-B7A7-EB794923EF12
+ MODULE_TYPE = MM_STANDALONE
+ PI_SPECIFICATION_VERSION = 0x00010032
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = DevicePathLib | MM_STANDALONE MM_CORE_STANDALONE
+
+
+#
+# VALID_ARCHITECTURES = IA32 X64 EBC
+#
+
+[Sources]
+ DevicePathUtilities.c
+ DevicePathUtilitiesStandaloneMm.c
+ DevicePathToText.c
+ DevicePathFromText.c
+ UefiDevicePathLib.c
+ UefiDevicePathLib.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ MemoryAllocationLib
+ DebugLib
+ BaseMemoryLib
+ PcdLib
+ PrintLib
+
+[Guids]
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVTUTF8Guid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVT100Guid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVT100PlusGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPcAnsiGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiUartDevicePathGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiSasDevicePathGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVirtualDiskGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiVirtualCdGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPersistentVirtualDiskGuid
+ ## SOMETIMES_CONSUMES ## GUID
+ gEfiPersistentVirtualCdGuid
+
+[Protocols]
+ gEfiDevicePathProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiDebugPortProtocolGuid ## UNDEFINED
+
+[Pcd]
+ gEfiMdePkgTokenSpaceGuid.PcdMaximumDevicePathNodeCount ## SOMETIMES_CONSUMES
--
2.31.1.windows.1


Re: [PATCH v3 0/2] Add EDKII_PCI_DEVICE_PPI support to EDK2

Wu, Hao A
 

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao
A
Sent: Tuesday, August 2, 2022 10:21 AM
To: devel@edk2.groups.io; Czajkowski, Maciej
<maciej.czajkowski@...>
Cc: Gao, Liming <gaoliming@...>; Ni, Ray <ray.ni@...>
Subject: Re: [edk2-devel] [PATCH v3 0/2] Add EDKII_PCI_DEVICE_PPI
support to EDK2

Thanks, the series look good to me. I have given my R-B tag for both of the
patches.
Will wait some time to see if there are comments from other reviewers. If no
further feedback received, I will merge the series before the end of this
week.

Best Regards,
Hao Wu

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Maciej
Czajkowski
Sent: Tuesday, August 2, 2022 1:00 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH v3 0/2] Add EDKII_PCI_DEVICE_PPI support
to
EDK2

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

The purpose of those changes is to introduce the way to enumerate and
assign resources in PEI for the systems with more than one PCI root.
Here is a need to have an interface that will support such a mechanizm.
For now, the part that performs the enumeration will be implemented in
the silicon code.
Sample code can be seen here: https://github.com/mczaj/edk2-
platforms/commit/d443062e58f9fba228869b54f2546d9735b3b506

v1: https://edk2.groups.io/g/devel/topic/91575907
v2: https://edk2.groups.io/g/devel/message/91893

v2 changes:
- collected Acked-by tag for no.1 commit
- followed-up with change suggestions in no.2 commit

v3 changes:
- no.1 commit: picked up reviewed-by tag
- no.2 commit: a few fixes based on v2 review

Maciej Czajkowski (2):
MdeModulePkg: Add EDKII_PCI_DEVICE_PPI definition
MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device

MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c | 505 +++++++++++++-------
MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c | 44 --
MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h | 57 ++-
MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf | 5 +-
MdeModulePkg/Include/Ppi/PciDevice.h | 32 ++
MdeModulePkg/MdeModulePkg.dec | 3 +
MdeModulePkg/MdeModulePkg.dsc | 1 +
7 files changed, 425 insertions(+), 222 deletions(-) create mode
100644 MdeModulePkg/Include/Ppi/PciDevice.h

--
2.27.0.windows.1

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII
Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP
957-
07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego
adresata i moze zawierac informacje poufne. W razie przypadkowego
otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale
jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest
zabronione.
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). If you are not the intended
recipient, please contact the sender and delete all copies; any review
or distribution by others is strictly prohibited.








Re: [PATCH 0/4] IntelFsp2(Wrapper)Pkg: Support FSP 2.4 MultiPhase.

Nate DeSimone
 

Hi Chasel,

 

I have a few comments for you.

 

First, we should have a platform provided LibraryClass for running code in between multi-phase actions. Right now you just have this comment in IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c:

 

    //

    // Platform handling can be added here to take care specific actions for each phase

    // before returning control back to FSP.

    //

 

I would like a new LibraryClass added: FspWrapperPlatformMultiPhaseLib

 

This would implement a single function:

 

VOID

EFIAPI

FspWrapperPlatformMultiPhaseHandler (

  IN UINT8     ComponentIndex,

  IN UINT32    PhaseIndex

 );

 

Add an implementation of this in IntelFsp2WrapperPkg/Library/BaseFspWrapperPlatformMultiPhaseLibSample. That .inf will provide an implementation of FspWrapperPlatformMultiPhaseHandler() that doesn’t do anything (just leave it empty).

 

Then, invoke FspWrapperPlatformMultiPhaseHandler() at the point where you have that comment above in FspWrapperMultiPhaseHandler().

 

The *BoardPkg can provide an SOC specific implementation. Typically the real *BoardPkg implementation will look something like this:

 

VOID

EFIAPI

FspWrapperPlatformMultiPhaseHandler (

  IN UINT8     ComponentIndex,

  IN UINT32    PhaseIndex

 )

{

  switch (ComponentIndex) {

  case FspMultiPhaseMemInitApiIndex:

    switch (PhaseIndex) {

      case 1:

        PeiServicesInstallPpi (mSomePlatformSpecificNotifyPpi1);

      break;

    }

    break;

  case FspMultiPhaseSiInitApiIndex:

    switch (PhaseIndex) {

      case 1:

        PeiServicesInstallPpi (mSomePlatformSpecificNotifyPpi2);

      break;

    }

    break;

  }

}

 

The exact specifics would vary by SOC design and are out of scope for this patch series. But you do need to provide the base case of “do nothing” in IntelFsp2WrapperPkg.

 

Second, in IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c:

 

      case EnumFspVariableRequestSetVariable:

        if (WriteVariableSupport) {

          Status = VariablePpi->SetVariable (

                                  VariablePpi,

                                  FspVariableRequestParams->VariableName,

                                  FspVariableRequestParams->VariableGuid,

                                  *FspVariableRequestParams->Attributes,

                                  (UINTN)*FspVariableRequestParams->DataSize,

                                  FspVariableRequestParams->Data

                                  );

        } else {

          return EFI_UNSUPPORTED;

        }

 

Instead of return EFI_UNSUPPORTED; it should be Status = EFI_UNSUPPORTED;. Same thing with EnumFspVariableRequestQueryVariableInfo.

 

Third, in FspWrapperVariableRequestHandler(), after you call EnumMultiPhaseCompleteVariableRequest you need to check if one of the FSP_STATUS_RESET_REQUIRED_* status codes is returned and if so invoke CallFspWrapperResetSystem().

 

Fourth, there is a bug in IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c:

 

    FspWrapperVariableRequestHandler (&FspHobListPtr, FspMultiPhaseMemInitApiIndex);

 

Should be this:

 

    FspWrapperVariableRequestHandler (&FspHobListPtr, FspMultiPhaseSiInitApiIndex);

 

Fifth, I noticed some spelling errors in IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c:

 

    // Firstly querry variable request informaiton from FSP.

 

Should be:

 

    // Get the variable request information from FSP.

 

And this:

 

  // Firstly querry FSP for how many phases supported.

 

Should be:

 

  // Query FSP for the number of phases supported.

 

Thanks,

Nate

 

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@...>
Sent: Thursday, August 4, 2022 5:20 PM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Zeng, Star <star.zeng@...>
Subject: [PATCH 0/4] IntelFsp2(Wrapper)Pkg: Support FSP 2.4 MultiPhase.

 

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

 

Add FSP 2.4 MultiPhase interfaces and implementation.

 

Cc: Nate DeSimone <nathaniel.l.desimone@...>

Cc: Star Zeng <star.zeng@...>

Signed-off-by: Chasel Chiu <chasel.chiu@...>

 

Chasel Chiu (4):

  IntelFsp2Pkg: Add FSP 2.4 MultiPhase interface.

  IntelFsp2WrapperPkg: Add FSP 2.4 MultiPhase interface.

  IntelFsp2Pkg: Adopt FSP 2.4 MultiPhase functions.

  IntelFsp2WrapperPkg: Implement FSP 2.4 MultiPhase wrapper handlers.

 

IntelFsp2Pkg/FspSecCore/SecFsp.c                                                               |   4 ++++

IntelFsp2Pkg/FspSecCore/SecFspApiChk.c                                                         |   9 +++++++++

IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/FspMultiPhaseLib.c                                   | 176 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c                                          |  33 +++++++++++++++++++++++++--------

IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c                                          |  27 +++++++++++++++++++++------

IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c | 337 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf                                                      |  75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf                                                      |  59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm                                               | 304 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryS.nasm                                               | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryCommon.nasm                                            |   3 +++

IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryM.nasm                                                | 303 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryS.nasm                                                | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/FspSecCore/X64/FspApiEntryCommon.nasm                                             |   3 +++

IntelFsp2Pkg/Include/FspEas/FspApi.h                                                           |  62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--

IntelFsp2Pkg/Include/FspGlobalData.h                                                           |   5 ++++-

IntelFsp2Pkg/Include/Library/FspMultiPhaseLib.h                                                |  54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/IntelFsp2Pkg.dec                                                                  |  12 ++++++++++--

IntelFsp2Pkg/IntelFsp2Pkg.dsc                                                                  |   4 ++++

IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/BaseFspMultiPhaseLib.inf                             |  50 ++++++++++++++++++++++++++++++++++++++++++++++++++

IntelFsp2Pkg/Tools/SplitFspBin.py                                                              |  48 +++++++++++++++++++++++++-----------------------

IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf                                        |   1 +

IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf                                        |   3 ++-

IntelFsp2WrapperPkg/Include/Library/FspWrapperMultiPhaseProcessLib.h                           |  38 ++++++++++++++++++++++++++++++++++++++

IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec                                                    |   6 +++++-

IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc                                                    |   4 +++-

IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/FspWrapperMultiPhaseProcessLib.inf  |  47 +++++++++++++++++++++++++++++++++++++++++++++++

27 files changed, 1831 insertions(+), 45 deletions(-)  create mode 100644 IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/FspMultiPhaseLib.c

create mode 100644 IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/PeiFspWrapperMultiPhaseProcessLib.c

create mode 100644 IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf

create mode 100644 IntelFsp2Pkg/FspSecCore/Fsp24SecCoreS.inf

create mode 100644 IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm

create mode 100644 IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryS.nasm

create mode 100644 IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryM.nasm

create mode 100644 IntelFsp2Pkg/FspSecCore/X64/Fsp24ApiEntryS.nasm

create mode 100644 IntelFsp2Pkg/Include/Library/FspMultiPhaseLib.h

create mode 100644 IntelFsp2Pkg/Library/BaseFspMultiPhaseLib/BaseFspMultiPhaseLib.inf

create mode 100644 IntelFsp2WrapperPkg/Include/Library/FspWrapperMultiPhaseProcessLib.h

create mode 100644 IntelFsp2WrapperPkg/Library/FspWrapperMultiPhaseProcessLib/FspWrapperMultiPhaseProcessLib.inf

 

--

2.35.0.windows.1

 


Re: [edk2-platforms][PATCH v3 0/6] Resolving SecureBootVariableLib dependency

Kun Qin
 

Hi Ard,

The term "Pipeline" is not applicable in the scope of edk2-platforms. Thanks for catching it.

A new version was sent for this fix: https://edk2.groups.io/g/devel/message/92128. Please let
me know if there is any other feedback.

Regards,
Kun

On 8/4/2022 3:26 AM, Ard Biesheuvel wrote:
On Wed, 3 Aug 2022 at 00:59, Kun Qin <kuqin12@...> wrote:
This v2 series is a follow up of previously submitted patches:
https://edk2.groups.io/g/devel/message/91669

The main update in v3 patches are:
- Updated a commit message to include Review-by tag
- Dumped a commit for Qemu platform as it was fixed by others

Patch v3 branch:https://github.com/kuqin12/edk2-platforms/tree/fix_sb_dep_v3
The patches look fine to me, but what does 'Pipeline' mean?

Cc: Ard Biesheuvel<ardb+tianocore@...>
Cc: Leif Lindholm<quic_llindhol@...>
Cc: Graeme Gregory<graeme@...>
Cc: Radoslaw Biernacki<rad@...>
Cc: Masami Hiramatsu<masami.hiramatsu@...>
Cc: Nhi Pham<nhi@...>
Cc: Vu Nguyen<vunguyen@...>
Cc: Thang Nguyen<thang@...>
Cc: Chuong Tran<chuong@...>
Cc: Thomas Abraham<thomas.abraham@...>
Cc: Sami Mujawar<sami.mujawar@...>
Cc: Abner Chang<abner.chang@...>
Cc: Gilbert Chen<gilbert.chen@...>
Cc: Daniel Schaefer<daniel.schaefer@...>
Cc: Jeremy Linton<jeremy.linton@...>
Cc: Peng Xie<xiepeng@...>
Cc: Ling Jia<jialing@...>
Cc: Yiqi Shu<shuyiqi@...>

Kun Qin (6):
RaspberryPi: Pipeline: Resolving newly introduced dependency
U5SeriesPkg: Pipeline: Resolving newly introduced dependency
VExpressPkg: Pipeline: Resolving newly introduced dependency
Socionext: Pipeline: Resolving newly introduced dependency
AmpereAltraPkg: Pipeline: Resolving newly introduced dependency
PhytiumCommonPkg: Pipeline: Resolving newly introduced dependency

Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 1 +
Platform/RaspberryPi/RPi3/RPi3.dsc | 1 +
Platform/RaspberryPi/RPi4/RPi4.dsc | 1 +
Platform/SiFive/U5SeriesPkg/FreedomU500VC707Board/U500.dsc | 1 +
Platform/SiFive/U5SeriesPkg/FreedomU540HiFiveUnleashedBoard/U540.dsc | 1 +
Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 1 +
Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc | 1 +
Silicon/Phytium/PhytiumCommonPkg/PhytiumCommonPkg.dsc.inc | 1 +
8 files changed, 8 insertions(+)

--
2.37.1.windows.1

4581 - 4600 of 96644