Re: [edk2-devel] [PATCH v3 03/11] MdePkg/Acpi65: Add _PSD/_CPC/State Coordination Types macros

[PierreGondois]

Hello Ray,

On 11/22/23 01:03, Ni, Ray wrote:

These ACPI methods were there since long ago.
Why do you only add the related definitions to 65.h?


I have only added the definitions to the latest ACPI definition file
(i.e. 6.5) because:
- as it was not defined before, I assumed it was not used previously
  in edk2 and definitions for older ACPI spec versions might not be
  necessary (i.e. no need to use EFI_ACPI_6_4_xxx definition if
  EFI_ACPI_6_5_xxx is available)
- this present patch-set only requires the latest definitions

Do you want to have definitions for all spec. versions ?




Cf. s8.3 Power, Performance, and Throttling State Dependencies


What does "Cf. s8.3" mean?


I used s8.3 for 'section 8.3' (in the ACPI 6.5 spec),



Thanks,
Ray

-Original Message-
From: devel@edk2.groups.io  On Behalf Of
PierreGondois
Sent: Wednesday, November 22, 2023 12:50 AM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel ; Leif Lindholm
; Sami Mujawar ;
Kinney, Michael D ; Gao, Liming

Subject: [edk2-devel] [PATCH v3 03/11] MdePkg/Acpi65: Add
_PSD/_CPC/State Coordination Types macros

Add macros for:
- _PSD version
- _CPC version
- C-state/T-state/P-state Coordination Types

These objects were present in previous ACPI specification version,
but are only added to the latest availbable version (6.5).

Signed-off-by: Pierre Gondois 
---
  MdePkg/Include/IndustryStandard/Acpi65.h | 23
+++
  1 file changed, 23 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/Acpi65.h
b/MdePkg/Include/IndustryStandard/Acpi65.h
index d79d0b2b5f9c..ec7c34e8e190 100644
--- a/MdePkg/Include/IndustryStandard/Acpi65.h
+++ b/MdePkg/Include/IndustryStandard/Acpi65.h
@@ -18,6 +18,29 @@
  //

  #pragma pack(1)



+///

+/// C-state/T-state/P-state Coordination Types

+/// Cf. s8.3 Power, Performance, and Throttling State Dependencies

+///

+#define EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ALL  0xFC

+#define EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ANY  0xFD

+#define EFI_ACPI_6_5_AML_STATE_COORD_TYPE_HW_ALL  0xFE

+

+///

+/// _PSD Revision

+/// Cf. s8.4.5.5 _PSD (P-State Dependency)

+///

+#define EFI_ACPI_6_5_AML_PSD_REVISION_V0  0

+

+///

+/// _CPC Revision

+/// Cf. s8.4.6.1 _CPC (Continuous Performance Control)

+///

+#define EFI_ACPI_6_5_AML_CPC_REVISION_V0  0

+#define EFI_ACPI_6_5_AML_CPC_REVISION_V1  1

+#define EFI_ACPI_6_5_AML_CPC_REVISION_V2  2

+#define EFI_ACPI_6_5_AML_CPC_REVISION_V3  3


Also a personal note, I realize this doesn't really make sense to define
versions like this, as there is only one revision for ACPI 6.5. This should
be instead:

#define EFI_ACPI_6_5_AML_CPC_REVISION  3

(same comment for EFI_ACPI_6_5_AML_PSD_REVISION_V0 above)

Regards,
Pierre


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111598): https://edk2.groups.io/g/devel/message/111598
Mute This Topic: https://groups.io/mt/102732018/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Patch V3 3/3] UefiCpuPkg/MpInitLib: Extract Dump Microcode Revision as function.

[Yuanhao Xie]

There is no functional changes, only extract DumpMicrocodeRevision since
only in PEI BSP will detect, apply microcode, and APs will sync
microcode.

Cc: Ray Ni 
Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Tom Lendacky 
Cc: Laszlo Ersek 
Signed-off-by: Yuanhao Xie 
---
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 91 
+--
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 31 ++-
 UefiCpuPkg/Library/MpInitLib/MpLib.h | 31 ++-
 3 files changed, 57 insertions(+), 96 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 11720560af..c0ca85543a 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -322,65 +322,64 @@ OnExit:
 }
 
 /**
-  Shadow the required microcode patches data into memory.
+Dump the microcode revision for each core.
 
-  @param[in, out]  CpuMpDataThe pointer to CPU MP Data structure.
-**/
+@param[in] CpuMpData  Pointer to CPU MP Data
+  **/
 VOID
