All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.