From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by mx.groups.io with SMTP id smtpd.web12.13759.1595515139529338729 for ; Thu, 23 Jul 2020 07:38:59 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=pNnl+XaX; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.128.67, mailfrom: pete@akeo.ie) Received: by mail-wm1-f67.google.com with SMTP id p14so4816014wmg.1 for ; Thu, 23 Jul 2020 07:38:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=ceH9qM5phyWLruKnydTUbxjIdInAxjOobHPPv9uRHyM=; b=pNnl+XaXD9fHx/rpWMsV2S4JxU+TyB1N9VsC/5rYzC/LSLvB6viVc0DK4frttZONW4 3LhyH/odj11gxo6TaXJ+Lc8uBSFDxdw2ndTSwFq/0zUPY/QP0g2VRHuZdQTwu06SQFXX XUQOanPIy8agqemBJhlBbrbWUqb7JF5S39bN6DAu+D0yDowgoHIEdqTvpy8Oauf1MCx4 2IOlqdDX9JlnE7HJPTot7MWm2XUti2nNX79gIuPCOvz7WxhlpifMYRuhzJshhG07G55Y n9ykEjRNhYGxQtvtqNrzxDWeBSG0im6G4ke8V4De+Owto7QKNdqzTz7RwSA2NEhmaJRp AalA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=ceH9qM5phyWLruKnydTUbxjIdInAxjOobHPPv9uRHyM=; b=Ia5Yabsv6oHVbg6mBg3Ogjn8QAEvqnKILHCxmqdyp8nsD37bMcteVgd+saK8nyvuhy UqJ/eVVQWJLLUioTpFMCiPX6xhsQMw7DVfSKeAs/eqteaR5MSQZ+4CEO81dn0FRdtG1S Sj/b6vqdk49JQ1QxHKv+IZ3SUgkx1xJhJ5uQvP4DShYrMsWq0WiVepRVmyEHok5XDhGB pAPQFhRBr8LmZvKkv7totxa0khj9s/ivRx91UOG4Ot9A4bHapYSOfEirb18ZZbRoZWtO E800CKL0GPf/NpP98YfbgN3BhJUp2isj+XxNGWWrjrY0cJxW0kRJMJ3iCAEFtHcPaQoZ juQA== X-Gm-Message-State: AOAM530DlVUkRiLyBgKPkbHQJYR1N3UlXxt5p0ZdrHwoca7qtEjU+Kan voougfNeuFJgtFCzGY8V+gm3Cg== X-Google-Smtp-Source: ABdhPJw+XMulbwwjD6EKOuEo8tFPhyNRT1qNLdtEg39JGd/B3XvELcCb7Mv30cd7Vf5odj7hwO1J6w== X-Received: by 2002:a1c:6809:: with SMTP id d9mr4430754wmc.34.1595515138068; Thu, 23 Jul 2020 07:38:58 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.55.132]) by smtp.googlemail.com with ESMTPSA id e23sm3750997wme.35.2020.07.23.07.38.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 23 Jul 2020 07:38:56 -0700 (PDT) Subject: Re: [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer To: Leif Lindholm Cc: devel@edk2.groups.io, ard.biesheuvel@arm.com, awarkentin@vmware.com, samer.el-haj-mahmoud@arm.com References: <20200720161507.14352-1-pete@akeo.ie> <20200723110826.GM1337@vanye> From: "Pete Batard" Message-ID: Date: Thu, 23 Jul 2020 15:38:55 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20200723110826.GM1337@vanye> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit 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 >> --- >> 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 >>