-ShadowMicrocodeUpdatePatch (
-  IN OUT CPU_MP_DATA  *CpuMpData
+DumpMicrocodeRevision (
+  IN CPU_MP_DATA  *CpuMpData
   )
 {
-  EFI_STATUS  Status;
+  UINT32   ThreadId;
+  UINT32   ExpectedMicrocodeRevision;
+  CPU_INFO_IN_HOB  *CpuInfoInHob;
+  UINTNIndex;
 
-  Status = PlatformShadowMicrocode (CpuMpData);
-  if (EFI_ERROR (Status)) {
-ShadowMicrocodePatchByPcd (CpuMpData);
+  //
+  // Dump the microcode revision for each core.
+  //
+  DEBUG_CODE_BEGIN ();
+  CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
+  for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+GetProcessorLocationByApicId (CpuInfoInHob[Index].InitialApicId, NULL, 
NULL, );
+if (ThreadId == 0) {
+  //
+  // MicrocodeDetect() loads microcode in first thread of each core, so,
+  // CpuMpData->CpuData[Index].MicrocodeEntryAddr is initialized only for 
first thread of each core.
+  //
+  ExpectedMicrocodeRevision = 0;
+  if (CpuMpData->CpuData[Index].MicrocodeEntryAddr != 0) {
+ExpectedMicrocodeRevision = ((CPU_MICROCODE_HEADER 
*)(UINTN)CpuMpData->CpuData[Index].MicrocodeEntryAddr)->UpdateRevision;
+  }
+
+  DEBUG ((
+DEBUG_INFO,
+"CPU[%04d]: Microcode revision = %08x, expected = %08x\n",
+Index,
+CpuMpData->CpuData[Index].MicrocodeRevision,
+ExpectedMicrocodeRevision
+));
+}
   }
+
+  DEBUG_CODE_END ();
 }
 
 /**
-  Get the cached microcode patch base address and size from the microcode patch
-  information cache HOB.
-
-  @param[out] Address   Base address of the microcode patches data.
-It will be updated if the microcode patch
-information cache HOB is found.
-  @param[out] RegionSizeSize of the microcode patches data.
-It will be updated if the microcode patch
-information cache HOB is found.
-
-  @retval  TRUE The microcode patch information cache HOB is found.
-  @retval  FALSEThe microcode patch information cache HOB is not found.
+  Shadow the required microcode patches data into memory.
 
+  @param[in, out]  CpuMpDataThe pointer to CPU MP Data structure.
 **/
-BOOLEAN
-GetMicrocodePatchInfoFromHob (
-  UINT64  *Address,
-  UINT64  *RegionSize
+VOID
+ShadowMicrocodeUpdatePatch (
+  IN OUT CPU_MP_DATA  *CpuMpData
   )
 {
-  EFI_HOB_GUID_TYPE  *GuidHob;
-  EDKII_MICROCODE_PATCH_HOB  *MicrocodePathHob;
+  EFI_STATUS  Status;
 
-  GuidHob = GetFirstGuidHob ();
-  if (GuidHob == NULL) {
-DEBUG ((DEBUG_INFO, "%a: Microcode patch cache HOB is not found.\n", 
__func__));
-return FALSE;
+  Status = PlatformShadowMicrocode (CpuMpData);
+  if (EFI_ERROR (Status)) {
+ShadowMicrocodePatchByPcd (CpuMpData);
   }
-
-  MicrocodePathHob = GET_GUID_HOB_DATA (GuidHob);
-
-  *Address= MicrocodePathHob->MicrocodePatchAddress;
-  *RegionSize = MicrocodePathHob->MicrocodePatchRegionSize;
-
-  DEBUG ((
-DEBUG_INFO,
-"%a: MicrocodeBase = 0x%lx, MicrocodeSize = 0x%lx\n",
-__func__,
-*Address,
-*RegionSize
-));
-
-  return TRUE;
 }
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 538095d3bb..2fd96bf0bc 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2282,37 +2282,10 @@ MpInitLibInitialize (
 }
   }
 
-  //
-  // Dump the microcode revision for each core.
-  //
-  DEBUG_CODE_BEGIN ();
-  UINT32  ThreadId;
-  UINT32  ExpectedMicrocodeRevision;
-
-  CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
-  for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
-GetProcessorLocationByApicId (CpuInfoInHob[Index].InitialApicId, NULL, 
NULL, );
-if (ThreadId == 0) {
-  //
-  // 

[edk2-devel] [Patch V3 2/3] UefiCpuPkg/MpInitLib: Store MTRRs settings only when CpuCount>1

[Yuanhao Xie]

Store BSP's MTRR setting only when CpuCount>1.

Cc: Ray Ni 
Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Tom Lendacky 
Cc: Laszlo Ersek 
Signed-off-by: Yuanhao Xie 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index c26a17e1db..538095d3bb 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2243,15 +2243,14 @@ MpInitLibInitialize (
 MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
   }
 
-  //
-  // Store BSP's MTRR setting
-  //
-  MtrrGetAllMtrrs (>MtrrTable);
-
   //
   // Wakeup APs to do some AP initialize sync (MTRR and/or Microcode).
   //
   if (CpuMpData->CpuCount > 1) {
+//
+// Store BSP's MTRR setting
+//
+MtrrGetAllMtrrs (>MtrrTable);
 if (MpHandOff != NULL) {
   //
   // Only needs to use this flag for DXE phase to update the wake up
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111596): https://edk2.groups.io/g/devel/message/111596
Mute This Topic: https://groups.io/mt/102744600/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Patch V3 1/3] UefiCpuPkg/MpInitLib: Eliminate redundant microcode loading in DXE.

[Yuanhao Xie]

The DXE stage's Microcode loading process has been elimincated by:

1. Let ShadowMicrocodeUpdatePatch and MicrocodeDetect for BSP performed
only during the PEI phase. DXE skip those actions.
2. BSP in DXE WakeUpAp only for synchronizing MTRR settings, not loading
microcode.

Synchronizing the MTRR table to the AP is always essential.
During the DXE phase, it cannot be omitted like loading microcode,
as the PEI and DXE may be in different bit modes.

Cc: Ray Ni 
Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Tom Lendacky 
Cc: Laszlo Ersek 
Signed-off-by: Yuanhao Xie 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 
+++---
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9a6ec5db5c..c26a17e1db 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -451,12 +451,19 @@ ApInitializeSync (
   CpuMpData = (CPU_MP_DATA *)Buffer;
   Status= GetProcessorNumber (CpuMpData, );
   ASSERT_EFI_ERROR (Status);
+  ASSERT (CpuMpData->InitFlag == ApInitReconfig || CpuMpData->InitFlag == 
ApInitDone);
+  if (CpuMpData->InitFlag != ApInitReconfig) {
+//
+// Load microcode on AP for PEI phase.
+// During the DXE phase, it cannot omitted.
+//
+MicrocodeDetect (CpuMpData, ProcessorNumber);
+  }
+
   //
-  // Load microcode on AP
-  //
-  MicrocodeDetect (CpuMpData, ProcessorNumber);
-  //
-  // Sync BSP's MTRR table to AP
+  // Synchronizing the MTRR table to the AP is always essential.
+  // During the DXE phase, it cannot be omitted like loading microcode,
+  // as the PEI and DXE may be in different bit modes.
   //
   MtrrSetAllMtrrs (>MtrrTable);
 }
@@ -2224,29 +2231,25 @@ MpInitLibInitialize (
 }
   }
 
-  if (!GetMicrocodePatchInfoFromHob (
- >MicrocodePatchAddress,
- >MicrocodePatchRegionSize
- ))
-  {
+  if (MpHandOff == NULL) {
 //
 // The microcode patch information cache HOB does not exist, which means
 // the microcode patches data has not been loaded into memory yet
 //
 ShadowMicrocodeUpdatePatch (CpuMpData);
+//
+// Detect and apply Microcode on BSP
+//
+MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
   }
 
-  //
-  // Detect and apply Microcode on BSP
-  //
-  MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
   //
   // Store BSP's MTRR setting
   //
   MtrrGetAllMtrrs (>MtrrTable);
 
   //
-  // Wakeup APs to do some AP initialize sync (Microcode & MTRR)
+  // Wakeup APs to do some AP initialize sync (MTRR and/or Microcode).
   //
   if (CpuMpData->CpuCount > 1) {
 if (MpHandOff != NULL) {
@@ -2258,6 +2261,11 @@ MpInitLibInitialize (
   CpuMpData->InitFlag = ApInitReconfig;
 }
 
+//
+// Wake up the AP to perform some AP initialization synchronization.
+// 1. For PEI stage, load microcode and synchronize MTRR,
+// 2. For the DXE phase, only synchronize MTRR.
+//
 WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
 //
 // Wait for all APs finished initialization
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111595): https://edk2.groups.io/g/devel/message/111595
Mute This Topic: https://groups.io/mt/102744598/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Patch V3 0/3] UefiCpuPkg/MpInitLib: Eliminate redundant microcode loading in DXE.

[Yuanhao Xie]

The DXE stage's Microcode loading process has been eliminated.
Store BSP's MTRR setting only when CpuCount>1.
Compare with V2, V3 also Extract Dump Microcode Revision as function

xieyuanh (3):
  UefiCpuPkg/MpInitLib: Eliminate redundant microcode loading in DXE.
  UefiCpuPkg/MpInitLib: Store MTRRs settings only when CpuCount>1
  UefiCpuPkg/MpInitLib: Extract Dump Microcode Revision as function.

 UefiCpuPkg/Library/MpInitLib/Microcode.c | 91 
+--
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 78 
+-
 UefiCpuPkg/Library/MpInitLib/MpLib.h | 31 ++-
 3 files changed, 84 insertions(+), 116 deletions(-)

--
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111594): https://edk2.groups.io/g/devel/message/111594
Mute This Topic: https://groups.io/mt/102744597/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/5] Designware MMCDXE changes and enhancement

[John Chew]

Hi Leif, Ard,

By any chance you have time to review this series of patches?

Thank you.
Regards,
John

-Original Message-
From: YuinYee Chew  
Sent: Friday, November 3, 2023 10:51 AM
To: devel@edk2.groups.io
Cc: YuinYee Chew ; Sunil V L 
; Leif Lindholm ; Ard 
Biesheuvel ; Michael D Kinney 
; Li Yong 
Subject: [PATCH v3 0/5] Designware MMCDXE changes and enhancement

v3:
  - CC Ard in this patch series [John Chew]

v2:
  - Added maintainer to all patch in this series [Sunil]
  - Implement addition memory allocation strategy, where dma buffer
is force to allocation below 4GB (Please refer to pathc 0005)
[Sunil]

v1:
  This patch series involves the following changes:
1. Remove DMA enable in CTRL register
2. Added DMA polling handling for RX/TX 
3. Integrate SD MMC handling into the driver

Cc: Sunil V L 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Michael D Kinney 
Cc: Li Yong 

John Chew (1):
  DesignWare/DwEmmcDxe: Force DMA buffer to allocate below 4GB

mindachen1987 (4):
  DesignWare/DwEmmcDxe: Enabled Internal IDMAC interrupt RX/TX register
  DesignWare/DwEmmcDxe: Add CPU little endian option
  DesignWare/DwEmmcDxe: Remove ARM dependency library
  DesignWare/DwEmmcDxe: Add handling for SDMMC

 Silicon/Synopsys/DesignWare/DesignWare.dec 
 |   3 +
 Silicon/Synopsys/DesignWare/DesignWare.dsc 
 |   1 +
 Silicon/Synopsys/DesignWare/Drivers/DwEmmcDxe/DwEmmc.h 
 |  70 +++--
 Silicon/Synopsys/DesignWare/Drivers/DwEmmcDxe/DwEmmcDxe.c  
 | 315 +++-
 Silicon/Synopsys/DesignWare/Drivers/DwEmmcDxe/DwEmmcDxe.inf
 |   3 +-
 Silicon/Synopsys/DesignWare/Drivers/DwEmmcDxe/{DwEmmcDxe.inf => 
DwSdmmcDxe.inf} |  11 +-
 6 files changed, 283 insertions(+), 120 deletions(-)  copy 
Silicon/Synopsys/DesignWare/Drivers/DwEmmcDxe/{DwEmmcDxe.inf => DwSdmmcDxe.inf} 
(74%)

--
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111593): https://edk2.groups.io/g/devel/message/111593
Mute This Topic: https://groups.io/mt/102357267/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 09/39] MdePkg: Add a new library named PeiServicesTablePointerLibReg

[Chao Li]

Hi Laszlo,


Thanks,
Chao
On 2023/11/21 22:37, Laszlo Ersek wrote:

On 11/17/23 10:59, Chao Li wrote:

Since some ARCH or platform not require execute code on memory during
PEI phase, some values may transferred via CPU registers.

Adding PeiServcieTablePointerLibReg to allow set and get the PEI service
table pointer depend by a CPU register, this library can accommodate lot
of platforms who not require execte code on memory during PEI phase.

Adding PeiServiceTablePointerLibReg to allows setting and getting the
PEI service table pointer via CPU registers, and the library can
accommodate many platforms that do not need to execute code on memory
during the PEI phase.

The idea of this library is derived from
ArmPkg/Library/PeiServicesTablePointerLib/

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=4584

Cc: Michael D Kinney
Cc: Liming Gao
Cc: Zhiguang Liu
Cc: Leif Lindholm
Cc: Ard Biesheuvel
Cc: Sami Mujawar
Cc: Laszlo Ersek
Cc: Sunil V L
Signed-off-by: Chao Li
---
  .../Library/PeiServicesTablePointerLib.h  | 37 +++-
  .../PeiServicesTablePointer.c | 86 +++
  .../PeiServicesTablePointerLib.uni| 20 +
  .../PeiServicesTablePointerLibReg.inf | 40 +
  MdePkg/MdePkg.dsc |  1 +
  5 files changed, 180 insertions(+), 4 deletions(-)
  create mode 100644 
MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointer.c
  create mode 100644 
MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointerLib.uni
  create mode 100644 
MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointerLibReg.inf

In my opinion, the PeiServicesTablePointerLib class header should not be
extended with new interfaces. I understand that the generality is
attractive, but it is not put to use; only the loongarch architecture
applies the new interfaces (in the subsequent patch), and for example
the ARM code (ArmPkg/Library/PeiServicesTablePointerLib) is not reworked
in terms of these new interfaces.


This libarary have ability of accommodate more ARCH why not? I checked 
the PI SPEC, all ARCH except IA32 and X64 using the register mechanism, 
if this library can be approved, all of them can moved into this 
libraryso that code con be reused more, I think this library is fine.




What's more, the new library interfaces, even though they are exposed in
the lib class header, are not implemented for other architectures, so
they aren't even callable on those arches.
The patch 10 in this series has added LoongArch instance of this 
library, please check.


I'm commenting on this patch and the subsequent patch in the series
together, as seen squashed together. NB I'm not an MdePkg maintainer, so
this is just my opinion.

So, Mike and Liming, what do your think?


(1) As noted above, the library class should not be modified.

(2) Modifying the *comments* in
"MdePkg/Include/Library/PeiServicesTablePointerLib.h" is welcome, I
think, but then we might want to add a (separate!) patch for removing
the Itanium language, as edk2 no longer supports Itanium.

(3) The PeiServicesTablePointerLibReg instance should be called
PeiServicesTablePointerLibCsrKs0 or just PeiServicesTablePointerLibKs0.
This library will be a public libray which using the reigster mechanism, 
so the name like PeiServiceTablePointerLibCsrKs0 would not appropriate.


This follows the example of the lib instance name
"PeiServicesTablePointerLibIdt". The whole library instance should be
loongaarch-specific IMO; there isn't much code that's being duplicated,
so the extra interfaces (internal or external) do not help with code
unification.

(4) "PeiServicesTablePointerLib.uni" should be named similarly (suffix
missing).

(5) BASE_NAME in the library instance INF file should be defined
similarly (suffix missing).

(6) The contents of the UNI file should be loongarch-specific, i.e. be
explicit about CSR KS0, in both comments and string constants.

(7) The comments in the library instance INF file should be similarly
loongarch-specific.

(8) I suggest dropping VALID_ARCHITECTURES altogether. If we want to
keep it, it should exclusively say LOONGARCH64.

(9) The new library instance should be listed in
[Components.LOONGARCH64] in MdePkg.dec.

This section does not exist yet; I suggest introducing it under
[Components.RISCV64].
No, it is RISC-V area, not LOONGARCH64. And I do not recommend going 
this way. I believe this library should be a public library for register 
mechanism.


(10) There need not / should not be two separate C source files; just
access the KS0 CSR in SetPeiServicesTablePointer() and
GetPeiServicesTablePointer() directly.

(11) The new library instance should probably not introduce new
references to Itanium.

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111592): https://edk2.groups.io/g/devel/message/111592
Mute This Topic: https://groups.io/mt/102644754/21656
Group 

Re: [edk2-devel] [PATCH] Modified ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c to follow error handling instead of success handling

[Gao, Zhichao]

Please follow 
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format for 
the commit message.

And introduce why you need to make the change. The error handling would be 
taken care later.

Thanks,
Zhichao

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo
> Ersek
> Sent: Wednesday, November 22, 2023 12:25 AM
> To: devel@edk2.groups.io; daniel.ngu...@arm.com
> Cc: Gao, Zhichao 
> Subject: Re: [edk2-devel] [PATCH] Modified
> ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c to follow error
> handling instead of success handling
> 
> On 11/16/23 07:16, Daniel Nguyen wrote:
> > Signed-off-by: Daniel Nguyen 
> > ---
> >  .../UefiShellLevel2CommandsLib/Reset.c| 43 +++
> >  1 file changed, 24 insertions(+), 19 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> > index 57ba3c90f3..361c47e430 100644
> > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> > @@ -79,30 +79,35 @@ ShellCommandRunReset (
> >,
> >
> >);
> > -if (!EFI_ERROR (Status)) {
> > -  if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
> > -DataSize = sizeof (OsIndications);
> > -Status   = gRT->GetVariable (
> > -  EFI_OS_INDICATIONS_VARIABLE_NAME,
> > -  ,
> > -  ,
> > -  ,
> > -  
> > -  );
> > -if (!EFI_ERROR (Status)) {
> > -  OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> > -} else {
> > -  OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> > -}
> > -
> > -Status = gRT->SetVariable (
> > +
> > +if (EFI_ERROR (Status)) {
> > +  ShellStatus = SHELL_UNSUPPORTED;
> > +  goto Error;
> > +}
> > +
> > +if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
> > +  DataSize = sizeof (OsIndications);
> > +  Status   = gRT->GetVariable (
> >  EFI_OS_INDICATIONS_VARIABLE_NAME,
> >  ,
> > -EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> > -sizeof (OsIndications),
> > +,
> > +,
> >  
> >  );
> > +
> > +  if (EFI_ERROR (Status)) {
> > +OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> > +  } else {
> > +OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> >}
> > +
> > +  Status = gRT->SetVariable (
> > +  EFI_OS_INDICATIONS_VARIABLE_NAME,
> > +  ,
> > +  EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> > +  sizeof (OsIndications),
> > +  
> > +  );
> >  }
> >
> >  if (EFI_ERROR (Status)) {
> 
> Adding Zhichao to the CC list.
> 
> From a quick skim, the subject line and the commit message are not great
> (the former is too long, the latter is empty).
> 
> Laszlo
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111591): https://edk2.groups.io/g/devel/message/111591
Mute This Topic: https://groups.io/mt/102698984/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 03/11] MdePkg/Acpi65: Add _PSD/_CPC/State Coordination Types macros

[Ni, Ray]

These ACPI methods were there since long ago.
Why do you only add the related definitions to 65.h?

> Cf. s8.3 Power, Performance, and Throttling State Dependencies

What does "Cf. s8.3" mean?

Thanks,
Ray
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of
> PierreGondois
> Sent: Wednesday, November 22, 2023 12:50 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Leif Lindholm
> ; Sami Mujawar ;
> Kinney, Michael D ; Gao, Liming
> 
> Subject: [edk2-devel] [PATCH v3 03/11] MdePkg/Acpi65: Add
> _PSD/_CPC/State Coordination Types macros
> 
> Add macros for:
> - _PSD version
> - _CPC version
> - C-state/T-state/P-state Coordination Types
> 
> These objects were present in previous ACPI specification version,
> but are only added to the latest availbable version (6.5).
> 
> Signed-off-by: Pierre Gondois 
> ---
>  MdePkg/Include/IndustryStandard/Acpi65.h | 23
> +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Acpi65.h
> b/MdePkg/Include/IndustryStandard/Acpi65.h
> index d79d0b2b5f9c..ec7c34e8e190 100644
> --- a/MdePkg/Include/IndustryStandard/Acpi65.h
> +++ b/MdePkg/Include/IndustryStandard/Acpi65.h
> @@ -18,6 +18,29 @@
>  //
> 
>  #pragma pack(1)
> 
> 
> 
> +///
> 
> +/// C-state/T-state/P-state Coordination Types
> 
> +/// Cf. s8.3 Power, Performance, and Throttling State Dependencies
> 
> +///
> 
> +#define EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ALL  0xFC
> 
> +#define EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ANY  0xFD
> 
> +#define EFI_ACPI_6_5_AML_STATE_COORD_TYPE_HW_ALL  0xFE
> 
> +
> 
> +///
> 
> +/// _PSD Revision
> 
> +/// Cf. s8.4.5.5 _PSD (P-State Dependency)
> 
> +///
> 
> +#define EFI_ACPI_6_5_AML_PSD_REVISION_V0  0
> 
> +
> 
> +///
> 
> +/// _CPC Revision
> 
> +/// Cf. s8.4.6.1 _CPC (Continuous Performance Control)
> 
> +///
> 
> +#define EFI_ACPI_6_5_AML_CPC_REVISION_V0  0
> 
> +#define EFI_ACPI_6_5_AML_CPC_REVISION_V1  1
> 
> +#define EFI_ACPI_6_5_AML_CPC_REVISION_V2  2
> 
> +#define EFI_ACPI_6_5_AML_CPC_REVISION_V3  3
> 
> +
> 
>  ///
> 
>  /// ACPI 6.5 Generic Address Space definition
> 
>  ///
> 
> --
> 2.25.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#111570):
> https://edk2.groups.io/g/devel/message/111570
> Mute This Topic: https://groups.io/mt/102732018/1712937
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray...@intel.com]
> -=-=-=-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111590): https://edk2.groups.io/g/devel/message/111590
Mute This Topic: https://groups.io/mt/102732018/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Fix typos in Bus and Core

[Page Chen via groups.io]

Thanks for the feedback. I will break down into smaller pieces.

Page

> On Nov 21, 2023, at 8:17 AM, Laszlo Ersek  wrote:
> 
> On 11/21/23 03:07, Page Chen via groups.io wrote:
>> begining -> beginning
>> Cabability -> Capability
>> CHNAGED -> CHANGED
>> compatability -> compatibility
>> concident -> coincident
>> correspoding -> corresponding
>> defered -> deferred
>> Dispacher -> Dispatcher
>> execuing -> executing
>> exhausive -> exhaustive
>> functio -> function
>> funtion -> function
>> funtions -> functions
>> notificaiton -> notification
>> Regsitered -> Registered
>> SUCESS -> SUCCESS
>> sucessfully -> successfully
>> writting -> writing
> 
> this patch is easy to post and terribly hard to review.
> 
> If it is supposed to be reviewed by a human reviewer, then please split
> it up.
> 
> the reason being that a human reviewer gets de-sensitivized to such
> changes in a very short time, and then doesn't notice any mistakes.
> Therefore, a human needs *frequent* interruption and resumption points,
> within a series of changes of this type. Lumping them all together is a
> terrible idea. I certainly can't review 62 changes of this type in one go.
> 
> according to the diffstat, we have ~10 modules that are affected. That
> provides, IMO, a much better resolution for this work; a reviewer can
> freely stop mid-series at any point and pick up later.
> 
> anyway -- I don't insist for this to be respun, I just can't review it
> like this.
> 
> Laszlo
> 
>> 
>> Signed-off-by: Page Chen 
>> Cc: Liming Gao 
>> Cc: Ray Ni 
>> Cc: Andrew Fish 
>> ---
>> .../Bus/Ata/AhciPei/AhciPeiStorageSecurity.c   |  2 +-
>> .../Bus/Ata/AhciPei/AhciPeiStorageSecurity.h   |  2 +-
>> .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c  | 18 +-
>> .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h   |  4 ++--
>> .../Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.c  | 10 +-
>> .../Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.h  |  8 
>> .../Bus/Pci/NvmExpressPei/NvmExpressPeiHci.c   |  4 ++--
>> .../Bus/Pci/NvmExpressPei/NvmExpressPeiHci.h   |  4 ++--
>> .../NvmExpressPeiStorageSecurity.c |  2 +-
>> .../NvmExpressPeiStorageSecurity.h |  2 +-
>> .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c|  4 ++--
>> .../Bus/Pci/PciBusDxe/PciOptionRomSupport.h|  2 +-
>> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h |  2 +-
>> MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c   |  2 +-
>> MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c   |  2 +-
>> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c | 18 +-
>> MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h |  4 ++--
>> MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c  |  2 +-
>> MdeModulePkg/Core/Dxe/DxeMain.h|  8 
>> MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c|  2 +-
>> MdeModulePkg/Core/Dxe/Hand/Notify.c|  2 +-
>> MdeModulePkg/Core/Dxe/Image/Image.c|  2 +-
>> MdeModulePkg/Core/Dxe/Mem/Page.c   |  6 +++---
>> .../SectionExtraction/CoreSectionExtraction.c  |  6 +++---
>> MdeModulePkg/Core/DxeIplPeim/DxeLoad.c |  4 ++--
>> MdeModulePkg/Core/Pei/PeiMain.h|  2 +-
>> 26 files changed, 62 insertions(+), 62 deletions(-)
>> 
>> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c 
>> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
>> index 40e042082cde..6381c371698f 100644
>> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
>> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
>> @@ -302,7 +302,7 @@ AhciStorageSecurityReceiveData (
>> 
>>   If the security protocol command completes without an error, the function 
>> shall
>>   return EFI_SUCCESS. If the security protocol command completes with an 
>> error,
>> -  the functio shall return EFI_DEVICE_ERROR.
>> +  the function shall return EFI_DEVICE_ERROR.
>> 
>>   @param[in] This  The PPI instance pointer.
>>   @param[in] DeviceIndex   The ID of the device.
>> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h 
>> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
>> index a13533ea57ea..8bc367da21d8 100644
>> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
>> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
>> @@ -190,7 +190,7 @@ AhciStorageSecurityReceiveData (
>> 
>>   If the security protocol command completes without an error, the function 
>> shall
>>   return EFI_SUCCESS. If the security protocol command completes with an 
>> error,
>> -  the functio shall return EFI_DEVICE_ERROR.
>> +  the function shall return EFI_DEVICE_ERROR.
>> 
>>   @param[in] This  The PPI instance pointer.
>>   @param[in] DeviceIndex   The ID of the device.
>> diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c 
>> b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
>> index b1ab17af3788..535f234916c0 100644
>> --- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
>> +++ 

Re: [edk2-devel] [PATCH v2] OvmfPkg/MemEncryptSevLib: Fix address overflow during PVALIDATE

[Laszlo Ersek]

On 11/20/23 08:55, Gerd Hoffmann wrote:
> On Fri, Nov 17, 2023 at 10:39:13PM +0100, Laszlo Ersek wrote:
>> On 11/17/23 12:42, Gerd Hoffmann wrote:
>>> On Fri, Nov 17, 2023 at 10:16:10AM +0100, Laszlo Ersek wrote:
 (+Liming +Mike)

 On 11/16/23 10:01, Gerd Hoffmann wrote:
> On Wed, Nov 15, 2023 at 11:51:53AM -0600, Michael Roth wrote:
>> The struct used for GHCB-based page-state change requests uses a 40-bit
>> bit-field for the GFN, which is shifted by PAGE_SHIFT to generate a
>> 64-bit address. However, anything beyond 40-bits simply gets shifted off
>> when doing this, which will cause issues when dealing with 1TB+
>> addresses. Fix this by casting the 40-bit GFN values to 64-bit ones
>> prior to shifting it by PAGE_SHIFT.
>>
>> Fixes: ade62c18f474 ("OvmfPkg/MemEncryptSevLib: add support to validate 
>> system RAM")
>> Signed-off-by: Michael Roth 
>
> Reviewed-by: Gerd Hoffmann 
>
> take care,
>   Gerd

 Is this hard feature freeze material?
>>>
>>> It is a clear bugfix, so IMHO it qualifies.
>>>
 Also, the patch looks garbled to me on-list (superfluous line breaks).
>>>
>>> Patch applies fine here.  I see mutt breaking the long line, but
>>> that is just the local display rendering, the mail good.
>>
>> Can you check the raw message? I did that and it seems broken.
>> Superfluous newlines. I see *double* CRLFs.
> 
> Hmm, everything looks fine here, and 'git am' accepts the mail without
> problems.  Pushed a branch:
> 
> https://github.com/kraxel/edk2/commits/b4/v2-20231115-michael-roth-ovmfpkg-memencryptsevlib-fix-address-overflow-during-pvalidate

Thanks. I hope that suffices for Liming to merge the patch.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111588): https://edk2.groups.io/g/devel/message/111588
Mute This Topic: https://groups.io/mt/102610323/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 4/5] MdeModulePkg/Bus/Pci/PciBusDxe: Fix NULL_RETURNS Coverity issue

[Laszlo Ersek]

On 11/20/23 04:57, Ranbir Singh wrote:
> 
> 
> On Wed, Nov 15, 2023 at 3:20 PM Laszlo Ersek  > wrote:
> 
> On 11/14/23 17:21, Kinney, Michael D wrote:
> > Hi Ranbir,
> >
> >  
> >
> > First I want to recognize your efforts to collect Coverity issues and
> > propose changes to address
> > them.
> >
> > I still disagree with adding CpuDealLoop() for any static analysis
> issues.
> >
> > There have been previous discussions about adding a PANIC() or FATAL()
> > macro that would
> > perform platform specific actions if a condition is detected where the
> > boot of the platform
> > can not continue.  A platform get to make the choice to log or
> reboot or
> > hang, etc.  Not the
> > code that detected the condition.
> >
> > https://edk2.groups.io/g/devel/message/86597
> 
> >  >
> 
> This is indeed a great idea.
> 
> I didn't know about that discussion. Perhaps a new DebugLib API would
> handle this (i.e., "panic").
> 
> I've been certainly proposing CpuDeadLoop() as a means to panic.
> 
> Did the thread conclude anything tangible?
> 
> > Unfortunately, in order to fix some of these static analysis issues
> > correctly, we are going
> > to have to identify the use of ASSERT() that really is a fatal
> condition
> > that can not continue
> 
> Absolutely.
> 
> > and introduce an implementation approach that provides a platform
> > handler and
> > satisfies the static analysis tools.
> 
> The "platform handler" is the difficult part. If the above-noted
> discussion from 2022 didn't produce an agreement, should we really block
> the static analyzer fixes on an agreed-upon panic API? I'm concerned
> that would just cause these fixes to get stuck. And I don't consider
> CpuDeadLoop()s added for this purpose serious technical debt. They are
> easy to find and update later, assuming we also add comments.
> 
> 
> I agree with the approach to not gate current fixes adding
> CpuDeadLoop(). Later on, it can be updated with the desired panic API
> and I can contribute for those required changes related to patches
> submitted by me.
> 
> I can update current patches to carry additional comment in suffix form
> to ease later search like
>     CpuDeadLoop (); // TBD: replace with Panic API in future
> 
> Laszlo, Mike - Let me know if that works for now.

It works for me.

Of course the risk is always that the proper panic API might never
materialize, and then we'll be stuck with these comments forever as yet
another piece of technical debt. From that perspective minimally, it
would be reasonable for Mike not to accept these reminder comments. (A
reminder BZ, with the commit hash and exact code locations listed, could
be a stronger reminder, but we've seen such BZs too fall through the
cracks over time.)

So, for me, I'm OK; but if Mike doesn't like this approach, I'll
certainly accept that (and then we can't fix the coverity warnings until
the panic API arrives).

Thanks
Laszlo

>  
> 
> > We also have to evaluate if a return error status with a DEBUG_ERROR
> > message would be a better
> > choice than an ASSERT() that can be filtered out by build options.
> 
> I agree 100% that this would be better for the codebase, but the work
> needed for this is (IMO) impossible to contain. ASSERT() has been abused
> for a long time *because* it seemed to allow the programmer to ignore
> any related error paths. If we now want to implement those error paths
> retroactively (which would be absolutely the right thing to do from a
> software engineering perspective), then immense amounts of work are
> going to be needed (patch review and regression testing), and I think
> it's just not practical to dump all that on someone that wants to
> improve the status quo. Replacing an invalid ASSERT() with a panic is
> honest about the current situation, is safer regarding RELEASE builds,
> and its work demand (regression testing, patch review) is tolerable.
> 
> I do agree that, if the error path mostly exists already, then returning
> errors for data/environment-based error conditions (i.e., not for
> algorithmic invariant failures) is best.
> 
> Where we need to be extremely vigilant is new patches. We must
> uncompromisingly reject *new* abuses of ASSERT(), in new patches.
> 
> Anyway, it seems that we've been trying to steer Ranbir in opposite
> directions. I'll let you take the lead on this; for one, I've not been
> aware of the panic api discussion for 2022!
> 
> (I don't feel especially pushy about fixing coverity issues, it's just
> that Ranbir has been contributing such patches, which I've found very
> 

Re: [edk2-devel] [PATCH v3 10/11] DynamicTablesPkg: Add DynamicTablesScmiInfoLib

[Leif Lindholm]

On Tue, Nov 21, 2023 at 17:50:06 +0100, Pierre Gondois wrote:
> The SCP holds some power information that could be advertised
> through the _CPC object. The communication with the SCP is done
> through SCMI protocols (c.f. ArmScmiDxe).
> 
> Use the SCMI protocols to query information and feed it to
> the DynamicTablesPkg.
> 
> Signed-off-by: Pierre Gondois 

Acked-by: Leif Lindholm 

/
Leif

> ---
>  DynamicTablesPkg/DynamicTables.dsc.inc|   1 +
>  DynamicTablesPkg/DynamicTablesPkg.dec |   3 +
>  DynamicTablesPkg/DynamicTablesPkg.dsc |   1 +
>  .../Library/DynamicTablesScmiInfoLib.h|  33 ++
>  .../DynamicTablesScmiInfoLib.c| 297 ++
>  .../DynamicTablesScmiInfoLib.inf  |  31 ++
>  6 files changed, 366 insertions(+)
>  create mode 100644 
> DynamicTablesPkg/Include/Library/DynamicTablesScmiInfoLib.h
>  create mode 100644 
> DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.c
>  create mode 100644 
> DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.inf
> 
> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc 
> b/DynamicTablesPkg/DynamicTables.dsc.inc
> index 9d4312c4e87d..41947012b92b 100644
> --- a/DynamicTablesPkg/DynamicTables.dsc.inc
> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc
> @@ -15,6 +15,7 @@ [BuildOptions]
>  [LibraryClasses.common]
>
> AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
> +  
> DynamicTablesScmiInfoLib|DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.inf
>
> SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
>
> SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
>
> TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec 
> b/DynamicTablesPkg/DynamicTablesPkg.dec
> index cfbcbb9569f1..aa422ce9f6fd 100644
> --- a/DynamicTablesPkg/DynamicTablesPkg.dec
> +++ b/DynamicTablesPkg/DynamicTablesPkg.dec
> @@ -42,6 +42,9 @@ [LibraryClasses]
>##  @libraryclass  Defines a set of SMBIOS string helper methods.
>SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
>  
> +  ##  @libraryclass  Defines a set of APIs to populate CmObj using SCMI.
> +  DynamicTablesScmiInfoLib|Include/Library/DynamicTablesScmiInfoLib.h
> +
>  [Protocols]
># Configuration Manager Protocol GUID
>gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 
> 0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
> diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc 
> b/DynamicTablesPkg/DynamicTablesPkg.dsc
> index bd5084a9008f..853ea2a1e4c2 100644
> --- a/DynamicTablesPkg/DynamicTablesPkg.dsc
> +++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
> @@ -39,6 +39,7 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
>PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>  
>  [Components.common]
> +  
> DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.inf
>DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
>DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
>DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
> diff --git a/DynamicTablesPkg/Include/Library/DynamicTablesScmiInfoLib.h 
> b/DynamicTablesPkg/Include/Library/DynamicTablesScmiInfoLib.h
> new file mode 100644
> index ..ff6b47d51fe8
> --- /dev/null
> +++ b/DynamicTablesPkg/Include/Library/DynamicTablesScmiInfoLib.h
> @@ -0,0 +1,33 @@
> +/** @file
> +  Arm SCMI Info Library.
> +
> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef ARM_SCMI_INFO_LIB_H_
> +#define ARM_SCMI_INFO_LIB_H_
> +
> +#include 
> +
> +/** Populate a AML_CPC_INFO object based on SCMI information.
> +
> +  @param[in]  DomainIdIdentifier for the performance domain.
> +  @param[out] CpcInfo If success, this structure was populated from
> +  information queried to the SCP.
> +
> +  @retval EFI_SUCCESS Success.
> +  @retval EFI_DEVICE_ERRORDevice error.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_TIMEOUT Time out.
> +  @retval EFI_UNSUPPORTED Unsupported.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DynamicTablesScmiInfoGetFastChannel (
> +  IN  UINT32DomainId,
> +  OUT AML_CPC_INFO  *CpcInfo
> +  );
> +
> +#endif // ARM_SCMI_INFO_LIB_H_
> diff --git 
> a/DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.c
>  
> b/DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.c
> new file mode 100644
> index ..b59da3d6bb51
> --- /dev/null
> +++ 
> 

Re: [edk2-devel] [PATCH v3 1/3] Platform/ARM: Juno: Fix typo

[PierreGondois]

On 11/21/23 18:00, Leif Lindholm wrote:

On Tue, Nov 21, 2023 at 17:51:33 +0100, PierreGondois wrote:

Fix a typo.

Change-Id: I84807f18855ff4c0a2f34a3556edc4698eea2170


You've accidentally included your internal Change-Id in the commit
messages for all patches in this set.

/
 Leif


Yes exact ...
I will wait a bit before re-submitting the patches with this fixed,

Regards,
Pierre




Signed-off-by: Pierre Gondois 
---
  .../ConfigurationManagerDxe/ConfigurationManager.c  | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
 
b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index 91f035480dcf..2fdb15cc4caf 100644
--- 
a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ 
b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -993,7 +993,7 @@ GetGicCInfo (
return EFI_NOT_FOUND;
  }
  
-/** Return Lpi State Infor.

+/** Return Lpi State Info.
  
@param [in]  This   Pointer to the Configuration Manager Protocol.

@param [in]  CmObjectId The Object ID of the CM object requested
--
2.25.1









-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111585): https://edk2.groups.io/g/devel/message/111585
Mute This Topic: https://groups.io/mt/102732064/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 1/3] Platform/ARM: Juno: Fix typo

[Leif Lindholm]

On Tue, Nov 21, 2023 at 17:51:33 +0100, PierreGondois wrote:
> Fix a typo.
> 
> Change-Id: I84807f18855ff4c0a2f34a3556edc4698eea2170

You've accidentally included your internal Change-Id in the commit
messages for all patches in this set.

/
Leif

> Signed-off-by: Pierre Gondois 
> ---
>  .../ConfigurationManagerDxe/ConfigurationManager.c  | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git 
> a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
>  
> b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> index 91f035480dcf..2fdb15cc4caf 100644
> --- 
> a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> +++ 
> b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> @@ -993,7 +993,7 @@ GetGicCInfo (
>return EFI_NOT_FOUND;
>  }
>  
> -/** Return Lpi State Infor.
> +/** Return Lpi State Info.
>  
>@param [in]  This   Pointer to the Configuration Manager 
> Protocol.
>@param [in]  CmObjectId The Object ID of the CM object requested
> -- 
> 2.25.1
> 
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111583): https://edk2.groups.io/g/devel/message/111583
Mute This Topic: https://groups.io/mt/102732064/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION

[Leif Lindholm]

On Tue, Nov 21, 2023 at 17:49:57 +0100, Pierre Gondois wrote:
> Rename PERFORMANCE_PROTOCOL_VERSION to reflect the different
> versions of the protocol. The macro is neither used in edk2 nor
> in edk2-platforms.
> 
> Signed-off-by: Pierre Gondois 

Reviewed-by: Leif Lindholm 

> ---
>  .../Include/Protocol/ArmScmiPerformanceProtocol.h   | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h 
> b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> index 7e548e4765c2..a28f0f766e37 100644
> --- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> +++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
> @@ -1,12 +1,12 @@
>  /** @file
>  
> -  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
> +  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> -  System Control and Management Interface V1.0
> -http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
> -DEN0056A_System_Control_and_Management_Interface.pdf
> +  System Control and Management Interface V3.2, latest version at:
> +  - https://developer.arm.com/documentation/den0056/latest/
> +
>  **/
>  
>  #ifndef ARM_SCMI_PERFORMANCE_PROTOCOL_H_
> @@ -14,7 +14,10 @@
>  
>  #include 
>  
> -#define PERFORMANCE_PROTOCOL_VERSION  0x1
> +/// Arm Scmi performance protocol versions.
> +#define PERFORMANCE_PROTOCOL_VERSION_V1  0x1
> +#define PERFORMANCE_PROTOCOL_VERSION_V2  0x2
> +#define PERFORMANCE_PROTOCOL_VERSION_V3  0x3
>  
>  #define ARM_SCMI_PERFORMANCE_PROTOCOL_GUID  { \
>0x9b8ba84, 0x3dd3, 0x49a6, {0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 0x7b, 
> 0xad} \
> -- 
> 2.25.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111584): https://edk2.groups.io/g/devel/message/111584
Mute This Topic: https://groups.io/mt/102732016/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 3/3] Platform/ARM: Juno: Generate _CPC objects for JunoR2

[PierreGondois]

The SsdtCpuTopologyGenerator can generate _CPC objects.
This is done by querying the SCP for the relevant performance
state information through SCMI. CM_ARM_CPC_INFO are then populated
and used to generate _CPC objects in the Ssdt Cpu topology.

Use the DynamicTablesScmiInfoLib and add the handling to generate
_CPC information.

Note that using _CPC is only possible if SCP is correctly tuned
to advertise performance levels on an abstract and unified scale.
A basic check is done to prevent the _CPC generation otherwise.

Change-Id: Ifac3ff21a58c53da41e7444c035cb0e3f73acfad
Signed-off-by: Pierre Gondois 
---
 .../ConfigurationManager.c| 233 +-
 .../ConfigurationManager.h|   3 +
 .../ConfigurationManagerDxe.inf   |   1 +
 3 files changed, 234 insertions(+), 3 deletions(-)

diff --git 
a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
 
b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index 6a4f6e626d3f..283ff92bd027 100644
--- 
a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ 
b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -756,6 +757,8 @@ EDKII_PLATFORM_REPOSITORY_INFO 
ArmJunoPlatformRepositoryInfo = {
   4,
 },
   },
+  { // CPC info, dynamically populated.
+  },
 };
 
 /** A helper function for returning the Configuration Manager Objects.
@@ -1212,6 +1215,55 @@ GetPsdInfo (
   return EFI_SUCCESS;
 }
 
+/** Return Cpc Info.
+
+  @param [in]  This   Pointer to the Configuration Manager 
Protocol.
+  @param [in]  CmObjectId The Object ID of the CM object requested
+  @param [in]  SearchTokenA unique token for identifying the requested
+  CM_ARM_PCI_INTERRUPT_MAP_INFO object.
+  @param [in, out] CmObject   Pointer to the Configuration Manager Object
+  descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS Success.
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+  @retval EFI_NOT_FOUND   The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetCpcInfo (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  * CONST This,
+  IN  CONST CM_OBJECT_ID  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN   SearchToken,
+  IN  OUT   CM_OBJ_DESCRIPTOR * CONST CmObject
+  )
+{
+  EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
+  UINT32TotalObjCount;
+  UINT32ObjIndex;
+
+  if ((This == NULL) || (CmObject == NULL)) {
+ASSERT (This != NULL);
+ASSERT (CmObject != NULL);
+return EFI_INVALID_PARAMETER;
+  }
+
+  PlatformRepo = This->PlatRepoInfo;
+
+  TotalObjCount = ARRAY_SIZE (PlatformRepo->CpcInfo);
+
+  for (ObjIndex = 0; ObjIndex < TotalObjCount; ObjIndex++) {
+if (SearchToken == (CM_OBJECT_TOKEN)>CpcInfo[ObjIndex]) {
+  CmObject->ObjectId = CmObjectId;
+  CmObject->Size = sizeof (PlatformRepo->CpcInfo[ObjIndex]);
+  CmObject->Data = (VOID*)>CpcInfo[ObjIndex];
+  CmObject->Count = 1;
+  return EFI_SUCCESS;
+}
+  }
+
+  return EFI_SUCCESS;
+}
+
 /** Return a list of Configuration Manager object references pointed to by the
 given input token.
 
@@ -1633,6 +1685,19 @@ GetArmNameSpaceObject (
  );
   break;
 
+case EArmObjCpcInfo:
+  Status = HandleCmObjectRefByToken (
+ This,
+ CmObjectId,
+ PlatformRepo->CpcInfo,
+ sizeof (PlatformRepo->CpcInfo),
+ ARRAY_SIZE (PlatformRepo->CpcInfo),
+ Token,
+ GetCpcInfo,
+ CmObject
+ );
+  break;
+
 default: {
   Status = EFI_NOT_FOUND;
   DEBUG ((
@@ -1788,6 +1853,151 @@ EDKII_CONFIGURATION_MANAGER_PROTOCOL 
ArmJunoPlatformConfigManagerProtocol = {
   
 };
 
+/** Clear Cpc information.
+
+  If populating _CPC information fails, remove GicC tokens pointing
+  to Cpc CmObj to avoid creating corrupted _CPC objects.
+
+  @param [in] PlatformRepo   Platfom Info repository.
+
+  @retval EFI_SUCCESS   Success.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ClearCpcInfo (
+  EDKII_PLATFORM_REPOSITORY_INFO  *PlatformRepo
+  )
+{
+  CM_ARM_GICC_INFO  *GicCInfo;
+
+  GicCInfo = (CM_ARM_GICC_INFO*)>GicCInfo;
+
+  GicCInfo[0].CpcToken = CM_NULL_TOKEN;
+  GicCInfo[1].CpcToken = CM_NULL_TOKEN;
+  GicCInfo[2].CpcToken = CM_NULL_TOKEN;
+  GicCInfo[3].CpcToken = CM_NULL_TOKEN;
+  GicCInfo[4].CpcToken = CM_NULL_TOKEN;
+  GicCInfo[5].CpcToken = CM_NULL_TOKEN;
+
+  return EFI_SUCCESS;
+}
+
+/** Use the SCMI protocol to populate CPC objects dynamically.
+
+  @param [in] 

[edk2-devel] [PATCH v3 0/3] Platform/ARM: Enable _CPC/_PSD generation on Juno-r2

[PierreGondois]

v1:
- https://edk2.groups.io/g/devel/message/104127
v2:
- https://edk2.groups.io/g/devel/message/110048
- Rebase patches on latest version.
- Correct frequency values which were advertised in KHz
  when CPPC spec require them to be in MHz.
v3:
- Rebase on latest master
- Use ACPI 6.5 definitions instead of 6.4 previously
- Renamed ArmScmiInfoLib to DynamicTablesScmiInfoLib

A recent patchset [1] allows the generation of _PSD/_CPC ACPI objects by
querying the relevant information through the SCMI protocol.
- The _PSD object describes to which performance domain a CPU belongs.
  CPUs in the same performance domain have the same frequency.
- The _CPC object describes an interface to control the performance level
  of a CPU (i.e. its frequency).

Demonstrate the _PSD/_CPC generation functionality on Juno-r2 platforms.

In order to work, the SCP must describe performance levels on a unified
scale (i.e. not as frequencies). The following performance level values
where used for testing:
- little CPUs OPPs: [181, 322, 383] * 1000
- big CPUs OPPs: [512, 833, 1024] * 1024

The SCP should also:
- have fast channel enabled
- set the `approximate_level` feature to allow performance level requests
  in between OPP leveles.

[1] https://edk2.groups.io/g/devel/message/111567

Pierre Gondois (3):
  Platform/ARM: Juno: Fix typo
  Platform/ARM: Juno: Generate _PSD objects
  Platform/ARM: Juno: Generate _CPC objects for JunoR2

 .../ConfigurationManager.c| 333 +-
 .../ConfigurationManager.h|  27 +-
 .../ConfigurationManagerDxe.inf   |   1 +
 3 files changed, 348 insertions(+), 13 deletions(-)

-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111579): https://edk2.groups.io/g/devel/message/111579
Mute This Topic: https://groups.io/mt/102732063/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 2/3] Platform/ARM: Juno: Generate _PSD objects

[PierreGondois]

The SsdtCpuTopologyGenerator can generate _PSD objects.
Add _PSD information and handling to the Configuration Manager
to generate them.

Change-Id: I4bb23582d250d43c173a3e6ee86eb6b0349f1b03
Signed-off-by: Pierre Gondois 
---
 .../ConfigurationManager.c| 98 +--
 .../ConfigurationManager.h| 24 -
 2 files changed, 113 insertions(+), 9 deletions(-)

diff --git 
a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
 
b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index 2fdb15cc4caf..6a4f6e626d3f 100644
--- 
a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ 
b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -129,12 +129,12 @@ EDKII_PLATFORM_REPOSITORY_INFO 
ArmJunoPlatformRepositoryInfo = {
  GIC_ENTRY (CPUInterfaceNumber, Mpidr, PmuIrq, VGicIrq, EnergyEfficiency)
   */
   {
-GICC_ENTRY (0, GET_MPID (0, 0), 34, 25, 1),
-GICC_ENTRY (1, GET_MPID (0, 1), 38, 25, 1),
-GICC_ENTRY (2, GET_MPID (1, 0), 50, 25, 0),
-GICC_ENTRY (3, GET_MPID (1, 1), 54, 25, 0),
-GICC_ENTRY (4, GET_MPID (1, 2), 58, 25, 0),
-GICC_ENTRY (5, GET_MPID (1, 3), 62, 25, 0)
+GICC_ENTRY (0, GET_MPID (0, 0), 34, 25, 1, REFERENCE_TOKEN 
(PsdInfo[PSD_BIG_DOMAIN_ID])),
+GICC_ENTRY (1, GET_MPID (0, 1), 38, 25, 1, REFERENCE_TOKEN 
(PsdInfo[PSD_BIG_DOMAIN_ID])),
+GICC_ENTRY (2, GET_MPID (1, 0), 50, 25, 0, REFERENCE_TOKEN 
(PsdInfo[PSD_LITTLE_DOMAIN_ID])),
+GICC_ENTRY (3, GET_MPID (1, 1), 54, 25, 0, REFERENCE_TOKEN 
(PsdInfo[PSD_LITTLE_DOMAIN_ID])),
+GICC_ENTRY (4, GET_MPID (1, 2), 58, 25, 0, REFERENCE_TOKEN 
(PsdInfo[PSD_LITTLE_DOMAIN_ID])),
+GICC_ENTRY (5, GET_MPID (1, 3), 62, 25, 0, REFERENCE_TOKEN 
(PsdInfo[PSD_LITTLE_DOMAIN_ID])),
   },
 
   // GIC Distributor Info
@@ -733,7 +733,29 @@ EDKII_PLATFORM_REPOSITORY_INFO 
ArmJunoPlatformRepositoryInfo = {
   {
 { REFERENCE_TOKEN (LpiInfo[1]) },
 { REFERENCE_TOKEN (LpiInfo[2]) },
-  }
+  },
+  { // Power domains
+{ // 0: big cores
+  // Revision
+  EFI_ACPI_6_5_AML_PSD_REVISION_V0,
+  // Domain
+  PSD_BIG_DOMAIN_ID,
+  // CoordType
+  EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ANY,
+  // NumProc
+  2,
+},
+{ // 1: little cores
+  // Revision
+  EFI_ACPI_6_5_AML_PSD_REVISION_V0,
+  // Domain
+  PSD_LITTLE_DOMAIN_ID,
+  // CoordType
+  EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ANY,
+  // NumProc
+  4,
+},
+  },
 };
 
 /** A helper function for returning the Configuration Manager Objects.
@@ -1141,6 +1163,55 @@ GetPciInterruptMapInfo (
   return EFI_NOT_FOUND;
 }
 
+/** Return Psd Info.
+
+  @param [in]  This   Pointer to the Configuration Manager 
Protocol.
+  @param [in]  CmObjectId The Object ID of the CM object requested
+  @param [in]  SearchTokenA unique token for identifying the requested
+  CM_ARM_PCI_INTERRUPT_MAP_INFO object.
+  @param [in, out] CmObject   Pointer to the Configuration Manager Object
+  descriptor describing the requested Object.
+
+  @retval EFI_SUCCESS Success.
+  @retval EFI_INVALID_PARAMETER   A parameter is invalid.
+  @retval EFI_NOT_FOUND   The required object information is not found.
+**/
+EFI_STATUS
+EFIAPI
+GetPsdInfo (
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  * CONST This,
+  IN  CONST CM_OBJECT_ID  CmObjectId,
+  IN  CONST CM_OBJECT_TOKEN   SearchToken,
+  IN  OUT   CM_OBJ_DESCRIPTOR * CONST CmObject
+  )
+{
+  EDKII_PLATFORM_REPOSITORY_INFO  * PlatformRepo;
+  UINT32TotalObjCount;
+  UINT32ObjIndex;
+
+  if ((This == NULL) || (CmObject == NULL)) {
+ASSERT (This != NULL);
+ASSERT (CmObject != NULL);
+return EFI_INVALID_PARAMETER;
+  }
+
+  PlatformRepo = This->PlatRepoInfo;
+
+  TotalObjCount = ARRAY_SIZE (PlatformRepo->PsdInfo);
+
+  for (ObjIndex = 0; ObjIndex < TotalObjCount; ObjIndex++) {
+if (SearchToken == (CM_OBJECT_TOKEN)>PsdInfo[ObjIndex]) {
+  CmObject->ObjectId = CmObjectId;
+  CmObject->Size = sizeof (PlatformRepo->PsdInfo[ObjIndex]);
+  CmObject->Data = (VOID*)>PsdInfo[ObjIndex];
+  CmObject->Count = 1;
+  return EFI_SUCCESS;
+}
+  }
+
+  return EFI_SUCCESS;
+}
+
 /** Return a list of Configuration Manager object references pointed to by the
 given input token.
 
@@ -1549,6 +1620,19 @@ GetArmNameSpaceObject (
  );
   break;
 
+case EArmObjPsdInfo:
+  Status = HandleCmObjectRefByToken (
+ This,
+ CmObjectId,
+ PlatformRepo->PsdInfo,
+ sizeof (PlatformRepo->PsdInfo),
+ ARRAY_SIZE 

[edk2-devel] [PATCH v3 1/3] Platform/ARM: Juno: Fix typo

[PierreGondois]

Fix a typo.

Change-Id: I84807f18855ff4c0a2f34a3556edc4698eea2170
Signed-off-by: Pierre Gondois 
---
 .../ConfigurationManagerDxe/ConfigurationManager.c  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
 
b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
index 91f035480dcf..2fdb15cc4caf 100644
--- 
a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
+++ 
b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
@@ -993,7 +993,7 @@ GetGicCInfo (
   return EFI_NOT_FOUND;
 }
 
-/** Return Lpi State Infor.
+/** Return Lpi State Info.
 
   @param [in]  This   Pointer to the Configuration Manager 
Protocol.
   @param [in]  CmObjectId The Object ID of the CM object requested
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111580): https://edk2.groups.io/g/devel/message/111580
Mute This Topic: https://groups.io/mt/102732064/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 11/11] DynamicTablesPkg: Remove check for _CPC field

[PierreGondois]

When generating _CPC objects, some fields are mandatory by spec [1].
Some fields cannot be supported by a the Juno platform, which is used
to test the _CPC generation. Therefore, don't prevent the generation
of _CPC objects if the fields below are missing, and let the OS handle
the missing information.

_CPC fields that are exempted from checks:
- PerformanceLimitedRegister
- ReferencePerformanceCounterRegister
- DeliveredPerformanceCounterRegister

Also warn if one of the following optional fields is missing:
- HighestPerformance[Buffer|Integer]
- NominalPerformance[Buffer|Integer]

[1] Cf. non-optional fields in:
ACPI 6.5, s8.4.6.1 '_CPC (Continuous Performance Control)'

Signed-off-by: Pierre Gondois 
---
 .../Common/AmlLib/CodeGen/AmlCodeGen.c| 29 ---
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c 
b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index a96d2b8a048a..9b72c4408016 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -3531,6 +3531,21 @@ AmlCreateCpcNode (
 return EFI_INVALID_PARAMETER;
   }
 
+  // The following fields are theoretically mandatory, but not supported
+  // by some platforms. Just warn if they are missing:
+  // - PerformanceLimitedRegister
+  // - ReferencePerformanceCounterRegister
+  // - DeliveredPerformanceCounterRegister
+  if (IsNullGenericAddress (>PerformanceLimitedRegister) ||
+  IsNullGenericAddress (>ReferencePerformanceCounterRegister) ||
+  IsNullGenericAddress (>DeliveredPerformanceCounterRegister)) {
+DEBUG ((
+  DEBUG_WARN,
+  "Missing PerformanceLimited|ReferencePerformanceCounter|"
+  "DeliveredPerformanceCounter field in _CPC object\n"
+  ));
+  }
+
   if ((IsNullGenericAddress (>HighestPerformanceBuffer) &&
(CpcInfo->HighestPerformanceInteger == 0)) ||
   (IsNullGenericAddress (>NominalPerformanceBuffer) &&
@@ -3539,13 +3554,19 @@ AmlCreateCpcNode (
(CpcInfo->LowestNonlinearPerformanceInteger == 0)) ||
   (IsNullGenericAddress (>LowestPerformanceBuffer) &&
(CpcInfo->LowestPerformanceInteger == 0)) ||
-  IsNullGenericAddress (>DesiredPerformanceRegister) ||
-  IsNullGenericAddress (>ReferencePerformanceCounterRegister) ||
-  IsNullGenericAddress (>DeliveredPerformanceCounterRegister) ||
-  IsNullGenericAddress (>PerformanceLimitedRegister))
+  IsNullGenericAddress (>DesiredPerformanceRegister))
   {
 ASSERT (0);
 return EFI_INVALID_PARAMETER;
+  } else if ((IsNullGenericAddress (>HighestPerformanceBuffer) &&
+  (CpcInfo->HighestPerformanceInteger == 0)) ||
+ (IsNullGenericAddress (>NominalPerformanceBuffer) &&
+  (CpcInfo->NominalPerformanceInteger == 0)))
+  {
+DEBUG ((
+  DEBUG_WARN,
+  "Missing Reference|Delivered performance field in _CPC object\n"
+  ));
   }
 
   CpcPackage = NULL;
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111578): https://edk2.groups.io/g/devel/message/111578
Mute This Topic: https://groups.io/mt/102732035/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 07/11] DynamicTablesPkg: Add PsdToken field to CM_ARM_GICC_INFO object

[PierreGondois]

The _PSD object (cf. ACPI 6.5, s8.4.5.5 _PSD (P-State Dependency)
allows to describe CPU's power state dependencies. Add a PsdToken
field to the CM_ARM_GICC_INFO object so that interdependent CPUs
can reference the same CM_ARM_PSD_INFO object.

Signed-off-by: Pierre Gondois 
---
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h   | 5 +
 .../Common/TableHelperLib/ConfigurationManagerObjectParser.c | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h 
b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 3748cb7c8085..3805c5824773 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -218,6 +218,11 @@ typedef struct CmArmGicCInfo {
   i.e. a token referencing a CM_ARM_ET_INFO object.
   */
   CM_OBJECT_TOKENEtToken;
