public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH 0/2] Set CPU model in SMBIOS
@ 2024-07-12  9:23 Marcin Juszkiewicz
  2024-07-12  9:23 ` [edk2-devel] [PATCH 1/2] ArmPkg: make PcdProcessorVersion dynamic Marcin Juszkiewicz
  2024-07-12  9:23 ` [edk2-devel] [PATCH edk2-platforms 2/2] SbsaQemu: set CPU model in SMBIOS Marcin Juszkiewicz
  0 siblings, 2 replies; 7+ messages in thread
From: Marcin Juszkiewicz @ 2024-07-12  9:23 UTC (permalink / raw)
  To: devel; +Cc: Leif Lindholm, Ard Biesheuvel, Sami Mujawar, Marcin Juszkiewicz

During last changes to SbsaQemu platform I noticed that it says
"arm-virt" as Processor Version in SMBIOS and decided to change it.

The problem is that this value is set in stone as PcdProcessorVersion is
FixedAtBuild one.

So first patch make it dynamic in ArmPkg and second reads MIDR and sets
cpu model name.

Marcin Juszkiewicz (2):
  ArmPkg: make PcdProcessorVersion dynamic
  SbsaQemu: set CPU model in SMBIOS

 ArmPkg/ArmPkg.dec                                             | 2 +-
 .../Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

 Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |  2 +-
 .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf       |  2 +
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 50 +++++++++++++++++++
 3 files changed, 53 insertions(+), 1 deletion(-)
--
2.45.2



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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [edk2-devel] [PATCH 1/2] ArmPkg: make PcdProcessorVersion dynamic
  2024-07-12  9:23 [edk2-devel] [PATCH 0/2] Set CPU model in SMBIOS Marcin Juszkiewicz
@ 2024-07-12  9:23 ` Marcin Juszkiewicz
  2024-07-12  9:23 ` [edk2-devel] [PATCH edk2-platforms 2/2] SbsaQemu: set CPU model in SMBIOS Marcin Juszkiewicz
  1 sibling, 0 replies; 7+ messages in thread
From: Marcin Juszkiewicz @ 2024-07-12  9:23 UTC (permalink / raw)
  To: devel; +Cc: Leif Lindholm, Ard Biesheuvel, Sami Mujawar, Marcin Juszkiewicz

On SbsaQemu platform we would like to be able to set this variable
during bootup as cpu model can be different on each system run.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 ArmPkg/ArmPkg.dec                                             | 2 +-
 .../Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index c0861140e858..cd922a04ddfa 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -198,7 +198,6 @@ [PcdsFixedAtBuild.common]
   gArmTokenSpaceGuid.PcdBaseBoardProductName|L""|VOID*|0x30000056
   gArmTokenSpaceGuid.PcdBaseBoardVersion|L""|VOID*|0x30000057
   gArmTokenSpaceGuid.PcdProcessorManufacturer|L""|VOID*|0x30000071
-  gArmTokenSpaceGuid.PcdProcessorVersion|L""|VOID*|0x30000072
   gArmTokenSpaceGuid.PcdProcessorSerialNumber|L""|VOID*|0x30000073
   gArmTokenSpaceGuid.PcdProcessorAssetTag|L""|VOID*|0x30000074
   gArmTokenSpaceGuid.PcdProcessorPartNumber|L""|VOID*|0x30000075
@@ -301,6 +300,7 @@ [PcdsFixedAtBuild.AARCH64]
 #
 [PcdsFixedAtBuild.common, PcdsDynamic.common, PcdsPatchableInModule.common]
 
+  gArmTokenSpaceGuid.PcdProcessorVersion|L""|VOID*|0x30000072
   # System Memory (DRAM): These PCDs define the region of in-built system memory
   # Some platforms can get DRAM extensions, these additional regions may be
   # declared to UEFI using separate resource descriptor HOBs
diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
index 4f2d42133756..4a787b603698 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
@@ -527,8 +527,8 @@ AllocateType4AndSetProcessorInformationStrings (
     OemUpdateSmbiosInfo (mHiiHandle, PartNumber, ProcessorPartNumType04);
   }
 
