[v10,0/3] Fix address translations on MPFS PCIe controller

Message

Daire McNamara Oct. 11, 2024, 2 p.m. UTC

From: Daire McNamara <[e-mail]>

Hi all,

On Microchip PolarFire SoC (MPFS), the PCIe controller is connected to the
CPU via one of three Fabric Interface Connectors (FICs).  Each FIC present
to the CPU complex as 64-bit AXI-M and 64-bit AXI-S.  To preserve
compatibility with other PolarFire family members, the PCIe controller is
connected to its encapsulating FIC via a 32-bit AXI-M and 32-bit AXI-S
interface.

Each FIC is implemented in FPGA logic and can incorporate logic along its 64-bit
AXI-M to 32-bit AXI-M chain (including address translation) and, likewise, along
its 32-bit AXI-S to 64-bit AXI-S chain (again including address translation).

In order to reduce the potential support space for the PCIe controller in
this environment, MPFS supports certain reference designs for these address
translations: reference designs for cache-coherent memory accesses
and reference designs for non-cache-coherent memory accesses. The precise
details of these reference designs and associated customer guidelines
recommending that customers adhere to the addressing schemes used in those
reference designs are available from Microchip, but the implication for the
PCIe controller address translation between CPU-space and PCIe-space are:

For outbound address translation, the PCIe controller address translation tables
are treated as if they are 32-bit only.  Any further address translation must
be done in FPGA fabric.

For inbound address translation, the PCIe controller is configurable for two
cases:
* In the case of cache-coherent designs, the base of the AXI-S side of the
  address translation must be set to 0 and the size should be 4 GiB wide. The
  FPGA fabric must complete any address translations based on that 0-based
  address translation.
* In the case of non-cache coherent designs, the base of AXI-S side of the
  address translation must be set to 0x8000'0000 and the size shall be 2 GiB
  wide.  The FPGA fabric must complete any address translation based on that
  0x80000000 base.

So, for example, in the non-cache-coherent case, with a device tree property
that maps an inbound range from 0x10'0000'0000 in PCIe space to 0x10'0000'0000
in CPU space, the PCIe rootport will translate a PCIe address of 0x10'0000'0000
to an intermediate 32-bit AXI-S address of 0x8000'0000 and the FIC is
responsible for translating that intermediate 32-bit AXI-S address of
0x8000'0000 to a 64-bit AXI-S address of 0x10'0000'0000.

And similarly, for example, in the cache-coherent case, with a device tree
property that maps an inbound range from 0x10'0000'0000 in PCIe space to
0x10'0000'0000 in CPU space, the PCIe rootport will translate a PCIe address
of 0x10'0000'0000 to an intermediate 32-bit AXI-S address of 0x0000'0000 and
the FIC is responsible for translating that intermediate 32-bit AXI-S address
of 0x0000'0000 to a 64-bit AXI-S address of 0x10'0000'0000.

See https://lore.kernel.org/all/[e-mail]/T/
for backstory.

Changes since v9:
- Dropped plda_setup_inbound_address_translation() from StarFive driver

Changes since v8:
- Edits suggested by BHelgass and Ilpo Jarvinen
- Dropped the setup_inbound_atr u64 change (passing on openrisc 32-bit without it)

Changes since v7:
- Rebased on top of 6.11rc1

Changes since v6:
- Added Reviewed-by: Ilpo tag to outbound patch
- Fixed typos/capitalisation/etc as suggested by Ilpo

Changes since v5:
- Reverted setup_inbound_atr size parameter to u64 as ci system reported
  SZ_4G getting truncated to 0 on mips when I try to use size_t or resource_size_t.
  Added Acked-by tags

Changes since v4:
- Added more cleanups suggested by Ilpo Jarvinen
  Added cleanups for inbound v4 and outbound v3.

Changes since v3:
- Added nice cleanups suggested by Ilpo Jarvinen

Changes since v2:
- Added <Signed-off-by: tag>

Changes since v1:
- added bindings patch to allow dma-noncoherent
- changed a size_t to u64 to pass 32-bit compile tests
- allowed 64-bit outbound pcie translations
- tied PCIe side of eCAM translation table to 0

Conor Dooley (1):
  dt-bindings: PCI: microchip,pcie-host: allow dma-noncoherent

Daire McNamara (2):
  PCI: microchip: Fix outbound address translation tables
  PCI: microchip: Fix inbound address translation tables

 .../bindings/pci/microchip,pcie-host.yaml     |   2 +
 .../pci/controller/plda/pcie-microchip-host.c | 123 +++++++++++++++++-
 drivers/pci/controller/plda/pcie-plda-host.c  |  17 ++-
 drivers/pci/controller/plda/pcie-plda.h       |   6 +-
 4 files changed, 139 insertions(+), 9 deletions(-)


base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b

Comments

Conor Dooley Nov. 13, 2024, 11:50 a.m. UTC | #1

Hey folks,

On Fri, Oct 11, 2024 at 03:00:40PM +0100, [e-mail] wrote:
> From: Daire McNamara <[e-mail]>
> 
> Hi all,
> 
> On Microchip PolarFire SoC (MPFS), the PCIe controller is connected to the
> CPU via one of three Fabric Interface Connectors (FICs).  Each FIC present
> to the CPU complex as 64-bit AXI-M and 64-bit AXI-S.  To preserve
> compatibility with other PolarFire family members, the PCIe controller is
> connected to its encapsulating FIC via a 32-bit AXI-M and 32-bit AXI-S
> interface.
> 
> Each FIC is implemented in FPGA logic and can incorporate logic along its 64-bit
> AXI-M to 32-bit AXI-M chain (including address translation) and, likewise, along
> its 32-bit AXI-S to 64-bit AXI-S chain (again including address translation).
> 
> In order to reduce the potential support space for the PCIe controller in
> this environment, MPFS supports certain reference designs for these address
> translations: reference designs for cache-coherent memory accesses
> and reference designs for non-cache-coherent memory accesses. The precise
> details of these reference designs and associated customer guidelines
> recommending that customers adhere to the addressing schemes used in those
> reference designs are available from Microchip, but the implication for the
> PCIe controller address translation between CPU-space and PCIe-space are:
> 
> For outbound address translation, the PCIe controller address translation tables
> are treated as if they are 32-bit only.  Any further address translation must
> be done in FPGA fabric.
> 
> For inbound address translation, the PCIe controller is configurable for two
> cases:
> * In the case of cache-coherent designs, the base of the AXI-S side of the
>   address translation must be set to 0 and the size should be 4 GiB wide. The
>   FPGA fabric must complete any address translations based on that 0-based
>   address translation.
> * In the case of non-cache coherent designs, the base of AXI-S side of the
>   address translation must be set to 0x8000'0000 and the size shall be 2 GiB
>   wide.  The FPGA fabric must complete any address translation based on that
>   0x80000000 base.
> 
> So, for example, in the non-cache-coherent case, with a device tree property
> that maps an inbound range from 0x10'0000'0000 in PCIe space to 0x10'0000'0000
> in CPU space, the PCIe rootport will translate a PCIe address of 0x10'0000'0000
> to an intermediate 32-bit AXI-S address of 0x8000'0000 and the FIC is
> responsible for translating that intermediate 32-bit AXI-S address of
> 0x8000'0000 to a 64-bit AXI-S address of 0x10'0000'0000.
> 
> And similarly, for example, in the cache-coherent case, with a device tree
> property that maps an inbound range from 0x10'0000'0000 in PCIe space to
> 0x10'0000'0000 in CPU space, the PCIe rootport will translate a PCIe address
> of 0x10'0000'0000 to an intermediate 32-bit AXI-S address of 0x0000'0000 and
> the FIC is responsible for translating that intermediate 32-bit AXI-S address
> of 0x0000'0000 to a 64-bit AXI-S address of 0x10'0000'0000.
> 
> See https://lore.kernel.org/all/[e-mail]/T/
> for backstory.
> 
> Changes since v9:
> - Dropped plda_setup_inbound_address_translation() from StarFive driver

Since I had some success bumping the other series for this driver, any
chance of some attention here?
AFAIK, Daire's addressed what's been pointed out by reviewers and
exempted the StarFive driver from overwriting the firmware-set values
with once calculated from DT as they requested.

Cheers,
Conor.