[RFC PATCH 0/4] PIO drivers and cache coherency

[RFC PATCH 0/4] PIO drivers and cache coherency

[Catalin Marinas]

Hi,

There were a couple of threads recently about PIO drivers and cache
coherency for page cache pages. I'll try to quickly summarise some of
the discussions and current API issues and propose some improvements
(please correct if I got it wrong). I'm assuming the key people are on
linux-arch but if I missed anyone please cc them.

In the ARM case, a PIO driver writes the data to a page cache page
dirtying the D-cache. This may cause either aliases with the user
mapping (if the hardware has such thing) or just incoherency with the
I-cache on Harvard architectures.

Two examples are the ISP1760 HCD driver and the libata-sff library for
PIO sector transfers. Without any changes to these drivers, rootfs on a
USB stick or CompactFlash (via pata-platform) cannot be used (it usually
fails in /sbin/init with illegal instruction).

Based on the Documentation/cachetlb.txt document, my first approach was
to use flush_dcache_page() in the corresponding drivers, though with
some drawbacks and this doesn't seem to be a popular choice.

These two examples actually have different ways of accessing the
transfer buffer. The HCD driver receives a pointer via
urb->transfer_buffer while libata-sff uses a page structure.

One of the proposals was to always use the kmap API even if the page
isn't a highmem one (or if the kernel doesn't support highmem) and do
the cache flushing in these functions. There may be performance
penalties with this approach on some architectures and for some drivers
the kmap() is called in upper layers (the HCD case).

Another proposal was to extend the kmap API with information about the
direction of the transfer so that it can be implemented more
efficiently (functions like kmap_pio_*). This approach requires the page
structure to always be available for passing to the kmap_pio_* functions
but that's not possible on HCD devices (and upper layers like USB mass
storage do not know whether the driver is going to use DMA or PIO).

IMHO, what we need is something that resembles the DMA API. Another
opinion (James Bottomley's) is that such API would be closer to the kmap
API. Any other suggestions are welcome.

My proposal is for a PIO mapping API similar to the DMA API that
architectures can implement as they like to deal with such situations.
The first patch is an example specification of such API intended to
start a discussion around it rather than a final proposal.

The second patch is a light-weight ARM implementation of such APIs.  It
is incomplete in the sense that it doesn't do additional checks for
highmem pages (that may be flushed via kunmap_atomic). It also doesn't
deal with D-cache aliasing correctly.

The third and fourth patches show example usages of such API in the HCD
and libata cases.

Regards.


Catalin Marinas (4):
      pio-mapping: Add generic support for PIO mapping API
      pio-mapping: Add ARM support for the PIO mapping API
      pio-mapping: Use the PIO mapping API in libata-sff.c
      pio-mapping: Use the PIO mapping API in the ISP1760 HCD driver


 arch/arm/Kconfig                   |    3 ++
 arch/arm/include/asm/pio-mapping.h |   52 +++++++++++++++++++++++++++++++
 drivers/ata/libata-sff.c           |   10 ++++--
 drivers/usb/host/isp1760-hcd.c     |    9 +++++
 include/linux/pio-mapping.h        |   61 ++++++++++++++++++++++++++++++++++++
 5 files changed, 131 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/include/asm/pio-mapping.h
 create mode 100644 include/linux/pio-mapping.h

-- 
Catalin

[RFC PATCH 1/4] pio-mapping: Add generic support for PIO mapping API

[Catalin Marinas]

Implement partial support for PIO mapping API. Currently only map/unmap
single and page functions are included. These functions are intended to
be used by device drivers doing PIO to page cache pages that may
potentially be mapped in user space and cause cache coherency issues.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 include/linux/pio-mapping.h |   61 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 61 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/pio-mapping.h

diff --git a/include/linux/pio-mapping.h b/include/linux/pio-mapping.h
new file mode 100644
index 0000000..9f53aff
--- /dev/null
+++ b/include/linux/pio-mapping.h
@@ -0,0 +1,61 @@
+/*
+ * include/linux/pio-mapping.h
+ *
+ * Copyright (C) 2010 ARM Ltd.
+ * Written by Catalin Marinas <catalin.marinas@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef PIO_MAPPING_H
+#define PIO_MAPPING_H
+
+#include <linux/mm.h>
+
+enum pio_data_direction {
+	PIO_BIDIRECTIONAL,
+	PIO_TO_DEVICE,
+	PIO_FROM_DEVICE,
+	PIO_NONE
+};
+
+#ifdef CONFIG_HAVE_ARCH_PIO
+#include <asm/pio-mapping.h>
+#else
+
+static inline void *pio_map_single(void *addr, size_t size,
+				   enum pio_data_direction dir)
+{
+	return addr;
+}
+
+static inline void pio_unmap_single(void *addr, size_t size,
+				    enum pio_data_direction dir)
+{
+}
+
+static inline void *pio_map_page(struct page *page, unsigned long offset,
+				 size_t size, enum pio_data_direction dir)
+{
+	return page_address(page) + offset;
+}
+
+static inline void pio_unmap_page(void *addr, size_t size,
+				  enum pio_data_direction dir)
+{
+}
+
+#endif	/* CONFIG_HAVE_ARCH_PIO */
+
+#endif	/* PIO_MAPPING_H */

[RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Catalin Marinas]

This patch introduces support for the PIO mapping API on the ARM
architecture. It is currently only meant as an example for discussions
and it can be further optimised.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/Kconfig                   |    3 ++
 arch/arm/include/asm/pio-mapping.h |   52 ++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/pio-mapping.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 4c33ca8..e48adcf 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -161,6 +161,9 @@ config ARCH_MTD_XIP
 config GENERIC_HARDIRQS_NO__DO_IRQ
 	def_bool y
 
+config HAVE_ARCH_PIO
+	def_bool y
+
 if OPROFILE
 
 config OPROFILE_ARMV6
diff --git a/arch/arm/include/asm/pio-mapping.h b/arch/arm/include/asm/pio-mapping.h
new file mode 100644
index 0000000..d7c866a
--- /dev/null
+++ b/arch/arm/include/asm/pio-mapping.h
@@ -0,0 +1,52 @@
+/*
+ * include/linux/pio-mapping.h
+ *
+ * Copyright (C) 2010 ARM Ltd.
+ * Written by Catalin Marinas <catalin.marinas@arm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef ASM_PIO_MAPPING_H
+#define ASM_PIO_MAPPING_H
+
+#include <asm/cacheflush.h>
+
+static inline void *pio_map_single(void *addr, size_t size,
+				   enum pio_data_direction dir)
+{
+	return addr;
+}
+
+static inline void pio_unmap_single(void *addr, size_t size,
+				    enum pio_data_direction dir)
+{
+	if (dir == PIO_FROM_DEVICE)
+		__cpuc_flush_dcache_area(addr, size);
+}
+
+static inline void *pio_map_page(struct page *page, unsigned long offset,
+				 size_t size, enum pio_data_direction dir)
+{
+	return page_address(page) + offset;
+}
+
+static inline void pio_unmap_page(void *addr, size_t size,
+				  enum pio_data_direction dir)
+{
+	if (dir == PIO_FROM_DEVICE)
+		__cpuc_flush_dcache_area(addr, size);
+}
+
+#endif	/* ASM_PIO_MAPPING_H */

[RFC PATCH 3/4] pio-mapping: Use the PIO mapping API in libata-sff.c

[Catalin Marinas]

This patch shows an example of PIO API usage for non-highmem pages. It
is assumed that kunmap_atomic() performs cache flushing though this may
not always be the case (needs checking with architecture people).

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 drivers/ata/libata-sff.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 741065c..5b72a92 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -36,6 +36,7 @@
 #include <linux/pci.h>
 #include <linux/libata.h>
 #include <linux/highmem.h>
+#include <linux/pio-mapping.h>
 
 #include "libata.h"
 
@@ -888,9 +889,12 @@ static void ata_pio_sector(struct ata_queued_cmd *qc)
 		kunmap_atomic(buf, KM_IRQ0);
 		local_irq_restore(flags);
 	} else {
-		buf = page_address(page);
-		ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size,
-				       do_write);
+		enum pio_data_direction dir = do_write ?
+			PIO_TO_DEVICE : PIO_FROM_DEVICE;
+
+		buf = pio_map_page(page, offset, qc->sect_size, dir);
+		ap->ops->sff_data_xfer(qc->dev, buf, qc->sect_size, do_write);
+		pio_unmap_page(buf, qc->sect_size, dir);
 	}
 
 	qc->curbytes += qc->sect_size;

