Re: [edk2-devel] [edk2-platform][PATCH v1 1/1] Platforms/RaspberryPi/RPi4: Add Asset Tag support

["Samer El-Haj-Mahmoud" <samer.el-haj-mahmoud@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 12238 bytes --]

Thanks Andrei. I submitted a v2 with the commit message changes. I will also send a follow-up patch to add examples for RPi3/4 config scripting using the Shell

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Andrei Warkentin via groups.io
Sent: Monday, June 8, 2020 11:53 AM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; devel@edk2.groups.io
Cc: Leif Lindholm <leif@nuviainc.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Pete Batard <pete@akeo.ie>; Rui Lopes <rgl@ruilopes.com>
Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 1/1] Platforms/RaspberryPi/RPi4: Add Asset Tag support

Looks good to me aside from minor typos in commit message. Also, note that this applies to Pi 3 as well.

Would it be possible to have a follow-up change to amend the readmes for RPi3/RPi4 with this setvar example to set AssetTag?

Reviewed-by:  Andrei Warkentin <andrey.warkentin@gmail.com<mailto:andrey.warkentin@gmail.com>>
________________________________
From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>
Sent: Thursday, June 4, 2020 7:27 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Leif Lindholm <leif@nuviainc.com<mailto:leif@nuviainc.com>>; Ard Biesheuvel <ard.biesheuvel@arm.com<mailto:ard.biesheuvel@arm.com>>; Pete Batard <pete@akeo.ie<mailto:pete@akeo.ie>>; Andrei Warkentin <awarkentin@vmware.com<mailto:awarkentin@vmware.com>>; Rui Lopes <rgl@ruilopes.com<mailto:rgl@ruilopes.com>>
Subject: [edk2-platform][PATCH v1 1/1] Platforms/RaspberryPi/RPi4: Add Asset Tag support

Add suppot for configuring the Chassis AssetTag in the UI as well as
UEFI Shell, and carry the confiugured value as the Asset Tag string
in SMBIOS Types 2 and 3.

To configure using the UEFI Shell, use 'setvar' comnmand to read/write
the UEFI variable with GUID = gConfigDxeFormSetGuid and Name="AssetTag".
For example:

Shell> setvar AssetTag -guid CD7CC258-31DB-22E6-9F22-63B0B8EED6B5 -bs
-rt -nv =L"ABC123" =0x0000

This resolves this Github issue: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4%2Fissues%2F54&amp;data=02%7C01%7Cawarkentin%40vmware.com%7C8d843c5163b24fe547d308d8088296c5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637268704259127440&amp;sdata=Le7LTzcWXvSNZlOGqqBsl0qiU7cUw1473yDYZ7Bvk%2FI%3D&amp;reserved=0

Cc: Leif Lindholm <leif@nuviainc.com<mailto:leif@nuviainc.com>>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com<mailto:ard.biesheuvel@arm.com>>
Cc: Pete Batard <pete@akeo.ie<mailto:pete@akeo.ie>>
Cc: Andrei Warkentin <awarkentin@vmware.com<mailto:awarkentin@vmware.com>>
Cc: Rui Lopes <rgl@ruilopes.com<mailto:rgl@ruilopes.com>>
Signed-off-by: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com<mailto:samer.el-haj-mahmoud@arm.com>>
---
 Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf |  2 ++
 Platform/RaspberryPi/Include/ConfigVars.h                            |  7 ++++++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr              | 14 +++++++++++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c                   | 23 ++++++++++++++---
 Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c   | 26 ++++++++++++++++++--
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni              |  4 +++
 6 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf
index 1ed6338c69cb..59b2fefdf0fd 100644
--- a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf
+++ b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf
@@ -43,7 +43,9 @@ [LibraryClasses]
 [Protocols]
   gEfiSmbiosProtocolGuid           # PROTOCOL SOMETIMES_CONSUMED
   gRaspberryPiFirmwareProtocolGuid ## CONSUMES
+
 [Guids]
+  gConfigDxeFormSetGuid

 [Depex]
   gEfiSmbiosProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