+
+  /** Optional field: Reference Token for the Psd info of this processor.
+  i.e. a token referencing a CM_ARM_PSD_INFO object.
+  */
+  CM_OBJECT_TOKENPsdToken;
 } CM_ARM_GICC_INFO;
 
 /** A structure that describes the
diff --git 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
index cceff56756fc..178e1821393d 100644
--- 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
+++ 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
@@ -85,7 +85,8 @@ STATIC CONST CM_OBJ_PARSER  CmArmGicCInfoParser[] = {
   { "AffinityFlags", 4,"0x%x",   NULL 
},
   { "CpcToken",  sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL 
},
   { "TRBEInterrupt", 2,"0x%x",   NULL 
},
-  { "EtToken",   sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL }
+  { "EtToken",   sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL 
},
+  { "PsdToken",  sizeof (CM_OBJECT_TOKEN), "0x%p",   NULL 
},
 };
 
 /** A parser for EArmObjGicDInfo.
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111574): https://edk2.groups.io/g/devel/message/111574
Mute This Topic: https://groups.io/mt/102732026/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 10/11] DynamicTablesPkg: Add DynamicTablesScmiInfoLib

[PierreGondois]

The SCP holds some power information that could be advertised
through the _CPC object. The communication with the SCP is done
through SCMI protocols (c.f. ArmScmiDxe).

Use the SCMI protocols to query information and feed it to
the DynamicTablesPkg.

Signed-off-by: Pierre Gondois 
---
 DynamicTablesPkg/DynamicTables.dsc.inc|   1 +
 DynamicTablesPkg/DynamicTablesPkg.dec |   3 +
 DynamicTablesPkg/DynamicTablesPkg.dsc |   1 +
 .../Library/DynamicTablesScmiInfoLib.h|  33 ++
 .../DynamicTablesScmiInfoLib.c| 297 ++
 .../DynamicTablesScmiInfoLib.inf  |  31 ++
 6 files changed, 366 insertions(+)
 create mode 100644 DynamicTablesPkg/Include/Library/DynamicTablesScmiInfoLib.h
 create mode 100644 
DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.c
 create mode 100644 
DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.inf

diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc 
b/DynamicTablesPkg/DynamicTables.dsc.inc
index 9d4312c4e87d..41947012b92b 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -15,6 +15,7 @@ [BuildOptions]
 [LibraryClasses.common]
   AcpiHelperLib|DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
   AmlLib|DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
+  
DynamicTablesScmiInfoLib|DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.inf
   
SsdtPcieSupportLib|DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
   
SsdtSerialPortFixupLib|DynamicTablesPkg/Library/Common/SsdtSerialPortFixupLib/SsdtSerialPortFixupLib.inf
   
TableHelperLib|DynamicTablesPkg/Library/Common/TableHelperLib/TableHelperLib.inf
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dec 
b/DynamicTablesPkg/DynamicTablesPkg.dec
index cfbcbb9569f1..aa422ce9f6fd 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dec
+++ b/DynamicTablesPkg/DynamicTablesPkg.dec
@@ -42,6 +42,9 @@ [LibraryClasses]
   ##  @libraryclass  Defines a set of SMBIOS string helper methods.
   SmbiosStringTableLib|Include/Library/SmbiosStringTableLib.h
 
+  ##  @libraryclass  Defines a set of APIs to populate CmObj using SCMI.
+  DynamicTablesScmiInfoLib|Include/Library/DynamicTablesScmiInfoLib.h
+
 [Protocols]
   # Configuration Manager Protocol GUID
   gEdkiiConfigurationManagerProtocolGuid = { 0xd85a4835, 0x5a82, 0x4894, { 
0xac, 0x2, 0x70, 0x6f, 0x43, 0xd5, 0x97, 0x8e } }
diff --git a/DynamicTablesPkg/DynamicTablesPkg.dsc 
b/DynamicTablesPkg/DynamicTablesPkg.dsc
index bd5084a9008f..853ea2a1e4c2 100644
--- a/DynamicTablesPkg/DynamicTablesPkg.dsc
+++ b/DynamicTablesPkg/DynamicTablesPkg.dsc
@@ -39,6 +39,7 @@ [LibraryClasses.ARM, LibraryClasses.AARCH64]
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
 
 [Components.common]
+  
DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.inf
   DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
   DynamicTablesPkg/Library/Common/AmlLib/AmlLib.inf
   DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.inf
diff --git a/DynamicTablesPkg/Include/Library/DynamicTablesScmiInfoLib.h 
b/DynamicTablesPkg/Include/Library/DynamicTablesScmiInfoLib.h
new file mode 100644
index ..ff6b47d51fe8
--- /dev/null
+++ b/DynamicTablesPkg/Include/Library/DynamicTablesScmiInfoLib.h
@@ -0,0 +1,33 @@
+/** @file
+  Arm SCMI Info Library.
+
+  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef ARM_SCMI_INFO_LIB_H_
+#define ARM_SCMI_INFO_LIB_H_
+
+#include 
+
+/** Populate a AML_CPC_INFO object based on SCMI information.
+
+  @param[in]  DomainIdIdentifier for the performance domain.
+  @param[out] CpcInfo If success, this structure was populated from
+  information queried to the SCP.
+
+  @retval EFI_SUCCESS Success.
+  @retval EFI_DEVICE_ERRORDevice error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT Time out.
+  @retval EFI_UNSUPPORTED Unsupported.
+**/
+EFI_STATUS
+EFIAPI
+DynamicTablesScmiInfoGetFastChannel (
+  IN  UINT32DomainId,
+  OUT AML_CPC_INFO  *CpcInfo
+  );
+
+#endif // ARM_SCMI_INFO_LIB_H_
diff --git 
a/DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.c 
b/DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.c
new file mode 100644
index ..b59da3d6bb51
--- /dev/null
+++ 
b/DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.c
@@ -0,0 +1,297 @@
+/** @file
+  Arm SCMI Info Library.
+
+  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
+
+  Arm Functional Fixed Hardware Specification:
+  - https://developer.arm.com/documentation/den0048/latest/
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include 
+#include 
+#include 

[edk2-devel] [PATCH v3 09/11] DynamicTablesPkg: Generate _PSD in SsdtCpuTopologyGenerator

[PierreGondois]

Make use of the newly added AmlCreatePsdNode() to generate
_PSD objects.

_PSD objects allow to describe 'performance control, P-state
or CPPC, logical processor dependency', Cf. ACPI 6.4,
s8.4.5.5 _PSD (P-State Dependency).

Signed-off-by: Pierre Gondois 
---
 .../SsdtCpuTopologyGenerator.c| 96 +++
 1 file changed, 96 insertions(+)

diff --git 
a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
 
b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
index 8228c7845ac0..ebcf9fb4e482 100644
--- 
a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
+++ 
b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c
@@ -42,6 +42,7 @@ Requirements:
   - EArmObjCmRef (OPTIONAL)
   - EArmObjLpiInfo (OPTIONAL)
   - GetEArmObjEtInfo (OPTIONAL)
+  - EArmObjPsdInfo (OPTIONAL)
 */
 
 /** This macro expands to a function that retrieves the GIC
@@ -103,6 +104,16 @@ GET_OBJECT_LIST (
   CM_ARM_ET_INFO
   );
 
+/**
+  This macro expands to a function that retrieves the PSD
+  information from the Configuration Manager.
+*/
+GET_OBJECT_LIST (
+  EObjNameSpaceArm,
+  EArmObjPsdInfo,
+  CM_ARM_PSD_INFO
+  );
+
 /** Initialize the TokenTable.
 
   One entry should be allocated for each CM_ARM_PROC_HIERARCHY_INFO
@@ -256,6 +267,75 @@ WriteAslName (
   return EFI_SUCCESS;
 }
 
+/** Create and add an _PSD Node to Cpu Node.
+
+  For instance, transform an AML node from:
+  Device (C002)
+  {
+  Name (_UID, 2)
+  Name (_HID, "ACPI0007")
+  }
+
+  To:
+  Device (C002)
+  {
+  Name (_UID, 2)
+  Name (_HID, "ACPI0007")
+  Name (_PSD, Package()
+  {
+NumEntries,  // Integer
+Revision,// Integer
+Domain,  // Integer
+CoordType,   // Integer
+NumProcessors,   // Integer
+  })
+  }
+
+  @param [in]  Generator  The SSDT Cpu Topology generator.
+  @param [in]  CfgMgrProtocol Pointer to the Configuration Manager
+  Protocol Interface.
+  @param [in]  GicCInfo   Pointer to the CM_ARM_GICC_INFO object
+  describing the Cpu.
+  @param [in]  Node   CPU Node to which the _CPC node is
+  attached.
+
+  @retval EFI_SUCCESS The function completed successfully.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+CreateAmlPsdNode (
+  IN  ACPI_CPU_TOPOLOGY_GENERATOR *Generator,
+  IN  CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
+  IN  CM_ARM_GICC_INFO*GicCInfo,
+  IN  AML_OBJECT_NODE_HANDLE  *Node
+  )
+{
+  EFI_STATUS   Status;
+  CM_ARM_PSD_INFO  *PsdInfo;
+
+  Status = GetEArmObjPsdInfo (
+ CfgMgrProtocol,
+ GicCInfo->PsdToken,
+ ,
+ NULL
+ );
+  if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+return Status;
+  }
+
+  Status = AmlCreatePsdNode (
+ PsdInfo,
+ Node,
+ NULL
+ );
+  ASSERT_EFI_ERROR (Status);
+  return Status;
+}
+
 /** Create and add an _CPC Node to Cpu Node.
 
   For instance, transform an AML node from:
@@ -839,6 +919,14 @@ CreateAmlCpuFromProcHierarchy (
 }
   }
 
+  if (GicCInfo->PsdToken != CM_NULL_TOKEN) {
+Status = CreateAmlPsdNode (Generator, CfgMgrProtocol, GicCInfo, CpuNode);
+if (EFI_ERROR (Status)) {
+  ASSERT_EFI_ERROR (Status);
+  return Status;
+}
+  }
+
   // If a CPC info is associated with the
   // GicCinfo, create an _CPC method returning them.
   if (GicCInfo->CpcToken != CM_NULL_TOKEN) {
@@ -1296,6 +1384,14 @@ CreateTopologyFromGicC (
   break;
 }
 
+if (GicCInfo->PsdToken != CM_NULL_TOKEN) {
+  Status = CreateAmlPsdNode (Generator, CfgMgrProtocol, GicCInfo, CpuNode);
+  if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+return Status;
+  }
+}
+
 // If a CPC info is associated with the
 // GicCinfo, create an _CPC method returning them.
 if (GicCInfo[Index].CpcToken != CM_NULL_TOKEN) {
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111576): https://edk2.groups.io/g/devel/message/111576
Mute This Topic: https://groups.io/mt/102732032/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 08/11] DynamicTablesPkg: Add AmlCreatePsdNode() to generate _PSD

[PierreGondois]

Add AmlCreatePsdNode() to the AmlLib to generate _PSD objects.
_PSD objects allow to describe 'performance control, P-state
or CPPC, logical processor dependency', Cf. ACPI 6.5,
s8.4.5.5 _PSD (P-State Dependency).

Signed-off-by: Pierre Gondois 
---
 .../Include/Library/AmlLib/AmlLib.h   |  35 +++-
 .../Common/AmlLib/CodeGen/AmlCodeGen.c| 188 +-
 2 files changed, 221 insertions(+), 2 deletions(-)

diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h 
b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index be78c00b6109..fb1b0e24faf3 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -1,7 +1,7 @@
 /** @file
   AML Lib.
 
-  Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.
+  Copyright (c) 2019 - 2023, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -1628,4 +1628,37 @@ AmlAddNameStringToNamedPackage (
   IN AML_OBJECT_NODE_HANDLE  NamedNode
   );
 
+/** Create a _PSD node.
+
+  Creates and optionally adds the following node
+   Name(_PSD, Package()
+   {
+NumEntries,  // Integer
+Revision,// Integer
+Domain,  // Integer
+CoordType,   // Integer
+NumProc, // Integer
+  })
+
+  Cf. ACPI 6.5, s8.4.5.5 _PSD (P-State Dependency)
+
+  @ingroup CodeGenApis
+
+  @param [in]  PsdInfo  PsdInfo object
+  @param [in]  ParentNode   If provided, set ParentNode as the parent
+of the node created.
+  @param [out] NewPsdNode   If success and provided, contains the created node.
+
+  @retval EFI_SUCCESS The function completed successfully.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCreatePsdNode (
+  IN  AML_PSD_INFO*PsdInfo,
+  IN  AML_NODE_HANDLE ParentNodeOPTIONAL,
+  OUT AML_OBJECT_NODE_HANDLE  *NewPsdNode   OPTIONAL
+  );
+
 #endif // AML_LIB_H_
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c 
b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 89ca2c8a92e7..a96d2b8a048a 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -1,7 +1,7 @@
 /** @file
   AML Code Generation.
 
-  Copyright (c) 2020 - 2022, Arm Limited. All rights reserved.
+  Copyright (c) 2020 - 2023, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -3849,3 +3849,189 @@ exit_handler:
 
   return Status;
 }
+
+/** Create a _PSD node.
+
+  Creates and optionally adds the following node
+   Name(_PSD, Package()
+   {
+NumEntries,  // Integer
+Revision,// Integer
+Domain,  // Integer
+CoordType,   // Integer
+NumProc, // Integer
+  })
+
+  Cf. ACPI 6.5, s8.4.5.5 _PSD (P-State Dependency)
+
+  @ingroup CodeGenApis
+
+  @param [in]  PsdInfo  PsdInfo object
+  @param [in]  ParentNode   If provided, set ParentNode as the parent
+of the node created.
+  @param [out] NewPsdNode   If success and provided, contains the created node.
+
+  @retval EFI_SUCCESS The function completed successfully.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCESFailed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCreatePsdNode (
+  IN  AML_PSD_INFO*PsdInfo,
+  IN  AML_NODE_HANDLE ParentNodeOPTIONAL,
+  OUT AML_OBJECT_NODE_HANDLE  *NewPsdNode   OPTIONAL
+  )
+{
+  EFI_STATUS  Status;
+  AML_OBJECT_NODE_HANDLE  PsdNode;
+  AML_OBJECT_NODE_HANDLE  PsdPackage;
+  AML_OBJECT_NODE_HANDLE  IntegerNode;
+  UINT32  NumberOfEntries;
+
+  if ((PsdInfo == NULL) ||
+  ((ParentNode == NULL) && (NewPsdNode == NULL)))
+  {
+Status = EFI_INVALID_PARAMETER;
+ASSERT_EFI_ERROR (Status);
+return Status;
+  }
+
+  // Revision 3 per ACPI 6.5 specification
+  if (PsdInfo->Revision == EFI_ACPI_6_5_AML_PSD_REVISION_V0) {
+// NumEntries 5 per ACPI 6.5 specification
+NumberOfEntries = 5;
+  } else {
+Status = EFI_INVALID_PARAMETER;
+ASSERT_EFI_ERROR (Status);
+return Status;
+  }
+
+  if (((PsdInfo->CoordType != EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ALL) &&
+   (PsdInfo->CoordType != EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ANY) &&
+   (PsdInfo->CoordType != EFI_ACPI_6_5_AML_STATE_COORD_TYPE_HW_ALL)) ||
+  (PsdInfo->NumProc == 0))
+  {
+Status = EFI_INVALID_PARAMETER;
+ASSERT_EFI_ERROR (Status);
+return Status;
+  }
+
+  Status = AmlCodeGenNamePackage ("_PSD", NULL, );
+  if (EFI_ERROR (Status)) {
+ASSERT_EFI_ERROR (Status);
+return Status;
+  }
+
+  // Get the Package object node of the _PSD node,
+  // which is the 2nd fixed argument (i.e. index 1).
+  PsdPackage = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
+ 

[edk2-devel] [PATCH v3 06/11] DynamicTablesPkg: Add CM_ARM_PSD_INFO object

[PierreGondois]

Add an object describing _PSD information, cf. ACPI 6.5,
s8.4.5.5 _PSD (P-State Dependency).
Also add the corresponding CmObjParser.

Signed-off-by: Pierre Gondois 
---
 DynamicTablesPkg/Include/AcpiObjects.h| 20 +++
 .../Include/ArmNameSpaceObjects.h | 10 ++
 .../ConfigurationManagerObjectParser.c| 11 ++
 3 files changed, 41 insertions(+)

diff --git a/DynamicTablesPkg/Include/AcpiObjects.h 
b/DynamicTablesPkg/Include/AcpiObjects.h
index 8981c229544a..2bdabb2724bd 100644
--- a/DynamicTablesPkg/Include/AcpiObjects.h
+++ b/DynamicTablesPkg/Include/AcpiObjects.h
@@ -1,6 +1,7 @@
 /** @file
 
   Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -119,6 +120,25 @@ typedef struct AmlCpcInfo {
   UINT32NominalFrequencyInteger;
 } AML_CPC_INFO;
 
+/** A structure that describes a
+P-State Dependency (PSD) Info.
+
+  Cf. ACPI 6.5, s8.4.5.5 _PSD (P-State Dependency).
+*/
+typedef struct AmlPsdInfo {
+  /// Revision.
+  UINT8 Revision;
+
+  /// Domain Id.
+  UINT32Domain;
+
+  /// Coordination type.
+  UINT32CoordType;
+
+  /// Number of processors belonging to the Domain.
+  UINT32NumProc;
+} AML_PSD_INFO;
+
 #pragma pack()
 
 #endif //AML_CPC_INFO_H_
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h 
b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 453cc3fe83fa..3748cb7c8085 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -72,6 +72,7 @@ typedef enum ArmObjectID {
   EArmObjPccSubspaceType4Info, ///< 47 - Pcc 
Subspace Type 4 Info
   EArmObjPccSubspaceType5Info, ///< 48 - Pcc 
Subspace Type 5 Info
   EArmObjEtInfo,   ///< 49 - 
Embedded Trace Extension/Module Info
+  EArmObjPsdInfo,  ///< 50 - 
P-State Dependency (PSD) Info
   EArmObjMax
 } EARM_OBJECT_ID;
 
@@ -1327,6 +1328,15 @@ typedef struct CmArmEtInfo {
   ARM_ET_TYPEEtType;
 } CM_ARM_ET_INFO;
 
+/** A structure that describes a
+P-State Dependency (PSD) Info.
+
+Cf. ACPI 6.5, s8.4.5.5 _PSD (P-State Dependency).
+
+ID: EArmObjPsdInfo
+*/
+typedef AML_PSD_INFO CM_ARM_PSD_INFO;
+
 #pragma pack()
 
 #endif // ARM_NAMESPACE_OBJECTS_H_
diff --git 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
index ce494816ed88..cceff56756fc 100644
--- 
a/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
+++ 
b/DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
@@ -669,6 +669,15 @@ STATIC CONST CM_OBJ_PARSER  CmArmEtInfo[] = {
   { "EtType", sizeof (ARM_ET_TYPE), "0x%x", NULL }
 };
 
+/** A parser for EArmObjPsdInfo.
+*/
+STATIC CONST CM_OBJ_PARSER  CmArmPsdInfoParser[] = {
+  { "Revision",  1, "0x%llx", NULL },
+  { "DomainId",  4, "0x%x",   NULL },
+  { "CoordType", 4, "0x%x",   NULL },
+  { "NumProc",   4, "0x%x",   NULL },
+};
+
 /** A parser for Arm namespace objects.
 */
 STATIC CONST CM_OBJ_PARSER_ARRAY  ArmNamespaceObjectParser[] = {
@@ -767,6 +776,8 @@ STATIC CONST CM_OBJ_PARSER_ARRAY  
ArmNamespaceObjectParser[] = {
 ARRAY_SIZE (CmArmPccSubspaceType5InfoParser) },
   { "EArmObjEtInfo",   CmArmEtInfo,
 ARRAY_SIZE (CmArmEtInfo) },
+  { "EArmObjPsdInfo",  CmArmPsdInfoParser,
+ARRAY_SIZE (CmArmPsdInfoParser) },
   { "EArmObjMax",  NULL,   
   0},
 };
 
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111573): https://edk2.groups.io/g/devel/message/111573
Mute This Topic: https://groups.io/mt/102732024/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 05/11] DynamicTablesPkg: Rename AmlCpcInfo.h to AcpiObjects.h

[PierreGondois]

The DynamicTables framework uses the AmlLib to generate some
Aml objects. It is done by using structured known by both
frameworks, e.g. the AML_CPC_INFO/CM_ARM_CPC_INFO structures.

To prepare adding similar structures (e.g. representing _PSD
information), rename AmlCpcInfo.h to AcpiObjects.h. This new
file will contain all the structures used by the AmlLib and
the DynamicTables framework.

Signed-off-by: Pierre Gondois 
---
 DynamicTablesPkg/Include/{AmlCpcInfo.h => AcpiObjects.h}| 0
 DynamicTablesPkg/Include/ArmNameSpaceObjects.h  | 2 +-
 DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h| 2 +-
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 2 +-
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename DynamicTablesPkg/Include/{AmlCpcInfo.h => AcpiObjects.h} (100%)

diff --git a/DynamicTablesPkg/Include/AmlCpcInfo.h 
b/DynamicTablesPkg/Include/AcpiObjects.h
similarity index 100%
rename from DynamicTablesPkg/Include/AmlCpcInfo.h
rename to DynamicTablesPkg/Include/AcpiObjects.h
diff --git a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h 
b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
index 8c00bdac20bb..453cc3fe83fa 100644
--- a/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
+++ b/DynamicTablesPkg/Include/ArmNameSpaceObjects.h
@@ -13,7 +13,7 @@
 #ifndef ARM_NAMESPACE_OBJECTS_H_
 #define ARM_NAMESPACE_OBJECTS_H_
 
-#include 
+#include 
 #include 
 
 #pragma pack(1)
diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h 
b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 71e8539b306c..be78c00b6109 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -36,7 +36,7 @@
   @}
 */
 
-#include 
+#include 
 #include 
 
 #ifndef AML_HANDLE
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c 
b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 1ff8430c29c3..89ca2c8a92e7 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -11,7 +11,7 @@
 #include 
 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111572): https://edk2.groups.io/g/devel/message/111572
Mute This Topic: https://groups.io/mt/102732022/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 04/11] DynamicTablesPkg: Use new CPC revision macro

[PierreGondois]

Make use of the newly added CPC revision macro.

Signed-off-by: Pierre Gondois 
---
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c 
b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index a6db34fb970f..1ff8430c29c3 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -3523,7 +3523,7 @@ AmlCreateCpcNode (
   }
 
   // Revision 3 per ACPI 6.4 specification
-  if (CpcInfo->Revision == 3) {
+  if (CpcInfo->Revision == EFI_ACPI_6_5_AML_CPC_REVISION_V3) {
 // NumEntries 23 per ACPI 6.4 specification
 NumberOfEntries = 23;
   } else {
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111571): https://edk2.groups.io/g/devel/message/111571
Mute This Topic: https://groups.io/mt/102732020/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 03/11] MdePkg/Acpi65: Add _PSD/_CPC/State Coordination Types macros

[PierreGondois]

Add macros for:
- _PSD version
- _CPC version
- C-state/T-state/P-state Coordination Types

These objects were present in previous ACPI specification version,
but are only added to the latest availbable version (6.5).

Signed-off-by: Pierre Gondois 
---
 MdePkg/Include/IndustryStandard/Acpi65.h | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/Acpi65.h 
b/MdePkg/Include/IndustryStandard/Acpi65.h
index d79d0b2b5f9c..ec7c34e8e190 100644
--- a/MdePkg/Include/IndustryStandard/Acpi65.h
+++ b/MdePkg/Include/IndustryStandard/Acpi65.h
@@ -18,6 +18,29 @@
 //
 #pragma pack(1)
 
+///
+/// C-state/T-state/P-state Coordination Types
+/// Cf. s8.3 Power, Performance, and Throttling State Dependencies
+///
+#define EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ALL  0xFC
+#define EFI_ACPI_6_5_AML_STATE_COORD_TYPE_SW_ANY  0xFD
+#define EFI_ACPI_6_5_AML_STATE_COORD_TYPE_HW_ALL  0xFE
+
+///
+/// _PSD Revision
+/// Cf. s8.4.5.5 _PSD (P-State Dependency)
+///
+#define EFI_ACPI_6_5_AML_PSD_REVISION_V0  0
+
+///
+/// _CPC Revision
+/// Cf. s8.4.6.1 _CPC (Continuous Performance Control)
+///
+#define EFI_ACPI_6_5_AML_CPC_REVISION_V0  0
+#define EFI_ACPI_6_5_AML_CPC_REVISION_V1  1
+#define EFI_ACPI_6_5_AML_CPC_REVISION_V2  2
+#define EFI_ACPI_6_5_AML_CPC_REVISION_V3  3
+
 ///
 /// ACPI 6.5 Generic Address Space definition
 ///
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111570): https://edk2.groups.io/g/devel/message/111570
Mute This Topic: https://groups.io/mt/102732018/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support

[PierreGondois]

The PERFORMANCE_DESCRIBE_FASTCHANNEL Scmi command is available
since SCMI v2.0 and allows to query information about the supported
fast-channels of the Scmi performance protocol.
Add support for this command.

Also move SCMI_MESSAGE_ID_PERFORMANCE enum definition up in the file
to use it in SCMI_PERFORMANCE_DESCRIBE_FASTCHANNEL function
declaration.

Reviewed-by: Leif Lindholm 
Signed-off-by: Pierre Gondois 
---
 .../ArmScmiDxe/ScmiPerformanceProtocol.c  | 80 +++--
 .../Protocol/ArmScmiPerformanceProtocol.h | 88 +--
 2 files changed, 154 insertions(+), 14 deletions(-)

diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c 
b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
index 0f89808fbdf9..91efce4bf22d 100644
--- a/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
+++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiPerformanceProtocol.c
@@ -1,12 +1,12 @@
 /** @file
 
-  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
+  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
-  System Control and Management Interface V1.0
-http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
-DEN0056A_System_Control_and_Management_Interface.pdf
+  System Control and Management Interface V3.2, latest version at:
+  - https://developer.arm.com/documentation/den0056/latest/
+
 **/
 
 #include 
@@ -416,6 +416,75 @@ PerformanceLevelGet (
   return EFI_SUCCESS;
 }
 
+/** Discover the attributes of the FastChannel for the specified
+performance domain and the specified message.
+
+  @param[in]  ThisA Pointer to SCMI_PERFORMANCE_PROTOCOL Instance.
+  @param[in]  DomainIdIdentifier for the performance domain.
+  @param[in]  MessageId   Message Id of the FastChannel to discover.
+  Must be one of:
+   - PERFORMANCE_LIMITS_SET
+   - PERFORMANCE_LIMITS_GET
+   - PERFORMANCE_LEVEL_SET
+   - PERFORMANCE_LEVEL_GET
+  @param[out] FastChannel If success, contains the FastChannel description.
+
+  @retval EFI_SUCCESS Performance level got successfully.
+  @retval EFI_DEVICE_ERRORSCP returns an SCMI error.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_TIMEOUT Time out.
+  @retval EFI_UNSUPPORTED Unsupported.
+**/
+EFI_STATUS
+DescribeFastchannel (
+  IN  SCMI_PERFORMANCE_PROTOCOL *This,
+  IN  UINT32DomainId,
+  IN  SCMI_MESSAGE_ID_PERFORMANCE   MessageId,
+  OUT SCMI_PERFORMANCE_FASTCHANNEL  *FastChannel
+  )
+{
+  EFI_STATUSStatus;
+  SCMI_COMMAND  Cmd;
+  UINT32PayloadLength;
+  UINT32*ReturnValues;
+  UINT32*MessageParams;
+
+  if ((This == NULL)  ||
+  (FastChannel == NULL))
+  {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Status = ScmiCommandGetPayload ();
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  *MessageParams++ = DomainId;
+  *MessageParams   = MessageId;
+
+  Cmd.ProtocolId = ScmiProtocolIdPerformance;
+  Cmd.MessageId  = ScmiMessageIdPerformanceDescribeFastchannel;
+  PayloadLength  = sizeof (DomainId) + sizeof (MessageId);
+
+  Status = ScmiCommandExecute (
+ ,
+ ,
+ 
+ );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  CopyMem (
+FastChannel,
+ReturnValues,
+sizeof (SCMI_PERFORMANCE_FASTCHANNEL)
+);
+
+  return Status;
+}
+
 // Instance of the SCMI performance management protocol.
 STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol = {
   PerformanceGetVersion,
@@ -425,7 +494,8 @@ STATIC CONST SCMI_PERFORMANCE_PROTOCOL  PerformanceProtocol 
= {
   PerformanceLimitsSet,
   PerformanceLimitsGet,
   PerformanceLevelSet,
-  PerformanceLevelGet
+  PerformanceLevelGet,
+  DescribeFastchannel,
 };
 
 /** Initialize performance management protocol and install on a given Handle.
diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h 
b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
index a28f0f766e37..c69efe7408e1 100644
--- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
+++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
@@ -79,8 +79,58 @@ typedef struct {
   UINT32RangeMin;
 } SCMI_PERFORMANCE_LIMITS;
 
+/// Doorbell Support bit.
+#define SCMI_PERF_FC_ATTRIB_HAS_DOORBELL  BIT0
+
+/// Performance protocol describe fastchannel
+typedef struct {
+  /// Attributes.
+  UINT32Attributes;
+
+  /// Rate limit.
+  UINT32RateLimit;
+
+  /// Lower 32 bits of the FastChannel address.
+  UINT32ChanAddrLow;
+
+  /// Higher 32 bits of the FastChannel address.
+  UINT32ChanAddrHigh;
+
+  /// Size of the FastChannel in bytes.
+  UINT32ChanSize;
+
+  /// Lower 32 bits of the doorbell address.
+  UINT32DoorbellAddrLow;
+
+  /// Higher 32 bits of the doorbell address.
+  UINT32DoorbellAddrHigh;
+
+  /// Mask of lower 32 

[edk2-devel] [PATCH v3 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION

[PierreGondois]

Rename PERFORMANCE_PROTOCOL_VERSION to reflect the different
versions of the protocol. The macro is neither used in edk2 nor
in edk2-platforms.

Signed-off-by: Pierre Gondois 
---
 .../Include/Protocol/ArmScmiPerformanceProtocol.h   | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h 
b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
index 7e548e4765c2..a28f0f766e37 100644
--- a/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
+++ b/ArmPkg/Include/Protocol/ArmScmiPerformanceProtocol.h
@@ -1,12 +1,12 @@
 /** @file
 
-  Copyright (c) 2017-2021, Arm Limited. All rights reserved.
+  Copyright (c) 2017-2023, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
-  System Control and Management Interface V1.0
-http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/
-DEN0056A_System_Control_and_Management_Interface.pdf
+  System Control and Management Interface V3.2, latest version at:
+  - https://developer.arm.com/documentation/den0056/latest/
+
 **/
 
 #ifndef ARM_SCMI_PERFORMANCE_PROTOCOL_H_
@@ -14,7 +14,10 @@
 
 #include 
 
-#define PERFORMANCE_PROTOCOL_VERSION  0x1
+/// Arm Scmi performance protocol versions.
+#define PERFORMANCE_PROTOCOL_VERSION_V1  0x1
+#define PERFORMANCE_PROTOCOL_VERSION_V2  0x2
+#define PERFORMANCE_PROTOCOL_VERSION_V3  0x3
 
 #define ARM_SCMI_PERFORMANCE_PROTOCOL_GUID  { \
   0x9b8ba84, 0x3dd3, 0x49a6, {0xa0, 0x5a, 0x31, 0x34, 0xa5, 0xf0, 0x7b, 0xad} \
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111568): https://edk2.groups.io/g/devel/message/111568
Mute This Topic: https://groups.io/mt/102732016/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 00/11] DynamicTablesPkg: Enable _PSD/_CPC generation using SCMI

[PierreGondois]

v1:
- https://edk2.groups.io/g/devel/message/104115
v2:
- https://edk2.groups.io/g/devel/message/104115
- Rebase patches on latest version.
v3:
[PATCH 01/11] ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION
- Note the referenced spec spec is SCMI v3.2
[PATCH 02/11] ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support
- Note the referenced spec spec is SCMI v3.2
- Move SCMI_MESSAGE_ID_PERFORMANCE enum definition out of #pragma pack(1)
[PATCH 03/11] MdePkg/Acpi65: Add _PSD/_CPC/State Coordination Types macros
- Macros used to be added to ACPI 6.4 definition file, move them to the
  latest ACPI 6.5 definition file
[PATCH 10/11] DynamicTablesPkg: Add DynamicTablesScmiInfoLib
- Renamed ArmScmiInfoLib to DynamicTablesScmiInfoLib
[PATCH 11/11] DynamicTablesPkg: Remove check for _CPC field
- Add warning when missing one of the bypassed field
All patches:
- Rebase on latest master
- Use ACPI 6.5 definitions instead of 6.4 previously

The SCMI performance protocol allows to query the Operating Performance
Points (OPPs) available for each performance domain. Each OPP has a
specific frequency/power consumption/performance.

On Device Tree (DT) based platforms, the SCMI protocol is directly
available from the OS.
On ACPI based platforms, the _PSD objects allows to identify CPUs
belonging
to the same performance domain. CPUs belonging to the same performance
domain have the same frequency. The _CPC object allows to
describe/control the performance level/frequency of a CPU (i.e. its
frequency).

This patchset:
- Add support for 'DescribeFastchannel' SCMI performance protocol
- Add a object to the DynamicTablesPkg to describe the PSD information
- Allows to generate _PSD objects in the SsdtCpuTopologyGenerator
- Add a ArmScmiInfoLib library populating DynamicTablesPkg CPC objects,
  relying on the SCMI protocol to fetch the relevant information

A patchset for Juno-r2 platforms will be submitted to demonstrate
this functionality.

Pierre Gondois (11):
  ArmPkg/ArmScmiDxe: Rename PERFORMANCE_PROTOCOL_VERSION
  ArmPkg/ArmScmiDxe: Add PERFORMANCE_DESCRIBE_FASTCHANNEL support
  MdePkg/Acpi65: Add _PSD/_CPC/State Coordination Types macros
  DynamicTablesPkg: Use new CPC revision macro
  DynamicTablesPkg: Rename AmlCpcInfo.h to AcpiObjects.h
  DynamicTablesPkg: Add CM_ARM_PSD_INFO object
  DynamicTablesPkg: Add PsdToken field to CM_ARM_GICC_INFO object
  DynamicTablesPkg: Add AmlCreatePsdNode() to generate _PSD
  DynamicTablesPkg: Generate _PSD in SsdtCpuTopologyGenerator
  DynamicTablesPkg: Add DynamicTablesScmiInfoLib
  DynamicTablesPkg: Remove check for _CPC field

 .../ArmScmiDxe/ScmiPerformanceProtocol.c  |  80 -
 .../Protocol/ArmScmiPerformanceProtocol.h | 101 +-
 DynamicTablesPkg/DynamicTables.dsc.inc|   1 +
 DynamicTablesPkg/DynamicTablesPkg.dec |   3 +
 DynamicTablesPkg/DynamicTablesPkg.dsc |   1 +
 .../Include/{AmlCpcInfo.h => AcpiObjects.h}   |  20 ++
 .../Include/ArmNameSpaceObjects.h |  17 +-
 .../Include/Library/AmlLib/AmlLib.h   |  37 ++-
 .../Library/DynamicTablesScmiInfoLib.h|  33 ++
 .../SsdtCpuTopologyGenerator.c|  96 ++
 .../Common/AmlLib/CodeGen/AmlCodeGen.c| 221 -
 .../ConfigurationManagerObjectParser.c|  14 +-
 .../DynamicTablesScmiInfoLib.c| 297 ++
 .../DynamicTablesScmiInfoLib.inf  |  31 ++
 MdePkg/Include/IndustryStandard/Acpi65.h  |  23 ++
 15 files changed, 945 insertions(+), 30 deletions(-)
 rename DynamicTablesPkg/Include/{AmlCpcInfo.h => AcpiObjects.h} (88%)
 create mode 100644 DynamicTablesPkg/Include/Library/DynamicTablesScmiInfoLib.h
 create mode 100644 
DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.c
 create mode 100644 
DynamicTablesPkg/Library/DynamicTablesScmiInfoLib/DynamicTablesScmiInfoLib.inf

-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111567): https://edk2.groups.io/g/devel/message/111567
Mute This Topic: https://groups.io/mt/102732014/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 1/1] ArmPkg,MdePkg: move ArmLib.h to MdePkg

[Leif Lindholm]

Related to https://bugzilla.tianocore.org/show_bug.cgi?id=4121, but not
resolving it. (Nearly?) all of ArmPkg describes industry standard
behaviour, and hence according to general rules, ought to live in MdePkg.

Addressing this will however be a substantial task.
Take a first step by moving the ArmLib interface definition to MdePkg,
as discussed in
https://edk2.groups.io/g/devel/topic/patch_v5_2_6/102725178

Cc: Pierre Gondois 
Cc: Jiewen Yao 
Cc: Ard Biesheuvel 
Cc: Liming Gao 
Cc: Michael D Kinney 
Cc: Sami Mujawar 
Cc: Zhiguang Liu 
Signed-off-by: Leif Lindholm 
---

This should have no functional differences (and the set of
platforms I have test built didn't find any problems).
This may result in some modules depending on ArmPkg only
for ArmLib now being able to drop ArmPkg dependency.

 ArmPkg/ArmPkg.dec   | 4 
 MdePkg/MdePkg.dec   | 5 +
 {ArmPkg => MdePkg}/Include/Library/ArmLib.h | 0
 Maintainers.txt | 6 ++
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 7fe2b9bca43b..4ce59f3e1fbd 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -50,10 +50,6 @@ [LibraryClasses.common]
   #
   ArmHvcLib|Include/Library/ArmHvcLib.h
 
-  ##  @libraryclass  Provides an interface to Arm registers.
-  #
-  ArmLib|Include/Library/ArmLib.h
-
   ##  @libraryclass  Provides a Mmu interface.
   #
   ArmMmuLib|Include/Library/ArmMmuLib.h
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index ac54338089e8..78e18ee444cd 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -339,6 +339,11 @@ [LibraryClasses.RISCV64]
   ##  @libraryclass  Provides function to make ecalls to SBI
   BaseRiscVSbiLib|Include/Library/BaseRiscVSbiLib.h
 
+[LibraryClasses.ARM, LibraryClasses.AARCH64]
+  ##  @libraryclass  Provides an interface to Arm registers.
+  #
+  ArmLib|Include/Library/ArmLib.h
+
 [Guids]
   #
   # GUID defined in UEFI2.1/UEFI2.0/EFI1.1
diff --git a/ArmPkg/Include/Library/ArmLib.h b/MdePkg/Include/Library/ArmLib.h
similarity index 100%
rename from ArmPkg/Include/Library/ArmLib.h
rename to MdePkg/Include/Library/ArmLib.h
diff --git a/Maintainers.txt b/Maintainers.txt
index 7c0b4cb58cfd..0315fa23dfce 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -450,6 +450,12 @@ M: Abner Chang  [changab]
 R: Abdul Lateef Attar  [abdattar]
 R: Nickle Wang  [nicklela]
 
+MdePkg: ARM/AARCH64 standard interfaces
+F: MdePkg/Include/Library/ArmLib.h
+M: Leif Lindholm  [leiflindholm]
+M: Ard Biesheuvel  [ardbiesheuvel]
+R: Sami Mujawar  [samimujawar]
+
 NetworkPkg
 F: NetworkPkg/
 W: https://github.com/tianocore/tianocore.github.io/wiki/NetworkPkg
-- 
2.30.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111566): https://edk2.groups.io/g/devel/message/111566
Mute This Topic: https://groups.io/mt/102731845/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Bug] Building NetworkPkg fails due to missing SynchronizationLib dependency of BaseCryptLib

[Michael D Kinney]

Hi Sean and Michael,

The NetworkPkg.ci.yaml is pointing at a DSC file in the NetworkPkg.

If I try a local build of that DSC file, it does fail immediately with
a missing library mapping.

Any ideas on how this was not seen by CI?  Either for PR or Push actions?

Thanks,

