From mboxrd@z Thu Jan  1 00:00:00 1970
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
Date: Fri, 22 Jan 2016 23:57:04 +0000
Subject: Unhandled fault: page domain fault (0x81b) at 0x00e41008
In-Reply-To: <56A2B811.8010306@free.fr>
References: <56A268E7.7040004@free.fr>
 <20160122174814.GD19062@n2100.arm.linux.org.uk>
 <56A27C00.5060004@free.fr>
 <20160122193408.GE19062@n2100.arm.linux.org.uk>
 <56A2B811.8010306@free.fr>
Message-ID: <20160122235704.GF19062@n2100.arm.linux.org.uk>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

On Sat, Jan 23, 2016 at 12:15:29AM +0100, Mason wrote:
> On 22/01/2016 20:34, Russell King - ARM Linux wrote:
> > What do these "block_copy8, block_copy16, block_copy32" functions
> > do?  Does the "8", "16" and "32" refer to the access size?  Why do
> > they need to make accesses to userspace using these specific sizes?
> > What causes this restriction?
> 
> Interfaces are somewhat arbitrary. The problem statement
> was as follows.
> 
> Implement functions for copying a range of addresses
> FROM user-space, TO physical addresses,
> (and also the other way around)
> in access size of 8, 16, 32 bits.
> 
> So, a little over a decade ago, someone decided that these
> functions would have the following prototype:
> 
> int read_data8  (u8  *user_addr, u8  *phys_addr, int count)
> int read_data16 (u16 *user_addr, u16 *phys_addr, int count)
> int read_data32 (u32 *user_addr, u32 *phys_addr, int count)
> 
> int write_data8 (u8  *user_addr, u8  *phys_addr, int count)
> int write_data16(u16 *user_addr, u16 *phys_addr, int count)
> int write_data32(u32 *user_addr, u32 *phys_addr, int count)

Of course, physical addresses are _integers_ and not pointers... (I can't
help but say that because every time I see that mistake, I'm duty bound
to educate to prevent anyone thinking this kind of thing is a good idea.)

> (Note: the following code is simplified, as count may be
> larger than vmalloc space, so the operation needs to be
> "chunked" or "tiled".)
> 
> int read_data8 (u8 *user_addr, u8 *phys_addr, int count)
> {
>   int i, err = 0;
> 
>   /* map phys_addr into kernel VA */
>   void *va = ioremap(phys_addr, count);
>   if (va == NULL) return some_error;
> 
>   for (i = 0; i < count; ++i) {
>     u8 val = readb(va + i);
>     err = put_user(val, user_addr + i);
>     if (err) break;
>   }
> 
>   iounmap(va);
>   return err;
> }
> 
> Is this what you are suggesting?
> 
> I expect this to be quite slow.

That probably will be quite slow.  How about this instead:

int read_data8(u8 __user *user_addr, phys_addr_t phys_addr, size_t bytes)
{
	const size_t batch_size = PAGE_SIZE;
	void __user *user_pos = user_addr;
	void __iomem *va;
	size_t i, j;
	u8 *buf;
	int err;

	va = ioremap(phys_addr, bytes);
	buf = kmalloc(batch_size);
	if (!va || !buf) {
		iounmap(va);
		kfree(buf);
		return -ENOMEM;
	}

	for (i = 0; i < bytes; i += batch_size) {
		size_t len = bytes - i;
		if (len > batch_size)
			len = batch_size;

		for (j = 0; j < len; j += sizeof(*buf))
			buf[j / sizeof(*buf)] = readb_relaxed(va + i + j);

		if (copy_to_user(user_pos, buf, len)) {
			err = -EFAULT;
			break;
		}

		user_pos += len;
	}

	iounmap(va);
	kfree(buf);
	return err;
}

You can change the batch size by altering the "batch_size" variable,
though I suspect you'll find that the above may be reasonably fast.

You should only need to change the "u8" data types and the iomem
accessor for your other read functions.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up
according to speedtest.net.