-  if (StrLen ((CHAR16 *)FixedPcdGetPtr (PcdProcessorVersion)) > 0) {
-    HiiSetString (mHiiHandle, ProcessorVersion, (CHAR16 *)FixedPcdGetPtr (PcdProcessorVersion), NULL);
+  if (StrLen ((CHAR16 *)PcdGetPtr (PcdProcessorVersion)) > 0) {
+    HiiSetString (mHiiHandle, ProcessorVersion, (CHAR16 *)PcdGetPtr (PcdProcessorVersion), NULL);
   } else {
     OemUpdateSmbiosInfo (mHiiHandle, ProcessorVersion, ProcessorVersionType04);
   }
--
2.45.2



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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [edk2-devel] [PATCH edk2-platforms 2/2] SbsaQemu: set CPU model in SMBIOS
  2024-07-12  9:23 [edk2-devel] [PATCH 0/2] Set CPU model in SMBIOS Marcin Juszkiewicz
  2024-07-12  9:23 ` [edk2-devel] [PATCH 1/2] ArmPkg: make PcdProcessorVersion dynamic Marcin Juszkiewicz
@ 2024-07-12  9:23 ` Marcin Juszkiewicz
  2024-07-12 10:01   ` Ard Biesheuvel
  1 sibling, 1 reply; 7+ messages in thread
From: Marcin Juszkiewicz @ 2024-07-12  9:23 UTC (permalink / raw)
  To: devel; +Cc: Leif Lindholm, Ard Biesheuvel, Sami Mujawar, Marcin Juszkiewicz

SbsaQemu platform have several cpu models available. Since start it
reported "arm-virt" one in SMBIOS instead of real one.

This change replaces it with the real cpu model like Cortex-A57,
Neoverse-N2 etc.

Requires change in EDK2 to make PcdProcessorVersion dynamic.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |  2 +-
 .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf       |  2 +
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 50 +++++++++++++++++++
 3 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index e78f1d71998a..a823a2967525 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -485,7 +485,6 @@ [PcdsFixedAtBuild.common]
   # These values are fixed for now, but should be configurable via
   # something like an emulated SCP.
   gArmTokenSpaceGuid.PcdProcessorManufacturer|L"QEMU"
-  gArmTokenSpaceGuid.PcdProcessorVersion|L"arm-virt"
   gArmTokenSpaceGuid.PcdProcessorSerialNumber|L"SN0000"
   gArmTokenSpaceGuid.PcdProcessorAssetTag|L"AT0000"
   gArmTokenSpaceGuid.PcdProcessorPartNumber|L"PN0000"
@@ -495,6 +494,7 @@ [PcdsFixedAtBuild.common]
 
 [PcdsDynamicDefault.common]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
+  gArmTokenSpaceGuid.PcdProcessorVersion|L"                                "
 
   # Core and Cluster Count
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount|1
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
index 727c8e82d16e..616296f5a485 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
@@ -53,6 +53,8 @@ [Pcd]
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase
 
+  gArmTokenSpaceGuid.PcdProcessorVersion
+
 [Depex]
   gEfiAcpiTableProtocolGuid                       ## CONSUMES
 
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index e0eef54ff907..b19edf5a4ff8 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -845,6 +845,51 @@ DisableXhciOnOlderPlatVer (
   return Status;
 }
 
+EFI_STATUS
+UpdateCpuNameInSmbios (
+  )
+{
+  EFI_STATUS  Status;
+  UINT64      ProcessorId;
+  UINT16      PartNum;
+  UINTN       CpuLen;
+  CHAR16      CpuName[16];
+
+  ProcessorId = ArmReadMidr ();
+  PartNum     = (UINT16)((ProcessorId >> 4) & 0xFFF);
+
+  switch (PartNum) {
+    case 0x51:
+      StrCpyS (CpuName, 16, L"QEMU Max");
+      break;
+    case 0xd07:
+      StrCpyS (CpuName, 16, L"Arm Cortex-A57");
+      break;
+    case 0xd08:
+      StrCpyS (CpuName, 16, L"Arm Cortex-A72");
+      break;
+    case 0xd0c:
+      StrCpyS (CpuName, 16, L"Arm Neoverse-N1");
+      break;
+    case 0xd40:
+      StrCpyS (CpuName, 16, L"Arm Neoverse-V1");
+      break;
+    case 0xd49:
+      StrCpyS (CpuName, 16, L"Arm Neoverse-N2");
+      break;
+    default:
+      StrCpyS (CpuName, 16, L"virtual cpu");
+      break;
+  }
+
+  CpuLen = sizeof (CHAR16) * StrLen (CpuName);
+
+  Status = PcdSetPtrS (PcdProcessorVersion, &CpuLen, CpuName);
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
+
 EFI_STATUS
 EFIAPI
 InitializeSbsaQemuAcpiDxe (
@@ -907,5 +952,10 @@ InitializeSbsaQemuAcpiDxe (
     DEBUG ((DEBUG_ERROR, "Failed to handle XHCI enablement\n"));
   }
 
+  Status = UpdateCpuNameInSmbios ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to set CPU name in SMBIOS\n"));
+  }
+
   return EFI_SUCCESS;
 }
--
2.45.2



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



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms 2/2] SbsaQemu: set CPU model in SMBIOS
  2024-07-12  9:23 ` [edk2-devel] [PATCH edk2-platforms 2/2] SbsaQemu: set CPU model in SMBIOS Marcin Juszkiewicz
@ 2024-07-12 10:01   ` Ard Biesheuvel
  2024-07-12 10:58     ` Marcin Juszkiewicz
  2024-07-15  3:24     ` Nhi Pham via groups.io
  0 siblings, 2 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2024-07-12 10:01 UTC (permalink / raw)
  To: Marcin Juszkiewicz; +Cc: devel, Leif Lindholm, Ard Biesheuvel, Sami Mujawar

On Fri, 12 Jul 2024 at 11:24, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> wrote:
>
> SbsaQemu platform have several cpu models available. Since start it
> reported "arm-virt" one in SMBIOS instead of real one.
>
> This change replaces it with the real cpu model like Cortex-A57,
> Neoverse-N2 etc.
>
> Requires change in EDK2 to make PcdProcessorVersion dynamic.
>
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

I am going to make the same remark as I did before in a different
context: how is it guaranteed that the code that sets the PCD executes
before the code that consumes it?

Making PCDs dynamic is usually not the solution here, unless they are
set in the PEI phase and consumed in the DXE phase or after.

The EFI SMBIOS protocol has an 'update string' method that you can
call from a platform driver. That might be a better fit here, and the
PCD can remain fixed, and carry a fallback default value.


> ---
>  Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |  2 +-
>  .../SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf       |  2 +
>  .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 50 +++++++++++++++++++
>  3 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> index e78f1d71998a..a823a2967525 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> @@ -485,7 +485,6 @@ [PcdsFixedAtBuild.common]
>    # These values are fixed for now, but should be configurable via
>    # something like an emulated SCP.
>    gArmTokenSpaceGuid.PcdProcessorManufacturer|L"QEMU"
> -  gArmTokenSpaceGuid.PcdProcessorVersion|L"arm-virt"
>    gArmTokenSpaceGuid.PcdProcessorSerialNumber|L"SN0000"
>    gArmTokenSpaceGuid.PcdProcessorAssetTag|L"AT0000"
>    gArmTokenSpaceGuid.PcdProcessorPartNumber|L"PN0000"
> @@ -495,6 +494,7 @@ [PcdsFixedAtBuild.common]
>
>  [PcdsDynamicDefault.common]
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
> +  gArmTokenSpaceGuid.PcdProcessorVersion|L"                                "
>
>    # Core and Cluster Count
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount|1
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> index 727c8e82d16e..616296f5a485 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.inf
> @@ -53,6 +53,8 @@ [Pcd]
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdSmmuBase
>
> +  gArmTokenSpaceGuid.PcdProcessorVersion
> +
>  [Depex]
>    gEfiAcpiTableProtocolGuid                       ## CONSUMES
>
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> index e0eef54ff907..b19edf5a4ff8 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
> @@ -845,6 +845,51 @@ DisableXhciOnOlderPlatVer (
>    return Status;
>  }
>
> +EFI_STATUS
> +UpdateCpuNameInSmbios (
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINT64      ProcessorId;
> +  UINT16      PartNum;
> +  UINTN       CpuLen;
> +  CHAR16      CpuName[16];
> +
> +  ProcessorId = ArmReadMidr ();
> +  PartNum     = (UINT16)((ProcessorId >> 4) & 0xFFF);
> +
> +  switch (PartNum) {
> +    case 0x51:
> +      StrCpyS (CpuName, 16, L"QEMU Max");
> +      break;
> +    case 0xd07:
> +      StrCpyS (CpuName, 16, L"Arm Cortex-A57");
> +      break;
> +    case 0xd08:
> +      StrCpyS (CpuName, 16, L"Arm Cortex-A72");
> +      break;
> +    case 0xd0c:
> +      StrCpyS (CpuName, 16, L"Arm Neoverse-N1");
> +      break;
> +    case 0xd40:
> +      StrCpyS (CpuName, 16, L"Arm Neoverse-V1");
> +      break;
> +    case 0xd49:
> +      StrCpyS (CpuName, 16, L"Arm Neoverse-N2");
> +      break;
> +    default:
> +      StrCpyS (CpuName, 16, L"virtual cpu");
> +      break;
> +  }
> +
> +  CpuLen = sizeof (CHAR16) * StrLen (CpuName);
> +
> +  Status = PcdSetPtrS (PcdProcessorVersion, &CpuLen, CpuName);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  InitializeSbsaQemuAcpiDxe (
> @@ -907,5 +952,10 @@ InitializeSbsaQemuAcpiDxe (
>      DEBUG ((DEBUG_ERROR, "Failed to handle XHCI enablement\n"));
>    }
>
> +  Status = UpdateCpuNameInSmbios ();
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to set CPU name in SMBIOS\n"));
> +  }
> +
>    return EFI_SUCCESS;
>  }
> --
> 2.45.2
>


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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms 2/2] SbsaQemu: set CPU model in SMBIOS
  2024-07-12 10:01   ` Ard Biesheuvel
@ 2024-07-12 10:58     ` Marcin Juszkiewicz
  2024-07-15  3:24     ` Nhi Pham via groups.io
  1 sibling, 0 replies; 7+ messages in thread
From: Marcin Juszkiewicz @ 2024-07-12 10:58 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: devel, Leif Lindholm, Ard Biesheuvel, Sami Mujawar

On 12.07.2024 12:01, Ard Biesheuvel wrote:
>> SbsaQemu platform have several cpu models available. Since start it
>> reported "arm-virt" one in SMBIOS instead of real one.
>>
>> This change replaces it with the real cpu model like Cortex-A57,
>> Neoverse-N2 etc.
>>
>> Requires change in EDK2 to make PcdProcessorVersion dynamic.
>>
>> Signed-off-by: Marcin Juszkiewicz<marcin.juszkiewicz@linaro.org>
> I am going to make the same remark as I did before in a different
> context: how is it guaranteed that the code that sets the PCD executes
> before the code that consumes it?
> 
> Making PCDs dynamic is usually not the solution here, unless they are
> set in the PEI phase and consumed in the DXE phase or after.
> 
> The EFI SMBIOS protocol has an 'update string' method that you can
> call from a platform driver. That might be a better fit here, and the
> PCD can remain fixed, and carry a fallback default value.

Thanks. Will take a look.


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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms 2/2] SbsaQemu: set CPU model in SMBIOS
  2024-07-12 10:01   ` Ard Biesheuvel
  2024-07-12 10:58     ` Marcin Juszkiewicz
