RaspberryPi/RPiFirmwareDxe: Use the real serial number on RPi4

[valtzu]

Fetching the real serial number on Raspberry Pi 4 was not working due to compiler optimization / struct padding.

Related to comment in

edk2-platforms/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c

Lines 395 to 396 in 6cfae9d

	// Some platforms return 0 or 0x0000000010000000 for serial.
	// For those, try to use the MAC address.

After this change, the reported serial number will be the actual board serial number and not the MAC address – so this would be a breaking change.

 cat /sys/class/dmi/id/board_serial
-0000E4xxxxxxxx5F
+10000000Dxxxxxx1

---

Tested on RPi 4B 8G Rev 1.4 & 1.5.

[leiflindholm]

@jlinton

[jlinton]

Oh! I wonder if this is the result of changing VC firmware levels and/or rpi3 vs 4 because the original code/etc seems pretty specific about resetting the pack option.

[leiflindholm]

Oh! I wonder if this is the result of changing VC firmware levels and/or rpi3 vs 4 because the original code/etc seems pretty specific about resetting the pack option.

Looks to me like the existing code meant to do
#pragma pack(1)
struct...
#pragma pack()

And I see this pattern repeated through the file. In fact, only the first instance remembers to do the pack(1).

@jlinton or @valtzu, could one of you possibly rework this to change all the affected locations and verify that doesn't break anything?
Preferably by grouping them all together at the top of the file within a single #pragma block.

[valtzu]

Looks to me like the existing code meant to do #pragma pack(1) struct... #pragma pack()

And I see this pattern repeated through the file. In fact, only the first instance remembers to do the pack(1).

I tried this and it does seem to be working. This is a lot bigger change though.

and verify that doesn't break anything?

I'm not sure how to do that – the device boots normally, I see correct serial number etc. but I don't know if some other feature broke.

[leiflindholm]

Looks to me like the existing code meant to do #pragma pack(1) struct... #pragma pack()
And I see this pattern repeated through the file. In fact, only the first instance remembers to do the pack(1).

I tried this and it does seem to be working. This is a lot bigger change though.

It's bigger, but it's just fixing the same bug in all places it's repeated in the file.

Thanks a lot for doing that!

and verify that doesn't break anything?

I'm not sure how to do that – the device boots normally, I see correct serial number etc. but I don't know if some other feature broke.

I mean, it fixes what are clearly bugs and "seems to work". That's enough for me to be willing to merge it. As I see it, anything that breaks was already broken but hidden due to a cut&paste error.

@jlinton ?

[leiflindholm]

@valtzu oh, err, maybe update the commit message to match though?

Something along the lines of "fix and consolidate incorrect pragma pack blocks",
still calling out the serial number thing.

[leiflindholm]

@valtzu thanks! I'll give until tomorrow for someone to pitch in and object. (And then merge if no one does.)

[jlinton]

Ok, yes I wanted to do some testing on it myself with various versions of the FW, but I think Leif is right, the pragma's were wrong and they were just copy/pasted that way.

So, if nothing else, the code is now more correct. If it turns out one of the FW revisions is packing it differently, then we can fix that when it happens.