From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web10.23449.1633427638570505444 for ; Tue, 05 Oct 2021 02:53:58 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=sDNqsdtr; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 437F961507 for ; Tue, 5 Oct 2021 09:53:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1633427637; bh=5OmBqtGQyKW8go6r6gRuaMwkG0klb20dUoNazFIbIrw=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=sDNqsdtrna2SQ80lMbjHLiy+OTzFqZnKFP7SOFpqU6BPa7h6CNn2vGaE7447ebzqo Pv4ziQ+fHDjyM8apnR1ZhFc1x0fWSOGoEz0aTkLppbyga9IzcdMo+XdDb07uewL9pB 1Zp+e/cV0SpDiMUg5MJuZLskgnix7dQ2c9HruiCKxCxZ0X9uE+QoP7m+IlkjVSUo5s UBvcnQfH9tY2yiw9xpE6kdWj/uokffohm+xxtr0xa10WXoub/q6c2FBIXXn5FZAiMW Isw01K4ptoYf9YOhX5nM+PpraOAJLt0yBfuMq+gTDezU8P9BbsXB26OutvQYNkyqlp 1hygTBzT0P4pA== Received: by mail-oi1-f182.google.com with SMTP id s69so25405155oie.13 for ; Tue, 05 Oct 2021 02:53:57 -0700 (PDT) X-Gm-Message-State: AOAM531FPLMysE6LeHPZcs93Cfvyocp6vUTybklHGRKIUVKhRNy89foc aK1PZ2IJKcOLqg1HWQMeZBjF6r7yWC2O6cRDpKQ= X-Google-Smtp-Source: ABdhPJyQi8OvpNml9t8nR7mOSOJqwi/F3JdVvWkdY35wgYCInJYqfsSj/MZGzqDsmGkQsyyWl0glT5wDvQiCx/HmxEE= X-Received: by 2002:aca:3114:: with SMTP id x20mr1712089oix.174.1633427636589; Tue, 05 Oct 2021 02:53:56 -0700 (PDT) MIME-Version: 1.0 References: <20211002005214.40227-1-jeremy.linton@arm.com> <20211002005214.40227-2-jeremy.linton@arm.com> In-Reply-To: <20211002005214.40227-2-jeremy.linton@arm.com> From: "Ard Biesheuvel" Date: Tue, 5 Oct 2021 11:53:45 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 1/1] Platform/RaspberryPi: Always use non translating DMA in DT mode To: Jeremy Linton Cc: edk2-devel-groups-io , Peter Batard , Ard Biesheuvel , Leif Lindholm , Andrei Warkentin , Sunny Wang , Samer El-Haj-Mahmoud Content-Type: text/plain; charset="UTF-8" On Sat, 2 Oct 2021 at 02:52, Jeremy Linton 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 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 >