pftf / RPi4

Raspberry Pi 4 UEFI Firmware Images
https://rpi4-uefi.dev
Other
1.19k stars 140 forks source link

RPi 4B firmware misreports SoC model GPIO MMIO space #105

Open thebel1 opened 3 years ago

thebel1 commented 3 years ago

While working on a native GPIO driver for ESXi, I noticed that the vmkernel misreports the size of the GPIO MMIO space for the BCM2711 SoC.

I've talked to the ESXi on Arm folks and they believe it's a firmware bug.

I've tested this on two different Raspberry Pi 4Bs, and the MMIO space is reported as being 180 bytes in size on both. This size corresponds to the BCM2835 (an older chip). The BCM2711 has an MMIO space of 244 bytes, to make room for the pull-up/pull-down config registers.

References:

samerhaj commented 3 years ago

This is due to the RPi4 sharing the same GPIO code from RPi3:https://github.com/pftf/edk2-platforms/tree/master/Silicon/Broadcom/Bcm283x/Library/GpioLib

Specifically: https://github.com/pftf/edk2-platforms/blob/master/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836Gpio.h#L18

I scrubbed the definitions against the BCM2711 documentation and it seems that everything matches, except for the following:

#define GPIO_GPPUD         (GPIO_BASE_ADDRESS + 0x94)
#define GPIO_GPPUDCLK0     (GPIO_BASE_ADDRESS + 0x98)
#define GPIO_GPPUDCLK1     (GPIO_BASE_ADDRESS + 0x9C)
#define GPIO_GPPUPPDN0     (GPIO_BASE_ADDRESS + 0xE4)
#define GPIO_GPPUPPDN1     (GPIO_BASE_ADDRESS + 0xE8)
#define GPIO_GPPUPPDN2     (GPIO_BASE_ADDRESS + 0xEC)
#define GPIO_GPPUPPDN3     (GPIO_BASE_ADDRESS + 0xF0)

Anything else?

TheMindVirus commented 3 years ago

DMAR Table would only help to tell drivers in the OS that there has been a change to the DMA Mapping but UEFI drivers will still remain hard coded at old addresses.

Raspberry Pi OS uses Device Tree Boot to tell it which board it is running on and where everything has been mapped but UEFI is using ACPI and hardcoded drivers.

I'm not personally a fan of either boot scheme but maybe UEFI can start picking up on what's in a decompiled dtb file at boot.

I'd much prefer it if a simple text file could describe the board more dynamically (with hash checksum), in a way which both people and software could understand.

jlinton commented 3 years ago

We probably need to be a bit careful with the GPIO export in ACPI mode. That is because if the firmware is managing HATs (say the fan hat). It will expect to access GPIO/etc pins from AML/etc.

So, if we are going to enable a GPIO device on the ACPI/rpi4, it probably needs to be in its own SSDT and only exported if everything in the firmware using GPIO is disabled.

andysan commented 3 years ago

I noticed a related issue when poking around in the Linux pinctrl driver. I have a patch that makes it detect the current I have a patch to make it detect the BCM2845 _HID value. However, I wasn't able to reliably detect the revision of the IP block. The revision in the BCM2835 and the BCM2711 could be distinguished based on size (seems a bit dirty, but it would work). The driver currently supports a third chip, BCM7211, which seems similar to the 2711, but supports additional interrupts.

Is there better, more general, solution to distinguish between the different revisions of this IP block? ~Should we start assigning different _HID values to different revisions of the same IP block to make it possible to distinguish between them?~ Would adding a _HRV field be a good option? Another option that would work for Linux would be to add PRP0001 to the _CID list and rely on DT compatible strings to distinguish different revisions.

andysan commented 3 years ago

We probably need to be a bit careful with the GPIO export in ACPI mode. That is because if the firmware is managing HATs (say the fan hat). It will expect to access GPIO/etc pins from AML/etc.

So, if we are going to enable a GPIO device on the ACPI/rpi4, it probably needs to be in its own SSDT and only exported if everything in the firmware using GPIO is disabled.

Won't the OS need access to the GPIO block to honour PinFunction descriptors? I might have misunderstood how PinFunction is supposed to work though.