[RFC PATCH 4/4] pio-mapping: Use the PIO mapping API in the ISP1760 HCD driver

[Catalin Marinas]

This patch provides another example of PIO API usage. In this case,
there is no struct page information in the HCD driver and the
pio_*_single() functions are used.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 drivers/usb/host/isp1760-hcd.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
index 27b8f7c..e55cb90 100644
--- a/drivers/usb/host/isp1760-hcd.c
+++ b/drivers/usb/host/isp1760-hcd.c
@@ -17,6 +17,7 @@
 #include <linux/debugfs.h>
 #include <linux/uaccess.h>
 #include <linux/io.h>
+#include <linux/pio-mapping.h>
 #include <asm/unaligned.h>
 
 #include "../core/hcd.h"
@@ -904,6 +905,10 @@ __acquires(priv->lock)
 			status = 0;
 	}
 
+	pio_unmap_single(urb->transfer_buffer, urb->transfer_buffer_length,
+			 usb_pipein(urb->pipe) ?
+			 PIO_FROM_DEVICE : PIO_TO_DEVICE);
+
 	/* complete() can reenter this HCD */
 	usb_hcd_unlink_urb_from_ep(priv_to_hcd(priv), urb);
 	spin_unlock(&priv->lock);
@@ -1536,7 +1541,9 @@ static struct list_head *qh_urb_transaction(struct isp1760_hcd *priv,
 	/*
 	 * data transfer stage:  buffer setup
 	 */
-	buf = urb->transfer_buffer;
+	buf = pio_map_single(urb->transfer_buffer, urb->transfer_buffer_length,
+			     usb_pipein(urb->pipe) ?
+			     PIO_FROM_DEVICE : PIO_TO_DEVICE);
 
 	if (is_input)
 		token |= IN_PID;

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[James Bottomley]

On Fri, 2010-02-05 at 16:31 +0000, Catalin Marinas wrote:
> This patch introduces support for the PIO mapping API on the ARM
> architecture. It is currently only meant as an example for discussions
> and it can be further optimised.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm/Kconfig                   |    3 ++
>  arch/arm/include/asm/pio-mapping.h |   52 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/include/asm/pio-mapping.h
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 4c33ca8..e48adcf 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -161,6 +161,9 @@ config ARCH_MTD_XIP
>  config GENERIC_HARDIRQS_NO__DO_IRQ
>  	def_bool y
>  
> +config HAVE_ARCH_PIO
> +	def_bool y
> +
>  if OPROFILE
>  
>  config OPROFILE_ARMV6
> diff --git a/arch/arm/include/asm/pio-mapping.h b/arch/arm/include/asm/pio-mapping.h
> new file mode 100644
> index 0000000..d7c866a
> --- /dev/null
> +++ b/arch/arm/include/asm/pio-mapping.h
> @@ -0,0 +1,52 @@
> +/*
> + * include/linux/pio-mapping.h
> + *
> + * Copyright (C) 2010 ARM Ltd.
> + * Written by Catalin Marinas <catalin.marinas@arm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#ifndef ASM_PIO_MAPPING_H
> +#define ASM_PIO_MAPPING_H
> +
> +#include <asm/cacheflush.h>
> +
> +static inline void *pio_map_single(void *addr, size_t size,
> +				   enum pio_data_direction dir)
> +{
> +	return addr;

This API is a bit semantically nasty to use, isn't it?  What we usually
get in the I/O path is a scatter gather list of pages and offsets (or
just a page in the network case).

In a highmem kernel, we'd have to kmap the page before it actually had a
kernel virtual address, so now the use case of the API becomes

vaddr = kmap_...(page)
pio_map_single(vaddr)

and the reverse on unmap.

Why not just combine the two since we always have to do them anyway and
do

kmap_pio ... with the various atomic versions?

enum pio_data_direction dir looks a bit wrong too ... why not just use
the exsiting enum dma_data_direction since that's what the dma API uses?

your pio_map_page is going to have to contain a kmap in the arch
implementation anyway ...

James

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Catalin Marinas]

On Fri, 2010-02-05 at 16:43 +0000, James Bottomley wrote:
> On Fri, 2010-02-05 at 16:31 +0000, Catalin Marinas wrote:
> > This patch introduces support for the PIO mapping API on the ARM
> > architecture. It is currently only meant as an example for discussions
> > and it can be further optimised.
[...]
> > diff --git a/arch/arm/include/asm/pio-mapping.h b/arch/arm/include/asm/pio-mapping.h
> > new file mode 100644
> > index 0000000..d7c866a
> > --- /dev/null
> > +++ b/arch/arm/include/asm/pio-mapping.h
[...]
> > +static inline void *pio_map_single(void *addr, size_t size,
> > +                                enum pio_data_direction dir)
> > +{
> > +     return addr;
> 
> This API is a bit semantically nasty to use, isn't it?  What we usually
> get in the I/O path is a scatter gather list of pages and offsets (or
> just a page in the network case).
> 
> In a highmem kernel, we'd have to kmap the page before it actually had a
> kernel virtual address, so now the use case of the API becomes
> 
> vaddr = kmap_...(page)
> pio_map_single(vaddr)
> 
> and the reverse on unmap.
> 
> Why not just combine the two since we always have to do them anyway and
> do
> 
> kmap_pio ... with the various atomic versions?

For most cases, what we need looks close enough to the kmap semantics
but this wouldn't work for the HCD case - the USB mass storage may use
kmap() but it cannot easily be converted to the kmap_pio API since it's
only the HCD driver that knows what kind of transfer it would do (and
this only gets a (void *) pointer).

We could allow a kmap_pio API to also take (void *) arguments in
addition to (struct page *) but would this be confusing? It diverges
slightly from the kmap API. Do you have a better suggestion?

> enum pio_data_direction dir looks a bit wrong too ... why not just use
> the exsiting enum dma_data_direction since that's what the dma API uses?

I just didn't want to confuse things with the DMA_* name but that's a
valid point.

> your pio_map_page is going to have to contain a kmap in the arch
> implementation anyway ...

In my approach, the only case for pio_map_page() would be for
non-highmem pages, otherwise you would need to use pio_map_single() on
the value returned by kmap() (as you pointed out, it doesn't look nice
but I don't have a solution to accommodate a kmap API with the HCD
drivers).

Thanks.

-- 
Catalin

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[James Bottomley]

On Fri, 2010-02-05 at 17:20 +0000, Catalin Marinas wrote:
> On Fri, 2010-02-05 at 16:43 +0000, James Bottomley wrote:
> > On Fri, 2010-02-05 at 16:31 +0000, Catalin Marinas wrote:
> > > This patch introduces support for the PIO mapping API on the ARM
> > > architecture. It is currently only meant as an example for discussions
> > > and it can be further optimised.
> [...]
> > > diff --git a/arch/arm/include/asm/pio-mapping.h b/arch/arm/include/asm/pio-mapping.h
> > > new file mode 100644
> > > index 0000000..d7c866a
> > > --- /dev/null
> > > +++ b/arch/arm/include/asm/pio-mapping.h
> [...]
> > > +static inline void *pio_map_single(void *addr, size_t size,
> > > +                                enum pio_data_direction dir)
> > > +{
> > > +     return addr;
> > 
> > This API is a bit semantically nasty to use, isn't it?  What we usually
> > get in the I/O path is a scatter gather list of pages and offsets (or
> > just a page in the network case).
> > 
> > In a highmem kernel, we'd have to kmap the page before it actually had a
> > kernel virtual address, so now the use case of the API becomes
> > 
> > vaddr = kmap_...(page)
> > pio_map_single(vaddr)
> > 
> > and the reverse on unmap.
> > 
> > Why not just combine the two since we always have to do them anyway and
> > do
> > 
> > kmap_pio ... with the various atomic versions?
> 
> For most cases, what we need looks close enough to the kmap semantics
> but this wouldn't work for the HCD case - the USB mass storage may use
> kmap() but it cannot easily be converted to the kmap_pio API since it's
> only the HCD driver that knows what kind of transfer it would do (and
> this only gets a (void *) pointer).

OK, so I'm not very familiar with the mechanics of useb, but if it gets
a pointer to the kernel virtual address what's wrong with just doing
virt_to_page() on that?  Note that virt_to_page() only really works if
the page is really a proper offset mapped kernel address, but my little
reading in drivers/usb seems to indicate it's a kmalloc buffer.

> We could allow a kmap_pio API to also take (void *) arguments in
> addition to (struct page *) but would this be confusing? It diverges
> slightly from the kmap API. Do you have a better suggestion?
> 
> > enum pio_data_direction dir looks a bit wrong too ... why not just use
> > the exsiting enum dma_data_direction since that's what the dma API uses?
> 
> I just didn't want to confuse things with the DMA_* name but that's a
> valid point.

So this is an interesting question of what will confuse driver writers
least.  Clearly if we can just pass in the dma data direction and have
done, that's easiest for them.

The flip side is that when we pass the flag DMA_FROM_DEVICE, the arch
people have to remember that this means we're writing to the page ... 

> > your pio_map_page is going to have to contain a kmap in the arch
> > implementation anyway ...
> 
> In my approach, the only case for pio_map_page() would be for
> non-highmem pages, otherwise you would need to use pio_map_single() on
> the value returned by kmap() (as you pointed out, it doesn't look nice
> but I don't have a solution to accommodate a kmap API with the HCD
> drivers).

So the distinction between higmemem and non-highmem is really one I'd
like to contain within the API ... otherwise we get drivers peppered
with

if (PageHighMem(page)) {
...
} else {
...
}

which leads to code proliferation and potentially more bugs.

James

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Catalin Marinas]

On Fri, 2010-02-05 at 17:36 +0000, James Bottomley wrote:
> On Fri, 2010-02-05 at 17:20 +0000, Catalin Marinas wrote:
> > On Fri, 2010-02-05 at 16:43 +0000, James Bottomley wrote:
> > > On Fri, 2010-02-05 at 16:31 +0000, Catalin Marinas wrote:
> > > > This patch introduces support for the PIO mapping API on the ARM
> > > > architecture. It is currently only meant as an example for discussions
> > > > and it can be further optimised.
> > [...]
> > > > diff --git a/arch/arm/include/asm/pio-mapping.h b/arch/arm/include/asm/pio-mapping.h
> > > > new file mode 100644
> > > > index 0000000..d7c866a
> > > > --- /dev/null
> > > > +++ b/arch/arm/include/asm/pio-mapping.h
> > [...]
> > > > +static inline void *pio_map_single(void *addr, size_t size,
> > > > +                                enum pio_data_direction dir)
> > > > +{
> > > > +     return addr;
> > >
> > > This API is a bit semantically nasty to use, isn't it?  What we usually
> > > get in the I/O path is a scatter gather list of pages and offsets (or
> > > just a page in the network case).
> > >
> > > In a highmem kernel, we'd have to kmap the page before it actually had a
> > > kernel virtual address, so now the use case of the API becomes
> > >
> > > vaddr = kmap_...(page)
> > > pio_map_single(vaddr)
> > >
> > > and the reverse on unmap.
> > >
> > > Why not just combine the two since we always have to do them anyway and
> > > do
> > >
> > > kmap_pio ... with the various atomic versions?
> >
> > For most cases, what we need looks close enough to the kmap semantics
> > but this wouldn't work for the HCD case - the USB mass storage may use
> > kmap() but it cannot easily be converted to the kmap_pio API since it's
> > only the HCD driver that knows what kind of transfer it would do (and
> > this only gets a (void *) pointer).
> 
> OK, so I'm not very familiar with the mechanics of useb, but if it gets
> a pointer to the kernel virtual address what's wrong with just doing
> virt_to_page() on that?  Note that virt_to_page() only really works if
> the page is really a proper offset mapped kernel address, but my little
> reading in drivers/usb seems to indicate it's a kmalloc buffer.

According to this commit - 96983d2d8 - the USB mass storage only passes
lowmem pages to the non-DMA HCD drivers, so virt_to_page() is safe.

The only problem is that the transfer buffer passed to HCD may be bigger
than a page (I've seen it going to 64K) and you would need to call
kmap_pio() in the enqueue() function for each page, store the pointers
somewhere and later (once filled) unmap them via kunmap_pio() in the
dequeue() function (my temporary solution is this -
http://lkml.org/lkml/2010/2/2/142).
> 
> > > your pio_map_page is going to have to contain a kmap in the arch
> > > implementation anyway ...
> >
> > In my approach, the only case for pio_map_page() would be for
> > non-highmem pages, otherwise you would need to use pio_map_single() on
> > the value returned by kmap() (as you pointed out, it doesn't look nice
> > but I don't have a solution to accommodate a kmap API with the HCD
> > drivers).
> 
> So the distinction between higmemem and non-highmem is really one I'd
> like to contain within the API ... otherwise we get drivers peppered
> with
> 
> if (PageHighMem(page)) {
> ...
> } else {
> ...
> }
> 
> which leads to code proliferation and potentially more bugs.

I agree. Some more thinking needed over the weekend.

Thanks.

-- 
Catalin

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Catalin Marinas]

On Fri, 2010-02-05 at 17:36 +0000, James Bottomley wrote:
> On Fri, 2010-02-05 at 17:20 +0000, Catalin Marinas wrote:
> > On Fri, 2010-02-05 at 16:43 +0000, James Bottomley wrote:
> > > On Fri, 2010-02-05 at 16:31 +0000, Catalin Marinas wrote:
> > > > This patch introduces support for the PIO mapping API on the ARM
> > > > architecture. It is currently only meant as an example for discussions
> > > > and it can be further optimised.
> > [...]
> > > > diff --git a/arch/arm/include/asm/pio-mapping.h b/arch/arm/include/asm/pio-mapping.h
> > > > new file mode 100644
> > > > index 0000000..d7c866a
> > > > --- /dev/null
> > > > +++ b/arch/arm/include/asm/pio-mapping.h
> > [...]
> > > > +static inline void *pio_map_single(void *addr, size_t size,
> > > > +                                enum pio_data_direction dir)
> > > > +{
> > > > +     return addr;
> > >
> > > This API is a bit semantically nasty to use, isn't it?  What we usually
> > > get in the I/O path is a scatter gather list of pages and offsets (or
> > > just a page in the network case).
> > >
> > > In a highmem kernel, we'd have to kmap the page before it actually had a
> > > kernel virtual address, so now the use case of the API becomes
> > >
> > > vaddr = kmap_...(page)
> > > pio_map_single(vaddr)
> > >
> > > and the reverse on unmap.
> > >
> > > Why not just combine the two since we always have to do them anyway and
> > > do
> > >
> > > kmap_pio ... with the various atomic versions?
> >
> > For most cases, what we need looks close enough to the kmap semantics
> > but this wouldn't work for the HCD case - the USB mass storage may use
> > kmap() but it cannot easily be converted to the kmap_pio API since it's
> > only the HCD driver that knows what kind of transfer it would do (and
> > this only gets a (void *) pointer).
> 
> OK, so I'm not very familiar with the mechanics of useb, but if it gets
> a pointer to the kernel virtual address what's wrong with just doing
> virt_to_page() on that?  Note that virt_to_page() only really works if
> the page is really a proper offset mapped kernel address, but my little
> reading in drivers/usb seems to indicate it's a kmalloc buffer.

I got your point about a kmap-like API which seems to be the case with
many block device drivers. The USB HCD is a different case where kmap
isn't used, only a pointer to a buffer that may span across multiple
pages.

What about something like below (just copying the code, no patch yet). I
used the "pio_" prefix rather than "kmap_" prefix since the last two
functions are used for address ranges rather than page structures for
cases like HCD.

The pio_data_direction could be dropped and use the DMA one. We could
also use pio_kmap_read/pio_kmap_write or similar but we have to triple
the number of functions, so I prefer the additional argument.


#include <linux/highmem.h>

enum pio_data_direction {
	PIO_BIDIRECTIONAL,
	PIO_TO_DEVICE,
	PIO_FROM_DEVICE,
	PIO_NONE
};

#ifdef CONFIG_HAVE_ARCH_PIO
#include <asm/pio-mapping.h>
#else

static inline void *pio_kmap(struct page *page, enum pio_data_direction dir)
{
	return kmap(page);
}

static inline void pio_kunmap(struct page *page, enum pio_data_direction dir)
{
	kunmap(page);
}

static inline void *pio_kmap_atomic(struct page *page, enum km_type idx,
				    enum pio_data_direction dir)
{
	return kmap_atomic(page, idx);
}

static inline void pio_kunmap_atomic(void *addr, enum km_type idx,
				     enum pio_data_direction dir)
{
	kunmap_atomic(page, idx);
}

static inline void *pio_map_range(void *start, size_t size,
				  enum pio_data_direction dir)
{
	return start;
}

static inline void pio_unmap_range(void *start, size_t size,
				   enum pio_data_direction dir)
{
}

#endif	/* CONFIG_HAVE_ARCH_PIO */

-- 
Catalin

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Russell King]

