On Mon, Jun 03, 2024 at 01:45:16PM -0500, Bjorn Helgaas wrote:
> On Fri, May 31, 2024 at 09:53:32AM +0100, Daire McNamara wrote:
> > On Microchip PolarFire SoC (MPFS) the PCIe Root Port can be behind one of
> > three general-purpose Fabric Interface Controller (FIC) buses that
> > encapsulate an AXI-M interface. That FIC is responsible for managing
> > the translations of the upper 32-bits of the AXI-M address. On MPFS,
> > the Root Port driver needs to take account of that outbound address
> > translation done by the parent FIC bus before setting up its own
> > outbound address translation tables. In all cases on MPFS,
> > the remaining outbound address translation tables are 32-bit only.
> >
> > Limit the outbound address translation tables to 32-bit only.
> >
> > Fixes: 6f15a9c9f941 ("PCI: microchip: Add Microchip Polarfire PCIe controller driver")
> >
> > Signed-off-by: Daire McNamara <daire.mcnamara@xxxxxxxxxxxxx>
> > ---
> > drivers/pci/controller/pcie-microchip-host.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-microchip-host.c b/drivers/pci/controller/pcie-microchip-host.c
> > index 137fb8570ba2..0795cd122a4a 100644
> > --- a/drivers/pci/controller/pcie-microchip-host.c
> > +++ b/drivers/pci/controller/pcie-microchip-host.c
> > @@ -983,7 +983,8 @@ static int mc_pcie_setup_windows(struct platform_device *pdev,
> > if (resource_type(entry->res) == IORESOURCE_MEM) {
> > pci_addr = entry->res->start - entry->offset;
> > mc_pcie_setup_window(bridge_base_addr, index,
> > - entry->res->start, pci_addr,
> > + entry->res->start & 0xffffffff,
> > + pci_addr & 0xffffffff,
> > resource_size(entry->res));
>
> Is this masking something that the PCI core needs to be aware of when
> it allocates address space for BARs?
I don't believe so.
>
> The PCI core knows about the CPU physical address range of each bridge
> window and the corresponding PCI address range. From this patch, it
> looks like only the low 32 bits of the CPU address are used by the
> Root Port. That might not be a problem as long as the windows
> described by DT are correct and none of them overlap after masking out
> the upper 32 bits. But for example, if DT has windows like this:
>
> [mem 0x1'0000'0000-0x1'8000'0000]
> [mem 0x2'0000'0000-0x2'8000'0000]
>
> the PCI core will assume they are valid and non-overlapping, when
> IIUC, they *do* overlap.
True, but I can't see how that could happen on any real system - in my mind,
a PolarFire Soc designer (or indeed any designer on any chip) will know where
its rootport is actually attached in its memory map. On PolarFire SoC, for
example, a designer can only reach a rootport over a FIC, and - if they were
to attach to the rootport over two FICs at the same time, that would be a
blunder and would be picked up during design phase. I can't imagine any
reason anyone would release a product with that arrangement.
>
> But also only the low 32 bits of the PCI address are used, and it
> seems like the PCI core will need to know that so it doesn't program a
> 64-bit BAR with a value above 4GB?
Yeah, I'll send around a v2 shortly to address this - I was rather
over-zealous when I prevented that.
>
> > index++;
> > }
> > @@ -1117,8 +1118,8 @@ static int mc_platform_init(struct pci_config_window *cfg)
> > int ret;
> >
> > /* Configure address translation table 0 for PCIe config space */
> > - mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start,
> > - cfg->res.start,
> > + mc_pcie_setup_window(bridge_base_addr, 0, cfg->res.start & 0xffffffff,
> > + cfg->res.start & 0xffffffff,
> > resource_size(&cfg->res));
> >
> > /* Need some fixups in config space */
> > --
> > 2.34.1
> >
>