index cefddbafcd8f..9e5a69c7a657 100644
--- a/Platform/RaspberryPi/Include/ConfigVars.h
+++ b/Platform/RaspberryPi/Include/ConfigVars.h
@@ -1,6 +1,7 @@
 /** @file
  *
  *  Copyright (c) 2020, Andrei Warkentin <andrey.warkentin@gmail.com<mailto:andrey.warkentin@gmail.com>>
+ *  Copyright (c) 2020, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -82,6 +83,12 @@ typedef struct {
   UINT32 Mode;
 } SYSTEM_TABLE_MODE_VARSTORE_DATA;

+#define ASSET_TAG_STR_MAX_LEN       32
+#define ASSET_TAG_STR_STORAGE_SIZE  33
+typedef struct {
+  CHAR16 AssetTag[ASSET_TAG_STR_STORAGE_SIZE];
+} ADVANCED_ASSET_TAG_VARSTORE_DATA;
+
 typedef struct {
   /*
    * 0 - uSD slot routed to Broadcom SDHOST on Pi 3 or eMMC2 on Pi 4.
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index 72cc90ae0bec..b4b2a3a7abde 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -1,6 +1,7 @@
 /** @file
  *
  *  Copyright (c) 2018 Andrei Warkentin <andrey.warkentin@gmail.com<mailto:andrey.warkentin@gmail.com>>
+ *  Copyright (c) 2020, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -49,6 +50,11 @@ formset
       name  = SystemTableMode,
       guid  = CONFIGDXE_FORM_SET_GUID;

+    efivarstore ADVANCED_ASSET_TAG_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = AssetTag,
+      guid  = CONFIGDXE_FORM_SET_GUID;
+
     efivarstore MMC_SD_VARSTORE_DATA,
       attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
       name  = SdIsArasan,
@@ -172,6 +178,14 @@ formset
             option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_BOTH), value = SYSTEM_TABLE_MODE_BOTH, flags = DEFAULT;
             option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), value = SYSTEM_TABLE_MODE_DT, flags = DEFAULT;
         endoneof;
+
+        string varid = AssetTag.AssetTag,
+            prompt  = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_PROMPT),
+            help    = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_HELP),
+            flags   = INTERACTIVE | RESET_REQUIRED,
+            minsize = 0,
+            maxsize = ASSET_TAG_STR_MAX_LEN,
+        endstring;
     endform;

     form formid = 0x1003,
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 583f07c9b8bf..77a0b37529ea 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -186,9 +186,10 @@ SetupVariables (
   VOID
   )
 {
-  UINTN Size;
-  UINT8 Var8;
-  UINT32 Var32;
+  UINTN      Size;
+  UINT8      Var8;
+  UINT32     Var32;
+  CHAR16     AssetTagVar[ASSET_TAG_STR_STORAGE_SIZE] = L"";
   EFI_STATUS Status;

   /*
@@ -238,6 +239,22 @@ SetupVariables (
     PcdSet32 (PcdSystemTableMode, PcdGet32 (PcdSystemTableMode));
   }

+  Size = sizeof(AssetTagVar);
+
+  Status = gRT->GetVariable(L"AssetTag",
+                  &gConfigDxeFormSetGuid,
+                  NULL, &Size, AssetTagVar);
+
+  if (EFI_ERROR (Status)) {
+    Status = gRT->SetVariable (
+                    L"AssetTag",
+                    &gConfigDxeFormSetGuid,
+                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+                    sizeof(AssetTagVar),
+                    AssetTagVar
+                    );
+  }
+
   Size = sizeof (UINT32);
   Status = gRT->GetVariable (L"SdIsArasan",
                   &gConfigDxeFormSetGuid,
diff --git a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
index 3351fea2ec32..7b86e76a1248 100644
--- a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
+++ b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
@@ -20,6 +20,7 @@
  *  Copyright (c) 2013, Linaro.org
  *  Copyright (c) 2012, Apple Inc. All rights reserved.<BR>
  *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Copyright (c) 2020, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -40,7 +41,9 @@
 #include <Library/MemoryAllocationLib.h>
 #include <Library/TimeBaseLib.h>
 #include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
 #include <Library/PrintLib.h>
+#include <ConfigVars.h>

 #define SMB_IS_DIGIT(c)  (((c) >= '0') && ((c) <= '9'))

@@ -164,7 +167,7 @@ SMBIOS_TABLE_TYPE2 mBoardInfoType2 = {
   2,    // ProductName String
   3,    // Version String
   4,    // SerialNumber String
-  0,    // AssetTag String
+  5,    // AssetTag String
   {     // FeatureFlag
     1,    //  Motherboard           :1;
     0,    //  RequiresDaughterCard  :1;
@@ -179,11 +182,15 @@ SMBIOS_TABLE_TYPE2 mBoardInfoType2 = {
   0,                        // NumberOfContainedObjectHandles;
   { 0 }                     // ContainedObjectHandles[1];
 };
+
+CHAR8 mChassisAssetTag[128];
+
 CHAR8 *mBoardInfoType2Strings[] = {
   mSysInfoManufName,
   mSysInfoProductName,
   mSysInfoVersionName,
   mSysInfoSerial,
+  mChassisAssetTag,
   NULL
 };

@@ -196,7 +203,7 @@ SMBIOS_TABLE_TYPE3 mEnclosureInfoType3 = {
   MiscChassisEmbeddedPc,    // Type;
   2,                        // Version String
   3,                        // SerialNumber String
-  0,                        // AssetTag String
+  4,                        // AssetTag String
   ChassisStateSafe,         // BootupState;
   ChassisStateSafe,         // PowerSupplyState;
   ChassisStateSafe,         // ThermalState;
@@ -212,6 +219,7 @@ CHAR8 *mEnclosureInfoType3Strings[] = {
   mSysInfoManufName,
   mSysInfoProductName,
   mSysInfoSerial,
+  mChassisAssetTag,
   NULL
 };

@@ -760,6 +768,20 @@ BoardInfoUpdateSmbiosType2 (
   VOID
   )
 {
+  UINTN      Size;
+  CHAR16     AssetTagVar[ASSET_TAG_STR_STORAGE_SIZE] = L"";
+  EFI_STATUS Status;
+
+  Size = sizeof(AssetTagVar);
+  Status = gRT->GetVariable(L"AssetTag",
+                  &gConfigDxeFormSetGuid,
+                  NULL, &Size, AssetTagVar);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to get Asset Tag: %r\n", Status));
+  }
+  UnicodeStrToAsciiStrS(AssetTagVar, mChassisAssetTag, sizeof(mChassisAssetTag));
+  DEBUG ((DEBUG_INFO, "System Asset Tag : %a\n", mChassisAssetTag));
+
   LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mBoardInfoType2, mBoardInfoType2Strings, NULL);
 }

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 7195e497f986..2a468760c6a9 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -1,6 +1,7 @@
 /** @file
  *
  *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com<mailto:andrey.warkentin@gmail.com>>
+ *  Copyright (c) 2020, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -47,6 +48,9 @@
 #string STR_ADVANCED_SYSTAB_BOTH     #language en-US "ACPI + Devicetree"
 #string STR_ADVANCED_SYSTAB_DT       #language en-US "Devicetree"

+#string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
+#string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset Tag"
+
 /*
  * MMC/SD configuration.
  */
--
2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Attachment #2: Type: text/html, Size: 22443 bytes --]