On Mon, Feb 08, 2010 at 04:10:21PM +0000, Catalin Marinas wrote:
> The pio_data_direction could be dropped and use the DMA one. We could
> also use pio_kmap_read/pio_kmap_write or similar but we have to triple
> the number of functions, so I prefer the additional argument.

Do we need to do anything for reading a buffer for PIO _out_ to the
device?  My understanding is that this has never been a problem.

The only problem I'm aware of is where PIO writes to the kernel
mapping of a lowmem pages; highmem pages need the data flushed out
of the temporary atomic kmap mapping anyway.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[James Bottomley]

On Mon, 2010-02-08 at 16:10 +0000, Catalin Marinas wrote:
> On Fri, 2010-02-05 at 17:36 +0000, James Bottomley wrote:
> > On Fri, 2010-02-05 at 17:20 +0000, Catalin Marinas wrote:
> > > On Fri, 2010-02-05 at 16:43 +0000, James Bottomley wrote:
> > > > On Fri, 2010-02-05 at 16:31 +0000, Catalin Marinas wrote:
> > > > > This patch introduces support for the PIO mapping API on the ARM
> > > > > architecture. It is currently only meant as an example for discussions
> > > > > and it can be further optimised.
> > > [...]
> > > > > diff --git a/arch/arm/include/asm/pio-mapping.h b/arch/arm/include/asm/pio-mapping.h
> > > > > new file mode 100644
> > > > > index 0000000..d7c866a
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/include/asm/pio-mapping.h
> > > [...]
> > > > > +static inline void *pio_map_single(void *addr, size_t size,
> > > > > +                                enum pio_data_direction dir)
> > > > > +{
> > > > > +     return addr;
> > > >
> > > > This API is a bit semantically nasty to use, isn't it?  What we usually
> > > > get in the I/O path is a scatter gather list of pages and offsets (or
> > > > just a page in the network case).
> > > >
> > > > In a highmem kernel, we'd have to kmap the page before it actually had a
> > > > kernel virtual address, so now the use case of the API becomes
> > > >
> > > > vaddr = kmap_...(page)
> > > > pio_map_single(vaddr)
> > > >
> > > > and the reverse on unmap.
> > > >
> > > > Why not just combine the two since we always have to do them anyway and
> > > > do
> > > >
> > > > kmap_pio ... with the various atomic versions?
> > >
> > > For most cases, what we need looks close enough to the kmap semantics
> > > but this wouldn't work for the HCD case - the USB mass storage may use
> > > kmap() but it cannot easily be converted to the kmap_pio API since it's
> > > only the HCD driver that knows what kind of transfer it would do (and
> > > this only gets a (void *) pointer).
> > 
> > OK, so I'm not very familiar with the mechanics of useb, but if it gets
> > a pointer to the kernel virtual address what's wrong with just doing
> > virt_to_page() on that?  Note that virt_to_page() only really works if
> > the page is really a proper offset mapped kernel address, but my little
> > reading in drivers/usb seems to indicate it's a kmalloc buffer.
> 
> I got your point about a kmap-like API which seems to be the case with
> many block device drivers. The USB HCD is a different case where kmap
> isn't used, only a pointer to a buffer that may span across multiple
> pages.
> 
> What about something like below (just copying the code, no patch yet). I
> used the "pio_" prefix rather than "kmap_" prefix since the last two
> functions are used for address ranges rather than page structures for
> cases like HCD.
> 
> The pio_data_direction could be dropped and use the DMA one. We could
> also use pio_kmap_read/pio_kmap_write or similar but we have to triple
> the number of functions, so I prefer the additional argument.

Yes ... I think we don't want to confuse driver writers with two enums
that mean the same thing but have to be used in different places.

> #include <linux/highmem.h>
> 
> enum pio_data_direction {
> 	PIO_BIDIRECTIONAL,
> 	PIO_TO_DEVICE,
> 	PIO_FROM_DEVICE,
> 	PIO_NONE
> };
> 
> #ifdef CONFIG_HAVE_ARCH_PIO
> #include <asm/pio-mapping.h>
> #else
> 
> static inline void *pio_kmap(struct page *page, enum pio_data_direction dir)
> {
> 	return kmap(page);
> }
> 
> static inline void pio_kunmap(struct page *page, enum pio_data_direction dir)
> {
> 	kunmap(page);
> }
> 
> static inline void *pio_kmap_atomic(struct page *page, enum km_type idx,
> 				    enum pio_data_direction dir)
> {
> 	return kmap_atomic(page, idx);
> }
> 
> static inline void pio_kunmap_atomic(void *addr, enum km_type idx,
> 				     enum pio_data_direction dir)
> {
> 	kunmap_atomic(page, idx);
> }

Above are all perfect, thanks for doing this!

Below is where it gets tricky.

This API would have to cope with highmem too ... and because of the
limited mappings available, we can't just map an arbitrary sized buffer
(especially in atomics where the number of available slots is often only
two).

> static inline void *pio_map_range(void *start, size_t size,
> 				  enum pio_data_direction dir)
> {
> 	return start;
> }
> 
> static inline void pio_unmap_range(void *start, size_t size,
> 				   enum pio_data_direction dir)
> {
> }

I think really for range PIO, we need helpers to map and unmap a page at
a time ... sort of like the for_each_sg approach except this time we
loop over the pages in the range mapping and unmapping a single one.

James

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[James Bottomley]

On Mon, 2010-02-08 at 16:54 +0000, Russell King wrote:
> On Mon, Feb 08, 2010 at 04:10:21PM +0000, Catalin Marinas wrote:
> > The pio_data_direction could be dropped and use the DMA one. We could
> > also use pio_kmap_read/pio_kmap_write or similar but we have to triple
> > the number of functions, so I prefer the additional argument.
> 
> Do we need to do anything for reading a buffer for PIO _out_ to the
> device?  My understanding is that this has never been a problem.

Before outbound PIO, you need to flush all the aliases before sending.
However, this should *already* be done in the block path, so for block
I/O it should be unnecessary, yes.  All we need to do on the TO_DEVICE
case should be map and unmap the page if it's highmem.

> The only problem I'm aware of is where PIO writes to the kernel
> mapping of a lowmem pages; highmem pages need the data flushed out
> of the temporary atomic kmap mapping anyway.

Not quite.  in the PIO FROM_DEVICE case, we've created a dirty kernel
alias by reading the data from the device and writing it via the kernel
mapping.  We have to flush that alias whether it exists as a highmem
mapping or as a simple offset mapping before userspace will see the
data.  The block API assumes the FROM_DEVICE transfer went straight from
the device into main memory and didn't go via the kernel alias, so the
block use case won't flush it.

Additionally, for architectures that don't promise no movein on a
flushed but untouched page (unlike parisc, which does), we might need to
invalidate all the user aliases before passing the page back to kill any
stale data that may have been speculated into the alias caches).

James

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Russell King]

On Mon, Feb 08, 2010 at 11:20:49AM -0600, James Bottomley wrote:
> On Mon, 2010-02-08 at 16:54 +0000, Russell King wrote:
> > On Mon, Feb 08, 2010 at 04:10:21PM +0000, Catalin Marinas wrote:
> > > The pio_data_direction could be dropped and use the DMA one. We could
> > > also use pio_kmap_read/pio_kmap_write or similar but we have to triple
> > > the number of functions, so I prefer the additional argument.
> > 
> > Do we need to do anything for reading a buffer for PIO _out_ to the
> > device?  My understanding is that this has never been a problem.
> 
> Before outbound PIO, you need to flush all the aliases before sending.
> However, this should *already* be done in the block path, so for block
> I/O it should be unnecessary, yes.  All we need to do on the TO_DEVICE
> case should be map and unmap the page if it's highmem.
> 
> > The only problem I'm aware of is where PIO writes to the kernel
> > mapping of a lowmem pages; highmem pages need the data flushed out
> > of the temporary atomic kmap mapping anyway.
> 
> Not quite.

Yes quite; I disagree.

>  in the PIO FROM_DEVICE case, we've created a dirty kernel
> alias by reading the data from the device and writing it via the kernel
> mapping.

Here, I agree 100%.

>  We have to flush that alias whether it exists as a highmem
> mapping or as a simple offset mapping before userspace will see the
> data.

And this is where things get more complicated.  If you have a CPU
where aliases exist for different virtual addresses, and you have
highmem, when you unmap the highmem mapping, you have to flush the
dirty mapping.

That flush is there today.

The problem case is where you have a lowmem page cache page which
you're writing into - this is not kmapped, and so on kunmap*, there
is no flush.

>  The block API assumes the FROM_DEVICE transfer went straight from
> the device into main memory and didn't go via the kernel alias, so the
> block use case won't flush it.

50% correct.  As I point out above, with an aliasing cache + highmem,
kunmap has to already do cache maintainence.  To do additional
cache maintainence would be a pure duplication of what kunmap is
already doing.

> Additionally, for architectures that don't promise no movein on a
> flushed but untouched page (unlike parisc, which does), we might need to
> invalidate all the user aliases before passing the page back to kill any
> stale data that may have been speculated into the alias caches).

For a read-in, there should be zero user mappings of that page.

Do we really allow users to map a page before it's been written with
the corresponding data from disk?  What if it previously contained the
data from the passwd or login program?  I'd be a nice security hole if
we did allow this to happen.

For a user mapping to exist, we receive a fault.  If we don't have the
data in the page cache, we allocate a page and issue a request into the
fs layers to bring the data in from a block device, and wait for the
page to be up to date.  Only when it is up to date do we map the page
into userspace.

