Platform/RPi: Smbios reporting improvements

[edk2-devel] [edk2-platforms][PATCH 2/2] Platform/RPi/RPiFirmwareDxe: Fix serial number population for RPi4

Posted by Pete Batard 6 years, 1 month ago

Some (all?) Raspbery Pi 4 platforms report 0x0000000010000000 as their
board serial when queried through the VideoCore mailbox.

Fix this by using the MAC address then.

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

diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
index dd61ef089ca7..75826fdc0e53 100644
--- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
+++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
@@ -394,8 +394,9 @@ RpiFirmwareGetSerial (
   }
 
   *Serial = Cmd->TagBody.Serial;
-  // Some platforms return 0 for serial. For those, try to use the MAC address.
-  if (*Serial == 0) {
+  // Some platforms return 0 or 0x0000000010000000 for serial.
+  // For those, try to use the MAC address.
+  if ((*Serial == 0) || ((*Serial & 0xFFFFFFFF0FFFFFFFULL) == 0)) {
     Status = RpiFirmwareGetMacAddress ((UINT8*) Serial);
     // Convert to a more user-friendly value
     *Serial = SwapBytes64 (*Serial << 16);
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53029): https://edk2.groups.io/g/devel/message/53029
Mute This Topic: https://groups.io/mt/69532224/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] Platform/RPi/RPiFirmwareDxe: Fix serial number population for RPi4

Posted by Ard Biesheuvel 6 years, 1 month ago

On Wed, 8 Jan 2020 at 18:00, Pete Batard <pete@akeo.ie> wrote:
>
> Some (all?) Raspbery Pi 4 platforms report 0x0000000010000000 as their
> board serial when queried through the VideoCore mailbox.
>
> Fix this by using the MAC address then.
>
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>  Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> index dd61ef089ca7..75826fdc0e53 100644
> --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> @@ -394,8 +394,9 @@ RpiFirmwareGetSerial (
>    }
>
>    *Serial = Cmd->TagBody.Serial;
> -  // Some platforms return 0 for serial. For those, try to use the MAC address.
> -  if (*Serial == 0) {
> +  // Some platforms return 0 or 0x0000000010000000 for serial.
> +  // For those, try to use the MAC address.
> +  if ((*Serial == 0) || ((*Serial & 0xFFFFFFFF0FFFFFFFULL) == 0)) {

What is the point of using a mask here? Is it deliberately matching
0x0000000020000000 or 0x00000000F0000000 as well?

>      Status = RpiFirmwareGetMacAddress ((UINT8*) Serial);
>      // Convert to a more user-friendly value
>      *Serial = SwapBytes64 (*Serial << 16);
> --
> 2.21.0.windows.1
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53085): https://edk2.groups.io/g/devel/message/53085
Mute This Topic: https://groups.io/mt/69532224/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] Platform/RPi/RPiFirmwareDxe: Fix serial number population for RPi4

Posted by Pete Batard 6 years, 1 month ago

Hi Ard,

On 2020.01.09 14:44, Ard Biesheuvel wrote:
> On Wed, 8 Jan 2020 at 18:00, Pete Batard <pete@akeo.ie> wrote:
>>
>> Some (all?) Raspbery Pi 4 platforms report 0x0000000010000000 as their
>> board serial when queried through the VideoCore mailbox.
>>
>> Fix this by using the MAC address then.
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
>>   Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
>> index dd61ef089ca7..75826fdc0e53 100644
>> --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
>> +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
>> @@ -394,8 +394,9 @@ RpiFirmwareGetSerial (
>>     }
>>
>>     *Serial = Cmd->TagBody.Serial;
>> -  // Some platforms return 0 for serial. For those, try to use the MAC address.
>> -  if (*Serial == 0) {
>> +  // Some platforms return 0 or 0x0000000010000000 for serial.
>> +  // For those, try to use the MAC address.
>> +  if ((*Serial == 0) || ((*Serial & 0xFFFFFFFF0FFFFFFFULL) == 0)) {
> 
> What is the point of using a mask here? Is it deliberately matching
> 0x0000000020000000 or 0x00000000F0000000 as well?

My expectation is that if we're seeing 0x0000000010000000 as a board 
serial, there's not so insignificant chance are future boards may use 
0x0000000010000000, 0x0000000030000000 and so on.

In other words, when someone starts counting from 0 to 1 somewhere, it's 
not unreasonable to also expect them to reach 2 at some stage...

Now, if anything, the one change we could apply here is remove the 
(*Serial == 0) check, which of course is redundant. But I fail to see 
why we'd want to restrict ourselves to checking only for 
0x0000000010000000 here, when we have no clear idea how the value we 
read may evolve in the future, as anything that the mask condition 
matches is clearly not a serial we want to use anyway...

Regards,

/Pete


> 
>>       Status = RpiFirmwareGetMacAddress ((UINT8*) Serial);
>>       // Convert to a more user-friendly value
>>       *Serial = SwapBytes64 (*Serial << 16);
>> --
>> 2.21.0.windows.1
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53086): https://edk2.groups.io/g/devel/message/53086
Mute This Topic: https://groups.io/mt/69532224/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH 2/2] Platform/RPi/RPiFirmwareDxe: Fix serial number population for RPi4

Posted by Pete Batard 6 years ago

Could I have a ping on this, as well as 
https://edk2.groups.io/g/devel/topic/69331625#52611?

It looks to me like the latest RPi related patches have reached some 
kind of integration limbo, since I'm not seeing any of those being 
requested a v2.

Thanks,

/Pete

On 2020.01.09 15:02, Pete Batard wrote:
> Hi Ard,
> 
> On 2020.01.09 14:44, Ard Biesheuvel wrote:
>> On Wed, 8 Jan 2020 at 18:00, Pete Batard <pete@akeo.ie> wrote:
>>>
>>> Some (all?) Raspbery Pi 4 platforms report 0x0000000010000000 as their
>>> board serial when queried through the VideoCore mailbox.
>>>
>>> Fix this by using the MAC address then.
>>>
>>> Signed-off-by: Pete Batard <pete@akeo.ie>
>>> ---
>>>   Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git 
>>> a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c 
>>> b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
>>> index dd61ef089ca7..75826fdc0e53 100644
>>> --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
>>> +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
>>> @@ -394,8 +394,9 @@ RpiFirmwareGetSerial (
>>>     }
>>>
>>>     *Serial = Cmd->TagBody.Serial;
>>> -  // Some platforms return 0 for serial. For those, try to use the 
>>> MAC address.
>>> -  if (*Serial == 0) {
>>> +  // Some platforms return 0 or 0x0000000010000000 for serial.
>>> +  // For those, try to use the MAC address.
>>> +  if ((*Serial == 0) || ((*Serial & 0xFFFFFFFF0FFFFFFFULL) == 0)) {
>>
>> What is the point of using a mask here? Is it deliberately matching
>> 0x0000000020000000 or 0x00000000F0000000 as well?
> 
> My expectation is that if we're seeing 0x0000000010000000 as a board 
> serial, there's not so insignificant chance are future boards may use 
> 0x0000000010000000, 0x0000000030000000 and so on.
> 
> In other words, when someone starts counting from 0 to 1 somewhere, it's 
> not unreasonable to also expect them to reach 2 at some stage...
> 
> Now, if anything, the one change we could apply here is remove the 
> (*Serial == 0) check, which of course is redundant. But I fail to see 
> why we'd want to restrict ourselves to checking only for 
> 0x0000000010000000 here, when we have no clear idea how the value we 
> read may evolve in the future, as anything that the mask condition 
> matches is clearly not a serial we want to use anyway...
> 
> Regards,
> 
> /Pete
> 
> 
>>
>>>       Status = RpiFirmwareGetMacAddress ((UINT8*) Serial);
>>>       // Convert to a more user-friendly value
>>>       *Serial = SwapBytes64 (*Serial << 16);
>>> -- 
>>> 2.21.0.windows.1
>>>
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53416): https://edk2.groups.io/g/devel/message/53416
Mute This Topic: https://groups.io/mt/69532224/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-