[edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer

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

* [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer
@ 2020-07-20 16:15 Pete Batard
  2020-07-20 16:47 ` Andrei Warkentin
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Pete Batard @ 2020-07-20 16:15 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, awarkentin, samer.el-haj-mahmoud

Per SMBIOS specs, The Type 0 BIOS Release Date is not a free form field but
must be specified in a US middle-endian format (mm/dd/yyyy), so make sure
we populate it accordingly by converting gcc's __DATE__ string. This is
required for platforms like Windows, that fail to parse the date otherwise.

Also, the system manufacturer should not be set to the same value as the
board manufacturer for the Type 1 strings, as, on the Raspberry Pi, this is
not representative of the actual manufacturer of the system, which is the
Raspberry Pi Foundation always.

It should be noted that we do not expect other compilers than ones using
a __DATE__ format similar to gcc's to be used for the foreseeable future.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 31 ++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
index d5fb843d43ce..fb775d00feba 100644
--- a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
+++ b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
@@ -119,11 +119,12 @@ SMBIOS_TABLE_TYPE0 mBIOSInfoType0 = {
 
 CHAR8 mBiosVendor[128]  = "EDK2";
 CHAR8 mBiosVersion[128] = "EDK2-DEV";
+CHAR8 mBiosDate[12]     = "00/00/0000";
 
 CHAR8 *mBIOSInfoType0Strings[] = {
   mBiosVendor,              // Vendor
   mBiosVersion,             // Version
-  __DATE__ " " __TIME__,    // Release Date
+  mBiosDate,                // Release Date
   NULL
 };
 
@@ -149,7 +150,7 @@ CHAR8 mSysInfoSerial[sizeof (UINT64) * 2 + 1];
 CHAR8 mSysInfoSKU[sizeof (UINT64) * 2 + 1];
 
 CHAR8 *mSysInfoType1Strings[] = {
-  mSysInfoManufName,
+  "Raspberry Pi Foundation",
   mSysInfoProductName,
   mSysInfoVersionName,
   mSysInfoSerial,
@@ -626,6 +627,28 @@ BIOSInfoUpdateSmbiosType0 (
   INTN   i;
   INTN   State = 0;
   INTN   Value[2];
+  INTN   Year = (__DATE__[7] == '?' ? 1900  \
+           : (((__DATE__[7] - '0') * 1000 ) \
+           + (__DATE__[8] - '0') * 100      \
+           + (__DATE__[9] - '0') * 10       \
+           + __DATE__[10] - '0'));
+  INTN   Month = ( __DATE__ [2] == '?' ? 1  \
+           : __DATE__ [2] == 'n' ? (        \
+             __DATE__ [1] == 'a' ? 1 : 6)   \
+           : __DATE__ [2] == 'b' ? 2        \
+           : __DATE__ [2] == 'r' ? (        \
+             __DATE__ [0] == 'M' ? 3 : 4)   \
+           : __DATE__ [2] == 'y' ? 5        \
+           : __DATE__ [2] == 'l' ? 7        \
+           : __DATE__ [2] == 'g' ? 8        \
+           : __DATE__ [2] == 'p' ? 9        \
+           : __DATE__ [2] == 't' ? 10       \
+           : __DATE__ [2] == 'v' ? 11       \
+           : 12);
+  INTN   Day = ( __DATE__[4] == '?' ? 1     \
+           : ((__DATE__[4] == ' ' ? 0 :     \
+             ((__DATE__[4] - '0') * 10))    \
+           + __DATE__[5] - '0'));
 
   // Populate the Firmware major and minor.
   Status = mFwProtocol->GetFirmwareRevision (&EpochSeconds);
@@ -648,6 +671,10 @@ BIOSInfoUpdateSmbiosType0 (
     mBiosVendor, sizeof (mBiosVendor));
   UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
     mBiosVersion, sizeof (mBiosVersion));
+  ASSERT (Year >= 0 && Year <= 9999);
+  ASSERT (Month >= 1 && Month <= 12);
+  ASSERT (Day >= 1 && Day <= 31);
+  AsciiSPrint (mBiosDate, sizeof (mBiosDate), "%02d/%02d/%04d", Month, Day, Year);
 
   // Look for a "x.y" numeric string anywhere in mBiosVersion and
   // try to parse it to populate the BIOS major and minor.
-- 
2.21.0.windows.1


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

* Re: [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer
  2020-07-20 16:15 [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer Pete Batard
@ 2020-07-20 16:47 ` Andrei Warkentin
  2020-07-20 16:51 ` Samer El-Haj-Mahmoud
  2020-07-23 11:08 ` Leif Lindholm
  2 siblings, 0 replies; 8+ messages in thread
From: Andrei Warkentin @ 2020-07-20 16:47 UTC (permalink / raw)
  To: Pete Batard, devel@edk2.groups.io
  Cc: ard.biesheuvel@arm.com, leif@nuviainc.com,
	samer.el-haj-mahmoud@arm.com

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

Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>
________________________________
From: Pete Batard <pete@akeo.ie>
Sent: Monday, July 20, 2020 11:15 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@arm.com <ard.biesheuvel@arm.com>; leif@nuviainc.com <leif@nuviainc.com>; Andrei Warkentin <awarkentin@vmware.com>; samer.el-haj-mahmoud@arm.com <samer.el-haj-mahmoud@arm.com>
Subject: [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer

Per SMBIOS specs, The Type 0 BIOS Release Date is not a free form field but
must be specified in a US middle-endian format (mm/dd/yyyy), so make sure
we populate it accordingly by converting gcc's __DATE__ string. This is
required for platforms like Windows, that fail to parse the date otherwise.

Also, the system manufacturer should not be set to the same value as the
board manufacturer for the Type 1 strings, as, on the Raspberry Pi, this is
not representative of the actual manufacturer of the system, which is the
Raspberry Pi Foundation always.

It should be noted that we do not expect other compilers than ones using
a __DATE__ format similar to gcc's to be used for the foreseeable future.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 31 ++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
index d5fb843d43ce..fb775d00feba 100644
--- a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
+++ b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
@@ -119,11 +119,12 @@ SMBIOS_TABLE_TYPE0 mBIOSInfoType0 = {

 CHAR8 mBiosVendor[128]  = "EDK2";
 CHAR8 mBiosVersion[128] = "EDK2-DEV";
+CHAR8 mBiosDate[12]     = "00/00/0000";

 CHAR8 *mBIOSInfoType0Strings[] = {
   mBiosVendor,              // Vendor
   mBiosVersion,             // Version
-  __DATE__ " " __TIME__,    // Release Date
+  mBiosDate,                // Release Date
   NULL
 };

@@ -149,7 +150,7 @@ CHAR8 mSysInfoSerial[sizeof (UINT64) * 2 + 1];
 CHAR8 mSysInfoSKU[sizeof (UINT64) * 2 + 1];

 CHAR8 *mSysInfoType1Strings[] = {
-  mSysInfoManufName,
+  "Raspberry Pi Foundation",
   mSysInfoProductName,
   mSysInfoVersionName,
   mSysInfoSerial,
@@ -626,6 +627,28 @@ BIOSInfoUpdateSmbiosType0 (
   INTN   i;
   INTN   State = 0;
   INTN   Value[2];
+  INTN   Year = (__DATE__[7] == '?' ? 1900  \
+           : (((__DATE__[7] - '0') * 1000 ) \
+           + (__DATE__[8] - '0') * 100      \
+           + (__DATE__[9] - '0') * 10       \
+           + __DATE__[10] - '0'));
+  INTN   Month = ( __DATE__ [2] == '?' ? 1  \
+           : __DATE__ [2] == 'n' ? (        \
+             __DATE__ [1] == 'a' ? 1 : 6)   \
+           : __DATE__ [2] == 'b' ? 2        \
+           : __DATE__ [2] == 'r' ? (        \
+             __DATE__ [0] == 'M' ? 3 : 4)   \
+           : __DATE__ [2] == 'y' ? 5        \
+           : __DATE__ [2] == 'l' ? 7        \
+           : __DATE__ [2] == 'g' ? 8        \
+           : __DATE__ [2] == 'p' ? 9        \
+           : __DATE__ [2] == 't' ? 10       \
+           : __DATE__ [2] == 'v' ? 11       \
+           : 12);
+  INTN   Day = ( __DATE__[4] == '?' ? 1     \
+           : ((__DATE__[4] == ' ' ? 0 :     \
+             ((__DATE__[4] - '0') * 10))    \
+           + __DATE__[5] - '0'));

   // Populate the Firmware major and minor.
   Status = mFwProtocol->GetFirmwareRevision (&EpochSeconds);
@@ -648,6 +671,10 @@ BIOSInfoUpdateSmbiosType0 (
     mBiosVendor, sizeof (mBiosVendor));
   UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
     mBiosVersion, sizeof (mBiosVersion));
+  ASSERT (Year >= 0 && Year <= 9999);
+  ASSERT (Month >= 1 && Month <= 12);
+  ASSERT (Day >= 1 && Day <= 31);
+  AsciiSPrint (mBiosDate, sizeof (mBiosDate), "%02d/%02d/%04d", Month, Day, Year);

   // Look for a "x.y" numeric string anywhere in mBiosVersion and
   // try to parse it to populate the BIOS major and minor.
--
2.21.0.windows.1


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

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

* Re: [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer
  2020-07-20 16:15 [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer Pete Batard
  2020-07-20 16:47 ` Andrei Warkentin
@ 2020-07-20 16:51 ` Samer El-Haj-Mahmoud
  2020-07-23 11:08 ` Leif Lindholm
  2 siblings, 0 replies; 8+ messages in thread
From: Samer El-Haj-Mahmoud @ 2020-07-20 16:51 UTC (permalink / raw)
  To: Pete Batard, devel@edk2.groups.io
  Cc: Ard Biesheuvel, leif@nuviainc.com,
	Andrei Warkentin (awarkentin@vmware.com), Samer El-Haj-Mahmoud

Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

> -----Original Message-----
> From: Pete Batard <pete@akeo.ie>
> Sent: Monday, July 20, 2020 12:15 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com; Andrei
> Warkentin (awarkentin@vmware.com) <awarkentin@vmware.com>; Samer
> El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Subject: [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS
> Release Date and System Manufacturer
>
> Per SMBIOS specs, The Type 0 BIOS Release Date is not a free form field but
> must be specified in a US middle-endian format (mm/dd/yyyy), so make sure
> we populate it accordingly by converting gcc's __DATE__ string. This is
> required for platforms like Windows, that fail to parse the date otherwise.
>
> Also, the system manufacturer should not be set to the same value as the
> board manufacturer for the Type 1 strings, as, on the Raspberry Pi, this is not
> representative of the actual manufacturer of the system, which is the
> Raspberry Pi Foundation always.
>
> It should be noted that we do not expect other compilers than ones using a
> __DATE__ format similar to gcc's to be used for the foreseeable future.
>
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>  Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c |
> 31 ++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git
> a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> index d5fb843d43ce..fb775d00feba 100644
> ---
> a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> +++
> b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> @@ -119,11 +119,12 @@ SMBIOS_TABLE_TYPE0 mBIOSInfoType0 = {
>
>  CHAR8 mBiosVendor[128]  = "EDK2";
>  CHAR8 mBiosVersion[128] = "EDK2-DEV";
> +CHAR8 mBiosDate[12]     = "00/00/0000";
>
>  CHAR8 *mBIOSInfoType0Strings[] = {
>    mBiosVendor,              // Vendor
>    mBiosVersion,             // Version
> -  __DATE__ " " __TIME__,    // Release Date
> +  mBiosDate,                // Release Date
>    NULL
>  };
>
> @@ -149,7 +150,7 @@ CHAR8 mSysInfoSerial[sizeof (UINT64) * 2 + 1];
>  CHAR8 mSysInfoSKU[sizeof (UINT64) * 2 + 1];
>
>  CHAR8 *mSysInfoType1Strings[] = {
> -  mSysInfoManufName,
> +  "Raspberry Pi Foundation",
>    mSysInfoProductName,
>    mSysInfoVersionName,
>    mSysInfoSerial,
> @@ -626,6 +627,28 @@ BIOSInfoUpdateSmbiosType0 (
>    INTN   i;
>    INTN   State = 0;
>    INTN   Value[2];
> +  INTN   Year = (__DATE__[7] == '?' ? 1900  \
> +           : (((__DATE__[7] - '0') * 1000 ) \
> +           + (__DATE__[8] - '0') * 100      \
> +           + (__DATE__[9] - '0') * 10       \
> +           + __DATE__[10] - '0'));
> +  INTN   Month = ( __DATE__ [2] == '?' ? 1  \
> +           : __DATE__ [2] == 'n' ? (        \
> +             __DATE__ [1] == 'a' ? 1 : 6)   \
> +           : __DATE__ [2] == 'b' ? 2        \
> +           : __DATE__ [2] == 'r' ? (        \
> +             __DATE__ [0] == 'M' ? 3 : 4)   \
> +           : __DATE__ [2] == 'y' ? 5        \
> +           : __DATE__ [2] == 'l' ? 7        \
> +           : __DATE__ [2] == 'g' ? 8        \
> +           : __DATE__ [2] == 'p' ? 9        \
> +           : __DATE__ [2] == 't' ? 10       \
> +           : __DATE__ [2] == 'v' ? 11       \
> +           : 12);
> +  INTN   Day = ( __DATE__[4] == '?' ? 1     \
> +           : ((__DATE__[4] == ' ' ? 0 :     \
> +             ((__DATE__[4] - '0') * 10))    \
> +           + __DATE__[5] - '0'));
>
>    // Populate the Firmware major and minor.
>    Status = mFwProtocol->GetFirmwareRevision (&EpochSeconds); @@ -
> 648,6 +671,10 @@ BIOSInfoUpdateSmbiosType0 (
>      mBiosVendor, sizeof (mBiosVendor));
>    UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
>      mBiosVersion, sizeof (mBiosVersion));
> +  ASSERT (Year >= 0 && Year <= 9999);
> +  ASSERT (Month >= 1 && Month <= 12);
> +  ASSERT (Day >= 1 && Day <= 31);
> +  AsciiSPrint (mBiosDate, sizeof (mBiosDate), "%02d/%02d/%04d", Month,
> + Day, Year);
>
>    // Look for a "x.y" numeric string anywhere in mBiosVersion and
>    // try to parse it to populate the BIOS major and minor.
> --
> 2.21.0.windows.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.

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

* Re: [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer
  2020-07-20 16:15 [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer Pete Batard
  2020-07-20 16:47 ` Andrei Warkentin
  2020-07-20 16:51 ` Samer El-Haj-Mahmoud
@ 2020-07-23 11:08 ` Leif Lindholm
  2020-07-23 14:38   ` Pete Batard
  2 siblings, 1 reply; 8+ messages in thread
From: Leif Lindholm @ 2020-07-23 11:08 UTC (permalink / raw)
  To: Pete Batard; +Cc: devel, ard.biesheuvel, awarkentin, samer.el-haj-mahmoud

On Mon, Jul 20, 2020 at 17:15:07 +0100, Pete Batard wrote:
> Per SMBIOS specs, The Type 0 BIOS Release Date is not a free form field but
> must be specified in a US middle-endian format (mm/dd/yyyy), so make sure
> we populate it accordingly by converting gcc's __DATE__ string. This is
> required for platforms like Windows, that fail to parse the date otherwise.
> 
> Also, the system manufacturer should not be set to the same value as the
> board manufacturer for the Type 1 strings, as, on the Raspberry Pi, this is
> not representative of the actual manufacturer of the system, which is the
> Raspberry Pi Foundation always.
> 
> It should be noted that we do not expect other compilers than ones using
> a __DATE__ format similar to gcc's to be used for the foreseeable future.
> 
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>  Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 31 ++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> index d5fb843d43ce..fb775d00feba 100644
> --- a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> +++ b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> @@ -119,11 +119,12 @@ SMBIOS_TABLE_TYPE0 mBIOSInfoType0 = {
>  
>  CHAR8 mBiosVendor[128]  = "EDK2";
>  CHAR8 mBiosVersion[128] = "EDK2-DEV";
> +CHAR8 mBiosDate[12]     = "00/00/0000";
>  
>  CHAR8 *mBIOSInfoType0Strings[] = {
>    mBiosVendor,              // Vendor
>    mBiosVersion,             // Version
> -  __DATE__ " " __TIME__,    // Release Date
> +  mBiosDate,                // Release Date
>    NULL
>  };
>  
> @@ -149,7 +150,7 @@ CHAR8 mSysInfoSerial[sizeof (UINT64) * 2 + 1];
>  CHAR8 mSysInfoSKU[sizeof (UINT64) * 2 + 1];
>  
>  CHAR8 *mSysInfoType1Strings[] = {
> -  mSysInfoManufName,
> +  "Raspberry Pi Foundation",
>    mSysInfoProductName,
>    mSysInfoVersionName,
>    mSysInfoSerial,
> @@ -626,6 +627,28 @@ BIOSInfoUpdateSmbiosType0 (
>    INTN   i;
>    INTN   State = 0;
>    INTN   Value[2];
> +  INTN   Year = (__DATE__[7] == '?' ? 1900  \
> +           : (((__DATE__[7] - '0') * 1000 ) \
> +           + (__DATE__[8] - '0') * 100      \
> +           + (__DATE__[9] - '0') * 10       \
> +           + __DATE__[10] - '0'));
> +  INTN   Month = ( __DATE__ [2] == '?' ? 1  \
> +           : __DATE__ [2] == 'n' ? (        \
> +             __DATE__ [1] == 'a' ? 1 : 6)   \
> +           : __DATE__ [2] == 'b' ? 2        \
> +           : __DATE__ [2] == 'r' ? (        \
> +             __DATE__ [0] == 'M' ? 3 : 4)   \
> +           : __DATE__ [2] == 'y' ? 5        \
> +           : __DATE__ [2] == 'l' ? 7        \
> +           : __DATE__ [2] == 'g' ? 8        \
> +           : __DATE__ [2] == 'p' ? 9        \
> +           : __DATE__ [2] == 't' ? 10       \
> +           : __DATE__ [2] == 'v' ? 11       \
> +           : 12);
> +  INTN   Day = ( __DATE__[4] == '?' ? 1     \
> +           : ((__DATE__[4] == ' ' ? 0 :     \
> +             ((__DATE__[4] - '0') * 10))    \
> +           + __DATE__[5] - '0'));

So, this hunk is very neat, but nigh-on unreviewable.
I.e. we should defintely have it - but only once.

Could you break this up into some macros to go into some generic
helper lib? (I don't have a better idea than EmbeddedPkg TimeBaseLib,
but then that is already included in this module.)

Would you be OK to break that snippet out separate?

/
    Leif

>    // Populate the Firmware major and minor.
>    Status = mFwProtocol->GetFirmwareRevision (&EpochSeconds);
> @@ -648,6 +671,10 @@ BIOSInfoUpdateSmbiosType0 (
>      mBiosVendor, sizeof (mBiosVendor));
>    UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
>      mBiosVersion, sizeof (mBiosVersion));
> +  ASSERT (Year >= 0 && Year <= 9999);
> +  ASSERT (Month >= 1 && Month <= 12);
> +  ASSERT (Day >= 1 && Day <= 31);
> +  AsciiSPrint (mBiosDate, sizeof (mBiosDate), "%02d/%02d/%04d", Month, Day, Year);
>  
>    // Look for a "x.y" numeric string anywhere in mBiosVersion and
>    // try to parse it to populate the BIOS major and minor.
> -- 
> 2.21.0.windows.1
> 

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

* Re: [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer
  2020-07-23 11:08 ` Leif Lindholm
@ 2020-07-23 14:38   ` Pete Batard
  2020-07-23 15:47     ` Leif Lindholm
  0 siblings, 1 reply; 8+ messages in thread
From: Pete Batard @ 2020-07-23 14:38 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: devel, ard.biesheuvel, awarkentin, samer.el-haj-mahmoud

Hi Leif,

On 2020.07.23 12:08, Leif Lindholm wrote:
> On Mon, Jul 20, 2020 at 17:15:07 +0100, Pete Batard wrote:
>> Per SMBIOS specs, The Type 0 BIOS Release Date is not a free form field but
>> must be specified in a US middle-endian format (mm/dd/yyyy), so make sure
>> we populate it accordingly by converting gcc's __DATE__ string. This is
>> required for platforms like Windows, that fail to parse the date otherwise.
>>
>> Also, the system manufacturer should not be set to the same value as the
>> board manufacturer for the Type 1 strings, as, on the Raspberry Pi, this is
>> not representative of the actual manufacturer of the system, which is the
>> Raspberry Pi Foundation always.
>>
>> It should be noted that we do not expect other compilers than ones using
>> a __DATE__ format similar to gcc's to be used for the foreseeable future.
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
>>   Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 31 ++++++++++++++++++--
>>   1 file changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>> index d5fb843d43ce..fb775d00feba 100644
>> --- a/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>> +++ b/Platform/RaspberryPi/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>> @@ -119,11 +119,12 @@ SMBIOS_TABLE_TYPE0 mBIOSInfoType0 = {
>>   
>>   CHAR8 mBiosVendor[128]  = "EDK2";
>>   CHAR8 mBiosVersion[128] = "EDK2-DEV";
>> +CHAR8 mBiosDate[12]     = "00/00/0000";
>>   
>>   CHAR8 *mBIOSInfoType0Strings[] = {
>>     mBiosVendor,              // Vendor
>>     mBiosVersion,             // Version
>> -  __DATE__ " " __TIME__,    // Release Date
>> +  mBiosDate,                // Release Date
>>     NULL
>>   };
>>   
>> @@ -149,7 +150,7 @@ CHAR8 mSysInfoSerial[sizeof (UINT64) * 2 + 1];
>>   CHAR8 mSysInfoSKU[sizeof (UINT64) * 2 + 1];
>>   
>>   CHAR8 *mSysInfoType1Strings[] = {
>> -  mSysInfoManufName,
>> +  "Raspberry Pi Foundation",
>>     mSysInfoProductName,
>>     mSysInfoVersionName,
>>     mSysInfoSerial,
>> @@ -626,6 +627,28 @@ BIOSInfoUpdateSmbiosType0 (
>>     INTN   i;
>>     INTN   State = 0;
>>     INTN   Value[2];
>> +  INTN   Year = (__DATE__[7] == '?' ? 1900  \
>> +           : (((__DATE__[7] - '0') * 1000 ) \
>> +           + (__DATE__[8] - '0') * 100      \
>> +           + (__DATE__[9] - '0') * 10       \
>> +           + __DATE__[10] - '0'));
>> +  INTN   Month = ( __DATE__ [2] == '?' ? 1  \
>> +           : __DATE__ [2] == 'n' ? (        \
>> +             __DATE__ [1] == 'a' ? 1 : 6)   \
>> +           : __DATE__ [2] == 'b' ? 2        \
>> +           : __DATE__ [2] == 'r' ? (        \
>> +             __DATE__ [0] == 'M' ? 3 : 4)   \
>> +           : __DATE__ [2] == 'y' ? 5        \
>> +           : __DATE__ [2] == 'l' ? 7        \
>> +           : __DATE__ [2] == 'g' ? 8        \
>> +           : __DATE__ [2] == 'p' ? 9        \
>> +           : __DATE__ [2] == 't' ? 10       \
>> +           : __DATE__ [2] == 'v' ? 11       \
>> +           : 12);
>> +  INTN   Day = ( __DATE__[4] == '?' ? 1     \
>> +           : ((__DATE__[4] == ' ' ? 0 :     \
>> +             ((__DATE__[4] - '0') * 10))    \
>> +           + __DATE__[5] - '0'));
> 
> So, this hunk is very neat, but nigh-on unreviewable.
> I.e. we should defintely have it - but only once.
> 
> Could you break this up into some macros to go into some generic
> helper lib? (I don't have a better idea than EmbeddedPkg TimeBaseLib,
> but then that is already included in this module.)

So you would like to have a set of macros like:
TIME_GET_BUILD_YEAR, TIME_GET_BUILD_MONTH, TIME_GET_BUILD_DAY in 
TimeBaseLib.h that perform the above?

> Would you be OK to break that snippet out separate?

I think that's a good idea, especially as there is a potential 
underlying issue with the __DATE__ format being specific to each 
compiler, so we probably want to handle compiler detection somewhere, 
preferably globally. For instance, the Intel compiler's __DATE__ format 
would not work with the above, so I'll add some "vetted compiler" 
detection for the macros.

The one thing I am not planning to look into is that, of course, as long 
as you don't explicitly force the compiler to rebuild the sources where 
these macros are used, then you may end up with a very obsolete build 
date compared to the actual date of your last re-build. But that's 
mostly because I don't think there exists a generic solution we can ise 
to force recompilation of a file that uses a specific macro and also 
because our main goal with these is to ensure that the Pi firmwares, 
that we produce through AppVeyor, have a proper build date, and AppVeyor 
builds are are always clean.

I'll send an EDK2 patch for the macros, and then a revised patch for 
this when I get a chance.

Regards,

/Pete

> 
> /
>      Leif
> 
>>     // Populate the Firmware major and minor.
>>     Status = mFwProtocol->GetFirmwareRevision (&EpochSeconds);
>> @@ -648,6 +671,10 @@ BIOSInfoUpdateSmbiosType0 (
>>       mBiosVendor, sizeof (mBiosVendor));
>>     UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
>>       mBiosVersion, sizeof (mBiosVersion));
>> +  ASSERT (Year >= 0 && Year <= 9999);
>> +  ASSERT (Month >= 1 && Month <= 12);
>> +  ASSERT (Day >= 1 && Day <= 31);
>> +  AsciiSPrint (mBiosDate, sizeof (mBiosDate), "%02d/%02d/%04d", Month, Day, Year);
>>   
>>     // Look for a "x.y" numeric string anywhere in mBiosVersion and
>>     // try to parse it to populate the BIOS major and minor.
>> -- 
>> 2.21.0.windows.1
>>


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

* Re: [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer
  2020-07-23 14:38   ` Pete Batard
@ 2020-07-23 15:47     ` Leif Lindholm
  2020-07-23 17:15       ` [edk2-devel] " Andrew Fish
  0 siblings, 1 reply; 8+ messages in thread
From: Leif Lindholm @ 2020-07-23 15:47 UTC (permalink / raw)
  To: Pete Batard; +Cc: devel, ard.biesheuvel, awarkentin, samer.el-haj-mahmoud

Hi Pete,

On Thu, Jul 23, 2020 at 15:38:55 +0100, Pete Batard wrote:
> > > @@ -626,6 +627,28 @@ BIOSInfoUpdateSmbiosType0 (
> > >     INTN   i;
> > >     INTN   State = 0;
> > >     INTN   Value[2];
> > > +  INTN   Year = (__DATE__[7] == '?' ? 1900  \
> > > +           : (((__DATE__[7] - '0') * 1000 ) \
> > > +           + (__DATE__[8] - '0') * 100      \
> > > +           + (__DATE__[9] - '0') * 10       \
> > > +           + __DATE__[10] - '0'));
> > > +  INTN   Month = ( __DATE__ [2] == '?' ? 1  \
> > > +           : __DATE__ [2] == 'n' ? (        \
> > > +             __DATE__ [1] == 'a' ? 1 : 6)   \
> > > +           : __DATE__ [2] == 'b' ? 2        \
> > > +           : __DATE__ [2] == 'r' ? (        \
> > > +             __DATE__ [0] == 'M' ? 3 : 4)   \
> > > +           : __DATE__ [2] == 'y' ? 5        \
> > > +           : __DATE__ [2] == 'l' ? 7        \
> > > +           : __DATE__ [2] == 'g' ? 8        \
> > > +           : __DATE__ [2] == 'p' ? 9        \
> > > +           : __DATE__ [2] == 't' ? 10       \
> > > +           : __DATE__ [2] == 'v' ? 11       \
> > > +           : 12);
> > > +  INTN   Day = ( __DATE__[4] == '?' ? 1     \
> > > +           : ((__DATE__[4] == ' ' ? 0 :     \
> > > +             ((__DATE__[4] - '0') * 10))    \
> > > +           + __DATE__[5] - '0'));
> > 
> > So, this hunk is very neat, but nigh-on unreviewable.
> > I.e. we should defintely have it - but only once.
> > 
> > Could you break this up into some macros to go into some generic
> > helper lib? (I don't have a better idea than EmbeddedPkg TimeBaseLib,
> > but then that is already included in this module.)
> 
> So you would like to have a set of macros like:
> TIME_GET_BUILD_YEAR, TIME_GET_BUILD_MONTH, TIME_GET_BUILD_DAY in
> TimeBaseLib.h that perform the above?
> 
> > Would you be OK to break that snippet out separate?
> 
> I think that's a good idea, especially as there is a potential underlying
> issue with the __DATE__ format being specific to each compiler, so we
> probably want to handle compiler detection somewhere, preferably globally.
> For instance, the Intel compiler's __DATE__ format would not work with the
> above, so I'll add some "vetted compiler" detection for the macros.

Yes, thanks.
A friendly "your compiler probably won't know what to do with this"
#error would be friendlier than whatever non-GCC/clang would generate
on encountering this.

> The one thing I am not planning to look into is that, of course, as long as
> you don't explicitly force the compiler to rebuild the sources where these
> macros are used, then you may end up with a very obsolete build date
> compared to the actual date of your last re-build.

Yeah, that's fine.

> But that's mostly because
> I don't think there exists a generic solution we can ise to force
> recompilation of a file that uses a specific macro and also because our main
> goal with these is to ensure that the Pi firmwares, that we produce through
> AppVeyor, have a proper build date, and AppVeyor builds are are always
> clean.

Yeah, I really don't care about that.
Or the flipside, where someone modifies only (say) the Smbios
populating code a month after they built the rest of the tree.

If someone wants to put a *specific* date in, that should be done
manually, and if someone wants information on what was actually built,
they should put in the hashes of the top commits of all repositories
on PACKAGES_PATH as well as whether the state of each was clean.

Managing these bits through a rigid build process, like you are, is
very much valid.

> I'll send an EDK2 patch for the macros, and then a revised patch for this
> when I get a chance.

Thanks!

Regards,

Leif

> Regards,
> 
> /Pete
> 
> > 
> > /
> >      Leif
> > 
> > >     // Populate the Firmware major and minor.
> > >     Status = mFwProtocol->GetFirmwareRevision (&EpochSeconds);
> > > @@ -648,6 +671,10 @@ BIOSInfoUpdateSmbiosType0 (
> > >       mBiosVendor, sizeof (mBiosVendor));
> > >     UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
> > >       mBiosVersion, sizeof (mBiosVersion));
> > > +  ASSERT (Year >= 0 && Year <= 9999);
> > > +  ASSERT (Month >= 1 && Month <= 12);
> > > +  ASSERT (Day >= 1 && Day <= 31);
> > > +  AsciiSPrint (mBiosDate, sizeof (mBiosDate), "%02d/%02d/%04d", Month, Day, Year);
> > >     // Look for a "x.y" numeric string anywhere in mBiosVersion and
> > >     // try to parse it to populate the BIOS major and minor.
> > > -- 
> > > 2.21.0.windows.1
> > > 
> 

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

* Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer
  2020-07-23 15:47     ` Leif Lindholm
@ 2020-07-23 17:15       ` Andrew Fish
  2020-07-23 17:44         ` Pete Batard
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Fish @ 2020-07-23 17:15 UTC (permalink / raw)
  To: devel, leif; +Cc: Pete Batard, ard.biesheuvel, awarkentin, samer.el-haj-mahmoud

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



> On Jul 23, 2020, at 8:47 AM, Leif Lindholm <leif@nuviainc.com> wrote:
> 
> Hi Pete,
> 
> On Thu, Jul 23, 2020 at 15:38:55 +0100, Pete Batard wrote:
>>>> @@ -626,6 +627,28 @@ BIOSInfoUpdateSmbiosType0 (
>>>>    INTN   i;
>>>>    INTN   State = 0;
>>>>    INTN   Value[2];
>>>> +  INTN   Year = (__DATE__[7] == '?' ? 1900  \
>>>> +           : (((__DATE__[7] - '0') * 1000 ) \
>>>> +           + (__DATE__[8] - '0') * 100      \
>>>> +           + (__DATE__[9] - '0') * 10       \
>>>> +           + __DATE__[10] - '0'));
>>>> +  INTN   Month = ( __DATE__ [2] == '?' ? 1  \
>>>> +           : __DATE__ [2] == 'n' ? (        \
>>>> +             __DATE__ [1] == 'a' ? 1 : 6)   \
>>>> +           : __DATE__ [2] == 'b' ? 2        \
>>>> +           : __DATE__ [2] == 'r' ? (        \
>>>> +             __DATE__ [0] == 'M' ? 3 : 4)   \
>>>> +           : __DATE__ [2] == 'y' ? 5        \
>>>> +           : __DATE__ [2] == 'l' ? 7        \
>>>> +           : __DATE__ [2] == 'g' ? 8        \
>>>> +           : __DATE__ [2] == 'p' ? 9        \
>>>> +           : __DATE__ [2] == 't' ? 10       \
>>>> +           : __DATE__ [2] == 'v' ? 11       \
>>>> +           : 12);
>>>> +  INTN   Day = ( __DATE__[4] == '?' ? 1     \
>>>> +           : ((__DATE__[4] == ' ' ? 0 :     \
>>>> +             ((__DATE__[4] - '0') * 10))    \
>>>> +           + __DATE__[5] - '0'));
>>> 
>>> So, this hunk is very neat, but nigh-on unreviewable.
>>> I.e. we should defintely have it - but only once.
>>> 
>>> Could you break this up into some macros to go into some generic
>>> helper lib? (I don't have a better idea than EmbeddedPkg TimeBaseLib,
>>> but then that is already included in this module.)
>> 
>> So you would like to have a set of macros like:
>> TIME_GET_BUILD_YEAR, TIME_GET_BUILD_MONTH, TIME_GET_BUILD_DAY in
>> TimeBaseLib.h that perform the above?
>> 
>>> Would you be OK to break that snippet out separate?
>> 
>> I think that's a good idea, especially as there is a potential underlying
>> issue with the __DATE__ format being specific to each compiler, so we
>> probably want to handle compiler detection somewhere, preferably globally.
>> For instance, the Intel compiler's __DATE__ format would not work with the
>> above, so I'll add some "vetted compiler" detection for the macros.
> 
> Yes, thanks.
> A friendly "your compiler probably won't know what to do with this"
> #error would be friendlier than whatever non-GCC/clang would generate
> on encountering this.
> 

Leif,

I was thinking the "Mmm dd yyyy” form of __DATE__ was standard. Maybe all we need is a compile time assert that checks the format?

6.10.8.1 Mandatory macros 1 The following macro names shall be deﬁned by the implementation: _ _DATE_ _ The date of translation of the preprocessing translation unit: a character string literal of the form "Mmm dd yyyy", where the names of the months are the same as those generated by the asctime function, and the ﬁrst character of dd is a space character if the value is less than 10. If the date of translation is not available, an implementation-deﬁned valid date shall be supplied 

It seems likely some compilers had an alternate version and chose not to change it and break existing code when the format got standardized. You would hope those compilers had some kind of optional strict ANSI flag or some such….

Thanks,

Andrew Fish

>> The one thing I am not planning to look into is that, of course, as long as
>> you don't explicitly force the compiler to rebuild the sources where these
>> macros are used, then you may end up with a very obsolete build date
>> compared to the actual date of your last re-build.
> 
> Yeah, that's fine.
> 
>> But that's mostly because
>> I don't think there exists a generic solution we can ise to force
>> recompilation of a file that uses a specific macro and also because our main
>> goal with these is to ensure that the Pi firmwares, that we produce through
>> AppVeyor, have a proper build date, and AppVeyor builds are are always
>> clean.
> 
> Yeah, I really don't care about that.
> Or the flipside, where someone modifies only (say) the Smbios
> populating code a month after they built the rest of the tree.
> 
> If someone wants to put a *specific* date in, that should be done
> manually, and if someone wants information on what was actually built,
> they should put in the hashes of the top commits of all repositories
> on PACKAGES_PATH as well as whether the state of each was clean.
> 
> Managing these bits through a rigid build process, like you are, is
> very much valid.
> 
>> I'll send an EDK2 patch for the macros, and then a revised patch for this
>> when I get a chance.
> 
> Thanks!
> 
> Regards,
> 
> Leif
> 
>> Regards,
>> 
>> /Pete
>> 
>>> 
>>> /
>>>     Leif
>>> 
>>>>    // Populate the Firmware major and minor.
>>>>    Status = mFwProtocol->GetFirmwareRevision (&EpochSeconds);
>>>> @@ -648,6 +671,10 @@ BIOSInfoUpdateSmbiosType0 (
>>>>      mBiosVendor, sizeof (mBiosVendor));
>>>>    UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
>>>>      mBiosVersion, sizeof (mBiosVersion));
>>>> +  ASSERT (Year >= 0 && Year <= 9999);
>>>> +  ASSERT (Month >= 1 && Month <= 12);
>>>> +  ASSERT (Day >= 1 && Day <= 31);
>>>> +  AsciiSPrint (mBiosDate, sizeof (mBiosDate), "%02d/%02d/%04d", Month, Day, Year);
>>>>    // Look for a "x.y" numeric string anywhere in mBiosVersion and
>>>>    // try to parse it to populate the BIOS major and minor.
>>>> -- 
>>>> 2.21.0.windows.1
>>>> 
>> 
> 
> 
> 


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

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

* Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer
  2020-07-23 17:15       ` [edk2-devel] " Andrew Fish
@ 2020-07-23 17:44         ` Pete Batard
  0 siblings, 0 replies; 8+ messages in thread
From: Pete Batard @ 2020-07-23 17:44 UTC (permalink / raw)
  To: Andrew Fish, devel, leif; +Cc: ard.biesheuvel, awarkentin, samer.el-haj-mahmoud

Hi Andrew,

On 2020.07.23 18:15, Andrew Fish wrote:
> 
> 
>> On Jul 23, 2020, at 8:47 AM, Leif Lindholm <leif@nuviainc.com 
>> <mailto:leif@nuviainc.com>> wrote:
>>
>> Hi Pete,
>>
>> On Thu, Jul 23, 2020 at 15:38:55 +0100, Pete Batard wrote:
>>>>> @@ -626,6 +627,28 @@ BIOSInfoUpdateSmbiosType0 (
>>>>>    INTN   i;
>>>>>    INTN   State = 0;
>>>>>    INTN   Value[2];
>>>>> +  INTN   Year = (__DATE__[7] == '?' ? 1900  \
>>>>> +           : (((__DATE__[7] - '0') * 1000 ) \
>>>>> +           + (__DATE__[8] - '0') * 100      \
>>>>> +           + (__DATE__[9] - '0') * 10       \
>>>>> +           + __DATE__[10] - '0'));
>>>>> +  INTN   Month = ( __DATE__ [2] == '?' ? 1  \
>>>>> +           : __DATE__ [2] == 'n' ? (        \
>>>>> +             __DATE__ [1] == 'a' ? 1 : 6)   \
>>>>> +           : __DATE__ [2] == 'b' ? 2        \
>>>>> +           : __DATE__ [2] == 'r' ? (        \
>>>>> +             __DATE__ [0] == 'M' ? 3 : 4)   \
>>>>> +           : __DATE__ [2] == 'y' ? 5        \
>>>>> +           : __DATE__ [2] == 'l' ? 7        \
>>>>> +           : __DATE__ [2] == 'g' ? 8        \
>>>>> +           : __DATE__ [2] == 'p' ? 9        \
>>>>> +           : __DATE__ [2] == 't' ? 10       \
>>>>> +           : __DATE__ [2] == 'v' ? 11       \
>>>>> +           : 12);
>>>>> +  INTN   Day = ( __DATE__[4] == '?' ? 1     \
>>>>> +           : ((__DATE__[4] == ' ' ? 0 :     \
>>>>> +             ((__DATE__[4] - '0') * 10))    \
>>>>> +           + __DATE__[5] - '0'));
>>>>
>>>> So, this hunk is very neat, but nigh-on unreviewable.
>>>> I.e. we should defintely have it - but only once.
>>>>
>>>> Could you break this up into some macros to go into some generic
>>>> helper lib? (I don't have a better idea than EmbeddedPkg TimeBaseLib,
>>>> but then that is already included in this module.)
>>>
>>> So you would like to have a set of macros like:
>>> TIME_GET_BUILD_YEAR, TIME_GET_BUILD_MONTH, TIME_GET_BUILD_DAY in
>>> TimeBaseLib.h that perform the above?
>>>
>>>> Would you be OK to break that snippet out separate?
>>>
>>> I think that's a good idea, especially as there is a potential underlying
>>> issue with the __DATE__ format being specific to each compiler, so we
>>> probably want to handle compiler detection somewhere, preferably 
>>> globally.
>>> For instance, the Intel compiler's __DATE__ format would not work 
>>> with the
>>> above, so I'll add some "vetted compiler" detection for the macros.
>>
>> Yes, thanks.
>> A friendly "your compiler probably won't know what to do with this"
>> #error would be friendlier than whatever non-GCC/clang would generate
>> on encountering this.
>>
> 
> Leif,
> 
> I was thinking the "Mmm dd yyyy” form of __DATE__ was standard.

Just FYI, this is where I found that ICC might use something other than 
Mmm dd yyyy:

https://software.intel.com/content/www/us/en/develop/documentation/cpp-compiler-developer-guide-and-reference/top/compiler-reference/macros/iso-standard-predefined-macros.html

However, I am seeing information that ISO/IEC 9899, as far back as C99, 
does define __DATE__ as "Mmm dd yyyy", and I'm starting to think, since 
the intel doc mentions an 11 character date, whereas "mm dd yyyy" is 
only 10 chars, and it's supposed to highlight ISO compliance, that this 
might be an unfortunate typo on the intel web site.

I don't have ICC installed at the moment, so someone may want to check 
that ICC does indeed produce "Mmm dd yyyy".

> Maybe 
> all we need is a compile time assert that checks the format?
> 
> |6.10.8.1 Mandatory macros 1 The following macro names shall be deﬁned 
> by the implementation: _ _DATE_ _ The date of translation of the 
> preprocessing translation unit: a character string literal of the form 
> "Mmm dd yyyy", where the names of the months are the same as those 
> generated by the asctime function, and the ﬁrst character of dd is a 
> space character if the value is less than 10. If the date of translation 
> is not available, an implementation-deﬁned valid date shall be supplied|
> 
> It seems likely some compilers had an alternate version and chose not to 
> change it and break existing code when the format got standardized. You 
> would hope those compilers had some kind of optional strict ANSI flag or 
> some such….

If it turns out that the ICC documentation is only a typo, then I would 
think that the need for an ASSERT might be less important.

Regards,

/Pete

> 
> Thanks,
> 
> Andrew Fish
> 
>>> The one thing I am not planning to look into is that, of course, as 
>>> long as
>>> you don't explicitly force the compiler to rebuild the sources where 
>>> these
>>> macros are used, then you may end up with a very obsolete build date
>>> compared to the actual date of your last re-build.
>>
>> Yeah, that's fine.
>>
>>> But that's mostly because
>>> I don't think there exists a generic solution we can ise to force
>>> recompilation of a file that uses a specific macro and also because 
>>> our main
>>> goal with these is to ensure that the Pi firmwares, that we produce 
>>> through
>>> AppVeyor, have a proper build date, and AppVeyor builds are are always
>>> clean.
>>
>> Yeah, I really don't care about that.
>> Or the flipside, where someone modifies only (say) the Smbios
>> populating code a month after they built the rest of the tree.
>>
>> If someone wants to put a *specific* date in, that should be done
>> manually, and if someone wants information on what was actually built,
>> they should put in the hashes of the top commits of all repositories
>> on PACKAGES_PATH as well as whether the state of each was clean.
>>
>> Managing these bits through a rigid build process, like you are, is
>> very much valid.
>>
>>> I'll send an EDK2 patch for the macros, and then a revised patch for this
>>> when I get a chance.
>>
>> Thanks!
>>
>> Regards,
>>
>> Leif
>>
>>> Regards,
>>>
>>> /Pete
>>>
>>>>
>>>> /
>>>>     Leif
>>>>
>>>>>    // Populate the Firmware major and minor.
>>>>>    Status = mFwProtocol->GetFirmwareRevision (&EpochSeconds);
>>>>> @@ -648,6 +671,10 @@ BIOSInfoUpdateSmbiosType0 (
>>>>>      mBiosVendor, sizeof (mBiosVendor));
>>>>>    UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr 
>>>>> (PcdFirmwareVersionString),
>>>>>      mBiosVersion, sizeof (mBiosVersion));
>>>>> +  ASSERT (Year >= 0 && Year <= 9999);
>>>>> +  ASSERT (Month >= 1 && Month <= 12);
>>>>> +  ASSERT (Day >= 1 && Day <= 31);
>>>>> +  AsciiSPrint (mBiosDate, sizeof (mBiosDate), "%02d/%02d/%04d", 
>>>>> Month, Day, Year);
>>>>>    // Look for a "x.y" numeric string anywhere in mBiosVersion and
>>>>>    // try to parse it to populate the BIOS major and minor.
>>>>> -- 
>>>>> 2.21.0.windows.1
>>>>>
>>>
>>
>> 
>>
> 


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

end of thread, other threads:[~2020-07-23 17:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-20 16:15 [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer Pete Batard
2020-07-20 16:47 ` Andrei Warkentin
2020-07-20 16:51 ` Samer El-Haj-Mahmoud
2020-07-23 11:08 ` Leif Lindholm
2020-07-23 14:38   ` Pete Batard
2020-07-23 15:47     ` Leif Lindholm
2020-07-23 17:15       ` [edk2-devel] " Andrew Fish
2020-07-23 17:44         ` Pete Batard

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