@ 2024-07-15  3:24     ` Nhi Pham via groups.io
  2024-07-15  8:24       ` Ard Biesheuvel
  1 sibling, 1 reply; 7+ messages in thread
From: Nhi Pham via groups.io @ 2024-07-15  3:24 UTC (permalink / raw)
  To: devel, ardb, Marcin Juszkiewicz
  Cc: Leif Lindholm, Ard Biesheuvel, Sami Mujawar

On 7/12/2024 5:01 PM, Ard Biesheuvel via groups.io wrote:
> On Fri, 12 Jul 2024 at 11:24, Marcin Juszkiewicz
> <marcin.juszkiewicz@linaro.org> wrote:
>>
>> SbsaQemu platform have several cpu models available. Since start it
>> reported "arm-virt" one in SMBIOS instead of real one.
>>
>> This change replaces it with the real cpu model like Cortex-A57,
>> Neoverse-N2 etc.
>>
>> Requires change in EDK2 to make PcdProcessorVersion dynamic.
>>
>> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> 
> I am going to make the same remark as I did before in a different
> context: how is it guaranteed that the code that sets the PCD executes
> before the code that consumes it?

We could implement the setting in a 'PlatformInitDxe', which will be 
added to the 'APPRIORI DXE' list to ensure it is scheduled to run in the 
early DXE phase. This approach will help initialize the neccessary 
compoenents early in the DXE phase, providing a stable foundation for 
subsequent operations.

> 
> Making PCDs dynamic is usually not the solution here, unless they are
> set in the PEI phase and consumed in the DXE phase or after.
> 
> The EFI SMBIOS protocol has an 'update string' method that you can
> call from a platform driver. That might be a better fit here, and the
> PCD can remain fixed, and carry a fallback default value.
> 

In fact that the 'update string' method could eat up the boot time. 
Hence, the SMBIOS string patching is not prefered.

Regards,
Nhi


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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms 2/2] SbsaQemu: set CPU model in SMBIOS
  2024-07-15  3:24     ` Nhi Pham via groups.io
@ 2024-07-15  8:24       ` Ard Biesheuvel
  0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2024-07-15  8:24 UTC (permalink / raw)
  To: Nhi Pham
  Cc: devel, Marcin Juszkiewicz, Leif Lindholm, Ard Biesheuvel,
	Sami Mujawar

On Mon, 15 Jul 2024 at 05:25, Nhi Pham <nhi@os.amperecomputing.com> wrote:
>
> On 7/12/2024 5:01 PM, Ard Biesheuvel via groups.io wrote:
> > On Fri, 12 Jul 2024 at 11:24, Marcin Juszkiewicz
> > <marcin.juszkiewicz@linaro.org> wrote:
> >>
> >> SbsaQemu platform have several cpu models available. Since start it
> >> reported "arm-virt" one in SMBIOS instead of real one.
> >>
> >> This change replaces it with the real cpu model like Cortex-A57,
> >> Neoverse-N2 etc.
> >>
> >> Requires change in EDK2 to make PcdProcessorVersion dynamic.
> >>
> >> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> >
> > I am going to make the same remark as I did before in a different
> > context: how is it guaranteed that the code that sets the PCD executes
> > before the code that consumes it?
>
> We could implement the setting in a 'PlatformInitDxe', which will be
> added to the 'APPRIORI DXE' list to ensure it is scheduled to run in the
> early DXE phase. This approach will help initialize the neccessary
> compoenents early in the DXE phase, providing a stable foundation for
> subsequent operations.
>

