x86/mm, asm-generic: Add IOMMU ioremap_uc() variant default

Started by Luis R. Rodriguez on Jul 10, 2015 · 3 messages from 3 authors · Last reply Mar 16, 2017

Subsystems (3)

Maintainers (6)

Show 2 more
STALE3525d LANDED

Status: Landed in mainline as 8c7ea50c010b on 2015-07-21.

open the first message on its own page

Message details and related content
From: "Luis R. Rodriguez" <redacted>

We currently have no safe way of currently defining architecture
agnostic IOMMU ioremap_*() variants. The trend is for folks to
*assume* that ioremap_nocache() should be the default everywhere
and then add this mapping on each architectures -- this is not
correct today for a variety of reasons.

We have two options:

1) Sit and wait for every architecture in Linux to get a
   an ioremap_*() variant defined before including it upstream

2) Gather consensus on a safe architecture agnostic ioremap_*()
   default

Approach 1) introduces development latencies, and since 2) will take
time and work on clarifying semantics the only remaining sensible
thing to do to avoid issues is returning NULL on ioremap_*()
variants.

In order for this to work we must have all architectures declare
their own ioremap_*() variants as defined. This will take some work,
do this for ioremp_uc() to set the example as its only currently
implemented on x86. Document all this.

Signed-off-by: Luis R. Rodriguez <redacted>
---

Ingo,

I only provide implementation support for ioremap_uc() as the other ioremap_*()
variants are well defined all over the kernel for other architectures already.
Dan Williams is also providing declarations for them through another series [0]
This patch does not depend on that series, that patch series does not address
this specific issue.  With this patch in place now things won't fail to compile
if ioremap_uc() is used in the kernel on architectures lacking an ioremap_uc()
implementaiton and makes such calls simply fail. In the very near future there
is only one expected driver to use this, the atyfb framebuffer device drivers,
the series for which I've been respinning for a while now and it seems we

21 more lines

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index cc9c61bc1abe..7cfc085b6879 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -180,6 +180,8 @@ static inline unsigned int isa_virt_to_bus(volatile void *address)
  */
 extern void __iomem *ioremap_nocache(resource_size_t offset, unsigned long size);
 extern void __iomem *ioremap_uc(resource_size_t offset, unsigned long size);
+#define ioremap_uc ioremap_uc
+
 extern void __iomem *ioremap_cache(resource_size_t offset, unsigned long size);
 extern void __iomem *ioremap_prot(resource_size_t offset, unsigned long size,
 				unsigned long prot_val);
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index f56094cfdeff..eed3bbe88c8a 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -736,6 +736,35 @@ static inline void *phys_to_virt(unsigned long address)
 }
 #endif
 
+/**
+ * DOC: ioremap() and ioremap_*() variants
+ *
+ * If you have an IOMMU your architecture is expected to have both ioremap()
+ * and iounmap() implemented otherwise the asm-generic helpers will provide a
+ * direct mapping.
+ *
+ * There are ioremap_*() call variants, if you have no IOMMU we naturally will
+ * default to direct mapping for all of them, you can override these defaults.
+ * If you have an IOMMU you are highly encouraged to provide your own
+ * ioremap variant implementation as there currently is no safe architecture
+ * agnostic default. To avoid possible improper behaviour default asm-generic
+ * ioremap_*() variants all return NULL when an IOMMU is available. If you've
+ * defined your own ioremap_*() variant you must then declare your own
+ * ioremap_*() variant as defined to itself to avoid the default NULL return.
+ */
+
+#ifdef CONFIG_MMU
+
+#ifndef ioremap_uc
+#define ioremap_uc ioremap_uc
+static inline void __iomem *ioremap_uc(phys_addr_t offset, size_t size)
+{
+	return NULL;
+}
+#endif
+
+#else /* !CONFIG_MMU */
+
 /*
  * Change "struct page" to physical address.
  *
@@ -743,7 +772,6 @@ static inline void *phys_to_virt(unsigned long address)
  * you'll need to provide your own definitions.
  */
 
-#ifndef CONFIG_MMU
 #ifndef ioremap
 #define ioremap ioremap
 static inline void __iomem *ioremap(phys_addr_t offset, size_t size)
-- 
2.3.2.209.gd67f9d5.dirty
Message details and related content
On Thu, Jul 9, 2015 at 7:28 PM, Luis R. Rodriguez
[off-list ref] wrote:
quoted hunk
+/**
+ * DOC: ioremap() and ioremap_*() variants
+ *
+ * If you have an IOMMU your architecture is expected to have both ioremap()
+ * and iounmap() implemented otherwise the asm-generic helpers will provide a
+ * direct mapping.
+ *
+ * There are ioremap_*() call variants, if you have no IOMMU we naturally will
+ * default to direct mapping for all of them, you can override these defaults.
+ * If you have an IOMMU you are highly encouraged to provide your own
+ * ioremap variant implementation as there currently is no safe architecture
+ * agnostic default. To avoid possible improper behaviour default asm-generic
+ * ioremap_*() variants all return NULL when an IOMMU is available. If you've
+ * defined your own ioremap_*() variant you must then declare your own
+ * ioremap_*() variant as defined to itself to avoid the default NULL return.
Are the references above to "IOMMU" typos?  Should they say "MMU"
instead, so they match the #ifdef below?
quoted hunk
+ */
+
+#ifdef CONFIG_MMU
+
+#ifndef ioremap_uc
+#define ioremap_uc ioremap_uc
+static inline void __iomem *ioremap_uc(phys_addr_t offset, size_t size)
+{
+       return NULL;
+}
+#endif
+
+#else /* !CONFIG_MMU */
+
 /*
  * Change "struct page" to physical address.
  *
@@ -743,7 +772,6 @@ static inline void *phys_to_virt(unsigned long address)
  * you'll need to provide your own definitions.
  */

-#ifndef CONFIG_MMU
 #ifndef ioremap
 #define ioremap ioremap
 static inline void __iomem *ioremap(phys_addr_t offset, size_t size)
--
2.3.2.209.gd67f9d5.dirty

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Message details and related content
On Thu, Mar 16, 2017 at 03:46:51PM -0500, Bjorn Helgaas wrote:
On Thu, Jul 9, 2015 at 7:28 PM, Luis R. Rodriguez
[off-list ref] wrote:
quoted
+/**
+ * DOC: ioremap() and ioremap_*() variants
+ *
+ * If you have an IOMMU your architecture is expected to have both ioremap()
+ * and iounmap() implemented otherwise the asm-generic helpers will provide a
+ * direct mapping.
+ *
+ * There are ioremap_*() call variants, if you have no IOMMU we naturally will
+ * default to direct mapping for all of them, you can override these defaults.
+ * If you have an IOMMU you are highly encouraged to provide your own
+ * ioremap variant implementation as there currently is no safe architecture
+ * agnostic default. To avoid possible improper behaviour default asm-generic
+ * ioremap_*() variants all return NULL when an IOMMU is available. If you've
+ * defined your own ioremap_*() variant you must then declare your own
+ * ioremap_*() variant as defined to itself to avoid the default NULL return.
Are the references above to "IOMMU" typos?  Should they say "MMU"
instead, so they match the #ifdef below?
Yes. Patch welcomed.

  Luis
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help