Mike

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo
> Ersek
> Sent: Tuesday, November 21, 2023 7:19 AM
> To: Crossed Carpet ; devel@edk2.groups.io
> Cc: Gao, Liming 
> Subject: Re: [edk2-devel] [Bug] Building NetworkPkg fails due to
> missing SynchronizationLib dependency of BaseCryptLib
> 
> On 11/20/23 11:25, Crossed Carpet wrote:
> > Good morning Laszlo,
> > Thank you for your reply. I feared this was intentional due to
> believing
> > that it would have been caught with automated testing.
> > Doesn't the Azure Pipeline try to build all packages to make sure no
> > dependency broke?
> 
> Right, we have "NetworkPkg/NetworkPkg.ci.yaml". I'm unsure how this
> issue had slipped through.
> 
> Thanks
> Laszlo
> 
> >
> > Also Liming, would you do me the honour of creating a Bugzilla
> account
> > for this email?
> > Best regards,
> > CC
> > 
> 
> > *From:* Laszlo Ersek 
> > *Sent:* 17 November 2023 16:06
> > *To:* devel@edk2.groups.io ;
> > crossedcar...@hotmail.com 
> > *Cc:* Liming Gao (Byosoft address) 
> > *Subject:* Re: [edk2-devel] [Bug] Building NetworkPkg fails due to
> > missing SynchronizationLib dependency of BaseCryptLib
> >
> > On 11/17/23 13:49, CrossedCarpet wrote:
> >> Steps to reproduce:
> >> - download and setup edk2
> >> - run:
> >> build -a X64 -b DEBUG -t GCC -p NetworkPkg/NetworkPkg.dsc
> >>
> >> Get the error:
> >> build.py...
> >> /.../edk2/NetworkPkg/NetworkPkg.dsc(...): error 4000: Instance of
> >> library class [SynchronizationLib] is not found
> >> in [/.../edk2/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf]
> [X64]
> >> consumed by module [/.../edk2/NetworkPkg/TlsDxe/TlsDxe.inf]
> >>
> >> Adding this LibClass to NetworkPkg.dsc solves it:
> >>
> >>
> SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchroni
> zationLib.inf
> >
> > This is a regression from commit 5d5be45bd111 ("CryptPkg: Enable
> > CryptoPkg BaseCryptLib ParallelHash for PEI and DXE", 2022-12-02),
> which
> > was made for  > >.
> >
> > It added a new lib class dependency to "BaseCryptLib.inf", but it
> didn't
> > ensure that all DSC files in the tree that employed the
> > "BaseCryptLib.inf" instance had a resolution for the new lib class.
> >
> > Indeed it is not just NetworkPkg.dsc but also FmpDevicePkg.dsc
> that's
> > affected:
> >
> > $ git grep -l -F CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf --
> > '**dsc*' \
> >   | xargs -- grep --files-without-match -w SynchronizationLib --
> >
> > FmpDevicePkg/FmpDevicePkg.dsc
> > NetworkPkg/NetworkPkg.dsc
> >
> > It also *seems* to affect at least one platform in edk2-platforms:
> >
> > Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
> >
> > (although I realize this last DSC file is an "include" DSC, so the
> > missing dependency could be resolved in the DSC file that includes
> this
> > one.)
> >
> > Either way, thanks for catching this; the edk2 issue should be fixed
> > preferably during the current hard feature freeze (for NetworkPkg
> and
> > FmpDevicePkg).
> >
> >> I tried to open a bug in bugzilla but I wasn't able to log in or
> create
> >> an account. How should I do it next time?
> >
> > I think the bugzilla account creation was disabled due to spammer
> accounts.
> >
> > The way to request an account is described here (linked from the
> > bugzilla.tianocore.org landing page under link "Reporting issues"):
> >
> >   https://github.com/tianocore/tianocore.github.io/wiki/Reporting-
> Issues
> >  Issues>
> >
> > (I've added Liming to the CC list of this email.)
> >
> > Laszlo
> >
> >
> >>
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111565): https://edk2.groups.io/g/devel/message/111565
Mute This Topic: https://groups.io/mt/102646401/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] Fail to build boot with tools code failure

[Laszlo Ersek]

On 11/16/23 04:46, Jackie Lien (Temp) via groups.io wrote:
> Hi team,
> 
> Please help with the build error in BOOT.MXF.1.1.1. The command and log
> are listed below.
> 
>  
> 
> Sync & build command:
> 
>  1. python sync_crm.py BOOT.MXF.1.1.1-00175-Olympic-1
>  2. copy manifest to my local sync (copy
> 
> \\snowcone\builds906\PROD\BOOT.MXF.1.1.1-00175-Olympic-1\boot_images\boot_tools\manifest\build\manifest.xml
>  
> 
>  to 
> \\jacklien-linux\workspace\BOOT.MXF.1.1.1-00175-Olympic-1\boot_images\boot_tools\manifest\build\manifest.xml
>  
> )
>  3. python buildex.py -t Olympic -v LE -r DEBUG
> 
>  
> 
>  
> 
> Build log:
> build.py...
> 
> : error C0DE: Tools code failure
> 
>     Please send email to devel@edk2.groups.io
>  for help, attaching following call stack
> trace!
> 
>  
> 
> Traceback (most recent call last):
> 
>   File
> "/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/GenFds.py",
>  line 371, in GenFdsApi
> 
>     GenFds.GenFd('', FdfParserObj, BuildWorkSpace, ArchList)
> 
>   File
> "/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/GenFds.py",
>  line 510, in GenFd
> 
>     FdObj.GenFd()
> 
>   File
> "/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/Fd.py",
>  line 131, in GenFd
> 
>     RegionObj.AddToBuffer (FdBuffer, self.BaseAddress,
> self.BlockSizeList, self.ErasePolarity,
> GenFdsGlobalVariable.ImageBinDict, self.DefineVarDict, Flag=Flag)
> 
>   File
> "/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/Region.py",
>  line 134, in AddToBuffer
> 
>     FvObj.AddToBuffer(FvBuffer, FvBaseAddress, BlockSize, BlockNum,
> ErasePolarity, Flag=Flag)
> 
>   File
> "/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/Fv.py",
>  line 127, in AddToBuffer
> 
>     FileName = FfsFile.GenFfs(MacroDict, FvParentAddr=BaseAddress,
> IsMakefile=Flag, FvName=self.UiFvName)
> 
>   
> ^^
> 
>   File
> "/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/FfsInfStatement.py",
>  line 518, in GenFfs
> 
>     InputSectList, InputSectAlignments =
> self.__GenComplexFileSection__(Rule, FvChildAddr, FvParentAddr,
> IsMakefile=IsMakefile)
> 
> 
> ^^
> 
>   File
> "/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/FfsInfStatement.py",
>  line 969, in __GenComplexFileSection__
> 
>     SectList, Align = Sect.GenSection(self.OutputPath, self.ModuleGuid,
> SecIndex, self.KeyStringList, self, IsMakefile = IsMakefile)
> 
>  
> ^^
> 
>   File
> "/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/GenFds/EfiSection.py",
>  line 263, in GenSection
> 
>     ImageObj = PeImageClass (File)
> 
>    ^^^
> 
>   File
> "/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/Common/Misc.py",
>  line 1638, in __init__
> 
>     if ByteArray.tostring() != b'PE\0\0':
> 
>    ^^
> 
> AttributeError: 'array.array' object has no attribute 'tostring'
> 
>  
> 
>  
> 
>  
> 
> build.py...
> 
> : error 7000: Failed to execute command
> 
>  
> 
>  
> 
> - Failed -
> 
> Build end time: 11:15:43, Nov.16 2023
> 
> Build total time: 00:00:23
> 
>  
> 
> ERROR: buildex::Run  ['python',
> '/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2/BaseTools/Source/Python/build/build.py',
>  '-p', 'QcomPkg/SocPkg/Olympic/LE/Core.dsc', '-j', 
> 'QcomPkg/SocPkg/Olympic/LE/build_Core.log', '-w', '-a', 'ARM', '-b', 'DEBUG', 
> '-t', 'CLANG100LINUX', '-D', 
> 'BUILDROOT=/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images', 
> '-D', 
> 'BOOTROOT=/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/boot',
>  '-D', 'REL=DEBUG', '-D', 'PRODMODE=DEBUGMODE', '-D', 'VAR=LE', '-D', 
> 'COMPLIER=', '-D', 'FIRSTDLL=', '-D', 'SECONDDLL=', '-D', 'XBLSEC=', '-D', 
> 'TARGETROOT=QcomPkg/SocPkg/Olympic', '-D', 'TARGETID=SocOlympic', '-D', 
> 'TARGETPKG=SocPkg', '-D', 'QCOMDIR=', '-D', 
> 'BOOTTOOLS=/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/boot_tools',
>  '-D', 
> 'EDK2ROOT=/local/mnt/workspace/BOOT.MXF.1.1.1-00175-Olympic-1/boot_images/edk2',
>  '-D', 'SECTOOLS_DIR=/pkg/sectools/glue/latest', '-D', 
> 'SECTOOLSROOT=/pkg/sectools/glue/latest', '-D', 'COMPILER=CLANG100LINUX', 
> '-D', 'AARCH=ARM'] Build Subprocess failed with error...
> 
> None

The platform DSC 

Re: [edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

[Yao, Jiewen]

Cool, thanks for considering that!

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Wednesday, November 22, 2023 12:03 AM
> To: devel@edk2.groups.io; quic_llind...@quicinc.com
> Cc: Yao, Jiewen ; Pierre Gondois
> ; Li, Yi1 ; Lu, Xiaoyu1
> ; Jiang, Guomin ; Ard
> Biesheuvel ; Sami Mujawar
> ; Gerd Hoffmann 
> Subject: Re: [edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow
> dependency upon ArmPkg
> 
> On Tue, 21 Nov 2023 at 10:55, Leif Lindholm  wrote:
> >
> > On Tue, Nov 21, 2023 at 14:46:05 +, Yao, Jiewen wrote:
> > > This Bugzilla is filed in 2022-10-26. Now it is 2023-11-21.
> >
> > Oh, I'm sure I voiced the same opinion for many years before someone
> > (rightly) told me to go gile that bugzilla.
> >
> > > I agree with you that it is a big task. May I know what is the plan?
> > > E.g. who is working on that? When do you expect it will be done?
> >
> > On my list of "big items" to deal with, this comes after github PR
> > migration and line-ending conversion.
> >
> > > According to the dependency rule, what we need is only *interface*
> > > definition, but not *implementation*.
> > > That means the really requirement here is to move *interface* from
> > > ArmPkg to MdePkg, you can still keep the library implementation in
> > > ArmPkg. (It is just a subset of this Bugzilla)
> >
> > That ... is an option I had not previously considered.
> > Long-term we would still like to smash ArmLib into BaseLib, but if
> > MdePkg maintainers would be OK with moving ArmLib.h into MdePkg...
> >
> > > Also, I don’t think CPUID check really matters here - since it is only
> implementation.
> > > As long as, you have interface in MdePkg, then your INF can declare that
> interface.
> > > You can still put real implementation in ArmPkg - no requirement to move.
> > > That benefit is that you don’t need to add ArmPkg dependency in yaml.
> >
> > I can spin up a patch for that to get merged shortly after stable tag
> > to give plenty of time to catch any issues that may arise from moving
> > such a fundamental file. (These would likely be bugs, but
> > nevertheless...)
> >
> 
> This sounds like a reasonable solution to me for the short term.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111563): https://edk2.groups.io/g/devel/message/111563
Mute This Topic: https://groups.io/mt/102725178/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] Modified ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c to follow error handling instead of success handling

[Laszlo Ersek]

On 11/16/23 07:16, Daniel Nguyen wrote:
> Signed-off-by: Daniel Nguyen 
> ---
>  .../UefiShellLevel2CommandsLib/Reset.c| 43 +++
>  1 file changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c 
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> index 57ba3c90f3..361c47e430 100644
> --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Reset.c
> @@ -79,30 +79,35 @@ ShellCommandRunReset (
>,
>
>);
> -if (!EFI_ERROR (Status)) {
> -  if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
> -DataSize = sizeof (OsIndications);
> -Status   = gRT->GetVariable (
> -  EFI_OS_INDICATIONS_VARIABLE_NAME,
> -  ,
> -  ,
> -  ,
> -  
> -  );
> -if (!EFI_ERROR (Status)) {
> -  OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> -} else {
> -  OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> -}
> -
> -Status = gRT->SetVariable (
> +
> +if (EFI_ERROR (Status)) {
> +  ShellStatus = SHELL_UNSUPPORTED;
> +  goto Error;
> +}
> +
> +if ((OsIndications & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0) {
> +  DataSize = sizeof (OsIndications);
> +  Status   = gRT->GetVariable (
>  EFI_OS_INDICATIONS_VARIABLE_NAME,
>  ,
> -EFI_VARIABLE_NON_VOLATILE | 
> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> -sizeof (OsIndications),
> +,
> +,
>  
>  );
> +
> +  if (EFI_ERROR (Status)) {
> +OsIndications = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
> +  } else {
> +OsIndications |= EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
>}
> +
> +  Status = gRT->SetVariable (
> +  EFI_OS_INDICATIONS_VARIABLE_NAME,
> +  ,
> +  EFI_VARIABLE_NON_VOLATILE | 
> EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> +  sizeof (OsIndications),
> +  
> +  );
>  }
>  
>  if (EFI_ERROR (Status)) {

Adding Zhichao to the CC list.

>From a quick skim, the subject line and the commit message are not great
(the former is too long, the latter is empty).

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111562): https://edk2.groups.io/g/devel/message/111562
Mute This Topic: https://groups.io/mt/102698984/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 0/6] CryptoPkg: Enable Openssl native instruction support for AARCH64

[Ard Biesheuvel]

On Tue, 21 Nov 2023 at 03:48, Pierre Gondois  wrote:
>
> v5:
> - [PATCH v5 1/6] ArmPkg/ArmLib: Add macros/helper functions around AA64Isar0 
> register
>   - Remove unused ArmReadIdAA64Isar0() declaration
> - [PATCH v4 5/6] CryptoPkg/OpensslLib: Add AArch64Cap for arch specific hooks
>   - Removed unused GET_BITFIELD() macro
>   - Indentation
>

Thanks again for working on this.

The series looks good to me,

Reviewed-by: Ard Biesheuvel 

(but the MdePkg <-> ArmPkg discussion needs to conclude before this can go in)


> v4:
> - Added Acked-by from Gerd
> - Created bugzilla at:
>   https://bugzilla.tianocore.org/show_bug.cgi?id=4601
> - Replaced:
>   - [PATCH v3 1/6] ArmPkg/ArmLib: Move ArmReadIdAA64Isar0() to ArmLib
>   by:
>   - [PATCH v4 1/6] ArmPkg/ArmLib: Add macros/helper functions around 
> AA64Isar0 register
>   and:
>   - Moved ARM_ID_AA64ISAR0_EL1_* macros to ArmPkg/Include/Chipset/AArch64.h
>   - Added macros to get capabilities described in AA64ISAR0
>
> v3:
> - Use ArmPkg's function to access register:
>   - Dropped:
> - [PATCH v2 1/7] MdePkg/BaseLib: AARCH64: Add ArmReadCntPctReg()
> - [PATCH v2 2/7] MdePkg/BaseLib: AARCH64: Add ArmReadIdAA64Isar0Reg()
> - [PATCH v2 3/7] MdePkg/BaseRngLib: Prefer ArmReadIdAA64Isar0Reg() over 
> ArmReadIdIsar0()
>   - Added:
> - [PATCH v3 1/6] ArmPkg/ArmLib: Move ArmReadIdAA64Isar0() to ArmLib
> - Allow dependency of CryptoPkg over ArmPkg in CI:
>   - Added:
> - [PATCH v3 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon
>
> v2:
> - [PATCH v2 2/7] MdePkg/BaseLib: AARCH64: Add ArmReadIdAA64Isar0Reg()
>   - Correct bad mask values in MdePkg/Include/Library/BaseLib.h
> - [PATCH v2 4/7] CryptoPkg/OpensslLib: Add native instruction support:
>   - Add armcap.c to configure.py:sources_filter_fn() instead of
> manually commenting the file in .inf files
>
> Various OpensslLib implementations are available in edk2. The
> OpensslLibAccel.inf and OpensslLibFullAccel.inf ones use
> architecture specific instructions, e.g. AESE, PMULL, SHA256H, ...,
> allowing to improve speed.
>
> Enable support for Aarch64's native instructions:
> - Add ArmReadCntPctReg() and ArmReadIdAA64Isar0Reg() to
>   Aarch64's BaseLib.
> - Generate Aarch64's specific Openssl functions.
> - Add a OpensslStub/AArch64Cap.c file to allow Openssl
>   to probe Aarch64 native instruction support.
>
> This patch-set only enable support for GCC for now (MSFT support
> not added).
>
>  Testing 
>
> The tests run are based on the TestBaseCryptLibShell module.
> Each test is run 100 times, then the first 5 values
> (considered as warmup) are removed.
>
> The NoAccel column relies on the OpensslLibFull implementation,
> the Accel column relies on the OpensslLibFullAccel implementation.
> The 'Improvement' column is computed as:
>100 * ('Accel (ns)' - 'NoAccel (ns)') / 'NoAccel (ns)'
>
> The std deviation of the TestVerifyDhGenerateKey is big.
> It is due to [1] being called with the 'safe' parameter set, leading
> to the prime number taking more time to generate. It requires ~10
> iterations when safe=false, ~1000 iterations when safe=true.
>
> The test was run on a Juno-r2. The native Openssl implementation makes
> use of the following features (cf. [2]):
> - ARMV7_NEON
> - ARMV8_AES
> - ARMV8_SHA1
> - ARMV8_PMULL
> - ARMV8_SHA256
> and misses:
> - ARMV8_SHA512
>
> | TestName |   NoAccel (ns) |   NoAccel std |   Accel 
> (ns) |   Accel std |   Improvement |
> |:-|---:|--:|-:|:|--:|
> | mPkcs7EkuTest|   14757511 | 14370 | 
> 14947276 |   35677 |   1.28589 |
> | mAeadAesGcmTest  | 129667 |  2012 |   
> 113897 |1366 | -12.1619  |
> | mBlockCipherTest |   7325 |   102 | 
> 6487 |  81 | -11.4403  |
> | mAuthenticodeTest|   72852444 |   3097832 | 
> 67593102 | 3123627 |  -7.21917 |
> | mBnTest  | 771921 | 57966 |   
> 737656 |   61354 |  -4.43893 |
> | mDhTest  | 4082083501 |3340300622 |   
> 3502629757 |  3444890110 | -14.195   |
> | mEcTest  |   24666075 |191971 | 
> 23250301 |  178985 |  -5.73976 |
> | mHkdfTest| 848440 |  4295 |   
> 797966 |4320 |  -5.94904 |
> | mHmacTest| 235527 | 36284 |   
> 204823 |   37936 | -13.0363  |
> | mImageTimestampTest  |   12801070 | 18327 | 
> 12190046 |   23138 |  -4.77323 |
> | mOaepTest|   20032245 | 46525 | 
> 18671388 |   36399 |  -6.79333 |
> | mPkcs5Test   | 178624 |  1962 |   
> 

Re: [edk2-devel] BaseTools KeyError When Building EmulatorPkg

[Laszlo Ersek]

Adding Sean

Laszlo

On 11/19/23 23:43, ryderkeys via groups.io wrote:
> Hi all,
> 
> I am trying to build EDK2 from most recent Github (namely, EmulatorPkg) with 
> the new Stuart build system on Windows 10 22H2 64 bit with VS 2019. I'm 
> getting the following error which directed me to this mailing list. More 
> details on my build process below.  
> 
> INFO - build.py...
> INFO -  : error C0DE: Unknown fatal error when processing 
> [c:\users\ryder\edk2_2023\edk2\EmulatorPkg\EmulatorPkg.dsc]
> INFO - 
> INFO - (Please send email to devel@edk2.groups.io for help, attaching 
> following call stack trace!)
> INFO - 
> INFO - (Python 3.11.5 on win32) Traceback (most recent call last):
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
> 2692, in Main
> INFO - MyBuild = Build(Target, Workspace, Option,LogQ)
> INFO -   ^
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
> 815, in __init__
> INFO - self.InitPreBuild()
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
> 1015, in InitPreBuild
> INFO - self.LoadConfiguration()
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
> 971, in LoadConfiguration
> INFO - self.GetToolChainAndFamilyFromDsc (self.PlatformFile)
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\BaseTools\Source\Python\build\build.py", line 
> 905, in GetToolChainAndFamilyFromDsc
> INFO - dscobj = self.BuildDatabase[File, BuildArch]
> INFO -  ~~^
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\WorkspaceDatabase.py",
>  line 104, in __getitem__
> INFO - BuildObject = self.CreateBuildObject(FilePath, Arch, Target, 
> Toolchain)
> INFO -   
> ^
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\WorkspaceDatabase.py",
>  line 125, in CreateBuildObject
> INFO - BuildObject = self._GENERATOR_[FileType](
> INFO -   ^^^
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\DscBuildData.py",
>  line 231, in __init__
> INFO - self.SkuIdMgr = SkuClass(self.SkuName, self.SkuIds)
> INFO -  
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\DscBuildData.py",
>  line 510, in SkuName
> INFO - self._GetHeaderInfo()
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\DscBuildData.py",
>  line 310, in _GetHeaderInfo
> INFO - RecordList = self._RawData[MODEL_META_DATA_HEADER, self._Arch]
> INFO -  ~
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\MetaFileParser.py",
>  line 250, in __getitem__
> INFO - self._PostProcess()
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\MetaFileParser.py",
>  line 1429, in _PostProcess
> INFO - Processer[self._ItemType]()
> INFO -   File 
> "C:\Users\ryder\edk2_2023\edk2\.venv\Lib\site-packages\edk2basetools\Workspace\MetaFileParser.py",
>  line 1609, in __ProcessDirective
> INFO - __IncludeMacros['WORKSPACE'] = 
> GlobalData.gGlobalDefines['WORKSPACE']
> INFO -
> ~^
> INFO - KeyError: 'WORKSPACE'
> 
> I followed the directions on 
> 
>  and . 
> Are these the correct set of instructions to follow? If so, I entered the 
> following commands and received the error above.
> 
> git clone https://github.com/tianocore/edk2.git
> cd edk2
> python -m venv .venv
> .\.venv\Scripts\Activate.ps1
> stuart_setup -c EmulatorPkg/PlatformCI/PlatformBuild.py TOOL_CHAIN_TAG=VS2019 
> -a X64
> stuart_update -c EmulatorPkg/PlatformCI/PlatformBuild.py 
> TOOL_CHAIN_TAG=VS2019 -a X64
> stuart_build -c EmulatorPkg/PlatformCI/PlatformBuild.py TOOL_CHAIN_TAG=VS2019 
> -a X64
> 
> A quick couple of print statements in the python code of basetools reveal 
> that gGlobalDefines is an empty dictionary. Help would be greatly 
> appreciated. Let me know if you need more information.
> 
> Thank you,
> Ryder
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111560): https://edk2.groups.io/g/devel/message/111560
Mute This Topic: https://groups.io/mt/102698986/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 

Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Fix typos in Bus and Core

[Laszlo Ersek]

On 11/21/23 03:07, Page Chen via groups.io wrote:
> begining -> beginning
> Cabability -> Capability
> CHNAGED -> CHANGED
> compatability -> compatibility
> concident -> coincident
> correspoding -> corresponding
> defered -> deferred
> Dispacher -> Dispatcher
> execuing -> executing
> exhausive -> exhaustive
> functio -> function
> funtion -> function
> funtions -> functions
> notificaiton -> notification
> Regsitered -> Registered
> SUCESS -> SUCCESS
> sucessfully -> successfully
> writting -> writing

this patch is easy to post and terribly hard to review.

If it is supposed to be reviewed by a human reviewer, then please split
it up.

the reason being that a human reviewer gets de-sensitivized to such
changes in a very short time, and then doesn't notice any mistakes.
Therefore, a human needs *frequent* interruption and resumption points,
within a series of changes of this type. Lumping them all together is a
terrible idea. I certainly can't review 62 changes of this type in one go.

according to the diffstat, we have ~10 modules that are affected. That
provides, IMO, a much better resolution for this work; a reviewer can
freely stop mid-series at any point and pick up later.

anyway -- I don't insist for this to be respun, I just can't review it
like this.

Laszlo

> 
> Signed-off-by: Page Chen 
> Cc: Liming Gao 
> Cc: Ray Ni 
> Cc: Andrew Fish 
> ---
>  .../Bus/Ata/AhciPei/AhciPeiStorageSecurity.c   |  2 +-
>  .../Bus/Ata/AhciPei/AhciPeiStorageSecurity.h   |  2 +-
>  .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c  | 18 +-
>  .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h   |  4 ++--
>  .../Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.c  | 10 +-
>  .../Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.h  |  8 
>  .../Bus/Pci/NvmExpressPei/NvmExpressPeiHci.c   |  4 ++--
>  .../Bus/Pci/NvmExpressPei/NvmExpressPeiHci.h   |  4 ++--
>  .../NvmExpressPeiStorageSecurity.c |  2 +-
>  .../NvmExpressPeiStorageSecurity.h |  2 +-
>  .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c|  4 ++--
>  .../Bus/Pci/PciBusDxe/PciOptionRomSupport.h|  2 +-
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h |  2 +-
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c   |  2 +-
>  MdeModulePkg/Bus/Pci/XhciPei/XhciSched.c   |  2 +-
>  MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.c | 18 +-
>  MdeModulePkg/Bus/Usb/UsbKbDxe/EfiKey.h |  4 ++--
>  MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c  |  2 +-
>  MdeModulePkg/Core/Dxe/DxeMain.h|  8 
>  MdeModulePkg/Core/Dxe/FwVol/FwVolRead.c|  2 +-
>  MdeModulePkg/Core/Dxe/Hand/Notify.c|  2 +-
>  MdeModulePkg/Core/Dxe/Image/Image.c|  2 +-
>  MdeModulePkg/Core/Dxe/Mem/Page.c   |  6 +++---
>  .../SectionExtraction/CoreSectionExtraction.c  |  6 +++---
>  MdeModulePkg/Core/DxeIplPeim/DxeLoad.c |  4 ++--
>  MdeModulePkg/Core/Pei/PeiMain.h|  2 +-
>  26 files changed, 62 insertions(+), 62 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c 
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
> index 40e042082cde..6381c371698f 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.c
> @@ -302,7 +302,7 @@ AhciStorageSecurityReceiveData (
>  
>If the security protocol command completes without an error, the function 
> shall
>return EFI_SUCCESS. If the security protocol command completes with an 
> error,
> -  the functio shall return EFI_DEVICE_ERROR.
> +  the function shall return EFI_DEVICE_ERROR.
>  
>@param[in] This  The PPI instance pointer.
>@param[in] DeviceIndex   The ID of the device.
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h 
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
> index a13533ea57ea..8bc367da21d8 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiStorageSecurity.h
> @@ -190,7 +190,7 @@ AhciStorageSecurityReceiveData (
>  
>If the security protocol command completes without an error, the function 
> shall
>return EFI_SUCCESS. If the security protocol command completes with an 
> error,
> -  the functio shall return EFI_DEVICE_ERROR.
> +  the function shall return EFI_DEVICE_ERROR.
>  
>@param[in] This  The PPI instance pointer.
>@param[in] DeviceIndex   The ID of the device.
> diff --git a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c 
> b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
> index b1ab17af3788..535f234916c0 100644
> --- a/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
> +++ b/MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KbdTextIn.c
> @@ -81,7 +81,7 @@ PushEfikeyBufTail (
>  /**
>Judge whether is a registered key
>  
> -  @param RegsiteredData   A pointer to a buffer that is filled in with 
> the keystroke
> +  @param RegisteredData 

Re: [edk2-devel] [PATCH] Maintainers.txt: add Aaron Young as MptScsi and PvScsi reviewer

[Aaron Young via groups.io]

Thanks Laszlo. Yes, ajyoung-oracle is my github user ID.

Reviewed-by: Aaron Young 

Thanks,
-Aaron


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111558): https://edk2.groups.io/g/devel/message/111558
Mute This Topic: https://groups.io/mt/102728630/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Use processor extended information

[Laszlo Ersek]

On 11/20/23 13:42, Wu, Jiaxin wrote:
> For core id in cpu features library, I agree it should be not easy or
> simple change to 0x1f.
> 
>  
> 
> But in SMM CPU, there is no usage case depends on the number of cores
> retrieved from cupid 0x0b return value, only PackageId will be used. So,
> this patch doesn’t do bad things, should no regression issue. I agree
> with Ray’s explanation that  “CPUID.0B.PackageId == CPUID.1F.PackageId”,
> thus no need update for the PackageId update.
> 
>  
> 
> I checked the latest SDM:
> 
>  
> 
> “The sub-leaves of CPUID leaf 0BH describe an ordered hierarchy of
> logical processors starting from the smallest-scoped domain of a Logical
> Processor (sub-leaf index 0) to the Core domain (sub-leaf index 1) to
> the largest-scoped domain (the last valid sub-leaf index) **that is
> implicitly subordinate to the unenumerated highest-scoped domain of the
> processor package (socket)**”
> 
>  
> 
> Looks it already updated to indicate the largest-scoped domain is package.
> 
>  
> 
> With all above, I agree to drop this path, but other 2 patches in this
> set should be ok. Thanks Ray help clarify this.

This is precisely the reason why I originally requested the now-last
patch to be split off from the rest. I certainly didn't / couldn't go
into such depths of CPUID.0B versus CPUID.1F discussion, but still that
change looked very distinct from *populating* Location2 in the
SMM-add-processor protocol member function (upon CPU hotplug). So, FWIW,
I'm fine if the last patch in the series gets dropped.

Thanks
Laszlo

> 
>  
> 
> Thanks,
> 
> Jiaxin
> 
>  
> 
> *From:* Ni, Ray 
> *Sent:* Monday, November 20, 2023 9:45 AM
> *To:* Laszlo Ersek ; devel@edk2.groups.io; Wu, Jiaxin
> 
> *Cc:* Dong, Eric ; Kumar, Rahul R
> ; Gerd Hoffmann ; Zeng, Star
> 
> *Subject:* Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm:
> Use processor extended information
> 
>  
> 
> let me add more to explain:
> 
>  
> 
> 1. CPUID.0B.PackageId == CPUID.1F.PackageId
> 
>  
> 
> SDM clearly states the scope of every MSR (public): package, core, or
> thread.
> 
> But SDM doesn't emphasize that if a MSR is package scope, it's within
> the package defined by CPUID.0B or CPUID.1F.
> 
> That implies, CPUID.0B and CPUID.1F should return the same value for
> package ID.
> 
>  
> 
> Also, SDM has following statement to explain result of EAX for CPUID.0B
> and CPUID.1F:
> 
>     Bits 04-00: The number of bits that the x2APIC ID must be shifted to
> the right to address instances of the "*next higher-scoped"*​ domain.
> 
>  
> 
> That means when CPUID.0B returns the EAX[04:00], it returns the total
> bits of "thread", "core", "module", "tie", "die" because "package" is
> the next higher-scoped domain.
> 
>  
> 
> That also supports the idea: CPUID.0B.PackageId == CPUID.1F.PackageId.
> 
>  
> 
> 2. CPU Feature Initialization
> 
>  
> 
> In UefiCpuPkg/Include/RegisterCpuFeaturesLib.h, the following macros
> were added to support consumers of RegisterCpuFeaturesLib specify
> dependencies among different features.
> 
> For example, when feature #a PACKAGE_BEFORE feature #b, #b is performed
> in one thread of a package and after all threads have performed #a.
> 
> That means internally multi-thread-sync is used to guarantee the
> dependencies.
> 
> #define CPU_FEATURE_THREAD_BEFORE   BIT25
> 
> #define CPU_FEATURE_THREAD_AFTER    BIT26
> 
> #define CPU_FEATURE_CORE_BEFORE     BIT27
> 
> #define CPU_FEATURE_CORE_AFTER      BIT28
> 
> #define CPU_FEATURE_PACKAGE_BEFORE  BIT29
> 
> #define CPU_FEATURE_PACKAGE_AFTER   BIT30
> 
>  
> 
> But above 3 sets of macro only define the dependencies between 3 scopes:
> thread, core and package.
> 
> Other scopes were not supported as there is no MSR which belongs to
> other scopes at that moment, even today.
> 
> So, the cpu features library implementation also only depends on CPUID.0B.
> 
> If we update the code to get package id from CPUID.1F, to be consistent,
> we should also get the core id from CPUID.1F.
> 
> But if we do that, the number of cores which belong to the same domain
> could be less in CPUID.1F. As CPUID.1F returns
> the number of cores per module, instead of per package.
> 
> That will break the MP sync logic which depends on the number of cores
> per each domain.
> 
>  
> 
> Conclusion: we should not update code to use CPUID.1F as it will break
> the MP-sync logic in RegisterCpuFeaturesLib which is not aware of more
> than 3 layers of scopes.
> 
>  
> 
> Thanks,
> 
> Ray
> 
>  
> 
> 
> 
> *From:* Laszlo Ersek mailto:ler...@redhat.com>>
> *Sent:* Saturday, November 18, 2023 5:05 AM
> *To:* devel@edk2.groups.io
>  >; Ni, Ray  >; Wu, Jiaxin  >
> *Cc:* Dong, Eric mailto:eric.d...@intel.com>>;
> Kumar, Rahul R  >; Gerd Hoffmann  

Re: [edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

[Ard Biesheuvel]

On Tue, 21 Nov 2023 at 10:55, Leif Lindholm  wrote:
>
> On Tue, Nov 21, 2023 at 14:46:05 +, Yao, Jiewen wrote:
> > This Bugzilla is filed in 2022-10-26. Now it is 2023-11-21.
>
> Oh, I'm sure I voiced the same opinion for many years before someone
> (rightly) told me to go gile that bugzilla.
>
> > I agree with you that it is a big task. May I know what is the plan?
> > E.g. who is working on that? When do you expect it will be done?
>
> On my list of "big items" to deal with, this comes after github PR
> migration and line-ending conversion.
>
> > According to the dependency rule, what we need is only *interface*
> > definition, but not *implementation*.
> > That means the really requirement here is to move *interface* from
> > ArmPkg to MdePkg, you can still keep the library implementation in
> > ArmPkg. (It is just a subset of this Bugzilla)
>
> That ... is an option I had not previously considered.
> Long-term we would still like to smash ArmLib into BaseLib, but if
> MdePkg maintainers would be OK with moving ArmLib.h into MdePkg...
>
> > Also, I don’t think CPUID check really matters here - since it is only 
> > implementation.
> > As long as, you have interface in MdePkg, then your INF can declare that 
> > interface.
> > You can still put real implementation in ArmPkg - no requirement to move.
> > That benefit is that you don’t need to add ArmPkg dependency in yaml.
>
> I can spin up a patch for that to get merged shortly after stable tag
> to give plenty of time to catch any issues that may arise from moving
> such a fundamental file. (These would likely be bugs, but
> nevertheless...)
>

This sounds like a reasonable solution to me for the short term.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111556): https://edk2.groups.io/g/devel/message/111556
Mute This Topic: https://groups.io/mt/102725178/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

[Leif Lindholm]

On Tue, Nov 21, 2023 at 14:46:05 +, Yao, Jiewen wrote:
> This Bugzilla is filed in 2022-10-26. Now it is 2023-11-21.

Oh, I'm sure I voiced the same opinion for many years before someone
(rightly) told me to go gile that bugzilla.

> I agree with you that it is a big task. May I know what is the plan?
> E.g. who is working on that? When do you expect it will be done?

On my list of "big items" to deal with, this comes after github PR
migration and line-ending conversion.

> According to the dependency rule, what we need is only *interface*
> definition, but not *implementation*.
> That means the really requirement here is to move *interface* from
> ArmPkg to MdePkg, you can still keep the library implementation in
> ArmPkg. (It is just a subset of this Bugzilla)

That ... is an option I had not previously considered.
Long-term we would still like to smash ArmLib into BaseLib, but if
MdePkg maintainers would be OK with moving ArmLib.h into MdePkg...

> Also, I don’t think CPUID check really matters here - since it is only 
> implementation.
> As long as, you have interface in MdePkg, then your INF can declare that 
> interface.
> You can still put real implementation in ArmPkg - no requirement to move.
> That benefit is that you don’t need to add ArmPkg dependency in yaml.

I can spin up a patch for that to get merged shortly after stable tag
to give plenty of time to catch any issues that may arise from moving
such a fundamental file. (These would likely be bugs, but
nevertheless...)

Thanks!

/
Leif


> Thank you
> Yao, Jiewen
> 
> > -Original Message-
> > From: Leif Lindholm 
> > Sent: Tuesday, November 21, 2023 10:26 PM
> > To: Yao, Jiewen 
> > Cc: Pierre Gondois ; devel@edk2.groups.io; Li, Yi1
> > ; Lu, Xiaoyu1 ; Jiang, Guomin
> > ; Ard Biesheuvel ; Sami
> > Mujawar ; Gerd Hoffmann 
> > Subject: Re: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency
> > upon ArmPkg
> > 
> > Hi Jiewen,
> > 
> > On Tue, Nov 21, 2023 at 13:41:21 +, Yao, Jiewen wrote:
> > > Thanks to let me know the background.
> > >
> > > Please be aware that there is fundamental difference between
> > > dependency in INF and dependency in DSC.
> > >
> > > What we have previously in the ArmPkg in *DSC*. We don’t need add
> > > ArmPkg in yaml.
> > > However, what you try to introduce is ArmPkg in *INF*, e.g. your
> > > patch v5 5/6. Then we have to add ArmPkg in yaml.
> > >
> > > Personally, I don’t think it is a good idea to add ArmPkg to yaml,
> > > because it means that you have to pull ArmPkg when you build
> > > CryptoPkg,.
> > >
> > > As long as what you add is industry standard, it is OK to add to
> > > MdePkg, like what you did in v2. I would like to suggest this
> > > approach.
> > 
> > Ultimately, all of ArmPkg needs to migrate to MdePkg.
> > See https://bugzilla.tianocore.org/show_bug.cgi?id=4121
> > But this is a BIG task.
> > 
> > The reason I asked Pierre to add this functionality in ArmPkg rather
> > than MdePkg is because that is where the existing related discovery
> > code lives. (Think of it as CPUID.)
> > 
> > For historical reasons, predating mine and Ard's involvement with
> > edk2, this functionality (as well as other critical Arm functionality)
> > lives in a library called ArmLib, under ArmPkg.
> > For Ia32/X64, all such support lives in BaseLib, under MdePkg.
> > 
> > This is why I referred to ArmPkg as an exclave of MdePkg in my
> > original reply to Pierre. And until someone untangles this, it's not
> > realistic to treat ArmPkg as anything else.
> > 
> > And I don't think it's fair to expect Pierre to untangle this as part
> > of this series. But I also don't think "Arm architectures need to
> > duplicate their basic support code across multiple packages" is a
> > solution.
> > 
> > Regards,
> > 
> > Leif
> > 
> > > But I would like to have ARM expert to check if those are really ARM
> > > standard, and also have MdePkg owner check if it is acceptable.
> > >
> > > Thank you
> > > Yao, Jiewen
> > >
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Pierre Gondois 
> > > > Sent: Tuesday, November 21, 2023 8:59 PM
> > > > To: Yao, Jiewen ; devel@edk2.groups.io; Leif
> > Lindholm
> > > > 
> > > > Cc: Li, Yi1 ; Lu, Xiaoyu1 ; 
> > > > Jiang,
> > Guomin
> > > > ; Ard Biesheuvel ;
> > Sami
> > > > Mujawar ; Gerd Hoffmann 
> > > > Subject: Re: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow 
> > > > dependency
> > > > upon ArmPkg
> > > >
> > > > Hello Jiewen,
> > > >
> > > > On 11/21/23 12:27, Yao, Jiewen wrote:
> > > > > Why CryptoPkg needs to depend on ArmPkg?
> > > > >
> > > > > Can we move content to MdePkg?
> > > >
> > > > The OpensslLib needs to discover the native instruction supported by the
> > > > underlying platform before using them. This could also be done through 
> > > > the
> > > > MdePkg as you suggested. The v2 is implemented that way:
> > > > https://edk2.groups.io/g/devel/message/110953
> > > >
> > > > Also, as noted by Leif, it seems there is already 

Re: [edk2-devel] [Bug] Building NetworkPkg fails due to missing SynchronizationLib dependency of BaseCryptLib

[Laszlo Ersek]

On 11/20/23 11:25, Crossed Carpet wrote:
> Good morning Laszlo,
> Thank you for your reply. I feared this was intentional due to believing
> that it would have been caught with automated testing. 
> Doesn't the Azure Pipeline try to build all packages to make sure no
> dependency broke?

Right, we have "NetworkPkg/NetworkPkg.ci.yaml". I'm unsure how this
issue had slipped through.

Thanks
Laszlo

> 
> Also Liming, would you do me the honour of creating a Bugzilla account
> for this email?
> Best regards,
> CC
> 
> *From:* Laszlo Ersek 
> *Sent:* 17 November 2023 16:06
> *To:* devel@edk2.groups.io ;
> crossedcar...@hotmail.com 
> *Cc:* Liming Gao (Byosoft address) 
> *Subject:* Re: [edk2-devel] [Bug] Building NetworkPkg fails due to
> missing SynchronizationLib dependency of BaseCryptLib
>  
> On 11/17/23 13:49, CrossedCarpet wrote:
>> Steps to reproduce:
>> - download and setup edk2
>> - run:
>> build -a X64 -b DEBUG -t GCC -p NetworkPkg/NetworkPkg.dsc
>> 
>> Get the error:
>> build.py...
>> /.../edk2/NetworkPkg/NetworkPkg.dsc(...): error 4000: Instance of
>> library class [SynchronizationLib] is not found
>> in [/.../edk2/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf] [X64]
>> consumed by module [/.../edk2/NetworkPkg/TlsDxe/TlsDxe.inf]
>> 
>> Adding this LibClass to NetworkPkg.dsc solves it:
>>  
>> SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> 
> This is a regression from commit 5d5be45bd111 ("CryptPkg: Enable
> CryptoPkg BaseCryptLib ParallelHash for PEI and DXE", 2022-12-02), which
> was made for  >.
> 
> It added a new lib class dependency to "BaseCryptLib.inf", but it didn't
> ensure that all DSC files in the tree that employed the
> "BaseCryptLib.inf" instance had a resolution for the new lib class.
> 
> Indeed it is not just NetworkPkg.dsc but also FmpDevicePkg.dsc that's
> affected:
> 
> $ git grep -l -F CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf --
> '**dsc*' \
>   | xargs -- grep --files-without-match -w SynchronizationLib --
> 
> FmpDevicePkg/FmpDevicePkg.dsc
> NetworkPkg/NetworkPkg.dsc
> 
> It also *seems* to affect at least one platform in edk2-platforms:
> 
> Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
> 
> (although I realize this last DSC file is an "include" DSC, so the
> missing dependency could be resolved in the DSC file that includes this
> one.)
> 
> Either way, thanks for catching this; the edk2 issue should be fixed
> preferably during the current hard feature freeze (for NetworkPkg and
> FmpDevicePkg).
> 
>> I tried to open a bug in bugzilla but I wasn't able to log in or create
>> an account. How should I do it next time?
> 
> I think the bugzilla account creation was disabled due to spammer accounts.
> 
> The way to request an account is described here (linked from the
> bugzilla.tianocore.org landing page under link "Reporting issues"):
> 
>   https://github.com/tianocore/tianocore.github.io/wiki/Reporting-Issues
> 
> 
> (I've added Liming to the CC list of this email.)
> 
> Laszlo
> 
> 
>> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111554): https://edk2.groups.io/g/devel/message/111554
Mute This Topic: https://groups.io/mt/102646401/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V3 0/4] Add New Memory Attributes

[Laszlo Ersek]

On 11/20/23 05:59, Dhaval Sharma wrote:
> Did you mean to add it as a memory type by itself like
> EFI_MEMORY_TYPE_INFORMATION?

Yes, a new enum constant for EFI_MEMORY_TYPE.

(It wasn't a "loaded question", just wanted to understand the reasoning.)

> My interpretation of Memory Type is that it is more of SW usability
> construct while Memory Attr is more of HW behavioural construct.
> Together they define how a memory region can be used.

I guess it's hard to tell apart. The SPM definition (visible in the
context below, from your earlier email) does seem like a software
usability construct ("avoid allocating for ...").

But, I truly don't know. I guess I was only trying to gauge if I could
be a useful reviewer for this series; probably not.

Thanks!
Laszlo

> 
> On Fri, Nov 17, 2023 at 1:55 PM Laszlo Ersek  > wrote:
> 
> On 11/17/23 09:07, Dhaval Sharma wrote:
> > Hi,
> > I wanted to revisit this thread and I am maintaining the context as
> > there are a lot of details already mentioned here
> regarding EFI_MEMORY_SP.
> > Other than what has been addressed here, we also would like to have an
> > option in edk2 to *avoid* using this type of memory for its own
> > purposes. This seems like one of the motivations for original request
> > and is being honored by OS today but not edk2 as it does not have any
> > specific implementation today which takes this attribute into
> consideration.
> > I would like to add PCD based implementation which informs edk2 NOT to
> > use this memory for its own purposes and leave it alone (as still
> > available memory to OS).
> >
> > Specific-purpose memory (SPM). The memory is earmarked for
> > specific purposes such as for specific device drivers or applications.
> > The SPM attribute serves as a hint to the OS to avoid allocating this
> > memory for core OS data or code that can not be relocated.
> > Prolonged use of this memory for purposes other than the intended
> > purpose may result in suboptimal platform performance.
> >
> > Some more context:
> > https://lwn.net/Articles/784971/ 
> 
> Why was EFI_MEMORY_SP introduced as a memory attribute, rather than its
> own memory type?
> 
> Laszlo
> 
> 
> 
> -- 
> Thanks!
> =D



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111553): https://edk2.groups.io/g/devel/message/111553
Mute This Topic: https://groups.io/mt/75267363/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe, ConfigRouting.c, GetElementsFromRequest incorrect error handling.

[Laszlo Ersek]

On 11/18/23 14:06, Chip wrote:
> How many different patches are you looking for?
> 
> One patch to fix bug 2861 specifically, and a separate patch that fixes
> the uninitialized pointer issue?

I can't easily say, because I'm not familiar with HiiDatabaseDxe. So
looking at the present patch, I can only say it seems to be doing "too
much".

I generally prefer patches that do the possible *minimum* semantically.

Isolating the fix for the uninited pointer to one patch sounds good.

I'm not sure if the rest of the code code changes (i.e., the fix(es) for
bug 2861) belong to just *one* other patch though. If you think any one
of those changes makes no sense without the other changes, or else if
you think these changes are nearly identical all over, then keeping them
in one patch may be good.

Basically treat any patch (including commit message and code changes)
like an "explain like I'm five" lesson to a reviewer. Advance in small
steps, and explain liberally.

IMO it's not possible to write a patch that is "too didactic", only a
patch that's too terse.

> I provided the second previously (June
> and July 2023), and literally nobody commented in edk2-rfc or edk2-devel.

That's too bad, my apologies. The project has been facing challenges
with timely reviews.

Laszlo


> 
> Chip
> 
> 
> -Original Message- From: Laszlo Ersek
> Sent: Monday, November 13, 2023 9:59 AM
> To: devel@edk2.groups.io ; chip.program...@att.net
> Subject: Re: [edk2-devel] [PATCH v1 1/1] Bug 2861 - HiiDatabaseDxe,
> ConfigRouting.c, GetElementsFromRequest incorrect error handling.
> 
> Hi Charles,
> 
> On 10/26/23 03:05, Charles Hyde wrote:
>> From: Charles Hyde 
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2861
>>
>> I believe the attached ConfigRouting.txt patch will resolve bug 2861,
>> plus
>> resolve an uninitialized pointer issue in HiiConfigRoutingExportConfig().
>> The uninitialized pointer was identified when running the EDK2 Self
>> Certification Test with all tests selected, having caused the CPU to
>> issue
>> an exception error (most times) or completely trashed the system
>> (sometimes).
>>
>> I found a second instance of GetElementsFromRequest(), located in
>> HiiLib.c,
>> that also needed an update.  The attached patch should address this
>> bug and
>> more.
>>
>> Signed-off-by: Charles Hyde 
>> ---
> 
> Thanks for analyzing and fixing these bugs.
> 
> Can you please split the separate fixes to separate patches?
> 
> Also, the patch looks garbled; it shouldn't be attached / pasted but
> sent with git-send-email. Are you familiar with git-send-email?
> 
> Here's the official docs:
> 
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
> 
> and some unofficial tips:
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
> 
> Third, I suggest not to comment out, with /* */, dead code (such as a
> subcondition that always evaluates to false or true); instead, remove
> it, and explain in the commit message (or, if necessary, in a code
> comment) why that condition is a tautology. If the condition or argument
> is nontrivial, consider using an ASSERT().
> 
> Laszlo
> 
> 
>>
>> diff --git a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
>> b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
>> index 63a37ab59a..c3dc7bf558 100644
>> --- a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
>> +++ b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c
>> @@ -2272,8 +2272,14 @@ GetElementsFromRequest (
>> {
>>   EFI_STRING  TmpRequest;
>>
>> +  ASSERT (ConfigRequest != NULL);
>> +  if (ConfigRequest == NULL)
>> +    return FALSE;
>> +
>>   TmpRequest = StrStr (ConfigRequest, L"PATH=");
>>   ASSERT (TmpRequest != NULL);
>> +  if (TmpRequest == NULL)
>> +    return FALSE;
>>
>>   if ((StrStr (TmpRequest, L"=") != NULL) || (StrStr (TmpRequest,
>> L"&") != NULL)) {
>>     return TRUE;
>> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
>> b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
>> index 5ae6189a28..0b39f156f3 100644
>> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
>> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
>> @@ -420,14 +420,19 @@ AppendToMultiString (
>>   }
>>
>>   AppendStringSize = StrSize (AppendString);
>> +  if (AppendStringSize <= sizeof(*AppendString))    // If the string is
>> empty, there is no need to proceed further.
>> +    return EFI_SUCCESS;
>> +
>>   MultiStringSize  = StrSize (*MultiString);
>>   MaxLen   = MAX_STRING_LENGTH / sizeof (CHAR16);
>>
>>   //
>>   // Enlarge the buffer each time when length exceeds MAX_STRING_LENGTH.
>>   //
>> -  if ((MultiStringSize + AppendStringSize > MAX_STRING_LENGTH) ||
>> -  (MultiStringSize > MAX_STRING_LENGTH))
>> +  if ((MultiStringSize + AppendStringSize > MAX_STRING_LENGTH) /*||
>> +  (MultiStringSize > MAX_STRING_LENGTH)*/)  // There is no need to
>> check the second part.
>> +  

Re: [edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

[Yao, Jiewen]

This Bugzilla is filed in 2022-10-26. Now it is 2023-11-21.
I agree with you that it is a big task. May I know what is the plan?
E.g. who is working on that? When do you expect it will be done?



According to the dependency rule, what we need is only *interface* definition, 
but not *implementation*.
That means the really requirement here is to move *interface* from ArmPkg to 
MdePkg, you can still keep the library implementation in ArmPkg. (It is just a 
subset of this Bugzilla)
Also, I don’t think CPUID check really matters here - since it is only 
implementation.

As long as, you have interface in MdePkg, then your INF can declare that 
interface.
You can still put real implementation in ArmPkg - no requirement to move.
That benefit is that you don’t need to add ArmPkg dependency in yaml.

Thank you
Yao, Jiewen

> -Original Message-
> From: Leif Lindholm 
> Sent: Tuesday, November 21, 2023 10:26 PM
> To: Yao, Jiewen 
> Cc: Pierre Gondois ; devel@edk2.groups.io; Li, Yi1
> ; Lu, Xiaoyu1 ; Jiang, Guomin
> ; Ard Biesheuvel ; Sami
> Mujawar ; Gerd Hoffmann 
> Subject: Re: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency
> upon ArmPkg
> 
> Hi Jiewen,
> 
> On Tue, Nov 21, 2023 at 13:41:21 +, Yao, Jiewen wrote:
> > Thanks to let me know the background.
> >
> > Please be aware that there is fundamental difference between
> > dependency in INF and dependency in DSC.
> >
> > What we have previously in the ArmPkg in *DSC*. We don’t need add
> > ArmPkg in yaml.
> > However, what you try to introduce is ArmPkg in *INF*, e.g. your
> > patch v5 5/6. Then we have to add ArmPkg in yaml.
> >
> > Personally, I don’t think it is a good idea to add ArmPkg to yaml,
> > because it means that you have to pull ArmPkg when you build
> > CryptoPkg,.
> >
> > As long as what you add is industry standard, it is OK to add to
> > MdePkg, like what you did in v2. I would like to suggest this
> > approach.
> 
> Ultimately, all of ArmPkg needs to migrate to MdePkg.
> See https://bugzilla.tianocore.org/show_bug.cgi?id=4121
> But this is a BIG task.
> 
> The reason I asked Pierre to add this functionality in ArmPkg rather
> than MdePkg is because that is where the existing related discovery
> code lives. (Think of it as CPUID.)
> 
> For historical reasons, predating mine and Ard's involvement with
> edk2, this functionality (as well as other critical Arm functionality)
> lives in a library called ArmLib, under ArmPkg.
> For Ia32/X64, all such support lives in BaseLib, under MdePkg.
> 
> This is why I referred to ArmPkg as an exclave of MdePkg in my
> original reply to Pierre. And until someone untangles this, it's not
> realistic to treat ArmPkg as anything else.
> 
> And I don't think it's fair to expect Pierre to untangle this as part
> of this series. But I also don't think "Arm architectures need to
> duplicate their basic support code across multiple packages" is a
> solution.
> 
> Regards,
> 
> Leif
> 
> > But I would like to have ARM expert to check if those are really ARM
> > standard, and also have MdePkg owner check if it is acceptable.
> >
> > Thank you
> > Yao, Jiewen
> >
> >
> >
> >
> > > -Original Message-
> > > From: Pierre Gondois 
> > > Sent: Tuesday, November 21, 2023 8:59 PM
> > > To: Yao, Jiewen ; devel@edk2.groups.io; Leif
> Lindholm
> > > 
> > > Cc: Li, Yi1 ; Lu, Xiaoyu1 ; Jiang,
> Guomin
> > > ; Ard Biesheuvel ;
> Sami
> > > Mujawar ; Gerd Hoffmann 
> > > Subject: Re: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency
> > > upon ArmPkg
> > >
> > > Hello Jiewen,
> > >
> > > On 11/21/23 12:27, Yao, Jiewen wrote:
> > > > Why CryptoPkg needs to depend on ArmPkg?
> > > >
> > > > Can we move content to MdePkg?
> > >
> > > The OpensslLib needs to discover the native instruction supported by the
> > > underlying platform before using them. This could also be done through the
> > > MdePkg as you suggested. The v2 is implemented that way:
> > > https://edk2.groups.io/g/devel/message/110953
> > >
> > > Also, as noted by Leif, it seems there is already a dependency over 
> > > ArmPkg:
> > > # git grep ArmPkg CryptoPkg/
> > > CryptoPkg/CryptoPkg.dsc:  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > > CryptoPkg/CryptoPkg.dsc:
> > > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> > > CryptoPkg/CryptoPkg.dsc:
> > > ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
> > > CryptoPkg/CryptoPkgMbedTls.dsc:
> > > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> > > CryptoPkg/CryptoPkgMbedTls.dsc:
> > > ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
> > > CryptoPkg/CryptoPkgMbedTls.dsc:
> > >
> PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServic
> > > esTablePointerLib.inf
> > >
> > > Both solutions suit me (discovering capabilities through ArmPkg or 
> > > MdePkg),
> > > I just need to know which one is preferred,
> > >
> > > Regards,
> > > Pierre
> > >
> > > >
> > > >> -Original 

Re: [edk2-devel] [PATCH v3 09/39] MdePkg: Add a new library named PeiServicesTablePointerLibReg

[Laszlo Ersek]

On 11/17/23 10:59, Chao Li wrote:
> Since some ARCH or platform not require execute code on memory during
> PEI phase, some values may transferred via CPU registers.
> 
> Adding PeiServcieTablePointerLibReg to allow set and get the PEI service
> table pointer depend by a CPU register, this library can accommodate lot
> of platforms who not require execte code on memory during PEI phase.
> 
> Adding PeiServiceTablePointerLibReg to allows setting and getting the
> PEI service table pointer via CPU registers, and the library can
> accommodate many platforms that do not need to execute code on memory
> during the PEI phase.
> 
> The idea of this library is derived from
> ArmPkg/Library/PeiServicesTablePointerLib/
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4584
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Leif Lindholm 
> Cc: Ard Biesheuvel 
> Cc: Sami Mujawar 
> Cc: Laszlo Ersek 
> Cc: Sunil V L 
> Signed-off-by: Chao Li 
> ---
>  .../Library/PeiServicesTablePointerLib.h  | 37 +++-
>  .../PeiServicesTablePointer.c | 86 +++
>  .../PeiServicesTablePointerLib.uni| 20 +
>  .../PeiServicesTablePointerLibReg.inf | 40 +
>  MdePkg/MdePkg.dsc |  1 +
>  5 files changed, 180 insertions(+), 4 deletions(-)
>  create mode 100644 
> MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointer.c
>  create mode 100644 
> MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointerLib.uni
>  create mode 100644 
> MdePkg/Library/PeiServicesTablePointerLibReg/PeiServicesTablePointerLibReg.inf

In my opinion, the PeiServicesTablePointerLib class header should not be
extended with new interfaces. I understand that the generality is
attractive, but it is not put to use; only the loongarch architecture
applies the new interfaces (in the subsequent patch), and for example
the ARM code (ArmPkg/Library/PeiServicesTablePointerLib) is not reworked
in terms of these new interfaces.

What's more, the new library interfaces, even though they are exposed in
the lib class header, are not implemented for other architectures, so
they aren't even callable on those arches.

I'm commenting on this patch and the subsequent patch in the series
together, as seen squashed together. NB I'm not an MdePkg maintainer, so
this is just my opinion.

(1) As noted above, the library class should not be modified.

(2) Modifying the *comments* in
"MdePkg/Include/Library/PeiServicesTablePointerLib.h" is welcome, I
think, but then we might want to add a (separate!) patch for removing
the Itanium language, as edk2 no longer supports Itanium.

(3) The PeiServicesTablePointerLibReg instance should be called
PeiServicesTablePointerLibCsrKs0 or just PeiServicesTablePointerLibKs0.

This follows the example of the lib instance name
"PeiServicesTablePointerLibIdt". The whole library instance should be
loongaarch-specific IMO; there isn't much code that's being duplicated,
so the extra interfaces (internal or external) do not help with code
unification.

(4) "PeiServicesTablePointerLib.uni" should be named similarly (suffix
missing).

(5) BASE_NAME in the library instance INF file should be defined
similarly (suffix missing).

(6) The contents of the UNI file should be loongarch-specific, i.e. be
explicit about CSR KS0, in both comments and string constants.

(7) The comments in the library instance INF file should be similarly
loongarch-specific.

(8) I suggest dropping VALID_ARCHITECTURES altogether. If we want to
keep it, it should exclusively say LOONGARCH64.

(9) The new library instance should be listed in
[Components.LOONGARCH64] in MdePkg.dec.

This section does not exist yet; I suggest introducing it under
[Components.RISCV64].

(10) There need not / should not be two separate C source files; just
access the KS0 CSR in SetPeiServicesTablePointer() and
GetPeiServicesTablePointer() directly.

(11) The new library instance should probably not introduce new
references to Itanium.

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111550): https://edk2.groups.io/g/devel/message/111550
Mute This Topic: https://groups.io/mt/102644754/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

[Leif Lindholm]

Hi Jiewen,

On Tue, Nov 21, 2023 at 13:41:21 +, Yao, Jiewen wrote:
> Thanks to let me know the background.
> 
> Please be aware that there is fundamental difference between
> dependency in INF and dependency in DSC.
> 
> What we have previously in the ArmPkg in *DSC*. We don’t need add
> ArmPkg in yaml.
> However, what you try to introduce is ArmPkg in *INF*, e.g. your
> patch v5 5/6. Then we have to add ArmPkg in yaml.
> 
> Personally, I don’t think it is a good idea to add ArmPkg to yaml,
> because it means that you have to pull ArmPkg when you build
> CryptoPkg,.
> 
> As long as what you add is industry standard, it is OK to add to
> MdePkg, like what you did in v2. I would like to suggest this
> approach.

Ultimately, all of ArmPkg needs to migrate to MdePkg.
See https://bugzilla.tianocore.org/show_bug.cgi?id=4121
But this is a BIG task.

The reason I asked Pierre to add this functionality in ArmPkg rather
than MdePkg is because that is where the existing related discovery
code lives. (Think of it as CPUID.)

For historical reasons, predating mine and Ard's involvement with
edk2, this functionality (as well as other critical Arm functionality)
lives in a library called ArmLib, under ArmPkg.
For Ia32/X64, all such support lives in BaseLib, under MdePkg.

This is why I referred to ArmPkg as an exclave of MdePkg in my
original reply to Pierre. And until someone untangles this, it's not
realistic to treat ArmPkg as anything else.

And I don't think it's fair to expect Pierre to untangle this as part
of this series. But I also don't think "Arm architectures need to
duplicate their basic support code across multiple packages" is a
solution.

Regards,

Leif

> But I would like to have ARM expert to check if those are really ARM
> standard, and also have MdePkg owner check if it is acceptable.
> 
> Thank you
> Yao, Jiewen
> 
> 
> 
> 
> > -Original Message-
> > From: Pierre Gondois 
> > Sent: Tuesday, November 21, 2023 8:59 PM
> > To: Yao, Jiewen ; devel@edk2.groups.io; Leif Lindholm
> > 
> > Cc: Li, Yi1 ; Lu, Xiaoyu1 ; Jiang, 
> > Guomin
> > ; Ard Biesheuvel ; Sami
> > Mujawar ; Gerd Hoffmann 
> > Subject: Re: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency
> > upon ArmPkg
> > 
> > Hello Jiewen,
> > 
> > On 11/21/23 12:27, Yao, Jiewen wrote:
> > > Why CryptoPkg needs to depend on ArmPkg?
> > >
> > > Can we move content to MdePkg?
> > 
> > The OpensslLib needs to discover the native instruction supported by the
> > underlying platform before using them. This could also be done through the
> > MdePkg as you suggested. The v2 is implemented that way:
> > https://edk2.groups.io/g/devel/message/110953
> > 
> > Also, as noted by Leif, it seems there is already a dependency over ArmPkg:
> > # git grep ArmPkg CryptoPkg/
> > CryptoPkg/CryptoPkg.dsc:  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > CryptoPkg/CryptoPkg.dsc:
> > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> > CryptoPkg/CryptoPkg.dsc:
> > ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
> > CryptoPkg/CryptoPkgMbedTls.dsc:
> > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> > CryptoPkg/CryptoPkgMbedTls.dsc:
> > ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
> > CryptoPkg/CryptoPkgMbedTls.dsc:
> > PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServic
> > esTablePointerLib.inf
> > 
> > Both solutions suit me (discovering capabilities through ArmPkg or MdePkg),
> > I just need to know which one is preferred,
> > 
> > Regards,
> > Pierre
> > 
> > >
> > >> -Original Message-
> > >> From: Pierre Gondois 
> > >> Sent: Tuesday, November 21, 2023 4:47 PM
> > >> To: devel@edk2.groups.io
> > >> Cc: Yao, Jiewen ; Li, Yi1 ; Lu,
> > Xiaoyu1
> > >> ; Jiang, Guomin ; Leif
> > Lindholm
> > >> ; Ard Biesheuvel ;
> > >> Sami Mujawar ; Gerd Hoffmann
> > >> 
> > >> Subject: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency 
> > >> upon
> > >> ArmPkg
> > >>
> > >> Allow dependency upon ArmPkg to pass the dependency Check.
> > >>
> > >> Signed-off-by: Pierre Gondois 
> > >> ---
> > >>   CryptoPkg/CryptoPkg.ci.yaml | 1 +
> > >>   1 file changed, 1 insertion(+)
> > >>
> > >> diff --git a/CryptoPkg/CryptoPkg.ci.yaml b/CryptoPkg/CryptoPkg.ci.yaml
> > >> index f961d85927c0..3bbb220d3224 100644
> > >> --- a/CryptoPkg/CryptoPkg.ci.yaml
> > >> +++ b/CryptoPkg/CryptoPkg.ci.yaml
> > >> @@ -69,6 +69,7 @@
> > >>   },
> > >>
> > >>   "DependencyCheck": {
> > >>
> > >>   "AcceptableDependencies": [
> > >>
> > >> +"ArmPkg/ArmPkg.dec",
> > >>
> > >>   "MdePkg/MdePkg.dec",
> > >>
> > >>   "MdeModulePkg/MdeModulePkg.dec",
> > >>
> > >>   "CryptoPkg/CryptoPkg.dec",
> > >>
> > >> --
> > >> 2.25.1
> > >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111549): https://edk2.groups.io/g/devel/message/111549
Mute This Topic: 

Re: 回复: 回复: edk2-stable202311: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation

[Sami Mujawar]

Hi Liming,

Thanks a lot. I can see it in the mainline.

Regards,

Sami Mujawar

On 21/11/2023, 14:15, "gaoliming" mailto:gaolim...@byosoft.com.cn>> wrote:


Sami:
I have merged it. Please check. 


Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io   > 代表 Sami
> Mujawar
> 发送时间: 2023年11月21日 19:57
> 收件人: gaoliming mailto:gaolim...@byosoft.com.cn>>; 
> 'Laszlo Ersek'
> mailto:ler...@redhat.com>>; 'Michael Kinney' 
> mailto:michael.d.kin...@intel.com>>;
> devel@edk2.groups.io ; ashishsin...@nvidia.com 
> ; quic_llind...@quicinc.com 
> ;
> ardb+tianoc...@kernel.org ; Jeff Brasen 
> (jbra...@nvidia.com )
> mailto:jbra...@nvidia.com>>
> 抄送: nd mailto:n...@arm.com>>
> 主题: Re: 回复: edk2-stable202311: Re: [edk2-devel] [PATCH]
> DynamicTablesPkg: Fix ETE _UID Creation
> 
> Hi Liming,
> 
> On 21/11/2023, 01:05, "gaoliming"  
> >> wrote:
> 
> 
> Sami:
> I am OK to merge this patch for this stable bug. I will add push label for
> https://github.com/tianocore/edk2/pull/5061 
> 
>  
> 
> [SAMI] Thank you for picking this patch for the stable tag. Apparently, the
> patch does not seem to have merged.
> I am not sure if you have already applied the push label. Please let me know 
> if
> there are any issues that I can help with.
> 
> Regards,
> 
> Sami Mujawar
> 
> Thanks
> Liming
> > -邮件原件-
> > 发件人: Sami Mujawar mailto:sami.muja...@arm.com>
> >>
> > 发送时间: 2023年11月21日 0:07
> > 收件人: Laszlo Ersek mailto:ler...@redhat.com> 
> > >>;
> Liming Gao (Byosoft address)
> > mailto:gaolim...@byosoft.com.cn> 
> > >>; 
> > Michael
> Kinney mailto:michael.d.kin...@intel.com> 
> >>;
> > devel@edk2.groups.io  
> > >;
> ashishsin...@nvidia.com  
> >;
> quic_llind...@quicinc.com  
> >;
> > ardb+tianoc...@kernel.org  
> > >;
> jbra...@nvidia.com   >
> > 抄送: n...@arm.com   > >
> > 主题: edk2-stable202311: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix
> > ETE _UID Creation
> >
> > Hi Laszlo, Liming, Mike,
> >
> > Please see my response inline marked [SAMI].
> >
> > Regards,
> >
> > Sami Mujawar
> >
> > On 17/11/2023 09:20 am, Laszlo Ersek wrote:
> > > On 11/15/23 04:19, Ashish Singhal via groups.io wrote:
> > >> Just like CPU _UID, ETE UID also needs to be unique so
> > >> use AcpiProcessorUid instead of CpuName
> > >>
> > >> Signed-off-by: Ashish Singhal  > >> 
> >>
> > >> ---
> > >> .../Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c | 5
> > -
> > >> 1 file changed, 4 insertions(+), 1 deletion(-)
> > > Is this a fixup for the recent feature
> > >
> > > [PATCH v3 00/11] Update MADT for ACPI 6.5, and add TRBE & ETE support
> > > https://edk2.groups.io/g/devel/message/108996 
> > > 
>  
> 
> > >
> > > ?
> > >
> > > If so, then I *think* this qualifies to be merged during the hard
> > > feature freeze (+Liming +Mike), but:
> >
> > [SAMI] I raised a bugzilla for this issue at
> > https://bugzilla.tianocore.org/show_bug.cgi?id=4600 
> > 
>  
> 
> >
> > and have also created a pull request at
> > https://github.com/tianocore/edk2/pull/5061 
> > 
>  
> .
> >
> > This patch has also passed the CI checks when I did a draft pull request.
> >
> > I am not sure if I can apply the push label as we are in the code freeze
> > stage.
> >
> > Can you advise on how to proceed, please?
> >
> > [/SAMI]
> 

回复: 回复: edk2-stable202311: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation

[gaoliming via groups.io]

Sami:
  I have merged it. Please check. 

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Sami
> Mujawar
> 发送时间: 2023年11月21日 19:57
> 收件人: gaoliming ; 'Laszlo Ersek'
> ; 'Michael Kinney' ;
> devel@edk2.groups.io; ashishsin...@nvidia.com; quic_llind...@quicinc.com;
> ardb+tianoc...@kernel.org; Jeff Brasen (jbra...@nvidia.com)
> 
> 抄送: nd 
> 主题: Re: 回复: edk2-stable202311: Re: [edk2-devel] [PATCH]
> DynamicTablesPkg: Fix ETE _UID Creation
> 
> Hi Liming,
> 
> On 21/11/2023, 01:05, "gaoliming"  > wrote:
> 
> 
> Sami:
> I am OK to merge this patch for this stable bug. I will add push label for
> https://github.com/tianocore/edk2/pull/5061
> 
> [SAMI] Thank you for picking this patch for the stable tag. Apparently, the
> patch does not seem to have merged.
> I am not sure if you have already applied the push label. Please let me know 
> if
> there are any issues that I can help with.
> 
> Regards,
> 
> Sami Mujawar
> 
> Thanks
> Liming
> > -邮件原件-
> > 发件人: Sami Mujawar  >
> > 发送时间: 2023年11月21日 0:07
> > 收件人: Laszlo Ersek mailto:ler...@redhat.com>>;
> Liming Gao (Byosoft address)
> > mailto:gaolim...@byosoft.com.cn>>; Michael
> Kinney mailto:michael.d.kin...@intel.com>>;
> > devel@edk2.groups.io ;
> ashishsin...@nvidia.com ;
> quic_llind...@quicinc.com ;
> > ardb+tianoc...@kernel.org ;
> jbra...@nvidia.com 
> > 抄送: n...@arm.com 
> > 主题: edk2-stable202311: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix
> > ETE _UID Creation
> >
> > Hi Laszlo, Liming, Mike,
> >
> > Please see my response inline marked [SAMI].
> >
> > Regards,
> >
> > Sami Mujawar
> >
> > On 17/11/2023 09:20 am, Laszlo Ersek wrote:
> > > On 11/15/23 04:19, Ashish Singhal via groups.io wrote:
> > >> Just like CPU _UID, ETE UID also needs to be unique so
> > >> use AcpiProcessorUid instead of CpuName
> > >>
> > >> Signed-off-by: Ashish Singhal  >
> > >> ---
> > >> .../Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c | 5
> > -
> > >> 1 file changed, 4 insertions(+), 1 deletion(-)
> > > Is this a fixup for the recent feature
> > >
> > > [PATCH v3 00/11] Update MADT for ACPI 6.5, and add TRBE & ETE support
> > > https://edk2.groups.io/g/devel/message/108996
> 
> > >
> > > ?
> > >
> > > If so, then I *think* this qualifies to be merged during the hard
> > > feature freeze (+Liming +Mike), but:
> >
> > [SAMI] I raised a bugzilla for this issue at
> > https://bugzilla.tianocore.org/show_bug.cgi?id=4600
> 
> >
> > and have also created a pull request at
> > https://github.com/tianocore/edk2/pull/5061
> .
> >
> > This patch has also passed the CI checks when I did a draft pull request.
> >
> > I am not sure if I can apply the push label as we are in the code freeze
> > stage.
> >
> > Can you advise on how to proceed, please?
> >
> > [/SAMI]
> >
> > > - I think we should have a "Fixes:" tag in the commit message (for
> > > pointing out the commit that should have contained the code being
> > > added/updated now)
> > >
> > > - I think we should have a BZ too (also linked into the commit message).
> > >
> > > Laszlo
> > >
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111547): https://edk2.groups.io/g/devel/message/111547
Mute This Topic: https://groups.io/mt/102728858/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] Maintainers.txt: add Aaron Young as MptScsi and PvScsi reviewer

[Laszlo Ersek]

I asked Aaron if he'd be willing to take the "R" role for the MptScsi and
PvScsi drivers in OvmfPkg, given their low maintenance needs and previous
commit 1774a44ad91d ("Maintainers.txt: Remove MptScsi and PvScsi
reviewers", 2022-07-28). Designate Aaron as reviewer for those drivers,
with many thanks.

Cc: Aaron Young 
Cc: Andrew Fish 
Cc: Ard Biesheuvel 
Cc: Gerd Hoffmann 
Cc: Jiewen Yao 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Signed-off-by: Laszlo Ersek 
---

Notes:
Aaron, can you please confirm that your github user ID is indeed
"ajyoung-oracle"? I grabbed that with a simple web search, but didn't
verify it otherwise.

 Maintainers.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 7c0b4cb58cfd..a914a7bd6069 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -517,6 +517,11 @@ OvmfPkg: LsiScsi driver
 F: OvmfPkg/LsiScsiDxe/
 R: Gary Lin  [lcp]
 
+OvmfPkg: MptScsi and PVSCSI driver
+F: OvmfPkg/MptScsiDxe/
+F: OvmfPkg/PvScsiDxe/
+R: Aaron Young  [ajyoung-oracle]
+
 OvmfPkg: TCG- and TPM2-related modules
 F: OvmfPkg/Include/IndustryStandard/QemuTpm.h
 F: OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c

base-commit: 3db76e6476e493d3cda45b81bba99a645180cf35


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111546): https://edk2.groups.io/g/devel/message/111546
Mute This Topic: https://groups.io/mt/102728630/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

[Yao, Jiewen]

Thanks to let me know the background.

Please be aware that there is fundamental difference between dependency in INF 
and dependency in DSC.

What we have previously in the ArmPkg in *DSC*. We don’t need add ArmPkg in 
yaml.
However, what you try to introduce is ArmPkg in *INF*, e.g. your patch v5 5/6. 
Then we have to add ArmPkg in yaml.

Personally, I don’t think it is a good idea to add ArmPkg to yaml, because it 
means that you have to pull ArmPkg when you build CryptoPkg,.


As long as what you add is industry standard, it is OK to add to MdePkg, like 
what you did in v2. I would like to suggest this approach.

But I would like to have ARM expert to check if those are really ARM standard, 
and also have MdePkg owner check if it is acceptable.

Thank you
Yao, Jiewen




> -Original Message-
> From: Pierre Gondois 
> Sent: Tuesday, November 21, 2023 8:59 PM
> To: Yao, Jiewen ; devel@edk2.groups.io; Leif Lindholm
> 
> Cc: Li, Yi1 ; Lu, Xiaoyu1 ; Jiang, 
> Guomin
> ; Ard Biesheuvel ; Sami
> Mujawar ; Gerd Hoffmann 
> Subject: Re: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency
> upon ArmPkg
> 
> Hello Jiewen,
> 
> On 11/21/23 12:27, Yao, Jiewen wrote:
> > Why CryptoPkg needs to depend on ArmPkg?
> >
> > Can we move content to MdePkg?
> 
> The OpensslLib needs to discover the native instruction supported by the
> underlying platform before using them. This could also be done through the
> MdePkg as you suggested. The v2 is implemented that way:
> https://edk2.groups.io/g/devel/message/110953
> 
> Also, as noted by Leif, it seems there is already a dependency over ArmPkg:
> # git grep ArmPkg CryptoPkg/
> CryptoPkg/CryptoPkg.dsc:  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> CryptoPkg/CryptoPkg.dsc:
> NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> CryptoPkg/CryptoPkg.dsc:
> ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
> CryptoPkg/CryptoPkgMbedTls.dsc:
> NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> CryptoPkg/CryptoPkgMbedTls.dsc:
> ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
> CryptoPkg/CryptoPkgMbedTls.dsc:
> PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServic
> esTablePointerLib.inf
> 
> Both solutions suit me (discovering capabilities through ArmPkg or MdePkg),
> I just need to know which one is preferred,
> 
> Regards,
> Pierre
> 
> >
> >> -Original Message-
> >> From: Pierre Gondois 
> >> Sent: Tuesday, November 21, 2023 4:47 PM
> >> To: devel@edk2.groups.io
> >> Cc: Yao, Jiewen ; Li, Yi1 ; Lu,
> Xiaoyu1
> >> ; Jiang, Guomin ; Leif
> Lindholm
> >> ; Ard Biesheuvel ;
> >> Sami Mujawar ; Gerd Hoffmann
> >> 
> >> Subject: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon
> >> ArmPkg
> >>
> >> Allow dependency upon ArmPkg to pass the dependency Check.
> >>
> >> Signed-off-by: Pierre Gondois 
> >> ---
> >>   CryptoPkg/CryptoPkg.ci.yaml | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/CryptoPkg/CryptoPkg.ci.yaml b/CryptoPkg/CryptoPkg.ci.yaml
> >> index f961d85927c0..3bbb220d3224 100644
> >> --- a/CryptoPkg/CryptoPkg.ci.yaml
> >> +++ b/CryptoPkg/CryptoPkg.ci.yaml
> >> @@ -69,6 +69,7 @@
> >>   },
> >>
> >>   "DependencyCheck": {
> >>
> >>   "AcceptableDependencies": [
> >>
> >> +"ArmPkg/ArmPkg.dec",
> >>
> >>   "MdePkg/MdePkg.dec",
> >>
> >>   "MdeModulePkg/MdeModulePkg.dec",
> >>
> >>   "CryptoPkg/CryptoPkg.dec",
> >>
> >> --
> >> 2.25.1
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111545): https://edk2.groups.io/g/devel/message/111545
Mute This Topic: https://groups.io/mt/102725178/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

[PierreGondois]

Hello Jiewen,

On 11/21/23 12:27, Yao, Jiewen wrote:

Why CryptoPkg needs to depend on ArmPkg?

Can we move content to MdePkg?


The OpensslLib needs to discover the native instruction supported by the
underlying platform before using them. This could also be done through the
MdePkg as you suggested. The v2 is implemented that way:
https://edk2.groups.io/g/devel/message/110953

Also, as noted by Leif, it seems there is already a dependency over ArmPkg:
# git grep ArmPkg CryptoPkg/
CryptoPkg/CryptoPkg.dsc:  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
CryptoPkg/CryptoPkg.dsc:  
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
CryptoPkg/CryptoPkg.dsc:  
ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
CryptoPkg/CryptoPkgMbedTls.dsc:  
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
CryptoPkg/CryptoPkgMbedTls.dsc:  
ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
CryptoPkg/CryptoPkgMbedTls.dsc:  
PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf

Both solutions suit me (discovering capabilities through ArmPkg or MdePkg),
I just need to know which one is preferred,

Regards,
Pierre




-Original Message-
From: Pierre Gondois 
Sent: Tuesday, November 21, 2023 4:47 PM
To: devel@edk2.groups.io
Cc: Yao, Jiewen ; Li, Yi1 ; Lu, Xiaoyu1
; Jiang, Guomin ; Leif Lindholm
; Ard Biesheuvel ;
Sami Mujawar ; Gerd Hoffmann

Subject: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon
ArmPkg

Allow dependency upon ArmPkg to pass the dependency Check.

Signed-off-by: Pierre Gondois 
---
  CryptoPkg/CryptoPkg.ci.yaml | 1 +
  1 file changed, 1 insertion(+)

diff --git a/CryptoPkg/CryptoPkg.ci.yaml b/CryptoPkg/CryptoPkg.ci.yaml
index f961d85927c0..3bbb220d3224 100644
--- a/CryptoPkg/CryptoPkg.ci.yaml
+++ b/CryptoPkg/CryptoPkg.ci.yaml
@@ -69,6 +69,7 @@
  },

  "DependencyCheck": {

  "AcceptableDependencies": [

+"ArmPkg/ArmPkg.dec",

  "MdePkg/MdePkg.dec",

  "MdeModulePkg/MdeModulePkg.dec",

  "CryptoPkg/CryptoPkg.dec",

--
2.25.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111544): https://edk2.groups.io/g/devel/message/111544
Mute This Topic: https://groups.io/mt/102725178/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: 回复: edk2-stable202311: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix ETE _UID Creation

[Sami Mujawar]

Hi Liming,

On 21/11/2023, 01:05, "gaoliming" mailto:gaolim...@byosoft.com.cn>> wrote:


Sami:
I am OK to merge this patch for this stable bug. I will add push label for 
https://github.com/tianocore/edk2/pull/5061 
 
[SAMI] Thank you for picking this patch for the stable tag. Apparently, the 
patch does not seem to have merged. 
I am not sure if you have already applied the push label. Please let me know if 
there are any issues that I can help with.

Regards,

Sami Mujawar

Thanks
Liming
> -邮件原件-
> 发件人: Sami Mujawar mailto:sami.muja...@arm.com>>
> 发送时间: 2023年11月21日 0:07
> 收件人: Laszlo Ersek mailto:ler...@redhat.com>>; Liming Gao 
> (Byosoft address)
> mailto:gaolim...@byosoft.com.cn>>; Michael Kinney 
> mailto:michael.d.kin...@intel.com>>;
> devel@edk2.groups.io ; ashishsin...@nvidia.com 
> ; quic_llind...@quicinc.com 
> ;
> ardb+tianoc...@kernel.org ; 
> jbra...@nvidia.com 
> 抄送: n...@arm.com 
> 主题: edk2-stable202311: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Fix
> ETE _UID Creation
> 
> Hi Laszlo, Liming, Mike,
> 
> Please see my response inline marked [SAMI].
> 
> Regards,
> 
> Sami Mujawar
> 
> On 17/11/2023 09:20 am, Laszlo Ersek wrote:
> > On 11/15/23 04:19, Ashish Singhal via groups.io wrote:
> >> Just like CPU _UID, ETE UID also needs to be unique so
> >> use AcpiProcessorUid instead of CpuName
> >>
> >> Signed-off-by: Ashish Singhal  >> >
> >> ---
> >> .../Arm/AcpiSsdtCpuTopologyLibArm/SsdtCpuTopologyGenerator.c | 5
> -
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> > Is this a fixup for the recent feature
> >
> > [PATCH v3 00/11] Update MADT for ACPI 6.5, and add TRBE & ETE support
> > https://edk2.groups.io/g/devel/message/108996 
> > 
> >
> > ?
> >
> > If so, then I *think* this qualifies to be merged during the hard
> > feature freeze (+Liming +Mike), but:
> 
> [SAMI] I raised a bugzilla for this issue at
> https://bugzilla.tianocore.org/show_bug.cgi?id=4600 
> 
> 
> and have also created a pull request at
> https://github.com/tianocore/edk2/pull/5061 
> .
> 
> This patch has also passed the CI checks when I did a draft pull request.
> 
> I am not sure if I can apply the push label as we are in the code freeze
> stage.
> 
> Can you advise on how to proceed, please?
> 
> [/SAMI]
> 
> > - I think we should have a "Fixes:" tag in the commit message (for
> > pointing out the commit that should have contained the code being
> > added/updated now)
> >
> > - I think we should have a BZ too (also linked into the commit message).
> >
> > Laszlo
> >









-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111543): https://edk2.groups.io/g/devel/message/111543
Mute This Topic: https://groups.io/mt/102720225/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

[Yao, Jiewen]

Why CryptoPkg needs to depend on ArmPkg?

Can we move content to MdePkg?

> -Original Message-
> From: Pierre Gondois 
> Sent: Tuesday, November 21, 2023 4:47 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen ; Li, Yi1 ; Lu, 
> Xiaoyu1
> ; Jiang, Guomin ; Leif Lindholm
> ; Ard Biesheuvel ;
> Sami Mujawar ; Gerd Hoffmann
> 
> Subject: [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon
> ArmPkg
> 
> Allow dependency upon ArmPkg to pass the dependency Check.
> 
> Signed-off-by: Pierre Gondois 
> ---
>  CryptoPkg/CryptoPkg.ci.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/CryptoPkg/CryptoPkg.ci.yaml b/CryptoPkg/CryptoPkg.ci.yaml
> index f961d85927c0..3bbb220d3224 100644
> --- a/CryptoPkg/CryptoPkg.ci.yaml
> +++ b/CryptoPkg/CryptoPkg.ci.yaml
> @@ -69,6 +69,7 @@
>  },
> 
>  "DependencyCheck": {
> 
>  "AcceptableDependencies": [
> 
> +"ArmPkg/ArmPkg.dec",
> 
>  "MdePkg/MdePkg.dec",
> 
>  "MdeModulePkg/MdeModulePkg.dec",
> 
>  "CryptoPkg/CryptoPkg.dec",
> 
> --
> 2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111542): https://edk2.groups.io/g/devel/message/111542
Mute This Topic: https://groups.io/mt/102725178/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/3] ShellPkg/UefiShellDebug1CommandsLib: remove gEfiMpsTableGuid ref from DMEM

[Gao, Zhichao]

I would leave the decision to the right owner of LegacyBiosMpTable.h in MdePkg. 
Once pass review on MdePkg, I am fine with this patch.

Thanks,
Zhichao

> -Original Message-
> From: Laszlo Ersek 
> Sent: Thursday, November 9, 2023 8:06 PM
> To: devel@edk2.groups.io
> Cc: Gao, Zhichao 
> Subject: [PATCH 2/3] ShellPkg/UefiShellDebug1CommandsLib: remove
> gEfiMpsTableGuid ref from DMEM
> 
> We're removing . First, remove the
> gEfiMpsTableGuid system config table reference from the UEFI Shell's DMEM
> debug command.
> 
> Cc: Zhichao Gao 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1754
> Signed-off-by: Laszlo Ersek 
> ---
> 
> ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands
> Lib.uni | 1 -
> ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands
> Lib.inf | 1 -
>  ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c | 
> 9 
> -
>  3 files changed, 11 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> index 4041f0cd483e..155efc1a82f1 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> d
> +++ sLib.uni
> @@ -112,7 +112,6 @@
>"SAL System Table  
> %016LX\r\n"
>"ACPI Table
> %016LX\r\n"
>"ACPI 2.0 Table
> %016LX\r\n"
> -  "MPS Table 
> %016LX\r\n"
>"SMBIOS Table  
> %016LX\r\n"
>"DTB Table 
> %016LX\r\n"
>"Memory Attribute Table
> %016LX\r\n"
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.inf
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.inf
> index 3741dac5d94c..2074cb7d96da 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.inf
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> d
> +++ sLib.inf
> @@ -127,7 +127,6 @@ [Guids]
>gEfiGlobalVariableGuid  ## SOMETIMES_CONSUMES ## GUID
>gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ## SystemTable
>gEfiSmbios3TableGuid## SOMETIMES_CONSUMES ## SystemTable
> -  gEfiMpsTableGuid## SOMETIMES_CONSUMES ## SystemTable
>gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable
>gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable
>gShellDebug1HiiGuid ## SOMETIMES_CONSUMES ## HII
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
> index a609971f345e..39a59c195512 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dmem.c
> @@ -12,7 +12,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include   #include
>  @@ -113,7 +112,6 @@
> ShellCommandRunDmem (
>UINT64Acpi20TableAddress;
>UINT64SalTableAddress;
>UINT64SmbiosTableAddress;
> -  UINT64MpsTableAddress;
>UINT64DtbTableAddress;
>UINT64MemoryAttributesTableAddress;
>UINT64RtPropertiesTableAddress;
> @@ -190,7 +188,6 @@ ShellCommandRunDmem (
>AcpiTableAddress   = 0;
>SalTableAddress= 0;
>SmbiosTableAddress = 0;
> -  MpsTableAddress= 0;
>DtbTableAddress= 0;
>MemoryAttributesTableAddress   = 0;
>RtPropertiesTableAddress   = 0;
> @@ -224,11 +221,6 @@ ShellCommandRunDmem (
>continue;
>  }
> 
> -if (CompareGuid (
> >ConfigurationTable[TableWalker].VendorGuid, )) {
> -  MpsTableAddress = (UINT64)(UINTN)gST-
> >ConfigurationTable[TableWalker].VendorTable;
> -  continue;
> -}
> -
>  if (CompareGuid (
> >ConfigurationTable[TableWalker].VendorGuid,
> )) {
>MemoryAttributesTableAddress = (UINT64)(UINTN)gST-
> >ConfigurationTable[TableWalker].VendorTable;
>continue;
> @@ -292,7 +284,6 @@ ShellCommandRunDmem (
>  SalTableAddress,
>  AcpiTableAddress,
>  Acpi20TableAddress,
> -MpsTableAddress,
>  SmbiosTableAddress,
>  DtbTableAddress,
>  MemoryAttributesTableAddress,



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io 

Re: [edk2-devel] [PATCH v4 5/6] CryptoPkg/OpensslLib: Add AArch64Cap for arch specific hooks

[PierreGondois]

On 11/20/23 19:39, Leif Lindholm wrote:

On Mon, Nov 20, 2023 at 14:48:25 +0100, Pierre Gondois wrote:

Add AARCH64 specific implementations of:
- OPENSSL_cpuid_setup(), probing hardware capabilitie
   (presence of FEAT_AES, etc.)
- OPENSSL_rdtsc(), returning non-trusted entropy by accessing
   system counter.

Acked-by: Gerd Hoffmann 
Signed-off-by: Pierre Gondois 
---
  .../Library/OpensslLib/OpensslLibAccel.inf|  7 ++
  .../OpensslLib/OpensslLibFullAccel.inf|  7 ++
  .../OpensslLib/OpensslStub/AArch64Cap.c   | 84 +++
  3 files changed, 98 insertions(+)
  create mode 100644 CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c

diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
index 3d1a9638b1c1..08e8be6ea9e1 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
@@ -1329,6 +1329,7 @@ [Sources.X64]
  # Autogenerated files list ends here
  
  [Sources.AARCH64]

+  OpensslStub/AArch64Cap.c
  # Autogenerated files list starts here
$(OPENSSL_PATH)/crypto/aes/aes_cbc.c
$(OPENSSL_PATH)/crypto/aes/aes_cfb.c
@@ -1955,11 +1956,17 @@ [Packages]
MdePkg/MdePkg.dec
CryptoPkg/CryptoPkg.dec
  
+[Packages.AARCH64]

+  ArmPkg/ArmPkg.dec
+
  [LibraryClasses]
BaseLib
DebugLib
RngLib
  
+[LibraryClasses.AARCH64]

+  ArmLib
+
  [BuildOptions]
#
# Disables the following Visual Studio compiler warnings brought by openssl 
source,
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
index e7e83d419f4b..2a01ffe06bd7 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
@@ -1432,6 +1432,7 @@ [Sources.X64]
  # Autogenerated files list ends here
  
  [Sources.AARCH64]

+  OpensslStub/AArch64Cap.c
  # Autogenerated files list starts here
$(OPENSSL_PATH)/crypto/aes/aes_cbc.c
$(OPENSSL_PATH)/crypto/aes/aes_cfb.c
@@ -2107,11 +2108,17 @@ [Packages]
MdePkg/MdePkg.dec
CryptoPkg/CryptoPkg.dec
  
+[Packages.AARCH64]

+  ArmPkg/ArmPkg.dec
+
  [LibraryClasses]
BaseLib
DebugLib
RngLib
  
+[LibraryClasses.AARCH64]

+  ArmLib
+
  [BuildOptions]
#
# Disables the following Visual Studio compiler warnings brought by openssl 
source,
diff --git a/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c 
b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
new file mode 100644
index ..7468ef3ab54e
--- /dev/null
+++ b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
@@ -0,0 +1,84 @@
+/** @file
+  Arm capabilities probing.
+
+  Copyright (c) 2023, Arm Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include "crypto/arm_arch.h"
+
+#include 
+
+/** Get bits from a value.
+
+  Shift the input value from 'shift' bits and apply 'mask'.
+
+  @param   valueThe value to get the bits from.
+  @param   shiftIndex of the bits to read.
+  @param   mask Mask to apply to the value once shifted.
+
+  @return  The desired bitfield from the value.
+**/
+#define GET_BITFIELD(value, shift, mask)\
+  ((value >> shift) & mask)


(This macro appears unused here now.)


+
+UINT32  OPENSSL_armcap_P = 0;
+
+void
+OPENSSL_cpuid_setup (
+  void
+  )
+{
+  OPENSSL_armcap_P = 0;
+
+  /* Access to EL0 registers is possible from higher ELx. */
+  OPENSSL_armcap_P |= ARMV8_CPUID;
+  /* Access to Physical timer is possible. */
+  OPENSSL_armcap_P |= ARMV7_TICK;
+
+  /* Neon support is not guaranteed, but it is assumed to be present.
+ Arm ARM for Armv8, sA1.5 Advanced SIMD and floating-point support
+  */
+  OPENSSL_armcap_P |= ARMV7_NEON;
+
+  if (ArmHasAes ())
+  {


And I think that curly bracket is supposed to be on the previous line
(and similarly below), but this may be intional to align with nearby
code?


I think it was changed by uncrustify. I changed it back.
Thanks for the review,

Regards,
Pierre



Anyway, this is a big readability improvement, thank you very much!
Acked-by: Leif Lindholm 

/
 Leif


+OPENSSL_armcap_P |= ARMV8_AES;
+  }
+
+  if (ArmHasSha1 ())
+  {
+OPENSSL_armcap_P |= ARMV8_SHA1;
+  }
+
+  if (ArmHasSha256 ())
+  {
+OPENSSL_armcap_P |= ARMV8_SHA256;
+  }
+
+  if (ArmHasPmull ())
+  {
+OPENSSL_armcap_P |= ARMV8_PMULL;
+  }
+
+  if (ArmHasSha512 ())
+  {
+OPENSSL_armcap_P |= ARMV8_SHA512;
+  }
+}
+
+/** Read system counter value.
+
+  Used to get some non-trusted entropy.
+
+  @return Lower bits of the physical counter.
+**/
+uint32_t
+OPENSSL_rdtsc (
+  void
+  )
+{
+  return (UINT32)ArmReadCntPct ();
+}
--
2.25.1




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111540): https://edk2.groups.io/g/devel/message/111540
Mute This Topic: https://groups.io/mt/102707267/21656
Group Owner: devel+ow...@edk2.groups.io

[edk2-devel] [PATCH v5 6/6] CryptoPkg: Enable Openssl Accel builds for AARCH64

[PierreGondois]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4601

Enable the following modules builds for AARCH64:
- OpensslLibAccel.inf
- OpensslLibFullAccel.inf

Acked-by: Gerd Hoffmann 
Signed-off-by: Pierre Gondois 
---
 CryptoPkg/CryptoPkg.dsc | 23 ---
 CryptoPkg/Readme.md | 14 +++---
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
index 6a0104a3bb68..f08808d5d18e 100644
--- a/CryptoPkg/CryptoPkg.dsc
+++ b/CryptoPkg/CryptoPkg.dsc
@@ -5,6 +5,7 @@
 #  Copyright (c) 2009 - 2022, Intel Corporation. All rights reserved.
 #  Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights 
reserved.
 #  Copyright (c) 2022, Loongson Technology Corporation Limited. All rights 
reserved.
+#  Copyright (c) 2023, Arm Limited. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -327,7 +328,7 @@ [Components.IA32, Components.X64, Components.ARM, 
Components.AARCH64]
   MSFT:NOOPT_*_*_DLINK_FLAGS = 
/EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x1
   }
 
-[Components.IA32, Components.X64]
+[Components.IA32, Components.X64, Components.AARCH64]
   CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf {
 
   FILE_GUID = B91B9A95-4D52-4501-A98F-A1711C14ED93
@@ -396,9 +397,9 @@ [Components]
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
   }
 
-[Components.IA32, Components.X64]
+[Components.IA32, Components.X64, Components.AARCH64]
   #
-  # Build verification of IA32/X64 specific libraries
+  # Build verification of IA32/X64/AARCH64 specific libraries
   #
   CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
   CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
@@ -439,9 +440,9 @@ [Components.IA32, Components.X64, Components.ARM, 
Components.AARCH64]
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibFull.inf
   }
 
-[Components.IA32, Components.X64]
+[Components.IA32, Components.X64, Components.AARCH64]
   #
-  # CryptoPei with IA32/X64 performance optimized OpensslLib instance without 
EC services
+  # CryptoPei with IA32/X64/AARCH64 performance optimized OpensslLib instance 
without EC services
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoPei.inf {
@@ -455,7 +456,7 @@ [Components.IA32, Components.X64]
   }
 
   #
-  # CryptoPei with IA32/X64 performance optimized OpensslLib instance all 
services
+  # CryptoPei with IA32/X64/AARCH64 performance optimized OpensslLib instance 
all services
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoPei.inf {
@@ -505,9 +506,9 @@ [Components.IA32, Components.X64, Components.ARM, 
Components.AARCH64]
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibFull.inf
   }
 
-[Components.IA32, Components.X64]
+[Components.IA32, Components.X64, Components.AARCH64]
   #
-  # CryptoDxe with IA32/X64 performance optimized OpensslLib instance with no 
EC services
+  # CryptoDxe with IA32/X64/AARCH64 performance optimized OpensslLib instance 
with no EC services
   # with TLS feature enabled.
   # IA32/X64 assembly optimizations required larger alignments
   #
@@ -521,7 +522,7 @@ [Components.IA32, Components.X64]
   MSFT:*_*_X64_DLINK_FLAGS  = /ALIGN:256
   }
   #
-  # CryptoDxe with IA32/X64 performance optimized OpensslLib instance with all 
services.
+  # CryptoDxe with IA32/X64/AARCH64 performance optimized OpensslLib instance 
with all services.
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoDxe.inf {
@@ -561,7 +562,7 @@ [Components.IA32, Components.X64]
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibFull.inf
   }
   #
-  # CryptoSmm with IA32/X64 performance optimized OpensslLib instance with no 
EC services
+  # CryptoSmm with IA32/X64/AARCH64 performance optimized OpensslLib instance 
with no EC services
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoSmm.inf {
@@ -574,7 +575,7 @@ [Components.IA32, Components.X64]
   MSFT:*_*_X64_DLINK_FLAGS  = /ALIGN:256
   }
   #
-  # CryptoSmm with IA32/X64 performance optimized OpensslLib instance with all 
services
+  # CryptoSmm with IA32/X64/AARCH64 performance optimized OpensslLib instance 
with all services
   # IA32/X64 assembly optimizations required larger alignments
   #
   CryptoPkg/Driver/CryptoSmm.inf {
diff --git a/CryptoPkg/Readme.md b/CryptoPkg/Readme.md
index 5a68dfb6abd4..cb2228b6b898 100644
--- a/CryptoPkg/Readme.md
+++ b/CryptoPkg/Readme.md
@@ -246,13 +246,13 @@ specific set of enabled cryptographic services. If ECC 
services are not
 required, then the size can be reduced by using OpensslLib.inf instead of
 `OpensslLibFull.inf`. Performance optimization requires a size increase.
 
-| OpensslLib Instance | SSL | ECC | Perf Opt | CPU Arch | Size  |
-|:|:---:|:---:|::|::|:-:|
-| OpensslLibCrypto.inf

[edk2-devel] [PATCH v5 5/6] CryptoPkg/OpensslLib: Add AArch64Cap for arch specific hooks

[PierreGondois]

Add AARCH64 specific implementations of:
- OPENSSL_cpuid_setup(), probing hardware capabilitie
  (presence of FEAT_AES, etc.)
- OPENSSL_rdtsc(), returning non-trusted entropy by accessing
  system counter.

Acked-by: Gerd Hoffmann 
Acked-by: Leif Lindholm 
Signed-off-by: Pierre Gondois 
---
 .../Library/OpensslLib/OpensslLibAccel.inf|  7 ++
 .../OpensslLib/OpensslLibFullAccel.inf|  7 ++
 .../OpensslLib/OpensslStub/AArch64Cap.c   | 66 +++
 3 files changed, 80 insertions(+)
 create mode 100644 CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c

diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
index 3d1a9638b1c1..08e8be6ea9e1 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
@@ -1329,6 +1329,7 @@ [Sources.X64]
 # Autogenerated files list ends here
 
 [Sources.AARCH64]
+  OpensslStub/AArch64Cap.c
 # Autogenerated files list starts here
   $(OPENSSL_PATH)/crypto/aes/aes_cbc.c
   $(OPENSSL_PATH)/crypto/aes/aes_cfb.c
@@ -1955,11 +1956,17 @@ [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
 
+[Packages.AARCH64]
+  ArmPkg/ArmPkg.dec
+
 [LibraryClasses]
   BaseLib
   DebugLib
   RngLib
 
+[LibraryClasses.AARCH64]
+  ArmLib
+
 [BuildOptions]
   #
   # Disables the following Visual Studio compiler warnings brought by openssl 
source,
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
index e7e83d419f4b..2a01ffe06bd7 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
@@ -1432,6 +1432,7 @@ [Sources.X64]
 # Autogenerated files list ends here
 
 [Sources.AARCH64]
+  OpensslStub/AArch64Cap.c
 # Autogenerated files list starts here
   $(OPENSSL_PATH)/crypto/aes/aes_cbc.c
   $(OPENSSL_PATH)/crypto/aes/aes_cfb.c
@@ -2107,11 +2108,17 @@ [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
 
+[Packages.AARCH64]
+  ArmPkg/ArmPkg.dec
+
 [LibraryClasses]
   BaseLib
   DebugLib
   RngLib
 
+[LibraryClasses.AARCH64]
+  ArmLib
+
 [BuildOptions]
   #
   # Disables the following Visual Studio compiler warnings brought by openssl 
source,
diff --git a/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c 
b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
new file mode 100644
index ..846b004a4dd3
--- /dev/null
+++ b/CryptoPkg/Library/OpensslLib/OpensslStub/AArch64Cap.c
@@ -0,0 +1,66 @@
+/** @file
+  Arm capabilities probing.
+
+  Copyright (c) 2023, Arm Limited. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include "crypto/arm_arch.h"
+
+#include 
+
+UINT32  OPENSSL_armcap_P = 0;
+
+void
+OPENSSL_cpuid_setup (
+  void
+  )
+{
+  OPENSSL_armcap_P = 0;
+
+  /* Access to EL0 registers is possible from higher ELx. */
+  OPENSSL_armcap_P |= ARMV8_CPUID;
+  /* Access to Physical timer is possible. */
+  OPENSSL_armcap_P |= ARMV7_TICK;
+
+  /* Neon support is not guaranteed, but it is assumed to be present.
+ Arm ARM for Armv8, sA1.5 Advanced SIMD and floating-point support
+  */
+  OPENSSL_armcap_P |= ARMV7_NEON;
+
+  if (ArmHasAes ()) {
+OPENSSL_armcap_P |= ARMV8_AES;
+  }
+
+  if (ArmHasSha1 ()) {
+OPENSSL_armcap_P |= ARMV8_SHA1;
+  }
+
+  if (ArmHasSha256 ()) {
+OPENSSL_armcap_P |= ARMV8_SHA256;
+  }
+
+  if (ArmHasPmull ()) {
+OPENSSL_armcap_P |= ARMV8_PMULL;
+  }
+
+  if (ArmHasSha512 ()) {
+OPENSSL_armcap_P |= ARMV8_SHA512;
+  }
+}
+
+/** Read system counter value.
+
+  Used to get some non-trusted entropy.
+
+  @return Lower bits of the physical counter.
+**/
+uint32_t
+OPENSSL_rdtsc (
+  void
+  )
+{
+  return (UINT32)ArmReadCntPct ();
+}
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111538): https://edk2.groups.io/g/devel/message/111538
Mute This Topic: https://groups.io/mt/102725181/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v5 3/6] CryptoPkg/OpensslLib: Add native instruction support for AARCH64

[PierreGondois]

Add native instruction support for AARCH64.

Acked-by: Gerd Hoffmann 
Signed-off-by: Pierre Gondois 
---
 .../Library/OpensslLib/OpensslLibAccel.inf| 21 ++-
 .../OpensslLib/OpensslLibFullAccel.inf| 21 ++-
 CryptoPkg/Library/OpensslLib/UefiAsm.conf |  6 ++
 CryptoPkg/Library/OpensslLib/configure.py |  6 +-
 4 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
index a37347fbbfd6..a47e9087133b 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibAccel.inf
@@ -5,6 +5,7 @@
 #
 #  Copyright (c) 2010 - 2020, Intel Corporation. All rights reserved.
 #  (C) Copyright 2020 Hewlett Packard Enterprise Development LP
+#  Copyright (c) 2023, Arm Limited. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -24,9 +25,10 @@ [Defines]
   DEFINE OPENSSL_FLAGS   = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT 
-D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DEDK2_OPENSSL_NOEC=1
   DEFINE OPENSSL_FLAGS_IA32  = -DAES_ASM -DGHASH_ASM -DMD5_ASM 
-DOPENSSL_CPUID_OBJ -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DVPAES_ASM
   DEFINE OPENSSL_FLAGS_X64   = -DAES_ASM -DBSAES_ASM -DGHASH_ASM 
-DKECCAK1600_ASM -DMD5_ASM -DOPENSSL_CPUID_OBJ -DSHA1_ASM -DSHA256_ASM 
-DSHA512_ASM -DVPAES_ASM
+  DEFINE OPENSSL_FLAGS_AARCH64   =
 
 #
-#  VALID_ARCHITECTURES   = IA32 X64
+#  VALID_ARCHITECTURES   = IA32 X64 AARCH64
 #
 
 [Sources]
@@ -1326,6 +1328,10 @@ [Sources.X64]
   $(OPENSSL_GEN_PATH)/X64-GCC/crypto/sha/sha512-x86_64.s | GCC
 # Autogenerated files list ends here
 
+[Sources.AARCH64]
+# Autogenerated files list starts here
+# Autogenerated files list ends here
+
 [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
@@ -1403,3 +1409,16 @@ [BuildOptions]
   #   commit d9b8b89bec4480de3a10bdaf9425db371c19145b, and can be dropped 
then.)
   XCODE:*_*_IA32_CC_FLAGS   = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) 
$(OPENSSL_FLAGS_IA32) -w -std=c99 -Wno-error=uninitialized
   XCODE:*_*_X64_CC_FLAGS= -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) 
$(OPENSSL_FLAGS_X64) -w -std=c99 -Wno-error=uninitialized
+
+  GCC:*_*_AARCH64_CC_FLAGS= $(OPENSSL_FLAGS) $(OPENSSL_FLAGS_AARCH64) 
-Wno-error=format -Wno-format -D_BITS_STDINT_UINTN_H -D_BITS_STDINT_INTN_H
+
+  #
+  # AARCH64 uses strict alignment and avoids SIMD registers for code that may 
execute
+  # with the MMU off. This involves SEC, PEI_CORE and PEIM modules as well as 
BASE
+  # libraries, given that they may be included into such modules.
+  # This library, even though of the BASE type, is never used in such cases, 
and
+  # avoiding the SIMD register file (which is shared with the FPU) prevents the
+  # compiler from successfully building some of the OpenSSL source files that
+  # use floating point types, so clear the flags here.
+  #
+  GCC:*_*_AARCH64_CC_XIPFLAGS ==
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf 
b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
index 780d5febd72e..45a58c7d382e 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.inf
@@ -10,6 +10,7 @@
 #
 #  Copyright (c) 2010 - 2020, Intel Corporation. All rights reserved.
 #  (C) Copyright 2020 Hewlett Packard Enterprise Development LP
+#  Copyright (c) 2023, Arm Limited. All rights reserved.
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -29,9 +30,10 @@ [Defines]
   DEFINE OPENSSL_FLAGS   = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT 
-D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE
   DEFINE OPENSSL_FLAGS_IA32  = -DAES_ASM -DGHASH_ASM -DMD5_ASM 
-DOPENSSL_CPUID_OBJ -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DVPAES_ASM
   DEFINE OPENSSL_FLAGS_X64   = -DAES_ASM -DBSAES_ASM -DGHASH_ASM 
-DKECCAK1600_ASM -DMD5_ASM -DOPENSSL_CPUID_OBJ -DSHA1_ASM -DSHA256_ASM 
-DSHA512_ASM -DVPAES_ASM
+  DEFINE OPENSSL_FLAGS_AARCH64   =
 
 #
-#  VALID_ARCHITECTURES   = IA32 X64
+#  VALID_ARCHITECTURES   = IA32 X64 AARCH64
 #
 
 [Sources]
@@ -1429,6 +1431,10 @@ [Sources.X64]
   $(OPENSSL_GEN_PATH)/X64-GCC/crypto/sha/sha512-x86_64.s | GCC
 # Autogenerated files list ends here
 
+[Sources.AARCH64]
+# Autogenerated files list starts here
+# Autogenerated files list ends here
+
 [Packages]
   MdePkg/MdePkg.dec
   CryptoPkg/CryptoPkg.dec
@@ -1506,3 +1512,16 @@ [BuildOptions]
   #   commit d9b8b89bec4480de3a10bdaf9425db371c19145b, and can be dropped 
then.)
   XCODE:*_*_IA32_CC_FLAGS   = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) 
$(OPENSSL_FLAGS_IA32) -w -std=c99 -Wno-error=uninitialized
   XCODE:*_*_X64_CC_FLAGS= -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) 
$(OPENSSL_FLAGS_X64) -w -std=c99 -Wno-error=uninitialized
+
+  GCC:*_*_AARCH64_CC_FLAGS= $(OPENSSL_FLAGS) $(OPENSSL_FLAGS_AARCH64) 
-Wno-error=format 

[edk2-devel] [PATCH v5 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon ArmPkg

[PierreGondois]

Allow dependency upon ArmPkg to pass the dependency Check.

Signed-off-by: Pierre Gondois 
---
 CryptoPkg/CryptoPkg.ci.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CryptoPkg/CryptoPkg.ci.yaml b/CryptoPkg/CryptoPkg.ci.yaml
index f961d85927c0..3bbb220d3224 100644
--- a/CryptoPkg/CryptoPkg.ci.yaml
+++ b/CryptoPkg/CryptoPkg.ci.yaml
@@ -69,6 +69,7 @@
 },
 "DependencyCheck": {
 "AcceptableDependencies": [
+"ArmPkg/ArmPkg.dec",
 "MdePkg/MdePkg.dec",
 "MdeModulePkg/MdeModulePkg.dec",
 "CryptoPkg/CryptoPkg.dec",
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111535): https://edk2.groups.io/g/devel/message/111535
Mute This Topic: https://groups.io/mt/102725178/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v5 0/6] CryptoPkg: Enable Openssl native instruction support for AARCH64

[PierreGondois]

v5:
- [PATCH v5 1/6] ArmPkg/ArmLib: Add macros/helper functions around AA64Isar0 
register
  - Remove unused ArmReadIdAA64Isar0() declaration
- [PATCH v4 5/6] CryptoPkg/OpensslLib: Add AArch64Cap for arch specific hooks
  - Removed unused GET_BITFIELD() macro
  - Indentation

v4:
- Added Acked-by from Gerd
- Created bugzilla at:
  https://bugzilla.tianocore.org/show_bug.cgi?id=4601
- Replaced:
  - [PATCH v3 1/6] ArmPkg/ArmLib: Move ArmReadIdAA64Isar0() to ArmLib
  by:
  - [PATCH v4 1/6] ArmPkg/ArmLib: Add macros/helper functions around AA64Isar0 
register
  and:
  - Moved ARM_ID_AA64ISAR0_EL1_* macros to ArmPkg/Include/Chipset/AArch64.h
  - Added macros to get capabilities described in AA64ISAR0

v3:
- Use ArmPkg's function to access register:
  - Dropped:
- [PATCH v2 1/7] MdePkg/BaseLib: AARCH64: Add ArmReadCntPctReg()
- [PATCH v2 2/7] MdePkg/BaseLib: AARCH64: Add ArmReadIdAA64Isar0Reg()
- [PATCH v2 3/7] MdePkg/BaseRngLib: Prefer ArmReadIdAA64Isar0Reg() over 
ArmReadIdIsar0()
  - Added:
- [PATCH v3 1/6] ArmPkg/ArmLib: Move ArmReadIdAA64Isar0() to ArmLib
- Allow dependency of CryptoPkg over ArmPkg in CI:
  - Added:
- [PATCH v3 2/6] CryptoPkg/CryptoPkg.ci.yaml: Allow dependency upon

v2:
- [PATCH v2 2/7] MdePkg/BaseLib: AARCH64: Add ArmReadIdAA64Isar0Reg()
  - Correct bad mask values in MdePkg/Include/Library/BaseLib.h
- [PATCH v2 4/7] CryptoPkg/OpensslLib: Add native instruction support:
  - Add armcap.c to configure.py:sources_filter_fn() instead of
manually commenting the file in .inf files

Various OpensslLib implementations are available in edk2. The
OpensslLibAccel.inf and OpensslLibFullAccel.inf ones use
architecture specific instructions, e.g. AESE, PMULL, SHA256H, ...,
allowing to improve speed.

Enable support for Aarch64's native instructions:
- Add ArmReadCntPctReg() and ArmReadIdAA64Isar0Reg() to
  Aarch64's BaseLib.
- Generate Aarch64's specific Openssl functions.
- Add a OpensslStub/AArch64Cap.c file to allow Openssl
  to probe Aarch64 native instruction support.

This patch-set only enable support for GCC for now (MSFT support
not added).

 Testing 

The tests run are based on the TestBaseCryptLibShell module.
Each test is run 100 times, then the first 5 values
(considered as warmup) are removed.

The NoAccel column relies on the OpensslLibFull implementation,
the Accel column relies on the OpensslLibFullAccel implementation.
The 'Improvement' column is computed as:
   100 * ('Accel (ns)' - 'NoAccel (ns)') / 'NoAccel (ns)'

The std deviation of the TestVerifyDhGenerateKey is big.
It is due to [1] being called with the 'safe' parameter set, leading
to the prime number taking more time to generate. It requires ~10
iterations when safe=false, ~1000 iterations when safe=true.

The test was run on a Juno-r2. The native Openssl implementation makes
use of the following features (cf. [2]):
- ARMV7_NEON
- ARMV8_AES
- ARMV8_SHA1
- ARMV8_PMULL
- ARMV8_SHA256
and misses:
- ARMV8_SHA512

| TestName |   NoAccel (ns) |   NoAccel std |   Accel 
(ns) |   Accel std |   Improvement |
|:-|---:|--:|-:|:|--:|
| mPkcs7EkuTest|   14757511 | 14370 | 
14947276 |   35677 |   1.28589 |
| mAeadAesGcmTest  | 129667 |  2012 |   
113897 |1366 | -12.1619  |
| mBlockCipherTest |   7325 |   102 | 
6487 |  81 | -11.4403  |
| mAuthenticodeTest|   72852444 |   3097832 | 
67593102 | 3123627 |  -7.21917 |
| mBnTest  | 771921 | 57966 |   
737656 |   61354 |  -4.43893 |
| mDhTest  | 4082083501 |3340300622 |   
3502629757 |  3444890110 | -14.195   |
| mEcTest  |   24666075 |191971 | 
23250301 |  178985 |  -5.73976 |
| mHkdfTest| 848440 |  4295 |   
797966 |4320 |  -5.94904 |
| mHmacTest| 235527 | 36284 |   
204823 |   37936 | -13.0363  |
| mImageTimestampTest  |   12801070 | 18327 | 
12190046 |   23138 |  -4.77323 |
| mOaepTest|   20032245 | 46525 | 
18671388 |   36399 |  -6.79333 |
| mPkcs5Test   | 178624 |  1962 |   
114852 |1376 | -35.7018  |
| mPkcs7Test   |   28464572 | 70683 | 
25282753 |   82616 | -11.1782  |
| mPrngTest| 727013 |  3637 |   
460076 |2668 | -36.717   |
| mRsaCertTest |   39109865 | 90380 | 
36452412 |  220712 |  -6.79484 |
| mRsaTest |   22451367 

[edk2-devel] [PATCH v5 1/6] ArmPkg/ArmLib: Add macros/helper functions around AA64Isar0 register

[PierreGondois]

Add macro definitions and helper functions to access AArch64
capabilities described in the AA64Isar0 register.

Reviewed-by: Leif Lindholm 
Signed-off-by: Pierre Gondois 
---
 ArmPkg/Include/Chipset/AArch64.h   |  60 +++-
 ArmPkg/Include/Library/ArmLib.h| 218 +++-
 ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 367 +
 3 files changed, 643 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Include/Chipset/AArch64.h b/ArmPkg/Include/Chipset/AArch64.h
index 5390bf0a2774..97c20e71a811 100644
--- a/ArmPkg/Include/Chipset/AArch64.h
+++ b/ArmPkg/Include/Chipset/AArch64.h
@@ -1,7 +1,7 @@
 /** @file
 
   Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
-  Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.
+  Copyright (c) 2011 - 2023, Arm Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -127,6 +127,64 @@
 // build for ARMv8.0, we need to define the register here.
 #define ID_AA64MMFR2_EL1  S3_0_C0_C7_2
 
+//
+// Bit shifts for the ID_AA64ISAR0_EL1 register.
+//
+#define ARM_ID_AA64ISAR0_EL1_AES_SHIFT (4U)
+#define ARM_ID_AA64ISAR0_EL1_SHA1_SHIFT(8U)
+#define ARM_ID_AA64ISAR0_EL1_SHA2_SHIFT(12U)
+#define ARM_ID_AA64ISAR0_EL1_CRC32_SHIFT   (16U)
+#define ARM_ID_AA64ISAR0_EL1_ATOMIC_SHIFT  (20U)
+#define ARM_ID_AA64ISAR0_EL1_RDM_SHIFT (28U)
+#define ARM_ID_AA64ISAR0_EL1_SHA3_SHIFT(32U)
+#define ARM_ID_AA64ISAR0_EL1_SM3_SHIFT (36U)
+#define ARM_ID_AA64ISAR0_EL1_SM4_SHIFT (40U)
+#define ARM_ID_AA64ISAR0_EL1_DP_SHIFT  (44U)
+#define ARM_ID_AA64ISAR0_EL1_FHM_SHIFT (48U)
+#define ARM_ID_AA64ISAR0_EL1_TS_SHIFT  (52U)
+#define ARM_ID_AA64ISAR0_EL1_TLB_SHIFT (56U)
+#define ARM_ID_AA64ISAR0_EL1_RNDR_SHIFT(60U)
+
+//
+// Bit masks for the ID_AA64ISAR0_EL1 fields.
+//
+#define ARM_ID_AA64ISAR0_EL1_AES_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SHA1_MASK(0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SHA2_MASK(0xFU)
+#define ARM_ID_AA64ISAR0_EL1_CRC32_MASK   (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_ATOMIC_MASK  (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_RDM_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SHA3_MASK(0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SM3_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_SM4_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_DP_MASK  (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_FHM_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_TS_MASK  (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_TLB_MASK (0xFU)
+#define ARM_ID_AA64ISAR0_EL1_RNDR_MASK(0xFU)
+
+//
+// Bit masks for the ID_AA64ISAR0_EL1 field values.
+//
+#define ARM_ID_AA64ISAR0_EL1_AES_FEAT_AES_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_AES_FEAT_PMULL_MASK  (0x2U)
+#define ARM_ID_AA64ISAR0_EL1_SHA1_FEAT_SHA1_MASK  (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SHA2_FEAT_SHA256_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SHA2_FEAT_SHA512_MASK(0x2U)
+#define ARM_ID_AA64ISAR0_EL1_CRC32_HAVE_CRC32_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_ATOMIC_FEAT_LSE_MASK (0x2U)
+#define ARM_ID_AA64ISAR0_EL1_RDM_FEAT_RDM_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SHA3_FEAT_SHA3_MASK  (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SM3_FEAT_SM3_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_SM4_FEAT_SM4_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_DP_FEAT_DOTPROD_MASK (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_FHM_FEAT_FHM_MASK(0x1U)
+#define ARM_ID_AA64ISAR0_EL1_TS_FEAT_FLAGM_MASK   (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_TS_FEAT_FLAGM2_MASK  (0x2U)
+#define ARM_ID_AA64ISAR0_EL1_TLB_FEAT_TLBIOS_MASK (0x1U)
+#define ARM_ID_AA64ISAR0_EL1_TLB_FEAT_TLBIRANGE_MASK  (0x2U)
+#define ARM_ID_AA64ISAR0_EL1_RNDR_FEAT_RNG_MASK   (0x1U)
+
 #define VECTOR_BASE(tbl)  \
   .section .text.##tbl##,"ax";\
   .align 11;  \
diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 6aa8a48f07f3..25e89228c6a0 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -1,7 +1,7 @@
 /** @file
 
   Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
-  Copyright (c) 2011 - 2016, ARM Ltd. All rights reserved.
+  Copyright (c) 2011 - 2023, Arm Limited. All rights reserved.
   Copyright (c) 2020 - 2021, NUVIA Inc. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -805,6 +805,222 @@ ArmHasEte (
   VOID
   );
 
+/**
+  Checks whether the CPU implements FEAT_AES.
+
+  @retval TRUE  FEAT_AES is implemented.
+  @retval FALSE FEAT_AES is not implemented.
+**/
+BOOLEAN
+EFIAPI
+ArmHasAes (
+  VOID
+  );
+
+/**
+  Checks whether the CPU implements FEAT_PMULL.
+
+  @retval TRUE  FEAT_PMULL is implemented.
+  @retval FALSE FEAT_PMULL is not implemented.
+**/
+BOOLEAN
+EFIAPI
+ArmHasPmull (
+  VOID
+  );
+
+/**
+  Checks whether the CPU implements FEAT_SHA1.
+
+  @retval TRUE  FEAT_SHA1 is implemented.
+  @retval FALSE FEAT_SHA1 is not implemented.
+**/
+BOOLEAN
+EFIAPI
+ArmHasSha1 (
+  VOID
+  );
+
+/**
+  Checks 

Re: [edk2-devel] [PATCH v5 0/6] MPAM ACPI definitions and parser

[Gao, Zhichao]

Sorry, I finished review on my local, but forgot if I gave R-B or not. For 
ShellPkg part: Reviewed-by: Zhichao Gao 

Thanks,
Zhichao

> -Original Message-
> From: Rohit Mathew 
> Sent: Monday, November 20, 2023 8:17 PM
> To: devel@edk2.groups.io; Rohit Mathew 
> Cc: Kinney, Michael D ; Gao, Liming
> ; Thomas Abraham
> ; Sami Mujawar ;
> James Morse ; Ni, Ray ; Gao,
> Zhichao ; nd 
> Subject: RE: [edk2-devel] [PATCH v5 0/6] MPAM ACPI definitions and parser
> 
> Hi,
> 
> Gentle reminder on the series.
> 
> Regards,
> Rohit
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Rohit
> > Mathew via groups.io
> > Sent: Monday, October 2, 2023 6:14 PM
> > To: devel@edk2.groups.io
> > Cc: Michael D Kinney ; Liming Gao
> > ; Thomas Abraham
> ;
> > Sami Mujawar ; James Morse
> > ; Ray Ni ; Zhichao Gao
> > 
> > Subject: [edk2-devel] [PATCH v5 0/6] MPAM ACPI definitions and parser
> >
> > This series adds the following
> >  -  definitions corresponding to MPAM ACPI 2.0 specification.
> >  -  MPAM parser
> >
> > An MPAM ACPI table formulated using the newly added MPAM ACPI
> > definitions were validated on the linux kernel tree at [1]. The same
> > table was parsed via acpiview using the newly added parser. Certain
> > aspects of the MPAM ACPI specification are still not implemented by
> > the kernel tree. These aspects were verified only using acpiview.
> >
> > Changes since V1:
> >  -  Addressed comments on MPAM ACPI definitions from Sami.
> >  -  V1 did not incorporate the parser. V2 has this implemented.
> >
> > Changes since V2:
> >  -  Addressed comments related to Interface/Link type defintions from
> Sami.
> >
> > Changes since v3:
> >  -  Addressed comments from Pierre
> >
> > Changes since v4:
> >  -  Addressed comments from Sami regarding Parser.
> >
> > Changes can be found at:
> > https://github.com/rohit-arm/edk2/tree/mpam_acpi
> >
> > CI results can be found at:
> > https://github.com/tianocore/edk2/pull/4748
> >
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/log/?h
> > =mpa
> > m/snapshot/v6.5-rc1
> >
> >
> > Rohit Mathew (6):
> >   MdePkg/IndustryStandard: Add definitions for MPAM ACPI specification
> >   ShellPkg/AcpiView: Update field-validator prototype
> >   ShellPkg/AcpiView: Update print-formatter prototype
> >   ShellPkg: acpiview: Add routine to print 16 chars
> >   ShellPkg: acpiview: Add routines to print reserved fields
> >   ShellPkg/AcpiView: Add MPAM Parser
> >
> >  MdePkg/Include/IndustryStandard/Acpi65.h   
> >   |7 +-
> >  MdePkg/Include/IndustryStandard/Mpam.h 
> >   |  246
> 
> >  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c  
> >   |
> 199
> > ++-
> >  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h  
> >   |
> > 109 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Aest/AestParser.c
> > |   36 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> > |8 +-
> >
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Erst/ErstParser.c
> > |   28 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> > |   24 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> > |   14 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Hmat/HmatParser.
> c
> > |   12 +-
> >
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
> > |   32 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c
> > |   18 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamPars
> er
> > .c   | 1276 
> >
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pcct/PcctParser.c
> > |   56 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c
> > |   38 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
> > |   14 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
> > |   14 +-
> >
> > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
> > |   26 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> > dLib.c   |3 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> > dLib.inf |3 +-
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> > dLib.uni |3 +-
> >  21 files changed, 2033 insertions(+), 133 deletions(-)  create mode
> > 100644 MdePkg/Include/IndustryStandard/Mpam.h
> >  create mode 100644
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Mpam/MpamPars
> er
> > .c
> >
> > --
> > 2.34.1
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#109258):
> > https://edk2.groups.io/g/devel/message/109258
> > Mute This