Re: [edk2-devel] [edk2-platforms][PATCH 0/2] IpmiFeaturePkg: compilation fix

2024-06-21 Thread Nate DeSimone
The series has been pushed as 223bed9~..e70b333

-Original Message-
From: Mike Maslenkin  
Sent: Tuesday, June 18, 2024 3:32 PM
To: devel@edk2.groups.io; Desimone, Nathaniel L 
; gaolim...@byosoft.com.cn
Cc: Mike Maslenkin 
Subject: [edk2-platforms][PATCH 0/2] IpmiFeaturePkg: compilation fix

Signed-off-by: Mike Maslenkin 
Cc: Nate DeSimone 
Cc: Liming Gao 



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




Re: [edk2-devel] [edk2-platforms][PATCH 0/2] IpmiFeaturePkg: compilation fix

2024-06-21 Thread Nate DeSimone
Series Reviewed-by: Nate DeSimone 

-Original Message-
From: Mike Maslenkin  
Sent: Tuesday, June 18, 2024 3:32 PM
To: devel@edk2.groups.io; Desimone, Nathaniel L 
; gaolim...@byosoft.com.cn
Cc: Mike Maslenkin 
Subject: [edk2-platforms][PATCH 0/2] IpmiFeaturePkg: compilation fix

Signed-off-by: Mike Maslenkin 
Cc: Nate DeSimone 
Cc: Liming Gao 



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




[edk2-devel] [PATCH edk2-platforms v2 1/1] SbsaQemu: reformat all sources using uncrustify

2024-06-21 Thread Marcin Juszkiewicz
uncrustify is required in EDK2 repository. SbsaQemu (and other platforms
in edk2-platforms) code was free from using it IIRC.

Reformat all files to make new contributions easier. We can recommend
formatting sources without generating extra work for developers.

Signed-off-by: Marcin Juszkiewicz 
---
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h |  50 +-
 .../Include/IndustryStandard/SbsaQemuAcpi.h   |  86 +--
 .../SbsaQemuPlatformVersion.h |   2 +-
 .../Include/IndustryStandard/SbsaQemuSmc.h|  14 +-
 .../Include/Library/HardwareInfoLib.h |   8 +-
 .../Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c |  78 +--
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 490 +-
 .../SbsaQemuHighMemDxe/SbsaQemuHighMemDxe.c   |   8 +-
 .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c |  88 ++--
 .../SbsaQemuSmbiosDxe/SbsaQemuSmbiosDxe.c |  94 ++--
 .../SbsaQemuHardwareInfoLib.c |  66 +--
 .../Library/SbsaQemuLib/SbsaQemuLib.c |  23 +-
 .../Library/SbsaQemuLib/SbsaQemuMem.c |  49 +-
 .../SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c |  14 +-
 .../SbsaQemuPciHostBridgeLib.c|  90 ++--
 15 files changed, 603 insertions(+), 557 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
index 83a085cd86f4..e5f0748bb16e 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
@@ -11,60 +11,58 @@
 #define SBSAQEMU_ACPI_DXE_H
 
 typedef struct {
-  EFI_ACPI_6_0_IO_REMAPPING_ITS_NODENode;
-  UINT32Identifiers;
+  EFI_ACPI_6_0_IO_REMAPPING_ITS_NODENode;
+  UINT32Identifiers;
 } SBSA_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
 
-typedef struct
-{
-  EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE SmmuNode;
-  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE   SmmuIdMap;
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODESmmuNode;
+  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE  SmmuIdMap;
 } SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
 
-typedef struct
-{
-  EFI_ACPI_6_0_IO_REMAPPING_RC_NODERcNode;
-  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE   RcIdMap;
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_RC_NODE RcNode;
+  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLERcIdMap;
 } SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
 
 typedef struct {
-  EFI_ACPI_6_0_IO_REMAPPING_TABLE   Iort;
-  SBSA_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE   ItsNode;
-  SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE SmmuNode;
-  SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODERcNode;
+  EFI_ACPI_6_0_IO_REMAPPING_TABLE  Iort;
+  SBSA_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE  ItsNode;
+  SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODESmmuNode;
+  SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODE   RcNode;
 } SBSA_IO_REMAPPING_STRUCTURE;
 
 typedef struct {
-  EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE  mGtdt;
-  EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE mGwdt;
+  EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE mGtdt;
+  EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTUREmGwdt;
 } GENERIC_TIMER_DESCRIPTION_TABLES;
 
 #ifndef SYSTEM_TIMER_BASE_ADDRESS