It is my understanding that the situation that you describe can never
occur.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIOmapping API

[Catalin Marinas]

On Mon, 2010-02-08 at 16:54 +0000, Russell King wrote:
> On Mon, Feb 08, 2010 at 04:10:21PM +0000, Catalin Marinas wrote:
> > The pio_data_direction could be dropped and use the DMA one. We could
> > also use pio_kmap_read/pio_kmap_write or similar but we have to triple
> > the number of functions, so I prefer the additional argument.
> 
> Do we need to do anything for reading a buffer for PIO _out_ to the
> device?  My understanding is that this has never been a problem.

With PIPT caches there is no need for any flushing on the TO_DEVICE case
but VIPT may need this. As James said, it should already be handled in
the block path (though I haven't found where exactly).

But I added the functions for symmetry anyway.

> The only problem I'm aware of is where PIO writes to the kernel
> mapping of a lowmem pages; highmem pages need the data flushed out
> of the temporary atomic kmap mapping anyway.

If we do cache flushing in kunmap() we wouldn't need an additional cache
flushing for highmem pages, so you can implement pio_kunmap()
accordingly. James' point in a previous e-mail was that he doesn't want
drivers to have "if (!PageHighMem(page))".

Now, the ARM kunmap() function doesn't seem to do any flushing, only
kunmap_atomic(). Do I miss anything?

Also, for PIPT caches we don't actually create any D-cache aliases with
kmap, so I'm not sure why we need to always flush the cache in kunmap().

-- 
Catalin

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[James Bottomley]

On Mon, 2010-02-08 at 17:33 +0000, Russell King wrote:
> On Mon, Feb 08, 2010 at 11:20:49AM -0600, James Bottomley wrote:
> > On Mon, 2010-02-08 at 16:54 +0000, Russell King wrote:
> > > On Mon, Feb 08, 2010 at 04:10:21PM +0000, Catalin Marinas wrote:
> > > > The pio_data_direction could be dropped and use the DMA one. We could
> > > > also use pio_kmap_read/pio_kmap_write or similar but we have to triple
> > > > the number of functions, so I prefer the additional argument.
> > > 
> > > Do we need to do anything for reading a buffer for PIO _out_ to the
> > > device?  My understanding is that this has never been a problem.
> > 
> > Before outbound PIO, you need to flush all the aliases before sending.
> > However, this should *already* be done in the block path, so for block
> > I/O it should be unnecessary, yes.  All we need to do on the TO_DEVICE
> > case should be map and unmap the page if it's highmem.
> > 
> > > The only problem I'm aware of is where PIO writes to the kernel
> > > mapping of a lowmem pages; highmem pages need the data flushed out
> > > of the temporary atomic kmap mapping anyway.
> > 
> > Not quite.
> 
> Yes quite; I disagree.
> 
> >  in the PIO FROM_DEVICE case, we've created a dirty kernel
> > alias by reading the data from the device and writing it via the kernel
> > mapping.
> 
> Here, I agree 100%.
> 
> >  We have to flush that alias whether it exists as a highmem
> > mapping or as a simple offset mapping before userspace will see the
> > data.
> 
> And this is where things get more complicated.  If you have a CPU
> where aliases exist for different virtual addresses, and you have
> highmem, when you unmap the highmem mapping, you have to flush the
> dirty mapping.
> 
> That flush is there today.

Right, but only in the highmem case.

> The problem case is where you have a lowmem page cache page which
> you're writing into - this is not kmapped, and so on kunmap*, there
> is no flush.
> 
> >  The block API assumes the FROM_DEVICE transfer went straight from
> > the device into main memory and didn't go via the kernel alias, so the
> > block use case won't flush it.
> 
> 50% correct.  As I point out above, with an aliasing cache + highmem,
> kunmap has to already do cache maintainence.  To do additional
> cache maintainence would be a pure duplication of what kunmap is
> already doing.

Which is why the API is a proposed combination of pio+kmap ... for
highmem, in arch code, if you already do the flush it would be a pure
kmap.  For lowmem it would do the flush on unmap for FROM_DEVICE
transfers.

> > Additionally, for architectures that don't promise no movein on a
> > flushed but untouched page (unlike parisc, which does), we might need to
> > invalidate all the user aliases before passing the page back to kill any
> > stale data that may have been speculated into the alias caches).
> 
> For a read-in, there should be zero user mappings of that page.

Not necessarily.  For a faulted read in, yes.  If we're doing SG_IO on a
user page, the page will already exist in the user's space.  There are
other read paradigms that also do overwrite reads.

> Do we really allow users to map a page before it's been written with
> the corresponding data from disk?  What if it previously contained the
> data from the passwd or login program?  I'd be a nice security hole if
> we did allow this to happen.

Not map, no ... but if we're doing I/O to existing user areas, then yes.

> For a user mapping to exist, we receive a fault.  If we don't have the
> data in the page cache, we allocate a page and issue a request into the
> fs layers to bring the data in from a block device, and wait for the
> page to be up to date.  Only when it is up to date do we map the page
> into userspace.

Not necessarily, this doesn't happen for DIO reads which bypass the page
cache ... or for SG_IO ones which are essentially commands sent external
to the whole of the page/file cache system.

> It is my understanding that the situation that you describe can never
> occur.

James

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIOmapping API

[James Bottomley]

On Mon, 2010-02-08 at 18:02 +0000, Catalin Marinas wrote:
> If we do cache flushing in kunmap() we wouldn't need an additional cache
> flushing for highmem pages, so you can implement pio_kunmap()
> accordingly. James' point in a previous e-mail was that he doesn't want
> drivers to have "if (!PageHighMem(page))".
> 
> Now, the ARM kunmap() function doesn't seem to do any flushing, only
> kunmap_atomic(). Do I miss anything?
> 
> Also, for PIPT caches we don't actually create any D-cache aliases with
> kmap, so I'm not sure why we need to always flush the cache in kunmap().

