From: Mark Kettenis <hidden> Date: 2021-02-08 23:28:29
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Hector Martin <hidden> Date: 2021-02-09 00:26:58
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Feb 9, 2021 at 1:25 AM Hector Martin [off-list ref] wrote:
On 09/02/2021 08.20, Mark Kettenis wrote:
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.
Right, these are the same ranges that I found in the adt and that Mark
listed in his code snippet, so it seems we all see the same partitioning
of the address space. I also see them reflected in the
/defaults/pmap-io-ranges property in ADT, which seems to have an entry
for every register range that has some mmio registers, along with what
appears to be a bitmask of some attributes, and it clearly shows
the above ranges as having a distinct set of bits from the others
(in little-endian):
00000000 04000000 00000080 00000000 27000080 65494350
00000080 04000000 00000080 00000000 27000080 65494350
00000080 05000000 00000080 00000000 27000080 65494350
00000000 06000000 00000080 00000000 27000080 65494350
000000a0 06000000 00000020 00000000 27000080 65494350
000000c0 06000000 00000040 00000000 27000080 65494350
^64-bit address ^64-bit length ^ 64-bit flags?
As opposed to e.g.
0000f002 05000000 00400000 00000000 07400000 54524144
0000f802 05000000 00400000 00000000 07400000 54524144
00800021 05000000 00400000 00000000 07400000 44495344
0000a801 05000000 00400000 00000000 07400000 54524144
00000367 02000000 00400000 00000000 07400000 54524144
...
There is one more entry for the 0x700000000-0x7ffffffff range, which
has yet another distinct bitmask, but does not seem to correspond
to any registers listed in other nodes.
The first two are obviously the TB3 ports, and have more
features (three ranges instead of two, presumably IO port ranges are
supported on those, there's some extra DMA stuff, etc).
The PCI ranges property identifies these as 64-bit prefetchable (0x43),
32-bit non-prefetchable (0x02), and 32-bit pre prefetchable (0x42)
respectively. The third bus only lacks the 32-bit prefetchable range,
that is normally ok. Is this the NVMe host or something else?
None of them have an I/O space ranges though, only memory space.
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).
Ok, so if we want this to get encoded in a 'struct resource' flag, the PCI
resources should work just fine as these resources come from the
PCI layer rather than of_address_to_resource(). I think it would be
reasonable here to add something to of_address_to_resource() to
set such a flag if we can find an unused one, and then require the
drivers for this platform to go through devm_ioremap_resource()
or similar.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Feb 9, 2021 at 12:20 AM Mark Kettenis [off-list ref] wrote:
quoted
From: Arnd Bergmann <arnd@kernel.org>
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
}, {...
/* List terminator */
0,
}
};
Right, that list agrees mostly what I found, except I didn't see a
continuous 8GB range at 0x200000000 in the ADT ranges properties
but instead a 4GB range for most of the on-chip components plus a
2GB range for stuff that is related to the first PCIe host, starting at
0x380000000 and nothing inbetween. I suppose this makes no
practical difference.
For the PCIe ranges, I see that only one of them is non-prefetchable,
while the other two ranges are marked as prefetchable. These usually
get turned into ioremap_wc() or ioremap_wt() mappings in Linux,
which are more relaxed than MT_DEVICE_NGNRE. Having
stricter attributes in u-boot shouldn't hurt, it might just be slightly
slower than it has to be.
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.
From: Hector Martin <hidden> Date: 2021-02-09 11:25:20
On 09/02/2021 18.15, Arnd Bergmann wrote:
Right, these are the same ranges that I found in the adt and that Mark
listed in his code snippet, so it seems we all see the same partitioning
of the address space. I also see them reflected in the
/defaults/pmap-io-ranges property in ADT, which seems to have an entry
for every register range that has some mmio registers, along with what
appears to be a bitmask of some attributes, and it clearly shows
the above ranges as having a distinct set of bits from the others
(in little-endian):
00000000 04000000 00000080 00000000 27000080 65494350
00000080 04000000 00000080 00000000 27000080 65494350
00000080 05000000 00000080 00000000 27000080 65494350
00000000 06000000 00000080 00000000 27000080 65494350
000000a0 06000000 00000020 00000000 27000080 65494350
000000c0 06000000 00000040 00000000 27000080 65494350
^64-bit address ^64-bit length ^ 64-bit flags?
That's ASCII :-)
'PCIe'
As opposed to e.g.
0000f002 05000000 00400000 00000000 07400000 54524144
Ok, so if we want this to get encoded in a 'struct resource' flag, the PCI
resources should work just fine as these resources come from the
PCI layer rather than of_address_to_resource(). I think it would be
reasonable here to add something to of_address_to_resource() to
set such a flag if we can find an unused one, and then require the
drivers for this platform to go through devm_ioremap_resource()
or similar.
This sounds reasonable. For setting such a flag, I guess looking for a
property (inherited from parents) would make sense. `mmio-map-mode =
"nonposted"` or something like that?
--
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel