[edk2-devel] [PATCH 1/2] Platform/SbsaQemu: read platform version

Marcin Juszkiewicz posted 2 patches 1 year ago
There is a newer version of this series
[edk2-devel] [PATCH 1/2] Platform/SbsaQemu: read platform version
Posted by Marcin Juszkiewicz 1 year ago
Qemu has versioning for sbsa-ref platform. TF-A reads it from provided
DeviceTree and provides as SMC.

This change adds reading platform version into EDK2.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |  3 +++
 .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c | 27 +++++++++++++++++++
 .../SbsaQemuPlatformDxe.inf                   |  5 ++++
 Silicon/Qemu/SbsaQemu/SbsaQemu.dec            |  3 +++
 4 files changed, 38 insertions(+)

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index 0bd0df4f0239..b88729ad8ad6 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -558,6 +558,9 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisAssetTag|L"AT0000"
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisSKU|L"SK0000"
 
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor|0x0
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor|0x0
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
index b7270a07abbd..199766c7014a 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
@@ -7,15 +7,25 @@
 *
 **/
 
+#include <Library/ArmSmcLib.h>
 #include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
 #include <Library/NonDiscoverableDeviceRegistrationLib.h>
 #include <Library/PcdLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiDriverEntryPoint.h>
+#include <IndustryStandard/ArmStdSmc.h>
 
 #include <Protocol/FdtClient.h>
 
+/* those probably should go into IndustryStandard/ArmStdSmc.h */
+#define SMC_FASTCALL       0x80000000
+#define SMC64_FUNCTION     (SMC_FASTCALL   | 0x40000000)
+#define SIP_FUNCTION       (SMC64_FUNCTION | 0x02000000)
+#define SIP_FUNCTION_ID(n) (SIP_FUNCTION   | (n))
+
+#define SIP_SVC_VERSION  SIP_FUNCTION_ID(1)
+
 EFI_STATUS
 EFIAPI
 InitializeSbsaQemuPlatformDxe (
@@ -26,6 +36,9 @@ InitializeSbsaQemuPlatformDxe (
   EFI_STATUS                     Status;
   UINTN                          Size;
   VOID*                          Base;
+  UINTN                          Arg0;
+  UINTN                          Arg1;
+  UINTN                          Result;
 
   DEBUG ((DEBUG_INFO, "%a: InitializeSbsaQemuPlatformDxe called\n", __FUNCTION__));
 
@@ -51,5 +64,19 @@ InitializeSbsaQemuPlatformDxe (
     return Status;
   }
 
+  Result = ArmCallSmc0 (SIP_SVC_VERSION, &Arg0, &Arg1, NULL);
+  if (Result == SMC_ARCH_CALL_SUCCESS)
+  {
+        Result = PcdSet32S (PcdPlatformVersionMajor, Arg0);
+        ASSERT_EFI_ERROR (Result);
+        Result = PcdSet32S (PcdPlatformVersionMinor, Arg1);
+        ASSERT_EFI_ERROR (Result);
+  }
+
+  Arg0 = PcdGet32 (PcdPlatformVersionMajor);
+  Arg1 = PcdGet32 (PcdPlatformVersionMinor);
+
+  DEBUG ((DEBUG_INFO, "Platform version: %d.%d\n", Arg0, Arg1));
+
   return EFI_SUCCESS;
 }
diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
index 21d2135f6d17..1f2c8a9dd6af 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
@@ -20,6 +20,7 @@
   SbsaQemuPlatformDxe.c
 
 [Packages]
+  ArmPkg/ArmPkg.dec
   ArmVirtPkg/ArmVirtPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdeModulePkg/MdeModulePkg.dec
@@ -27,6 +28,7 @@
   Silicon/Qemu/SbsaQemu/SbsaQemu.dec
 
 [LibraryClasses]
+  ArmSmcLib
   PcdLib
   DebugLib
   NonDiscoverableDeviceRegistrationLib
@@ -36,6 +38,9 @@
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciBase
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciSize
 
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor
+
 [Depex]
   TRUE
 
diff --git a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
index 9448852967b6..5182978cf56d 100644
--- a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
+++ b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
@@ -67,3 +67,6 @@
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisManufacturer|L""|VOID*|0x0000011B
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisAssetTag|L""|VOID*|0x0000011C
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisSKU|L""|VOID*|0x0000011D
+
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor|0x0|UINT32|0x0000011E
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor|0x0|UINT32|0x0000011F
-- 
2.40.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104860): https://edk2.groups.io/g/devel/message/104860
Mute This Topic: https://groups.io/mt/98900379/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/2] Platform/SbsaQemu: read platform version
Posted by Leif Lindholm 12 months ago
On Mon, May 15, 2023 at 12:24:22 +0200, Marcin Juszkiewicz wrote:
> Qemu has versioning for sbsa-ref platform. TF-A reads it from provided
> DeviceTree and provides as SMC.
> 
> This change adds reading platform version into EDK2.
> 
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>  Platform/Qemu/SbsaQemu/SbsaQemu.dsc           |  3 +++
>  .../SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c | 27 +++++++++++++++++++
>  .../SbsaQemuPlatformDxe.inf                   |  5 ++++
>  Silicon/Qemu/SbsaQemu/SbsaQemu.dec            |  3 +++
>  4 files changed, 38 insertions(+)
> 
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> index 0bd0df4f0239..b88729ad8ad6 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> @@ -558,6 +558,9 @@ DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisAssetTag|L"AT0000"
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisSKU|L"SK0000"
>  
> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor|0x0
> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor|0x0
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> index b7270a07abbd..199766c7014a 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.c
> @@ -7,15 +7,25 @@
>  *
>  **/
>  
> +#include <Library/ArmSmcLib.h>
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/NonDiscoverableDeviceRegistrationLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/UefiDriverEntryPoint.h>
> +#include <IndustryStandard/ArmStdSmc.h>
>  
>  #include <Protocol/FdtClient.h>
>  
> +/* those probably should go into IndustryStandard/ArmStdSmc.h */
> +#define SMC_FASTCALL       0x80000000
> +#define SMC64_FUNCTION     (SMC_FASTCALL   | 0x40000000)
> +#define SIP_FUNCTION       (SMC64_FUNCTION | 0x02000000)
> +#define SIP_FUNCTION_ID(n) (SIP_FUNCTION   | (n))

Can you break this out into a separate patch for ArmStdSmc.h?
We're out of the freeze, so can merge without delay.

But please add SMC_ prefix on the last two macros for that.

> +
> +#define SIP_SVC_VERSION  SIP_FUNCTION_ID(1)

Could you break this out into a separate include file?
I'd suggest
Silicon/Qemu/SbsaQemu/Include/IndustryStandard/SbsaQemuSmc.h
(we'll be adding more calls there later).

/
    Leif

> +
>  EFI_STATUS
>  EFIAPI
>  InitializeSbsaQemuPlatformDxe (
> @@ -26,6 +36,9 @@ InitializeSbsaQemuPlatformDxe (
>    EFI_STATUS                     Status;
>    UINTN                          Size;
>    VOID*                          Base;
> +  UINTN                          Arg0;
> +  UINTN                          Arg1;
> +  UINTN                          Result;
>  
>    DEBUG ((DEBUG_INFO, "%a: InitializeSbsaQemuPlatformDxe called\n", __FUNCTION__));
>  
> @@ -51,5 +64,19 @@ InitializeSbsaQemuPlatformDxe (
>      return Status;
>    }
>  
> +  Result = ArmCallSmc0 (SIP_SVC_VERSION, &Arg0, &Arg1, NULL);
> +  if (Result == SMC_ARCH_CALL_SUCCESS)
> +  {
> +        Result = PcdSet32S (PcdPlatformVersionMajor, Arg0);
> +        ASSERT_EFI_ERROR (Result);
> +        Result = PcdSet32S (PcdPlatformVersionMinor, Arg1);
> +        ASSERT_EFI_ERROR (Result);
> +  }
> +
> +  Arg0 = PcdGet32 (PcdPlatformVersionMajor);
> +  Arg1 = PcdGet32 (PcdPlatformVersionMinor);
> +
> +  DEBUG ((DEBUG_INFO, "Platform version: %d.%d\n", Arg0, Arg1));
> +
>    return EFI_SUCCESS;
>  }
> diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
> index 21d2135f6d17..1f2c8a9dd6af 100644
> --- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
> +++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuPlatformDxe/SbsaQemuPlatformDxe.inf
> @@ -20,6 +20,7 @@
>    SbsaQemuPlatformDxe.c
>  
>  [Packages]
> +  ArmPkg/ArmPkg.dec
>    ArmVirtPkg/ArmVirtPkg.dec
>    EmbeddedPkg/EmbeddedPkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> @@ -27,6 +28,7 @@
>    Silicon/Qemu/SbsaQemu/SbsaQemu.dec
>  
>  [LibraryClasses]
> +  ArmSmcLib
>    PcdLib
>    DebugLib
>    NonDiscoverableDeviceRegistrationLib
> @@ -36,6 +38,9 @@
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciBase
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformAhciSize
>  
> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor
> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor
> +
>  [Depex]
>    TRUE
>  
> diff --git a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> index 9448852967b6..5182978cf56d 100644
> --- a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> +++ b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
> @@ -67,3 +67,6 @@
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisManufacturer|L""|VOID*|0x0000011B
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisAssetTag|L""|VOID*|0x0000011C
>    gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdChassisSKU|L""|VOID*|0x0000011D
> +
> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMajor|0x0|UINT32|0x0000011E
> +  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPlatformVersionMinor|0x0|UINT32|0x0000011F
> -- 
> 2.40.1
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105552): https://edk2.groups.io/g/devel/message/105552
Mute This Topic: https://groups.io/mt/98900379/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/2] Platform/SbsaQemu: read platform version
Posted by Rebecca Cran 1 year ago
On 5/15/23 04:24, Marcin Juszkiewicz wrote:
> +  Result = ArmCallSmc0 (SIP_SVC_VERSION, &Arg0, &Arg1, NULL);
> +  if (Result == SMC_ARCH_CALL_SUCCESS)
> +  {
> +        Result = PcdSet32S (PcdPlatformVersionMajor, Arg0);
> +        ASSERT_EFI_ERROR (Result);
> +        Result = PcdSet32S (PcdPlatformVersionMinor, Arg1);
> +        ASSERT_EFI_ERROR (Result);
> +  }
> +
> +  Arg0 = PcdGet32 (PcdPlatformVersionMajor);
> +  Arg1 = PcdGet32 (PcdPlatformVersionMinor);

I didn't notice this before, but PcdSet32S returns a RETURN_STATUS, not 
UINTN so Result should be type RETURN_STATUS (and probably a different 
variable used for the result of ArmCallSmc0).

Also, you should use ASSERT_RETURN_ERROR not ASSERT_EFI_ERROR (I checked 
ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.c 
to see how error handling is done there).


-- 
Rebecca Cran



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