-  #define SYSTEM_TIMER_BASE_ADDRESS MAX_ADDRESS
+#define SYSTEM_TIMER_BASE_ADDRESS  MAX_ADDRESS
 #endif
 
 #define GTDT_TIMER_LEVEL_TRIGGERED  0
 #define GTDT_TIMER_ACTIVE_LOW   
EFI_ACPI_6_3_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY
 #define GTDT_TIMER_ALWAYS_ON
EFI_ACPI_6_3_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY
 
-#define GTDT_GTIMER_FLAGS   (GTDT_TIMER_ACTIVE_LOW | \
+#define GTDT_GTIMER_FLAGS  (GTDT_TIMER_ACTIVE_LOW |  \
  GTDT_TIMER_LEVEL_TRIGGERED | \
  GTDT_TIMER_ALWAYS_ON)
 
-#define SBSA_PLATFORM_WATCHDOG_COUNT1
-#define SBSA_PLATFORM_TIMER_COUNT   (SBSA_PLATFORM_WATCHDOG_COUNT)
+#define SBSA_PLATFORM_WATCHDOG_COUNT  1
+#define SBSA_PLATFORM_TIMER_COUNT (SBSA_PLATFORM_WATCHDOG_COUNT)
 
-#define SBSAQEMU_WDT_REFRESH_FRAME_BASE  0x5001
-#define SBSAQEMU_WDT_CONTROL_FRAME_BASE  0x50011000
-#define SBSAQEMU_WDT_IRQ 48
+#define SBSAQEMU_WDT_REFRESH_FRAME_BASE  0x5001
+#define SBSAQEMU_WDT_CONTROL_FRAME_BASE  0x50011000
+#define SBSAQEMU_WDT_IRQ 48
 
 #define GTDT_WDTIMER_LEVEL_TRIGGERED  0
 #define GTDT_WDTIMER_ACTIVE_HIGH  0
 
-#define GTDT_WDTIMER_FLAGS  (GTDT_WDTIMER_ACTIVE_HIGH | 
GTDT_WDTIMER_LEVEL_TRIGGERED)
+#define GTDT_WDTIMER_FLAGS  (GTDT_WDTIMER_ACTIVE_HIGH | 
GTDT_WDTIMER_LEVEL_TRIGGERED)
 
 #define SBSAQEMU_ACPI_MEMORY_AFFINITY_STRUCTURE_INIT(  
   \
-  ProximityDomain, Base, Length, Flags)
   \
+   
   ProximityDomain, Base, Length, Flags)  

Re: [edk2-devel] [PATCH edk2-platforms 1/1] SbsaQemu: reformat all sources using uncrustify

2024-06-21 Thread Leif Lindholm
On Fri, Jun 21, 2024 at 15:39:01 +0200, Marcin Juszkiewicz wrote:
> uncrustify is required in EDK2 repository. SbsaQemu (and other platforms
> in edk2-platforms) code was free from using it IIRC.
> 
> Reformat all files to make new contributions easier. We can recommend
> formatting sources without generating extra work for developers.
> 
> Signed-off-by: Marcin Juszkiewicz 

Everything in here except one thing is improvement.

> ---
>  .../Include/Library/QemuOpenFwCfgLib.h|   7 +-
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h |  50 +-
>  .../Include/IndustryStandard/SbsaQemuAcpi.h   |  86 +--
>  .../SbsaQemuPlatformVersion.h |   2 +-
>  .../Include/IndustryStandard/SbsaQemuSmc.h|  14 +-
>  .../Include/Library/HardwareInfoLib.h |   8 +-
>  .../BoardBootManagerLib/BoardBootManager.c|  14 +-
>  .../Library/PeiReportFvLib/PeiReportFvLib.c   | 237 -
>  .../QemuOpenBoardPkg/PlatformInitPei/Memory.c |   4 +-
>  .../Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c |  78 +--
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 490 +-
>  .../SbsaQemuHighMemDxe/SbsaQemuHighMemDxe.c   |   8 +-
>  .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c |  88 ++--
>  .../SbsaQemuSmbiosDxe/SbsaQemuSmbiosDxe.c |  94 ++--
>  .../SbsaQemuHardwareInfoLib.c |  66 +--
>  .../Library/SbsaQemuLib/SbsaQemuLib.c |  23 +-
>  .../Library/SbsaQemuLib/SbsaQemuMem.c |  49 +-
>  .../SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c |  14 +-
>  .../SbsaQemuPciHostBridgeLib.c|  90 ++--
>  19 files changed, 735 insertions(+), 687 deletions(-)
> 


> --- a/Platform/Qemu/QemuOpenBoardPkg/Library/PeiReportFvLib/PeiReportFvLib.c
> +++ b/Platform/Qemu/QemuOpenBoardPkg/Library/PeiReportFvLib/PeiReportFvLib.c
> @@ -16,7 +16,7 @@
>  #include 
>  
>  // Use a FV pointer PCD to get a pointer to the FileSystemGuid in the FV 
> header
> -#define PCD_TO_FV_HEADER_FILE_SYSTEM_GUID(Pcd)   
> (&((EFI_FIRMWARE_VOLUME_HEADER *)(UINTN) PcdGet32 (Pcd))->FileSystemGuid)
> +#define PCD_TO_FV_HEADER_FILE_SYSTEM_GUID(Pcd)  
> (&((EFI_FIRMWARE_VOLUME_HEADER *)(UINTN) PcdGet32 (Pcd))->FileSystemGuid)
>  
>  /**
>Reports FVs necessary for MinPlarform pre-memory initialization
> @@ -38,13 +38,14 @@ ReportPreMemFv (
>  
>DEBUG_CODE (
>  for (Index = 0; Status == EFI_SUCCESS; Index++) {
> -  Status = PeiServicesLocatePpi (&gEfiPeiFirmwareVolumeInfo2PpiGuid, 
> Index, &Descriptor, (VOID**) &Ppi);
> -  if (!EFI_ERROR (Status)) {
> -FvHeader = (EFI_FIRMWARE_VOLUME_HEADER*) Ppi->FvInfo;
> -DEBUG ((DEBUG_INFO, "Found FV at 0x%x, size 0x%x\n", FvHeader, 
> FvHeader->FvLength));
> -  }
> +Status = PeiServicesLocatePpi (&gEfiPeiFirmwareVolumeInfo2PpiGuid, 
> Index, &Descriptor, (VOID **)&Ppi);
> +if (!EFI_ERROR (Status)) {
> +  FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)Ppi->FvInfo;
> +  DEBUG ((DEBUG_INFO, "Found FV at 0x%x, size 0x%x\n", FvHeader, 
> FvHeader->FvLength));

Uncrustify gets this hunk wrong.

>  }
> -  );
> +  }
> +
> +);

And this too.

Michael, do you have any known tweaks for this format?

Marcin, since the point was to clean up SbsaQemu, maybe you could
re-run this without including QemuOpenBoardPkg?

Regards,

Leif



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




[edk2-devel] [PATCH edk2-platforms 1/1] SbsaQemu: reformat all sources using uncrustify

2024-06-21 Thread Marcin Juszkiewicz
uncrustify is required in EDK2 repository. SbsaQemu (and other platforms
in edk2-platforms) code was free from using it IIRC.

Reformat all files to make new contributions easier. We can recommend
formatting sources without generating extra work for developers.

Signed-off-by: Marcin Juszkiewicz 
---
 .../Include/Library/QemuOpenFwCfgLib.h|   7 +-
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h |  50 +-
 .../Include/IndustryStandard/SbsaQemuAcpi.h   |  86 +--
 .../SbsaQemuPlatformVersion.h |   2 +-
 .../Include/IndustryStandard/SbsaQemuSmc.h|  14 +-
 .../Include/Library/HardwareInfoLib.h |   8 +-
 .../BoardBootManagerLib/BoardBootManager.c|  14 +-
 .../Library/PeiReportFvLib/PeiReportFvLib.c   | 237 -
 .../QemuOpenBoardPkg/PlatformInitPei/Memory.c |   4 +-
 .../Qemu/SbsaQemu/OemMiscLib/OemMiscLib.c |  78 +--
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 490 +-
 .../SbsaQemuHighMemDxe/SbsaQemuHighMemDxe.c   |   8 +-
 .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c |  88 ++--
 .../SbsaQemuSmbiosDxe/SbsaQemuSmbiosDxe.c |  94 ++--
 .../SbsaQemuHardwareInfoLib.c |  66 +--
 .../Library/SbsaQemuLib/SbsaQemuLib.c |  23 +-
 .../Library/SbsaQemuLib/SbsaQemuMem.c |  49 +-
 .../SbsaQemuNorFlashLib/SbsaQemuNorFlashLib.c |  14 +-
 .../SbsaQemuPciHostBridgeLib.c|  90 ++--
 19 files changed, 735 insertions(+), 687 deletions(-)

diff --git a/Platform/Qemu/QemuOpenBoardPkg/Include/Library/QemuOpenFwCfgLib.h 
b/Platform/Qemu/QemuOpenBoardPkg/Include/Library/QemuOpenFwCfgLib.h
index 8e5f8ccf70e2..828b272ca5ed 100644
--- a/Platform/Qemu/QemuOpenBoardPkg/Include/Library/QemuOpenFwCfgLib.h
+++ b/Platform/Qemu/QemuOpenBoardPkg/Include/Library/QemuOpenFwCfgLib.h
@@ -10,7 +10,6 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
-
 #ifndef QEMU_OPEN_BOARD_PKG_QEMU_FW_CFG_LIB_H_
 #define QEMU_OPEN_BOARD_PKG_QEMU_FW_CFG_LIB_H_
 
@@ -27,7 +26,7 @@
 #define FW_CFG_ID 0x0001
 #define FW_CFG_FILE_DIR   0x0019
 
-#define FW_CFG_QEMU_SIGNATURE SIGNATURE_32('Q', 'E', 'M', 'U')
+#define FW_CFG_QEMU_SIGNATURE  SIGNATURE_32('Q', 'E', 'M', 'U')
 
 typedef struct {
   UINT32Size;
@@ -98,8 +97,8 @@ QemuFwCfgReadBytes (
 EFI_STATUS
 EFIAPI
 QemuFwCfgFindFile (
-  IN  CHAR8  *String,
-  OUT QEMU_FW_CFG_FILE   *FWConfigFile
+  IN  CHAR8 *String,
+  OUT QEMU_FW_CFG_FILE  *FWConfigFile
   );
 
 #endif // QEMU_OPEN_BOARD_PKG_QEMU_FW_CFG_LIB_H_
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
index 83a085cd86f4..e5f0748bb16e 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
@@ -11,60 +11,58 @@
 #define SBSAQEMU_ACPI_DXE_H
 
 typedef struct {
-  EFI_ACPI_6_0_IO_REMAPPING_ITS_NODENode;
-  UINT32Identifiers;
+  EFI_ACPI_6_0_IO_REMAPPING_ITS_NODENode;
+  UINT32Identifiers;
 } SBSA_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE;
 
-typedef struct
-{
-  EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE SmmuNode;
-  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE   SmmuIdMap;
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODESmmuNode;
+  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE  SmmuIdMap;
 } SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE;
 
-typedef struct
-{
-  EFI_ACPI_6_0_IO_REMAPPING_RC_NODERcNode;
-  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE   RcIdMap;
+typedef struct {
+  EFI_ACPI_6_0_IO_REMAPPING_RC_NODE RcNode;
+  EFI_ACPI_6_0_IO_REMAPPING_ID_TABLERcIdMap;
 } SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODE;
 
 typedef struct {
-  EFI_ACPI_6_0_IO_REMAPPING_TABLE   Iort;
-  SBSA_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE   ItsNode;
-  SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODE SmmuNode;
-  SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODERcNode;
+  EFI_ACPI_6_0_IO_REMAPPING_TABLE  Iort;
+  SBSA_EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE  ItsNode;
+  SBSA_EFI_ACPI_6_0_IO_REMAPPING_SMMU3_NODESmmuNode;
+  SBSA_EFI_ACPI_6_0_IO_REMAPPING_RC_NODE   RcNode;
 } SBSA_IO_REMAPPING_STRUCTURE;
 
 typedef struct {
-  EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE  mGtdt;
-  EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE mGwdt;
+  EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE mGtdt;
+  EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTUREmGwdt;
 } GENERIC_TIMER_DESCRIPTION_TABLES;
 
 #ifndef SYSTEM_TIMER_BASE_ADDRESS
-  #define SYSTEM_TIMER_BASE_ADDRESS MAX_ADDRESS
+#define SYSTEM_TIMER_BASE_ADDRESS  MAX_ADDRESS
 #endif
 
 #define GTDT_TIMER_LEVEL_TRIGGERED  0
 #define GTDT_TIMER_ACTIVE_LOW   
EFI_ACPI_6_3_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY
 #define GTDT_TIMER_ALWAYS_ON
EFI_ACPI_6_3_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY
 
-#define GTDT_GTIMER_FLAGS   (GTDT_TIMER_ACTIVE_LOW | \
+#define GTDT_GTIMER_FLAGS  (GTDT_TIME

Re: [edk2-devel] [PATCH edk2-platforms 0/5] Platform,Silicon: drop use of PcdArmArchTimerFreqInHz

2024-06-21 Thread Leif Lindholm
On Thu, Jun 20, 2024 at 16:43:00 +0200, Ard Biesheuvel wrote:
> On Thu, 20 Jun 2024 at 16:33, Leif Lindholm  wrote:
> >
> > Related to https://github.com/tianocore/edk2/pull/5797
> >
> > PcdArmArchTimerFreqInHz is about to be removed, as it is now obsolete.
> > Its functionality has been partially broken, and mostly ignored, for
> > all AArch64 platforms since December 2020.
> >
> > This set cleans up some broken line endings in .dsc* files, then
> > drops all non-invasive references to the Pcd:
> > - .dsc* files setting it to 0 (which is the default in the definition,
> >   and means "just read it from the system register instead")
> > - .inf files declaring a dependency that is in fact not there in
> >   current code.
> >
> > Finally, it drops the setting of the Pcd for platforms that set it to
> > non-0. This has *never* done the right thing on these platforms since
> > they are all AArch64, but it may affect timer timeout, so deserves
> > deeper testing.
> >
> > Cc: Ard Biesheuvel 
> > Cc: Chuong Tran 
> > Cc: Graeme Gregory 
> > Cc: Marcin Juszkiewicz 
> > Cc: Marcin Wojtas 
> > Cc: Meenakshi Aggarwal 
> > Cc: Narinder Dhillon 
> > Cc: Nhi Pham 
> > Cc: Rebecca Cran 
> > Cc: Sami Mujawar 
> > Cc: Thomas Abraham 
> > Cc: Wenyi Xie 
> >
> > Leif Lindholm (5):
> >   Platform/SbsaQemu: fix .dsc line endings
> >   Platform,Silicon: drop redundant uses of PcdArmArchTimerFreqInHz
> >   Platform/ARM: drop use of PcdArmArchTimerFreqInHz
> >   Platform/Hisilicon: drop D05 use of PcdArmArchTimerFreqInHz
> >   Silicon/Marvell: drop use of PcdArmArchTimerFreqInHz
> >
> 
> Looks fine to me
> 
> Reviewed-by: Ard Biesheuvel 

Thanks!

Since 1-2 are just cleanup, I have now pushed those two commits,
with R-b:s given, as 3f08401365d6..b29e69a688d6.

I am tempted to now merge https://github.com/tianocore/edk2/pull/5797,
at which point the platforms affected by 3-5 will break.

Any objection?

/
Leif


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




Re: [edk2-devel] Regarding MOR Secure feature

2024-06-21 Thread Ard Biesheuvel
Hi Ray,


On Fri, 21 Jun 2024 at 10:03, Ni, Ray  wrote:
>
> Ard,
>
> 7 years ago, Laszlo added this commit 
> (https://github.com/tianocore/edk2/commit/fda8f631edbbf3823760542a06f12bd60fd39181)
>  to support some OS kernels that incorrectly "create" the MOR variable.
> The OS kernel bug is captured in this bugzilla 
> (bugzilla.redhat.com/show_bug.cgi?id=1498159). It seems to me the OS kernel 
> bugs only exist in Fedora 24 and 25 which are all EOL today. Fedora 26 has 
> the correct implementation that does NOT "create" the MOR variable. The 
> implementation is done by you here 
> (https://lore.kernel.org/all/20170825155019.6740-2-ard.biesheu...@linaro.org/T/#u).
>
> 5 years ago, you added StandaloneMm variable driver and 
> VariableHaveTcgProtocols() returns FALSE always in the standalone MM version. 
> (Commit: 
> https://github.com/tianocore/edk2/commit/a855f63e2fdd990837391b0e61e78b3f06b56916)
> As a result, MorLock variable is not created. It causes a bug that the BIOS 
> does not report the MOR Secure feature to OS.
>
> My questions are:
>
> can we revert Laszlo's commit? As the bug that commit fixes only exists in 
> Fedora 24/25 which are all EOL today.

Yes, I think we can revert it, although it is not clear from the
commit log what the erroneous behavior is.

> why is the MOR secure bug not found in ARM platform?
>

This is definitely a bug on ARM (and likely other users of standalone MM).

The problem is, of course, that standalone MM is standalone, and
cannot know for certain which EFI protocols are exposed by the DXE
core, nor invoke them directly.

>
> I think you are the best person to answer the questions because you not only 
> fixed the kernel, but also know details on the ARM standalone MM.
> I am so happy with that:)
>

:-)

To be honest, I need some time to page this all back into my brain,
but I am happy to help out.

MOR does not really rely on the TCG protocols, right? If standalone MM
can implement it without its ability to invoke those protocols, we
should just separate those. I guess that is what you are doing at the
moment?


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




Re: [edk2-devel] [PATCH 1/2] Platform/SbsaQemu: get the information of CPU topology via SMC calls

2024-06-21 Thread Marcin Juszkiewicz

W dniu 17.04.2024 o 13:26, Xiong Yining pisze:

Provide functions to check for CPU topology information:
- the number of sockets on sbsa-ref platform.
- the number of clusters in one socket.
- the number of cores in one cluster.
- the number of threads in one cores.

As SMC calls can return up to 4 return values. the number of
sockets, clusters and cores are read from TF-A using platform
specific SMC calls. And the number of threads is caluculated
using the total number of cpus and the number of sockets,
clusters and cores.

Signed-off-by: Xiong Yining 
---
  .../Include/IndustryStandard/SbsaQemuSmc.h|  1 +
  .../Include/Library/HardwareInfoLib.h | 26 +++
  .../SbsaQemuHardwareInfoLib.c | 33 +++
  3 files changed, 60 insertions(+)

diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h 
b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
index e3092007d27d..9d9780ca70fe 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
@@ -16,6 +16,7 @@
  #define SIP_SVC_GET_GIC_ITSSMC_SIP_FUNCTION_ID(101)
  #define SIP_SVC_GET_CPU_COUNT  SMC_SIP_FUNCTION_ID(200)
  #define SIP_SVC_GET_CPU_NODE   SMC_SIP_FUNCTION_ID(201)
+#define SIP_SVC_GET_CPU_TOPOLOGY SMC_SIP_FUNCTION_ID(202)
  #define SIP_SVC_GET_MEMORY_NODE_COUNT SMC_SIP_FUNCTION_ID(300)
  #define SIP_SVC_GET_MEMORY_NODE SMC_SIP_FUNCTION_ID(301)
  
diff --git a/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h b/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h

index 46fdad45353c..3e451ee344c7 100644
--- a/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
+++ b/Silicon/Qemu/SbsaQemu/Include/Library/HardwareInfoLib.h
@@ -15,6 +15,19 @@ typedef struct{
UINT64  AddressSize;
  } MemoryInfo;
  
+/**

+  Sockets: the number of sockets on sbsa-ref platform.
+  Clusters: the number of clusters in one socket.
+  Cores: the number of cores in one cluster.
+  Threads: the number of threads in one core.
+**/
+typedef struct{
+  UINT32 Sockets;
+  UINT32 Clusters;
+  UINT32 Cores;
+  UINT32 Threads;
+} CpuTopology;
+
  /**
Get CPU count from information passed by Qemu.
  
@@ -83,4 +96,17 @@ GetNumaNodeCount (

VOID
);
  
+/**

+  Get cpu topology(sockets, clusters, cores, threads) from device tree passed 
by Qemu.
+
+  @param [out]  CpuTopo A pointer to the cpu topology.
+
+
+  @retval   the information of cpu topology.
+**/
+VOID
+GetCpuTopology (
+  OUT CpuTopology *CpuTopo
+  );
+
  #endif /* HARDWARE_INFO_LIB */
diff --git 
a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
 
b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
index 4c22e7d6ee47..a12dc0244da5 100644
--- 
a/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
+++ 
b/Silicon/Qemu/SbsaQemu/Library/SbsaQemuHardwareInfoLib/SbsaQemuHardwareInfoLib.c
@@ -179,3 +179,36 @@ GetNumaNodeCount (
  
return NumberNumaNodes;

  }
+
+/**
+  Get CPU topology.
+**/
+VOID
+GetCpuTopology (
+  OUT CpuTopology *CpuTopo
+  )
+{
+  UINTN  SmcResult;
+  UINTN  Arg0;
+  UINTN  Arg1;
+  UINTN  Arg2;
+  UINT32 NumCores = GetCpuCount();
+
+  SmcResult = ArmCallSmc0(SIP_SVC_GET_CPU_TOPOLOGY, &Arg0, &Arg1, &Arg2);


Can you use ArmCallSmc() directly? TF-A returns 5 arguments for this SMC 
call, we use 4 and calculate 5th one. Once we improve SMCCC code in EDK2 
the 5th one would be used directly.



+  if (SmcResult != SMC_SIP_CALL_SUCCESS) {
+DEBUG ((DEBUG_ERROR, "%a: SIP_SVC_GET_CPU_TOPOLOGY call failed. We have no cpu 
topology information.\n", __FUNCTION__));
+ResetShutdown ();
+  } else {
+CpuTopo->Sockets = Arg0;
+CpuTopo->Clusters = Arg1;
+CpuTopo->Cores = Arg2;



+CpuTopo->Threads = NumCores / (CpuTopo->Sockets * CpuTopo->Clusters * 
CpuTopo->Cores);


Once SMCCC gets improved this would be read directly from TF-A.


+  }
+
+  DEBUG(( DEBUG_INFO, "%a: CPU Topology: sockets are %d, clusters are %d, cores are 
%d, threads are %d\n",


Can you replace " are" with ":" ("sockets: %d, clusters: %d")? Those 
numbers can be singular.



+  __FUNCTION__,
+  CpuTopo->Sockets,
+  CpuTopo->Clusters,
+  CpuTopo->Cores,
+  CpuTopo->Threads ));
+}
\ No newline at end of file




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119668): https://edk2.groups.io/g/devel/message/119668
Mute This Topic: https://groups.io/mt/105575034/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/2] Silicon/SbsaQemu: align the PPTT tables with qemu configuration

2024-06-21 Thread Marcin Juszkiewicz

W dniu 17.04.2024 o 13:26, Xiong Yining pisze:

To align the CPU topology information recognized by the operating system with 
the CPU topology
information configured by QEMU, we need to make use of the CPU topology 
information to create
complex PPTT tables setups.

We can get the CPU topology information via SMC.

Signed-off-by: Xiong Yining 


Thanks for submittion. Some issues to handle. Also: please call
uncrucify on your code.


---
  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h |  12 ++
  .../Include/IndustryStandard/SbsaQemuAcpi.h   |  32 
  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 140 +-
  3 files changed, 114 insertions(+), 70 deletions(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
index 83a085cd86f4..e29635b28938 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.h
@@ -90,4 +90,16 @@ typedef struct {
  ClockDomain /* Clock Domain */
\
}
  
+#define SBSAQEMU_ACPI_PROCESSOR_HIERARCHY_NODE_STRUCTURE_INIT(  \

+  Flags, Parent, ACPIProcessorID, NumberOfPrivateResources)
 \
+  {
 \
+EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR,  
  /* Type */ \
+sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) + NumberOfPrivateResources 
* sizeof (UINT32), /* Length */   \


First suggestion: please use latest ACPI version (6.5 at the moment).
There are 2-3 entries more inside of struct. One of them is CacheId
which can be anything as long as it is unique and > 0 (just do
incremental).


+{ EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE },
  /* Reserved */ \
+Flags, 
  /* Flags */\
+Parent,
  /* Parent */   \
+ACPIProcessorID,   
  /* ACPI Processor ID */\
+NumberOfPrivateResources   
  /* Number of private resources */  \
+  }
+
  #endif
