Re: [PATCH 13/18] arm64: ioremap: use nGnRnE mappings on platforms that require it

From: Arnd Bergmann <arnd@kernel.org>
Date: Mon, 8 Feb 2021 23:57:20 +0100

On Thu, Feb 4, 2021 at 9:39 PM Hector Martin [off-list ref] wrote:

quoted

(3) Do it at a lower level, in ioremap() itself. This requires that
    ioremap() somehow discriminates based on address range to pick what
    kind of mapping to make.

    Declaring these address ranges would be an issue. Options:

    a) An out of band list in a DT node, a la /reserved-memory

    b) Something based on the existing DT hierarchy, where we can scan
       bus ranges and locate buses with a property that says "nGnRnE" or
       "nGnRE" and dynamically build the list based on that.

    The advantage of this option is that it doesn't touch non-arch code.
    The disadvantage is that it adds a complete new bespoke mechanism to
    the DT, and that it does not let device drivers actually select the
    IO mode, which might be desirable in the future anyway for some
    devices.

I tried investigating further what this would look like, but scanning through
the ADT dump for what nodes use which register ranges. At first it seemed
the range 0x200000000-0x2ffffffff is used for all normal devices, while
the three PCI buses fall into the 0x380000000-0x4ffffffff,
0x500000000-0x67fffffff and 0x680000000-0x6ffffffff ranges
respectively. This would allow a nice abstraction where one node
contains all the devices in the 0x200000000-0x2ffffffff, and we do a
translation in of_address_to_resource(),  similar to what we have for
pci and isa nodes with their special addresses.

However, I did find that there are several nodes that use mmio
addresses next to the PCI addresses, e.g. apciec0, dart-apciec0,
apciec0-piodma, dart-acio0, acio0, acio-cpu0, atc0-dpin0, atc-phy0,
dart-usb0, and usb-drd0 in the 0x380000000-0x3ffffffff range, just
before the MMIO space of the first PCIe bus, so it gets a little
more complicated.

The actual device node could look something like

#define MAP_NONPOSTED 0x80000000

arm-io { /* name for adt, should be changed */
     compatible = "apple,m1-internal-bus";
     #address-cells = <2>; /* or maybe <3> if we want */
     #size-cells = <2>;
     ranges =
               /* on-chip MMIO */
               <(MAP_NONPOSTED | 0x2) 0x0   0x2 0x0 0x1 0x0>,

              /* first PCI: 2GB control, 4GB memory space */
             <(MAP_NONPOSTED | 0x3) 0x80000000  0x3 0x80000000  0x0 0x80000000>,
              <0x4 0x0   0x4 0x0  0x1 0x0>,

              /* second PCI: 2GB control, 4GB memory space */
             <(MAP_NONPOSTED | 0x5) 0x0  0x5 0x0  0x0 0x80000000>,
              <0x5 0x80000000  0x5 0x80000000  0x1>,

              /* third PCI  0.5GB control, 1.5GB memory space*/
             <(MAP_NONPOSTED | 0x6) 0x80000000  0x6 0x80000000  0x0 0x20000000>,
             <0x6 0xa0000000   0x6 0xa0000000  0x0 0x60000000>;
}

The MAP_NONPOSTED flag then gets interpreted by the .translate() and
.get_flags() callbacks of "struct of_bus" in the kernel, where it is put into
a "struct resource" flag, and interpreted when the resource gets mapped.

The PCI host controller nests inside of the node above, and (in theory)
uses the same trick to distinguish between prefetchable and non-prefetchable
memory, except in practice this is handled in device drivers that already
know whether to call ioremap() or ioremap_wc().

It is only PCI mmio space that needs to be nGnRE.  The PCI host
controller register space itself needs nGnRnE just like all other
integrated peripherals (or at least it works that way).

For U-Boot I'm using the following memory map:

static struct mm_region apple_mem_map[] = {
	{
		/* I/O */
		.virt = 0x200000000,
		.phys = 0x200000000,
		.size = 8UL * SZ_1G,
		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
			 PTE_BLOCK_NON_SHARE |
			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
	}, {
		/* I/O */
		.virt = 0x500000000,
		.phys = 0x500000000,
		.size = 2UL * SZ_1G,
		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
			 PTE_BLOCK_NON_SHARE |
			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
	}, {
		/* I/O */
		.virt = 0x680000000,
		.phys = 0x680000000,
		.size = SZ_512M,
		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
			 PTE_BLOCK_NON_SHARE |
			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
	}, {
		/* PCIE */
		.virt = 0x6a0000000,
		.phys = 0x6a0000000,
		.size = SZ_512M,
		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
			 PTE_BLOCK_INNER_SHARE |
			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
	}, {
		/* PCIE */
		.virt = 0x6c0000000,
		.phys = 0x6c0000000,
		.size = SZ_1G,
		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
			 PTE_BLOCK_INNER_SHARE |
			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
	}, {
		/* RAM */
		.virt = 0x800000000,
		.phys = 0x800000000,
		.size = 8UL * SZ_1G,
		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
			 PTE_BLOCK_INNER_SHARE
	}, {
		/* List terminator */
		0,
	}
};

struct mm_region *mem_map = apple_mem_map;

This seems to work so far.  It only has the regions for one PCIe
controller.  I suppose the other two are there to support the TB4
ports?

So there is one 512M region for "32-bit" mmio starting at 0x6a0000000
and one 1G region for "64-bit" mmio starting at 0x6c0000000.

Cheers,

Mark

On 09/02/2021 08.20, Mark Kettenis wrote:

It is only PCI mmio space that needs to be nGnRE.  The PCI host
controller register space itself needs nGnRnE just like all other
integrated peripherals (or at least it works that way).

This is correct. Actually, as I just discovered, nGnRE writes to MMIO 
are not silently blackholed, but rather raise an SError. A certain other 
Linux loader masks those SErrors in a vendor register completely 
unnecessarily, which is why this isn't apparent when you use it. I never 
noticed this myself until now because when I first ran into it, it was 
breaking the UART, so of course I'd never see the SErrors, and I didn't 
try again after I learned more about the L2C SError control mechanism :-)

Testing now, it seems we can actually fairly neatly figure out where 
nGnRE is allowed and where not, as writes that fail due to that raise a 
SError with L2C_ERR_INF=3.

I probed writing to i<<28 for i = [0..255], using nGnRE. This reveals 
that nGnRE writes are allowed (i.e. either succeed or error out 
differently) in the following ranges:

0x400000000 - 0x4ffffffff (apciec0)
0x580000000 - 0x67fffffff (apciec1)
0x6a0000000 - 0x6ffffffff (apcie)

Which matches the `ranges` properties of the respective apcie devices in 
the ADT. The first two are obviously the TB3 ports, amd have more 
features (three ranges instead of two, presumably IO port ranges are 
supported on those, there's some extra DMA stuff, etc).

So the hardware behavior is to block nGnRE everywhere except in those 
ranges (i.e. the nGnRnE fault takes precedence over other errors, like 
the address not existing at all).

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub