linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: Unhandled fault: page domain fault (0x81b) at 0x00e41008
Date: Fri, 22 Jan 2016 23:57:04 +0000	[thread overview]
Message-ID: <20160122235704.GF19062@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <56A2B811.8010306@free.fr>

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.

  reply	other threads:[~2016-01-22 23:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-22 17:37 Unhandled fault: page domain fault (0x81b) at 0x00e41008 Mason
2016-01-22 17:48 ` Russell King - ARM Linux
2016-01-22 18:59   ` Mason
2016-01-22 19:34     ` Russell King - ARM Linux
2016-01-22 23:15       ` Mason
2016-01-22 23:57         ` Russell King - ARM Linux [this message]
2016-01-23 11:14           ` Mason
2016-01-23 11:34             ` Russell King - ARM Linux
2016-01-23 20:53               ` Mason
2016-01-23 22:46                 ` Mason
2016-01-23 23:59                 ` Russell King - ARM Linux
2016-01-24 13:27                   ` Mason
2016-01-27 10:36                     ` Mason
2016-01-27 10:48                       ` Russell King - ARM Linux
2016-01-27 12:04                         ` Mason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160122235704.GF19062@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).