[PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*
@ 2022-11-28  5:40 Gerd Hoffmann
  2022-12-09 14:15 ` Yao, Jiewen
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2022-11-28  5:40 UTC (permalink / raw)
  To: devel
  Cc: Pawel Polawski, Anthony Perard, Jordan Justen, Jiewen Yao,
	Liming Gao, Julien Grall, Oliver Steffen, Jian J Wang,
	Ard Biesheuvel, Gerd Hoffmann

Instead of using hard-coded strings ("0.0.0" for BiosVersion etc)
which is mostly useless read the PCDs (PcdFirmwareVendor,
PcdFirmwareVersionString and PcdFirmwareReleaseDateString) and
build the string table dynamuically at runtime.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |   6 +
 .../XenSmbiosPlatformDxe.inf                  |   9 +-
 OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 115 +++++++++++-------
 3 files changed, 85 insertions(+), 45 deletions(-)

diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
index 0066bbc9229c..52689c96e5af 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
@@ -32,9 +32,12 @@ [Sources]
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
+  BaseLib
+  BaseMemoryLib
   DebugLib
   MemoryAllocationLib
   PcdLib
@@ -45,6 +48,9 @@ [LibraryClasses]
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString
 
 [Protocols]
   gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
diff --git a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
index 7f4588e33d1e..e646c88741b6 100644
--- a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
+++ b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
@@ -38,19 +38,26 @@ [Sources.ARM, Sources.AARCH64]
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
 
 [Packages.IA32, Packages.X64]
   OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
+  BaseLib
+  BaseMemoryLib
   DebugLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
 
 [LibraryClasses.IA32, LibraryClasses.X64]
-  BaseLib
   HobLib
 
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString
+
 [Protocols]
   gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
 
diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
index 94249d3ff1b0..dc1e6aed634f 100644
--- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
+++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
@@ -9,57 +9,43 @@
 **/
 
 #include <IndustryStandard/SmBios.h>          // SMBIOS_TABLE_TYPE0
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>                 // ASSERT_EFI_ERROR()
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
 #include <Library/UefiBootServicesTableLib.h> // gBS
 #include <Protocol/Smbios.h>                  // EFI_SMBIOS_PROTOCOL
 
 #include "SmbiosPlatformDxe.h"
 
-#define TYPE0_STRINGS \
-  "EFI Development Kit II / OVMF\0"     /* Vendor */ \
-  "0.0.0\0"                             /* BiosVersion */ \
-  "02/06/2015\0"                        /* BiosReleaseDate */
-//
-// Type definition and contents of the default Type 0 SMBIOS table.
-//
-#pragma pack(1)
-typedef struct {
-  SMBIOS_TABLE_TYPE0    Base;
-  UINT8                 Strings[sizeof (TYPE0_STRINGS)];
-} OVMF_TYPE0;
-#pragma pack()
-
-STATIC CONST OVMF_TYPE0  mOvmfDefaultType0 = {
+STATIC CONST SMBIOS_TABLE_TYPE0  mOvmfDefaultType0 = {
+  // SMBIOS_STRUCTURE Hdr
   {
-    // SMBIOS_STRUCTURE Hdr
-    {
-      EFI_SMBIOS_TYPE_BIOS_INFORMATION, // UINT8 Type
-      sizeof (SMBIOS_TABLE_TYPE0),      // UINT8 Length
-    },
-    1,      // SMBIOS_TABLE_STRING       Vendor
-    2,      // SMBIOS_TABLE_STRING       BiosVersion
-    0xE800, // UINT16                    BiosSegment
-    3,      // SMBIOS_TABLE_STRING       BiosReleaseDate
-    0,      // UINT8                     BiosSize
-    {      // MISC_BIOS_CHARACTERISTICS BiosCharacteristics
-      0,   // Reserved                                      :2
-      0,   // Unknown                                       :1
-      1,   // BiosCharacteristicsNotSupported               :1
-           // Remaining BiosCharacteristics bits left unset :60
-    },
-    {      // BIOSCharacteristicsExtensionBytes[2]
-      0,   // BiosReserved
-      0x1C // SystemReserved = VirtualMachineSupported |
-           //                  UefiSpecificationSupported |
-           //                  TargetContentDistributionEnabled
-    },
-    0,     // UINT8                     SystemBiosMajorRelease
-    0,     // UINT8                     SystemBiosMinorRelease
-    0xFF,  // UINT8                     EmbeddedControllerFirmwareMajorRelease
-    0xFF   // UINT8                     EmbeddedControllerFirmwareMinorRelease
+    EFI_SMBIOS_TYPE_BIOS_INFORMATION, // UINT8 Type
+    sizeof (SMBIOS_TABLE_TYPE0),      // UINT8 Length
   },
-  // Text strings (unformatted area)
-  TYPE0_STRINGS
+  1,      // SMBIOS_TABLE_STRING       Vendor
+  2,      // SMBIOS_TABLE_STRING       BiosVersion
+  0xE800, // UINT16                    BiosSegment
+  3,      // SMBIOS_TABLE_STRING       BiosReleaseDate
+  0,      // UINT8                     BiosSize
+  {      // MISC_BIOS_CHARACTERISTICS BiosCharacteristics
+    0,   // Reserved                                      :2
+    0,   // Unknown                                       :1
+    1,   // BiosCharacteristicsNotSupported               :1
+    // Remaining BiosCharacteristics bits left unset :60
+  },
+  {      // BIOSCharacteristicsExtensionBytes[2]
+    0,   // BiosReserved
+    0x1C // SystemReserved = VirtualMachineSupported |
+    //                  UefiSpecificationSupported |
+    //                  TargetContentDistributionEnabled
+  },
+  0,     // UINT8                     SystemBiosMajorRelease
+  0,     // UINT8                     SystemBiosMinorRelease
+  0xFF,  // UINT8                     EmbeddedControllerFirmwareMajorRelease
+  0xFF   // UINT8                     EmbeddedControllerFirmwareMinorRelease
 };
 
 /**
@@ -153,14 +139,55 @@ InstallAllStructures (
     //
     // Add OVMF default Type 0 (BIOS Information) table
     //
+    CHAR16  *VendStr, *VersStr, *DateStr;
+    UINTN   VendLen, VersLen, DateLen;
+    CHAR8   *Type0;
+
+    VendStr = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareVendor);
+    VendLen = StrLen (VendStr);
+    if (VendLen < 3) {
+      VendStr = L"unknown";
+      VendLen = StrLen (VendStr);
+    }
+
+    VersStr = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareVersionString);
+    VersLen = StrLen (VersStr);
+    if (VersLen < 3) {
+      VersStr = L"unknown";
+      VersLen = StrLen (VersStr);
+    }
+
+    DateStr = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareReleaseDateString);
+    DateLen = StrLen (DateStr);
+    if (DateLen < 3) {
+      DateStr = L"unknown";
+      DateLen = StrLen (DateStr);
+    }
+
+    DEBUG ((DEBUG_INFO, "FirmwareVendor:            \"%s\" (%d chars)\n", VendStr, VendLen));
+    DEBUG ((DEBUG_INFO, "FirmwareVersionString:     \"%s\" (%d chars)\n", VersStr, VersLen));
+    DEBUG ((DEBUG_INFO, "FirmwareReleaseDateString: \"%s\" (%d chars)\n", DateStr, DateLen));
+
+    Type0 = AllocateZeroPool (sizeof (mOvmfDefaultType0) + VendLen + VersLen + DateLen + 4);
+    if (Type0 == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    CopyMem (Type0, &mOvmfDefaultType0, sizeof (mOvmfDefaultType0));
+    UnicodeStrToAsciiStrS (VendStr, Type0 + sizeof (mOvmfDefaultType0), VendLen + 1);
+    UnicodeStrToAsciiStrS (VersStr, Type0 + sizeof (mOvmfDefaultType0) + VendLen + 1, VersLen + 1);
+    UnicodeStrToAsciiStrS (DateStr, Type0 + sizeof (mOvmfDefaultType0) + VendLen + VersLen + 2, DateLen + 1);
+
     SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
     Status       = Smbios->Add (
                              Smbios,
                              NULL,
                              &SmbiosHandle,
-                             (EFI_SMBIOS_TABLE_HEADER *)&mOvmfDefaultType0
+                             (EFI_SMBIOS_TABLE_HEADER *)Type0
                              );
     ASSERT_EFI_ERROR (Status);
+
+    FreePool (Type0);
   }
 
   return EFI_SUCCESS;
-- 
2.38.1


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

* Re: [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*
  2022-11-28  5:40 [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware* Gerd Hoffmann
@ 2022-12-09 14:15 ` Yao, Jiewen
       [not found] ` <172F259BA3D70E83.5373@groups.io>
  2023-03-30  8:04 ` Fiona Ebner
  2 siblings, 0 replies; 7+ messages in thread
From: Yao, Jiewen @ 2022-12-09 14:15 UTC (permalink / raw)
  To: Gerd Hoffmann, devel@edk2.groups.io
  Cc: Pawel Polawski, Anthony Perard, Justen, Jordan L, Gao, Liming,
	Julien Grall, Oliver Steffen, Wang, Jian J, Ard Biesheuvel

Looks good to me.

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

Any further discussion?



> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Monday, November 28, 2022 1:40 PM
> To: devel@edk2.groups.io
> Cc: Pawel Polawski <ppolawsk@redhat.com>; Anthony Perard
> <anthony.perard@citrix.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Julien Grall <julien@xen.org>; Oliver Steffen
> <osteffen@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann
> <kraxel@redhat.com>
> Subject: [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*
> 
> Instead of using hard-coded strings ("0.0.0" for BiosVersion etc)
> which is mostly useless read the PCDs (PcdFirmwareVendor,
> PcdFirmwareVersionString and PcdFirmwareReleaseDateString) and
> build the string table dynamuically at runtime.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |   6 +
>  .../XenSmbiosPlatformDxe.inf                  |   9 +-
>  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 115 +++++++++++---
> ----
>  3 files changed, 85 insertions(+), 45 deletions(-)
> 
> diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> index 0066bbc9229c..52689c96e5af 100644
> --- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> +++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> @@ -32,9 +32,12 @@ [Sources]
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
>    OvmfPkg/OvmfPkg.dec
> 
>  [LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
>    DebugLib
>    MemoryAllocationLib
>    PcdLib
> @@ -45,6 +48,9 @@ [LibraryClasses]
>  [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
>    gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString
> 
>  [Protocols]
>    gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
> diff --git a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
> b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
> index 7f4588e33d1e..e646c88741b6 100644
> --- a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
> +++ b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
> @@ -38,19 +38,26 @@ [Sources.ARM, Sources.AARCH64]
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> 
>  [Packages.IA32, Packages.X64]
>    OvmfPkg/OvmfPkg.dec
> 
>  [LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
>    DebugLib
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
> 
>  [LibraryClasses.IA32, LibraryClasses.X64]
> -  BaseLib
>    HobLib
> 
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString
> +
>  [Protocols]
>    gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
> 
> diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> index 94249d3ff1b0..dc1e6aed634f 100644
> --- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> +++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> @@ -9,57 +9,43 @@
>  **/
> 
>  #include <IndustryStandard/SmBios.h>          // SMBIOS_TABLE_TYPE0
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>                 // ASSERT_EFI_ERROR()
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
>  #include <Library/UefiBootServicesTableLib.h> // gBS
>  #include <Protocol/Smbios.h>                  // EFI_SMBIOS_PROTOCOL
> 
>  #include "SmbiosPlatformDxe.h"
> 
> -#define TYPE0_STRINGS \
> -  "EFI Development Kit II / OVMF\0"     /* Vendor */ \
> -  "0.0.0\0"                             /* BiosVersion */ \
> -  "02/06/2015\0"                        /* BiosReleaseDate */
> -//
> -// Type definition and contents of the default Type 0 SMBIOS table.
> -//
> -#pragma pack(1)
> -typedef struct {
> -  SMBIOS_TABLE_TYPE0    Base;
> -  UINT8                 Strings[sizeof (TYPE0_STRINGS)];
> -} OVMF_TYPE0;
> -#pragma pack()
> -
> -STATIC CONST OVMF_TYPE0  mOvmfDefaultType0 = {
> +STATIC CONST SMBIOS_TABLE_TYPE0  mOvmfDefaultType0 = {
> +  // SMBIOS_STRUCTURE Hdr
>    {
> -    // SMBIOS_STRUCTURE Hdr
> -    {
> -      EFI_SMBIOS_TYPE_BIOS_INFORMATION, // UINT8 Type
> -      sizeof (SMBIOS_TABLE_TYPE0),      // UINT8 Length
> -    },
> -    1,      // SMBIOS_TABLE_STRING       Vendor
> -    2,      // SMBIOS_TABLE_STRING       BiosVersion
> -    0xE800, // UINT16                    BiosSegment
> -    3,      // SMBIOS_TABLE_STRING       BiosReleaseDate
> -    0,      // UINT8                     BiosSize
> -    {      // MISC_BIOS_CHARACTERISTICS BiosCharacteristics
> -      0,   // Reserved                                      :2
> -      0,   // Unknown                                       :1
> -      1,   // BiosCharacteristicsNotSupported               :1
> -           // Remaining BiosCharacteristics bits left unset :60
> -    },
> -    {      // BIOSCharacteristicsExtensionBytes[2]
> -      0,   // BiosReserved
> -      0x1C // SystemReserved = VirtualMachineSupported |
> -           //                  UefiSpecificationSupported |
> -           //                  TargetContentDistributionEnabled
> -    },
> -    0,     // UINT8                     SystemBiosMajorRelease
> -    0,     // UINT8                     SystemBiosMinorRelease
> -    0xFF,  // UINT8                     EmbeddedControllerFirmwareMajorRelease
> -    0xFF   // UINT8                     EmbeddedControllerFirmwareMinorRelease
> +    EFI_SMBIOS_TYPE_BIOS_INFORMATION, // UINT8 Type
> +    sizeof (SMBIOS_TABLE_TYPE0),      // UINT8 Length
>    },
> -  // Text strings (unformatted area)
> -  TYPE0_STRINGS
> +  1,      // SMBIOS_TABLE_STRING       Vendor
> +  2,      // SMBIOS_TABLE_STRING       BiosVersion
> +  0xE800, // UINT16                    BiosSegment
> +  3,      // SMBIOS_TABLE_STRING       BiosReleaseDate
> +  0,      // UINT8                     BiosSize
> +  {      // MISC_BIOS_CHARACTERISTICS BiosCharacteristics
> +    0,   // Reserved                                      :2
> +    0,   // Unknown                                       :1
> +    1,   // BiosCharacteristicsNotSupported               :1
> +    // Remaining BiosCharacteristics bits left unset :60
> +  },
> +  {      // BIOSCharacteristicsExtensionBytes[2]
> +    0,   // BiosReserved
> +    0x1C // SystemReserved = VirtualMachineSupported |
> +    //                  UefiSpecificationSupported |
> +    //                  TargetContentDistributionEnabled
> +  },
> +  0,     // UINT8                     SystemBiosMajorRelease
> +  0,     // UINT8                     SystemBiosMinorRelease
> +  0xFF,  // UINT8                     EmbeddedControllerFirmwareMajorRelease
> +  0xFF   // UINT8                     EmbeddedControllerFirmwareMinorRelease
>  };
> 
>  /**
> @@ -153,14 +139,55 @@ InstallAllStructures (
>      //
>      // Add OVMF default Type 0 (BIOS Information) table
>      //
> +    CHAR16  *VendStr, *VersStr, *DateStr;
> +    UINTN   VendLen, VersLen, DateLen;
> +    CHAR8   *Type0;
> +
> +    VendStr = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareVendor);
> +    VendLen = StrLen (VendStr);
> +    if (VendLen < 3) {
> +      VendStr = L"unknown";
> +      VendLen = StrLen (VendStr);
> +    }
> +
> +    VersStr = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareVersionString);
> +    VersLen = StrLen (VersStr);
> +    if (VersLen < 3) {
> +      VersStr = L"unknown";
> +      VersLen = StrLen (VersStr);
> +    }
> +
> +    DateStr = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareReleaseDateString);
> +    DateLen = StrLen (DateStr);
> +    if (DateLen < 3) {
> +      DateStr = L"unknown";
> +      DateLen = StrLen (DateStr);
> +    }
> +
> +    DEBUG ((DEBUG_INFO, "FirmwareVendor:            \"%s\" (%d chars)\n",
> VendStr, VendLen));
> +    DEBUG ((DEBUG_INFO, "FirmwareVersionString:     \"%s\" (%d chars)\n",
> VersStr, VersLen));
> +    DEBUG ((DEBUG_INFO, "FirmwareReleaseDateString: \"%s\" (%d
> chars)\n", DateStr, DateLen));
> +
> +    Type0 = AllocateZeroPool (sizeof (mOvmfDefaultType0) + VendLen +
> VersLen + DateLen + 4);
> +    if (Type0 == NULL) {
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    CopyMem (Type0, &mOvmfDefaultType0, sizeof (mOvmfDefaultType0));
> +    UnicodeStrToAsciiStrS (VendStr, Type0 + sizeof (mOvmfDefaultType0),
> VendLen + 1);
> +    UnicodeStrToAsciiStrS (VersStr, Type0 + sizeof (mOvmfDefaultType0) +
> VendLen + 1, VersLen + 1);
> +    UnicodeStrToAsciiStrS (DateStr, Type0 + sizeof (mOvmfDefaultType0) +
> VendLen + VersLen + 2, DateLen + 1);
> +
>      SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
>      Status       = Smbios->Add (
>                               Smbios,
>                               NULL,
>                               &SmbiosHandle,
> -                             (EFI_SMBIOS_TABLE_HEADER *)&mOvmfDefaultType0
> +                             (EFI_SMBIOS_TABLE_HEADER *)Type0
>                               );
>      ASSERT_EFI_ERROR (Status);
> +
> +    FreePool (Type0);
>    }
> 
>    return EFI_SUCCESS;
> --
> 2.38.1


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

[parent not found: <172F259BA3D70E83.5373@groups.io>]

* Re: [edk2-devel] [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*
       [not found] ` <172F259BA3D70E83.5373@groups.io>
@ 2022-12-11  3:09   ` Yao, Jiewen
  0 siblings, 0 replies; 7+ messages in thread
From: Yao, Jiewen @ 2022-12-11  3:09 UTC (permalink / raw)
  To: devel@edk2.groups.io, Yao, Jiewen, Gerd Hoffmann
  Cc: Pawel Polawski, Anthony Perard, Justen, Jordan L, Gao, Liming,
	Julien Grall, Oliver Steffen, Wang, Jian J, Ard Biesheuvel

Merged https://github.com/tianocore/edk2/pull/3745

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> Sent: Friday, December 9, 2022 10:16 PM
> To: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io
> Cc: Pawel Polawski <ppolawsk@redhat.com>; Anthony Perard
> <anthony.perard@citrix.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>; Julien Grall <julien@xen.org>;
> Oliver Steffen <osteffen@redhat.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>
> Subject: Re: [edk2-devel] [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use
> PcdFirmware*
> 
> Looks good to me.
> 
> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> 
> Any further discussion?
> 
> 
> 
> > -----Original Message-----
> > From: Gerd Hoffmann <kraxel@redhat.com>
> > Sent: Monday, November 28, 2022 1:40 PM
> > To: devel@edk2.groups.io
> > Cc: Pawel Polawski <ppolawsk@redhat.com>; Anthony Perard
> > <anthony.perard@citrix.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>;
> > Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Julien Grall <julien@xen.org>; Oliver Steffen
> > <osteffen@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Ard
> > Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann
> > <kraxel@redhat.com>
> > Subject: [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*
> >
> > Instead of using hard-coded strings ("0.0.0" for BiosVersion etc)
> > which is mostly useless read the PCDs (PcdFirmwareVendor,
> > PcdFirmwareVersionString and PcdFirmwareReleaseDateString) and
> > build the string table dynamuically at runtime.
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |   6 +
> >  .../XenSmbiosPlatformDxe.inf                  |   9 +-
> >  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 115 +++++++++++---
> > ----
> >  3 files changed, 85 insertions(+), 45 deletions(-)
> >
> > diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> > b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> > index 0066bbc9229c..52689c96e5af 100644
> > --- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> > +++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> > @@ -32,9 +32,12 @@ [Sources]
> >
> >  [Packages]
> >    MdePkg/MdePkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> >    OvmfPkg/OvmfPkg.dec
> >
> >  [LibraryClasses]
> > +  BaseLib
> > +  BaseMemoryLib
> >    DebugLib
> >    MemoryAllocationLib
> >    PcdLib
> > @@ -45,6 +48,9 @@ [LibraryClasses]
> >  [Pcd]
> >    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
> >    gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString
> >
> >  [Protocols]
> >    gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
> > diff --git a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
> > b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
> > index 7f4588e33d1e..e646c88741b6 100644
> > --- a/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
> > +++ b/OvmfPkg/SmbiosPlatformDxe/XenSmbiosPlatformDxe.inf
> > @@ -38,19 +38,26 @@ [Sources.ARM, Sources.AARCH64]
> >
> >  [Packages]
> >    MdePkg/MdePkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> >
> >  [Packages.IA32, Packages.X64]
> >    OvmfPkg/OvmfPkg.dec
> >
> >  [LibraryClasses]
> > +  BaseLib
> > +  BaseMemoryLib
> >    DebugLib
> >    UefiBootServicesTableLib
> >    UefiDriverEntryPoint
> >
> >  [LibraryClasses.IA32, LibraryClasses.X64]
> > -  BaseLib
> >    HobLib
> >
> > +[Pcd]
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareReleaseDateString
> > +
> >  [Protocols]
> >    gEfiSmbiosProtocolGuid                      # PROTOCOL ALWAYS_CONSUMED
> >
> > diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> > b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> > index 94249d3ff1b0..dc1e6aed634f 100644
> > --- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> > +++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> > @@ -9,57 +9,43 @@
> >  **/
> >
> >  #include <IndustryStandard/SmBios.h>          // SMBIOS_TABLE_TYPE0
> > +#include <Library/BaseLib.h>
> > +#include <Library/BaseMemoryLib.h>
> >  #include <Library/DebugLib.h>                 // ASSERT_EFI_ERROR()
> > +#include <Library/MemoryAllocationLib.h>
> > +#include <Library/PcdLib.h>
> >  #include <Library/UefiBootServicesTableLib.h> // gBS
> >  #include <Protocol/Smbios.h>                  // EFI_SMBIOS_PROTOCOL
> >
> >  #include "SmbiosPlatformDxe.h"
> >
> > -#define TYPE0_STRINGS \
> > -  "EFI Development Kit II / OVMF\0"     /* Vendor */ \
> > -  "0.0.0\0"                             /* BiosVersion */ \
> > -  "02/06/2015\0"                        /* BiosReleaseDate */
> > -//
> > -// Type definition and contents of the default Type 0 SMBIOS table.
> > -//
> > -#pragma pack(1)
> > -typedef struct {
> > -  SMBIOS_TABLE_TYPE0    Base;
> > -  UINT8                 Strings[sizeof (TYPE0_STRINGS)];
> > -} OVMF_TYPE0;
> > -#pragma pack()
> > -
> > -STATIC CONST OVMF_TYPE0  mOvmfDefaultType0 = {
> > +STATIC CONST SMBIOS_TABLE_TYPE0  mOvmfDefaultType0 = {
> > +  // SMBIOS_STRUCTURE Hdr
> >    {
> > -    // SMBIOS_STRUCTURE Hdr
> > -    {
> > -      EFI_SMBIOS_TYPE_BIOS_INFORMATION, // UINT8 Type
> > -      sizeof (SMBIOS_TABLE_TYPE0),      // UINT8 Length
> > -    },
> > -    1,      // SMBIOS_TABLE_STRING       Vendor
> > -    2,      // SMBIOS_TABLE_STRING       BiosVersion
> > -    0xE800, // UINT16                    BiosSegment
> > -    3,      // SMBIOS_TABLE_STRING       BiosReleaseDate
> > -    0,      // UINT8                     BiosSize
> > -    {      // MISC_BIOS_CHARACTERISTICS BiosCharacteristics
> > -      0,   // Reserved                                      :2
> > -      0,   // Unknown                                       :1
> > -      1,   // BiosCharacteristicsNotSupported               :1
> > -           // Remaining BiosCharacteristics bits left unset :60
> > -    },
> > -    {      // BIOSCharacteristicsExtensionBytes[2]
> > -      0,   // BiosReserved
> > -      0x1C // SystemReserved = VirtualMachineSupported |
> > -           //                  UefiSpecificationSupported |
> > -           //                  TargetContentDistributionEnabled
> > -    },
> > -    0,     // UINT8                     SystemBiosMajorRelease
> > -    0,     // UINT8                     SystemBiosMinorRelease
> > -    0xFF,  // UINT8                     EmbeddedControllerFirmwareMajorRelease
> > -    0xFF   // UINT8                     EmbeddedControllerFirmwareMinorRelease
> > +    EFI_SMBIOS_TYPE_BIOS_INFORMATION, // UINT8 Type
> > +    sizeof (SMBIOS_TABLE_TYPE0),      // UINT8 Length
> >    },
> > -  // Text strings (unformatted area)
> > -  TYPE0_STRINGS
> > +  1,      // SMBIOS_TABLE_STRING       Vendor
> > +  2,      // SMBIOS_TABLE_STRING       BiosVersion
> > +  0xE800, // UINT16                    BiosSegment
> > +  3,      // SMBIOS_TABLE_STRING       BiosReleaseDate
> > +  0,      // UINT8                     BiosSize
> > +  {      // MISC_BIOS_CHARACTERISTICS BiosCharacteristics
> > +    0,   // Reserved                                      :2
> > +    0,   // Unknown                                       :1
> > +    1,   // BiosCharacteristicsNotSupported               :1
> > +    // Remaining BiosCharacteristics bits left unset :60
> > +  },
> > +  {      // BIOSCharacteristicsExtensionBytes[2]
> > +    0,   // BiosReserved
> > +    0x1C // SystemReserved = VirtualMachineSupported |
> > +    //                  UefiSpecificationSupported |
> > +    //                  TargetContentDistributionEnabled
> > +  },
> > +  0,     // UINT8                     SystemBiosMajorRelease
> > +  0,     // UINT8                     SystemBiosMinorRelease
> > +  0xFF,  // UINT8                     EmbeddedControllerFirmwareMajorRelease
> > +  0xFF   // UINT8                     EmbeddedControllerFirmwareMinorRelease
> >  };
> >
> >  /**
> > @@ -153,14 +139,55 @@ InstallAllStructures (
> >      //
> >      // Add OVMF default Type 0 (BIOS Information) table
> >      //
> > +    CHAR16  *VendStr, *VersStr, *DateStr;
> > +    UINTN   VendLen, VersLen, DateLen;
> > +    CHAR8   *Type0;
> > +
> > +    VendStr = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareVendor);
> > +    VendLen = StrLen (VendStr);
> > +    if (VendLen < 3) {
> > +      VendStr = L"unknown";
> > +      VendLen = StrLen (VendStr);
> > +    }
> > +
> > +    VersStr = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareVersionString);
> > +    VersLen = StrLen (VersStr);
> > +    if (VersLen < 3) {
> > +      VersStr = L"unknown";
> > +      VersLen = StrLen (VersStr);
> > +    }
> > +
> > +    DateStr = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareReleaseDateString);
> > +    DateLen = StrLen (DateStr);
> > +    if (DateLen < 3) {
> > +      DateStr = L"unknown";
> > +      DateLen = StrLen (DateStr);
> > +    }
> > +
> > +    DEBUG ((DEBUG_INFO, "FirmwareVendor:            \"%s\" (%d chars)\n",
> > VendStr, VendLen));
> > +    DEBUG ((DEBUG_INFO, "FirmwareVersionString:     \"%s\" (%d chars)\n",
> > VersStr, VersLen));
> > +    DEBUG ((DEBUG_INFO, "FirmwareReleaseDateString: \"%s\" (%d
> > chars)\n", DateStr, DateLen));
> > +
> > +    Type0 = AllocateZeroPool (sizeof (mOvmfDefaultType0) + VendLen +
> > VersLen + DateLen + 4);
> > +    if (Type0 == NULL) {
> > +      return EFI_OUT_OF_RESOURCES;
> > +    }
> > +
> > +    CopyMem (Type0, &mOvmfDefaultType0, sizeof (mOvmfDefaultType0));
> > +    UnicodeStrToAsciiStrS (VendStr, Type0 + sizeof (mOvmfDefaultType0),
> > VendLen + 1);
> > +    UnicodeStrToAsciiStrS (VersStr, Type0 + sizeof (mOvmfDefaultType0) +
> > VendLen + 1, VersLen + 1);
> > +    UnicodeStrToAsciiStrS (DateStr, Type0 + sizeof (mOvmfDefaultType0) +
> > VendLen + VersLen + 2, DateLen + 1);
> > +
> >      SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> >      Status       = Smbios->Add (
> >                               Smbios,
> >                               NULL,
> >                               &SmbiosHandle,
> > -                             (EFI_SMBIOS_TABLE_HEADER *)&mOvmfDefaultType0
> > +                             (EFI_SMBIOS_TABLE_HEADER *)Type0
> >                               );
> >      ASSERT_EFI_ERROR (Status);
> > +
> > +    FreePool (Type0);
> >    }
> >
> >    return EFI_SUCCESS;
> > --
> > 2.38.1
> 
> 
> 
> 
> 


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

* Re: [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*
  2022-11-28  5:40 [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware* Gerd Hoffmann
  2022-12-09 14:15 ` Yao, Jiewen
       [not found] ` <172F259BA3D70E83.5373@groups.io>
@ 2023-03-30  8:04 ` Fiona Ebner
  2023-03-30  8:53   ` Gerd Hoffmann
  2 siblings, 1 reply; 7+ messages in thread
From: Fiona Ebner @ 2023-03-30  8:04 UTC (permalink / raw)
  To: Gerd Hoffmann, devel
  Cc: Pawel Polawski, Anthony Perard, Jordan Justen, Jiewen Yao,
	Liming Gao, Julien Grall, Oliver Steffen, Jian J Wang,
	Ard Biesheuvel, dannf, Thomas Lamprecht

Am 28.11.22 um 06:40 schrieb Gerd Hoffmann:
> Instead of using hard-coded strings ("0.0.0" for BiosVersion etc)
> which is mostly useless read the PCDs (PcdFirmwareVendor,
> PcdFirmwareVersionString and PcdFirmwareReleaseDateString) and
> build the string table dynamuically at runtime.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |   6 +
>  .../XenSmbiosPlatformDxe.inf                  |   9 +-
>  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 115 +++++++++++-------
>  3 files changed, 85 insertions(+), 45 deletions(-)
> 

Hi,
after this patch, certain SMBIOS values are different[2]. We got a
report that the different vendor causes issues with hardware keys[0].
And, as I learned from the patch fixing it, the missing date can cause
issues with Windows[1].

To keep backwards compatibility for [0], it's necessary for us to
specify the old vendor value during build.

I assume this will affect other downstream distributors too. Since we
base our packaging off Debian, I asked there for how to best handle it
and am now passing along the question I got in return:

Am 29.03.23 um 18:51 schrieb dann frazier:
> It'd be
> good to understand what upstream's intention was there - are they
> expecting each distributor to set our own values, and if so, is there
> a scheme we should follow?

Best Regards,
Fiona

[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=4625
[1]: https://edk2.groups.io/g/devel/message/100922
[2]: dmidecode output differences in a Linux guest when running with a
build with OvmfPkg/OvmfPkgIa32X64.dsc:

edk2-stable202205:

Vendor: EFI Development Kit II / OVMF
Version: 0.0.0
Release Date: 02/06/2015

edk2-stable202302:

Vendor: EDK II
Version: unknown
Release Date: unknown


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

* Re: [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*
  2023-03-30  8:04 ` Fiona Ebner
@ 2023-03-30  8:53   ` Gerd Hoffmann
  2023-03-30 10:18     ` Fiona Ebner
  0 siblings, 1 reply; 7+ messages in thread
From: Gerd Hoffmann @ 2023-03-30  8:53 UTC (permalink / raw)
  To: Fiona Ebner
  Cc: devel, Pawel Polawski, Anthony Perard, Jordan Justen, Jiewen Yao,
	Liming Gao, Julien Grall, Oliver Steffen, Jian J Wang,
	Ard Biesheuvel, dannf, Thomas Lamprecht

On Thu, Mar 30, 2023 at 10:04:02AM +0200, Fiona Ebner wrote:
> Am 28.11.22 um 06:40 schrieb Gerd Hoffmann:
> > Instead of using hard-coded strings ("0.0.0" for BiosVersion etc)
> > which is mostly useless read the PCDs (PcdFirmwareVendor,
> > PcdFirmwareVersionString and PcdFirmwareReleaseDateString) and
> > build the string table dynamuically at runtime.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |   6 +
> >  .../XenSmbiosPlatformDxe.inf                  |   9 +-
> >  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 115 +++++++++++-------
> >  3 files changed, 85 insertions(+), 45 deletions(-)
> > 
> 
> Hi,
> after this patch, certain SMBIOS values are different[2]. We got a
> report that the different vendor causes issues with hardware keys[0].
> And, as I learned from the patch fixing it, the missing date can cause
> issues with Windows[1].

See a0f9628705e3 ("OvmfPkg/SmbiosPlatformDxe: tweak fallback release
date") for the windows issue.

> I assume this will affect other downstream distributors too. Since we
> base our packaging off Debian, I asked there for how to best handle it
> and am now passing along the question I got in return:

You can set those using
	'build --pcd PcdFirmwareVersionString="L${string}\\0" ...'
(same for the other pcds).

We had a patch introducing a define for that to allow the slightly more
convenient 'build -D FIRMWARE_VER="${string}" ...' but that was never
merged.

> > It'd be
> > good to understand what upstream's intention was there - are they
> > expecting each distributor to set our own values, and if so, is there
> > a scheme we should follow?

The intention is to allow setting version information to whatever makes
sense for you.  A hardcoded "0.0.0" version certainly isn't very useful
when it comes to bug reporting etc.

Fedora leaves vendor unchanged, sets release date to the commit date of
the edk2-stableyyyymm stable tag the build is based on and version to
the rpm package version, i.e. like this ...

    Handle 0x0000, DMI type 0, 26 bytes
    BIOS Information
	Vendor: EDK II
	Version: edk2-20230301gitf80f052277c8-1.test6.fc37
	Release Date: 03/01/2023
	[ ... ]

... which allows to easily identify the exact firmware build running.
The one listed above happens to be this one:

https://download.copr.fedorainfracloud.org/results/kraxel/edk2.testbuilds/fedora-37-x86_64/05727085-edk2/

HTH & take care,
  Gerd


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

* Re: [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*
  2023-03-30  8:53   ` Gerd Hoffmann
@ 2023-03-30 10:18     ` Fiona Ebner
  2023-03-30 10:50       ` Gerd Hoffmann
  0 siblings, 1 reply; 7+ messages in thread
From: Fiona Ebner @ 2023-03-30 10:18 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: devel, Pawel Polawski, Anthony Perard, Jordan Justen, Jiewen Yao,
	Liming Gao, Julien Grall, Oliver Steffen, Jian J Wang,
	Ard Biesheuvel, dannf, Thomas Lamprecht

Am 30.03.23 um 10:53 schrieb Gerd Hoffmann:
> On Thu, Mar 30, 2023 at 10:04:02AM +0200, Fiona Ebner wrote:
>> Am 28.11.22 um 06:40 schrieb Gerd Hoffmann:
>>> Instead of using hard-coded strings ("0.0.0" for BiosVersion etc)
>>> which is mostly useless read the PCDs (PcdFirmwareVendor,
>>> PcdFirmwareVersionString and PcdFirmwareReleaseDateString) and
>>> build the string table dynamuically at runtime.
>>>
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> ---
>>>  .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf   |   6 +
>>>  .../XenSmbiosPlatformDxe.inf                  |   9 +-
>>>  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 115 +++++++++++-------
>>>  3 files changed, 85 insertions(+), 45 deletions(-)
>>>
>>
>> Hi,
>> after this patch, certain SMBIOS values are different[2]. We got a
>> report that the different vendor causes issues with hardware keys[0].
>> And, as I learned from the patch fixing it, the missing date can cause
>> issues with Windows[1].
> 
> See a0f9628705e3 ("OvmfPkg/SmbiosPlatformDxe: tweak fallback release
> date") for the windows issue.
> 

Should the date string be "02/02/2022" rather than "2/2/2022" in that
commit?

I was wondering whether month or date comes first and found the
following in ([0], section 7.1):
> String number of the BIOS release date.
> The date string, if supplied, is in either
> mm/dd/yy or mm/dd/yyyy format. If the year
> portion of the string is two digits, the year is
> assumed to be 19yy.
> NOTE: The mm/dd/yyyy format is required for
> SMBIOS version 2.3 and later.


> You can set those using
> 	'build --pcd PcdFirmwareVersionString="L${string}\\0" ...'
> (same for the other pcds).

I had already found the build option in the discussion of v1 of this
patch, but now I'm wondering: is the "\\0" explicitly required? My build
yesterday seemed to work without it, but I guess, I'll just add it to
make sure.

>>> It'd be
>>> good to understand what upstream's intention was there - are they
>>> expecting each distributor to set our own values, and if so, is there
>>> a scheme we should follow?
> 
> The intention is to allow setting version information to whatever makes
> sense for you.  A hardcoded "0.0.0" version certainly isn't very useful
> when it comes to bug reporting etc.
> 
> Fedora leaves vendor unchanged, sets release date to the commit date of
> the edk2-stableyyyymm stable tag the build is based on and version to
> the rpm package version, i.e. like this ...
> 
>     Handle 0x0000, DMI type 0, 26 bytes
>     BIOS Information
> 	Vendor: EDK II
> 	Version: edk2-20230301gitf80f052277c8-1.test6.fc37
> 	Release Date: 03/01/2023
> 	[ ... ]
> 
> ... which allows to easily identify the exact firmware build running.
> The one listed above happens to be this one:
> 
> https://download.copr.fedorainfracloud.org/results/kraxel/edk2.testbuilds/fedora-37-x86_64/05727085-edk2/
> 

Thank you for the explanation!

Best regards,
Fiona

[0]:
https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.2.0.pdf


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

* Re: [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware*
  2023-03-30 10:18     ` Fiona Ebner
@ 2023-03-30 10:50       ` Gerd Hoffmann
  0 siblings, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2023-03-30 10:50 UTC (permalink / raw)
  To: Fiona Ebner
  Cc: devel, Pawel Polawski, Anthony Perard, Jordan Justen, Jiewen Yao,
	Liming Gao, Julien Grall, Oliver Steffen, Jian J Wang,
	Ard Biesheuvel, dannf, Thomas Lamprecht

On Thu, Mar 30, 2023 at 12:18:35PM +0200, Fiona Ebner wrote:
> Am 30.03.23 um 10:53 schrieb Gerd Hoffmann:
> > On Thu, Mar 30, 2023 at 10:04:02AM +0200, Fiona Ebner wrote:
> >> Am 28.11.22 um 06:40 schrieb Gerd Hoffmann:
> >> Hi,
> >> after this patch, certain SMBIOS values are different[2]. We got a
> >> report that the different vendor causes issues with hardware keys[0].
> >> And, as I learned from the patch fixing it, the missing date can cause
> >> issues with Windows[1].
> > 
> > See a0f9628705e3 ("OvmfPkg/SmbiosPlatformDxe: tweak fallback release
> > date") for the windows issue.
> > 
> 
> Should the date string be "02/02/2022" rather than "2/2/2022" in that
> commit?

Windows tests pass with "2/2/2022", but I have to admit didn't check the
specs.

take care,
  Gerd


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

end of thread, other threads:[~2023-03-30 10:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-28  5:40 [PATCH v3 1/1] OvmfPkg/SmbiosPlatformDxe: use PcdFirmware* Gerd Hoffmann
2022-12-09 14:15 ` Yao, Jiewen
     [not found] ` <172F259BA3D70E83.5373@groups.io>
2022-12-11  3:09   ` [edk2-devel] " Yao, Jiewen
2023-03-30  8:04 ` Fiona Ebner
2023-03-30  8:53   ` Gerd Hoffmann
2023-03-30 10:18     ` Fiona Ebner
2023-03-30 10:50       ` Gerd Hoffmann

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