Right ... this is why the implementation is arch specific.  The PIPT
implementation would be a nop for lowmem and directly equivalent to the
corresponding kmap for highmem (no flushes ... unless there are platform
semantics requiring them because of the mappings that are in the kmaps
anyway).

James

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Catalin Marinas]

On Mon, 2010-02-08 at 17:14 +0000, James Bottomley wrote:
> On Mon, 2010-02-08 at 16:10 +0000, Catalin Marinas wrote:
> 
> This API would have to cope with highmem too ... and because of the
> limited mappings available, we can't just map an arbitrary sized buffer
> (especially in atomics where the number of available slots is often only
> two).
> 
> > static inline void *pio_map_range(void *start, size_t size,
> >                                 enum pio_data_direction dir)
> > {
> >       return start;
> > }
> >
> > static inline void pio_unmap_range(void *start, size_t size,
> >                                  enum pio_data_direction dir)
> > {
> > }
> 
> I think really for range PIO, we need helpers to map and unmap a page at
> a time ... sort of like the for_each_sg approach except this time we
> loop over the pages in the range mapping and unmapping a single one.

These were not intended to create any additional mapping, more like the
dma_map_single() functions and only do the necessary flushing.

AFAICT, an HCD driver would call the pio_map_range() once to get the
address of the buffer and than wait for it to be filled in (e.g. via
interrupts). When it is full, it would call pio_unmap_range(). So even
if we do it per page, you still need to map the same amount of memory,
unless we modify the HCD drivers but I don't think this would be very
popular.

Maybe the name choosing is wrong. Would something like pio_begin() and
pio_end() work better for already mapped buffers?

-- 
Catalin

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Benjamin Herrenschmidt]

On Tue, 2010-02-09 at 18:03 +0000, Catalin Marinas wrote:
> 
> These were not intended to create any additional mapping, more like
> the
> dma_map_single() functions and only do the necessary flushing.
> 
> AFAICT, an HCD driver would call the pio_map_range() once to get the
> address of the buffer and than wait for it to be filled in (e.g. via
> interrupts). When it is full, it would call pio_unmap_range(). So even
> if we do it per page, you still need to map the same amount of memory,
> unless we modify the HCD drivers but I don't think this would be very
> popular.
> 
> Maybe the name choosing is wrong. Would something like pio_begin() and
> pio_end() work better for already mapped buffers? 

From your initial email, your problem isn't a VIVT cache aliasing but an
I$/D$ cache coherency problem with PIPT right ?

In that case, I would recommend you look at how this is already dealt
with on existing archs such as powerpc, using PG_arch1 in struct page to
keep track of whether a given page is clean for execution and mapping
pages that aren't non-exec so the kernel gets a chance to clean them
once when execution happens.

We have some faster path nowadays to do that on the first fault which
you take anyways (at least most of the time) at set_pte() time to avoid
double faults so the cost is minimum I believe.

Cheers,
Ben.

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Russell King]

On Wed, Feb 17, 2010 at 08:11:35PM +1100, Benjamin Herrenschmidt wrote:
> In that case, I would recommend you look at how this is already dealt
> with on existing archs such as powerpc, using PG_arch1 in struct page to
> keep track of whether a given page is clean for execution and mapping
> pages that aren't non-exec so the kernel gets a chance to clean them
> once when execution happens.

That would be fine if we weren't already using PG_arch_1 for delaying
D-cache flushes for pages which aren't mapped, in the same way that
Sparc64 does.

That doesn't cover this case though - the problem is not I/D cache
coherency - the problem has manifested itself as data corruption when
userspace reads the page.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [RFC PATCH 2/4] pio-mapping: Add ARM support for the PIO mapping API

[Benjamin Herrenschmidt]

On Wed, 2010-02-17 at 20:04 +0000, Russell King wrote:
> 
> That would be fine if we weren't already using PG_arch_1 for delaying
> D-cache flushes for pages which aren't mapped, in the same way that
> Sparc64 does.

Interesting. Maybe we need more arch bits in there :-)

> That doesn't cover this case though - the problem is not I/D cache
> coherency - the problem has manifested itself as data corruption when
> userspace reads the page.

Yup, let's stick to the other thread. I still don't see why a PIO
mapping API is needed, from what I've read so far, it's a case of
mis-using the existing DMA API that could be fixed at the driver level.
No ?

Cheers,
Ben.