APRIORI DXE should be avoided, so I don't want to see it used in a
reference implementation. (APRIORI DXE breaks the DXE dependency based
dispatch)

> >
> > Making PCDs dynamic is usually not the solution here, unless they are
> > set in the PEI phase and consumed in the DXE phase or after.
> >
> > The EFI SMBIOS protocol has an 'update string' method that you can
> > call from a platform driver. That might be a better fit here, and the
> > PCD can remain fixed, and carry a fallback default value.
> >
>
> In fact that the 'update string' method could eat up the boot time.
> Hence, the SMBIOS string patching is not prefered.
>

Let's implement it correctly first, and then, if boot time appears to
be impacted, think about how to optimize this.

Boot time is not a huge concern for an emulator.


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



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-07-15  8:24 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-12  9:23 [edk2-devel] [PATCH 0/2] Set CPU model in SMBIOS Marcin Juszkiewicz
2024-07-12  9:23 ` [edk2-devel] [PATCH 1/2] ArmPkg: make PcdProcessorVersion dynamic Marcin Juszkiewicz
2024-07-12  9:23 ` [edk2-devel] [PATCH edk2-platforms 2/2] SbsaQemu: set CPU model in SMBIOS Marcin Juszkiewicz
2024-07-12 10:01   ` Ard Biesheuvel
2024-07-12 10:58     ` Marcin Juszkiewicz
2024-07-15  3:24     ` Nhi Pham via groups.io
2024-07-15  8:24       ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox