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

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

On Sat, 2 Oct 2021 at 02:52, 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(?). Worse
> with just the DMA ranges corrected, the XHCI/QUIRK itself
> then causes the controller to start having what appear
> to be DMA issues.
>
> So again, simplify the situation and make all DT's
> provided by this firmware have a 3G DMA limit on the
> PCIe bus. Then remove the ability for linux/etc to
> trigger the quirk by remove the DT node attaching the
> reset controller to the XHCI. The latter seems somewhat
> questionable, since the DT/PCIe host bridge driver is
> doing what appears to be a PERST which in theory
> might then require a firmware reload, but at the
> moment seems to work without.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

I am not opposed to this, but I'd like some other folks to chime in please?

> ---
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 75 ++++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> index 0472d8ecf6..1a8fc7a134 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -334,6 +334,76 @@ 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;
> +  }
> +
> +  // Now that we are always running without DMA translation, and with
> +  // a 3G DMA limit, there shouldn't be a need to reset/reload
> +  // the XHCI for it to operate. The possible problem is that the PCIe root
> +  // port is itself being reset (by linux+DT), The rpi foundation claims
> +  // this is needed as a pre-req to reloading the XHCI firmware, which
> +  // also implies that a PCI fundamental reset should cause the XHCI itself
> +  // to reset. For sure isn't happening fully, otherwise reloading the
> +  // firmware would be mandatory. As it is, recent kernels actually start to
> +  // have problems following the XHCI reset notification mailbox!
> +  // So lets just stop the kernel from triggering the mailbox by removing
> +  // the node.
> +  Node = fdt_path_offset (mFdtImage, "/scb/pcie@7d500000/pci@1,0");
> +  if (Node < 0) {
> +    // This can happen on CM4/etc which doesn't have an onboard XHCI
> +    DEBUG ((DEBUG_ERROR, "%a: failed to locate /scb/pcie@7d500000/pci@1/usb@1\n", __FUNCTION__));
> +  } else {
> +    Retval = fdt_del_node (mFdtImage, Node);
> +    if (Retval != 0) {
> +      DEBUG ((DEBUG_ERROR, "Failed to remove /scb/pcie@7d500000/pci@1/usb@1\n"));
> +      return EFI_NOT_FOUND;
> +    }
> +  }
> +
> +#endif
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    @param  ImageHandle   of the loaded driver
>    @param  SystemTable   Pointer to the System Table
> @@ -431,6 +501,11 @@ FdtDxeInitialize (
>      Print (L"Failed to update USB compatible properties: %r\n", 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
>