Date:   Fri,  5 Feb 2021 05:39:46 +0900
From:   Hector Martin <marcan@...can.st>
To:     Hector Martin <marcan@...can.st>, soc@...nel.org
Cc:     linux-arm-kernel@...ts.infradead.org,
        Marc Zyngier <maz@...nel.org>, robh+dt@...nel.org,
        Arnd Bergmann <arnd@...nel.org>, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, Olof Johansson <olof@...om.net>
Subject: [PATCH 13/18] arm64: ioremap: use nGnRnE mappings on platforms that require it

This follows from the fixmap patch, but relates to the general case.
This is a hack, and incomplete. Read on for discussion.

The problem: on Apple ARM platforms, SoC MMIO needs to use nGnRnE
mappings: writes using nGnRE are blackholed. This seems to be by design,
and there doesn't seem to be any fabric configuration or other bit we
can flip to make the problem go away.

As an additional confounding factor, reportedly PCIe MMIO BAR mappings
conversely *do* need to use nGnRE to work properly. So we can't even get
away with a single ioremap setting, but need to discriminate based on
what bus the device is in. Since these devices have Thunderbolt, all PCI
devices in the tree are potentially in scope. Ugh.

Ideas:

(1) Set up some devicetree property to default to nGnRnE at the platform
    level, and then make PCI drivers use nGnRE.

    This will require changing the PCI code to make pci_ioremap_bar do
    something other than a plain ioremap().

    Unfortunately, of the ~630 PCI drivers in the tree, only ~90 use
    pci_ioremap_bar(). This would require a tree-wide cleanup to
    introduce something akin to pci_ioremap(), and make all PCI
    drivers use it instead of naked ioremap().

    Currently there are three ioremap variants:

    ioremap()
    ioremap_wc()
    ioremap_uc() (not normally used on arm64)

    None of these really capture the nGnRE vs nGnRnE distinction. If
    a new variant is introduced in common code, we'd have to provide
    a default implementation that falls back to regular ioremap() on
    other arches. Something like ioremap() vs. ioremap_np() (nonposted)?

(2) The converse of (1): keep the nGnRE default, but introduce special
    casing to the OF binding code to use nGnRnE when instructed to do so
    on these platforms. This means of_iomap() needs changing.

    The advantage of this approach is that the set of possible non-PCI
    drivers that are useful on these SoCs is bounded, so not all drivers
    that don't go through that path need to be fixed.

    Additionally, this could take advantage of the OF address
    translation stuff to be smarter about deciding to use nGnRnE, e.g.
    doing it based on a property of the parent bus node.

    Of note, some devices (like samsung_tty) go through the platform
    device framework, which eventually goes into devm code. So
    of_address_to_resource would need to set some flag on the struct
    resource, that can then be used by both of_iomap() and
    devm_ioremap_resource() and friends to eventually call the right
    ioremap variant.

    The ioremap considerations from (1) apply here too.

(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.

All discussion and additional ideas welcome.

Signed-off-by: Hector Martin <marcan@...can.st>
---
 arch/arm64/include/asm/io.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 5ea8656a2030..f2609a4f5019 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -167,7 +167,14 @@ extern void __iomem *__ioremap(phys_addr_t phys_addr, size_t size, pgprot_t prot
 extern void iounmap(volatile void __iomem *addr);
 extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
 
-#define ioremap(addr, size)		__ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
+/*
+ * Some platforms require nGnRnE for MMIO.
+ */
+extern bool arm64_use_ne_io;
+
+#define ioremap(addr, size)		__ioremap((addr), (size), \
+						  arm64_use_ne_io ? __pgprot(PROT_DEVICE_nGnRnE) \
+								  : __pgprot(PROT_DEVICE_nGnRE))
 #define ioremap_wc(addr, size)		__ioremap((addr), (size), __pgprot(PROT_NORMAL_NC))
 
 /*
-- 
2.30.0

Powered by blists - more mailing lists