Re: [PATCH] Platform/RaspberryPi: Always use non translating DMA in DT mode

["Ard Biesheuvel" <ardb@kernel.org>]

On Mon, 16 Aug 2021 at 22:27, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> One of the many issues with the PCIe on this platform is
> its inbound DMA is either constrained to the lower 3G, or
> on later SoC's a translation can be used. That translation
> was problematic with some of the OS's expected to boot
> on this platform. So, across the board a 3G DMA limit is
> enforced during boot. This itself causes problems because
> the later boards removed the SPI EEPROM used by the onboard
> XHCI controller, instead favoring using a block of RAM to
> load its firmware. Hence it is the lower level firmware's
> responsibility via a mailbox call, to read the bridge
> translation/configuration before telling the XHCI controller
> where it can find its firmware.
>
> Everything is great, except that it appears that Linux
> after reprogramming the bridge to match the DT (when using
> a translation) can't actually get the XHCI/QUIRK reset to
> function. Apparently, because the firmware only reads the
> bridge configuration the first time its called(?).
>
> So again, simplify the situation and make all DT's
> provided by this firmware have a 3G DMA limit on the
> PCIe bus.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

This looks reasonable to me. Anyone care to ack this as well?

> ---
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 51 ++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> index 0472d8ecf6..bc02cabe24 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -334,6 +334,52 @@ CleanSimpleFramebuffer (
>    return EFI_SUCCESS;
>  }
>
> +STATIC
> +EFI_STATUS
> +SyncPcie (
> +  VOID
> +  )
> +{
> +#if (RPI_MODEL == 4)
> +  INTN          Node;
> +  INTN          Retval;
> +  UINT32        DmaRanges[7];
> +
> +  Node = fdt_path_offset (mFdtImage, "pcie0");
> +  if (Node < 0) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to locate 'pcie0' alias\n", __FUNCTION__));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  // non translated 32-bit DMA window with a limit of 0xc0000000
> +  DmaRanges[0] = cpu_to_fdt32 (0x02000000);
> +  DmaRanges[1] = cpu_to_fdt32 (0x00000000);
> +  DmaRanges[2] = cpu_to_fdt32 (0x00000000);
> +  DmaRanges[3] = cpu_to_fdt32 (0x00000000);
> +  DmaRanges[4] = cpu_to_fdt32 (0x00000000);
> +  DmaRanges[5] = cpu_to_fdt32 (0x00000000);
> +  DmaRanges[6] = cpu_to_fdt32 (0xc0000000);
> +
> +  DEBUG ((DEBUG_INFO, "%a: Updating PCIe dma-ranges\n",  __FUNCTION__));
> +
> +  // Match dma-ranges with the edk2+ACPI setup we are using.
> +  // This works around a failure in linux to reset the XHCI correctly
> +  // when in DT mode following the linux kernel reprogramming the PCIe
> +  // subsystem to match the DT supplied inbound PCIe/DMA translation.
> +  // It appears the lower level firmware honors whatever it reads
> +  // during the first PCI/XHCI quirk call and uses that value until
> +  // rebooted rather than re-reading it on every mailbox command.
> +  Retval = fdt_setprop (mFdtImage, Node, "dma-ranges",
> +                        DmaRanges,  sizeof DmaRanges);
> +  if (Retval != 0) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to update 'dma-ranges' property (%d)\n",
> +      __FUNCTION__, Retval));
> +    return EFI_NOT_FOUND;
> +  }
> +#endif
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    @param  ImageHandle   of the loaded driver
>    @param  SystemTable   Pointer to the System Table
> @@ -431,6 +477,11 @@ FdtDxeInitialize (
>      Print (L"Failed to update USB compatible properties: %r\n", Status);
>    }
>
> +  Status = SyncPcie ();
> +  if (EFI_ERROR (Status)) {
> +    Print (L"Failed to update PCIe address ranges: %r\n", Status);
> +  }
> +
>    DEBUG ((DEBUG_INFO, "Installed devicetree at address %p\n", mFdtImage));
>    Status = gBS->InstallConfigurationTable (&gFdtTableGuid, mFdtImage);
>    if (EFI_ERROR (Status)) {
> --
> 2.13.7
>