diff --git a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h 
b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
index 61d8bce8c959..7ef85b7e2f79 100644
--- a/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
+++ b/Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuAcpi.h
@@ -166,36 +166,4 @@ typedef struct {
  64/* LineSize */  
 \
}
  
-#define SBSAQEMU_ACPI_PPTT_CLUSTER_STRUCT  {   \

-EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR,  
\
-sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR),
\
-{ EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE },
\
-{  
\
-  EFI_ACPI_6_3_PPTT_PACKAGE_PHYSICAL, /* PhysicalPackage */
\
-  EFI_ACPI_6_3_PPTT_PROCESSOR_ID_INVALID, /* AcpiProcessorIdValid */   
\
-  EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD,  /* Is not a Thread */
\
-  EFI_ACPI_6_3_PPTT_NODE_IS_NOT_LEAF, /* Not Leaf */   
\
-  EFI_ACPI_6_3_PPTT_IMPLEMENTATION_IDENTICAL, /* Identical Cores */
\
-}, 
\
-0,/* Parent */ 
\
-0,/* AcpiProcessorId */
\
-0,/* NumberOfPrivateResources */   
\
-  }
-
-#define SBSAQEMU_ACPI_PPTT_CORE_STRUCT  {  
\
-EFI_ACPI_6_3_PPTT_TYPE_PROCESSOR,  
\
-(sizeof (EFI_ACPI_6_3_PPTT_STRUCTURE_PROCESSOR) + (2 * sizeof (UINT32))),  
\
-{ EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE },
\
-{  
\
-  EFI_ACPI_6_3_PPTT_PACKAGE_NOT_PHYSICAL, /* PhysicalPackage */
\
-  EFI_ACPI_6_3_PPTT_PROCESSOR_ID_VALID,   /* AcpiProcessorValid */ 
\
-  EFI_ACPI_6_3_PPTT_PROCESSOR_IS_NOT_THREAD,  /* Is not a 

[edk2-devel] Regarding MOR Secure feature

2024-06-21 Thread Ni, Ray
Ard,

7 years ago, Laszlo added this commit 
(https://github.com/tianocore/edk2/commit/fda8f631edbbf3823760542a06f12bd60fd39181)
 to support some OS kernels that incorrectly "create" the MOR variable.
The OS kernel bug is captured in this bugzilla 
(bugzilla.redhat.com/show_bug.cgi?id=1498159).
 It seems to me the OS kernel bugs only exist in Fedora 24 and 25 which are all 
EOL today. Fedora 26 has the correct implementation that does NOT "create" the 
MOR variable. The implementation is done by you here 
(https://lore.kernel.org/all/20170825155019.6740-2-ard.biesheu...@linaro.org/T/#u).

5 years ago, you added StandaloneMm variable driver and 
VariableHaveTcgProtocols() returns FALSE always in the standalone MM version. 
(Commit: 
https://github.com/tianocore/edk2/commit/a855f63e2fdd990837391b0e61e78b3f06b56916)
As a result, MorLock variable is not created. It causes a bug that the BIOS 
does not report the MOR Secure feature to OS.

My questions are:

  1.
can we revert Laszlo's commit? As the bug that commit fixes only exists in 
Fedora 24/25 which are all EOL today.
  2.
why is the MOR secure bug not found in ARM platform?

I think you are the best person to answer the questions because you not only 
fixed the kernel, but also know details on the ARM standalone MM.
I am so happy with that:)

Thanks,
Ray


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