From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by mx.groups.io with SMTP id smtpd.web12.3712.1595526281211474551 for ; Thu, 23 Jul 2020 10:44:41 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=Y5bA87dw; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.128.65, mailfrom: pete@akeo.ie) Received: by mail-wm1-f65.google.com with SMTP id f139so5968145wmf.5 for ; Thu, 23 Jul 2020 10:44:40 -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=/uXQEOBg/EGWSVKndwlHBjTPA6xkMBQ6YyhRT2ZvHkA=; b=Y5bA87dwTd724xZpj1xc+qEF3lYdjLAwRT9EbzjnVtCxqjdrUGWoDH6/W9NoX3ki3E +ZnQwvKT3neCTGN2LtuU88k8LRyBRkPdepLDeF8vYgzsGVPaTbd3/8n0iP6KjfTieAaC FfKARqBL/H5UYW/nIL/GqapqLQF2VpNkr9HEPzbYJuUhC8LM672km7zEQ6EwXdJu3M+J p1M0sUpdxPPvIM3f8G0h0amEvjOegfmpjNsTVmKW730/An0d2qL5V1ZCUBPwnfBI0wTs xMTw7WY7gJhKzY+XbVgJvmlA3vFtpxdbB+qYYyv4rEGqFfx7h8Yx0AjFr6sYrs5B0/oh jIrA== 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=/uXQEOBg/EGWSVKndwlHBjTPA6xkMBQ6YyhRT2ZvHkA=; b=lwGIYAYkiS7KwrO8LhlZNjqRgKZMNPTyTdBVFP/6V5ICiTV92zpR42sA7+2lJ+5GOU R8ndB/a1psNRZMt5KTnXs1jhgar+kGKmiK5Js6j0mt/US82jAragSW0OTKUqo+X7s5Fq to605aMG53865n8O7gjGIMsRkxU5vRd2nn+vNjJPpVAC1bvkACRDltT595/LV8z3kdsh d5oqPz63h9xnNRb/NcAczAn9cdv5n3/h38IxaVLk8f5ZjccQ9p9ZvURV10Cc4IKj1qtm YxuXO+xZFHtCIpgMrswthNWmXEwBEpKAr7/Z1YtnBOfhEfypAY2BcdETuujtDFCy0CX2 bPBg== X-Gm-Message-State: AOAM531LC9u87neJ3uA185YrQmgJaXUkJibZPMDn/tO30/Wd8x4aod/+ INgJ+uUV3YKMNVc3q+UlQq7Ojw== X-Google-Smtp-Source: ABdhPJxdo942vNGfwBbxzFam0KOrKDEjaieI6Nj1+YHbqp/5iDptcJFuPdHc+eI48/XrvvUdQFsNlA== X-Received: by 2002:a05:600c:218f:: with SMTP id e15mr4940284wme.187.1595526279586; Thu, 23 Jul 2020 10:44:39 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.55.132]) by smtp.googlemail.com with ESMTPSA id y77sm4436074wmd.36.2020.07.23.10.44.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 23 Jul 2020 10:44:38 -0700 (PDT) Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: Fix BIOS Release Date and System Manufacturer To: Andrew Fish , devel@edk2.groups.io, leif@nuviainc.com Cc: ard.biesheuvel@arm.com, awarkentin@vmware.com, samer.el-haj-mahmoud@arm.com References: <20200720161507.14352-1-pete@akeo.ie> <20200723110826.GM1337@vanye> <20200723154735.GO1337@vanye> From: "Pete Batard" Message-ID: <6be12fcf-b9ae-ac11-b0f7-f236819076ce@akeo.ie> Date: Thu, 23 Jul 2020 18:44:36 +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: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit Hi Andrew, On 2020.07.23 18:15, Andrew Fish wrote: > > >> On Jul 23, 2020, at 8:47 AM, Leif Lindholm > > 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 defined > 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 first character of dd is a > space character if the value is less than 10. If the date of translation > is not available, an implementation-defined 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 >>>>> >>> >> >> >> >