arm64 N1SDP PCI root complex driver

[br]

Summary

This is a PCI root complex driver for ARM Neoverse N1 board.

The PCI controller has bugs (it was shipped untested). This driver adds required quirks:

1. remap config space of pci root complex (it has different location)
2. filter-out accesses to invalid devices (otherwise it faults)

Test Plan

tested on ARM PC box (N1SDP)
PCI-based peripherals (ethernet, xhci) work

MSI/MSI-X interrupts work fine with GIC ITS

[andrew]

You should add this to files.arm64

sys/dev/pci/pci_n1sdp.c

87 ↗

(On Diff #67252)

Why are you using bus_space_map? You should use kva_alloc and pmap_kenter to create a normal mapping. bus_space_map is for device memory.

[br]

sys/dev/pci/pci_n1sdp.c

87 ↗

(On Diff #67252)

Which memattr to use here, VM_MEMATTR_DEVICE ?
Should we use kva_alloc/pmap_kenter for paddr_rc too ?

[andrew]

sys/dev/pci/pci_n1sdp.c

87 ↗

(On Diff #67252)

This table seems to be mapped as VM_MEMATTR_UNCACHEABLE in uefi. paddr_rc looks like it's mapped as VM_MEMATTR_DEVICE so you can use pmap_kenter_device for it.

[br]

Address Andrew's comments: use kva_alloc/pmap_enter for non-device memory

[br]

Override MSI/MSIx pcib methods so MSI allocation fail and INTx are used.
MSI/MSIx don't work for now

[br]

Add comment for MSI/MSIx

[br]

Include pcI_n1sdp to the build

[mmel]

sys/dev/pci/pci_n1sdp.c

229 ↗

(On Diff #67509)

Extended registers are not supported? Or it should be PCIE_REGMAX?

235 ↗

(On Diff #67509)

'bus - sc->bus_start" looks strange, are you sure that this is OK?

237 ↗

(On Diff #67509)

I think that this condition is wrong. We should return 0xFFFFFFFF if bus is root bus and slot or function are non-zero. Also, i assuming from rest of code that root bus number is stored in sc->bus_start. Imho, this should be:

	if (bus == sc->bus_start) {
		if (slot != 0 || func != 0)
			return (~0U);
		data = *(uint32_t *)(rc_remapped_addr[sc_acpi->segment] +
		    (offset & ~3));
	} else {
		data = bus_space_read_4(sc->bst, sc->bsh, offset & ~3);
        }

241 ↗

(On Diff #67509)

Did you test this driver on card(s) with PCIe switch? I don't see how are type 1 configuration requests generated.

[br]

sys/dev/pci/pci_n1sdp.c

229 ↗

(On Diff #67509)

PCIE_REGMAX is 4095. Nothing above is supported since only 12 bits are available

235 ↗

(On Diff #67509)

Note 100% sure and never saw bus_start > 0 (it comes from FDT and ACPI tables)

237 ↗

(On Diff #67509)

We could do that I think

241 ↗

(On Diff #67509)

Yes, PCIe switch is used in ARM Neoverse N1

[br]

o Address mmel's comment.
o Don't override MSI methods since Andrew has fixed a bug in the ITS driver

[jhb]

As with the xilinx driver, can we please not put MD and chipset-specific code in sys/dev/pci?

[br]

In D23349#516026, @jhb wrote:

As with the xilinx driver, can we please not put MD and chipset-specific code in sys/dev/pci?

So where to put it then? Propose a path. What about sys/dev/pci/controller/pci_n1sdp.c

[jhb]

Are there other drivers for this chipset? If so, sys/arm64/n1sdp might be a good place for neoverse 1-specific drivers to live. sys/dev/n1sdp could also work (though on arm platforms we've tended to put them in sys/arm/<chipset|soc> rather than sys/dev)

[br]

move the driver by request from jhb@

[andrew]

There are no other SoC specific drivers. This driver is at most a quirk for a slightly broken but otherwise standard root complex.

[jhb]

I would still rather keep sys/dev/pci for "generic" PCI things such as the PCI-PCI bridge driver or the PCI bus driver, etc. sys/arm64/pci could also be a place (for x86 we use sys/{amd64,i386,x86}/pci to hold MD PCI drivers that don't belong elsewhere)

[br]

what about dev/pci/controller/? linux does that.
We have a similar subdirectory for MMC drivers: dev/mmc/host/ that works well and keeps dev/mmc/ clean

[andrew]

I would be happy to move the existing drivers from sys/dev/pci to sys/dev/pci/controller or similar. We have a similar structure for usb controllers.

[br]

Move the driver to dev/pci/controller/

[br]

The pcie_discovery_data is device specific, move it to the softc.
Move the root memory to the softc and switch it to use bus_space.
While here create a common function to get the correct bus_space
details for a given bus/slot/func/reg combination.

Submitted by: Andrew Turner <[e-mail]>