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

["Leif Lindholm" <leif@nuviainc.com>]

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
>