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: Wed, 27 Jan 2016 10:48:12 +0000 [thread overview]
Message-ID: <20160127104811.GX10826@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <56A89DC3.8010309@free.fr>
On Wed, Jan 27, 2016 at 11:36:51AM +0100, Mason wrote:
> On 24/01/2016 14:27, Mason wrote:
>
> > Our "software stack" provides the kernel API under discussion:
> >
> > {read,write}_data{8,16,32}
> >
> > These 6 functions must be implemented, because they are part
> > of the API we provide to customers. As the "page domain fault"
> > underscores, my own implementation is incorrect. I am grateful
> > for the implementation you suggested up-thread, and will test
> > its performance on Monday.
>
> For the record, I've now changed the implementation as follows.
> I'll benchmark performance as soon as I fix the other bug in
> the module.
>
> #define DEFINE_BLOCK_READ(N) \
> static int read##N(void __user *dest, void *buf, void __iomem *io, size_t len) \
> { \
> size_t i; u##N *temp = buf; \
> for (i = 0; i < len; i += N/8) temp[i] = RD##N(io + i); \
> return copy_to_user(dest, temp, len) ? -EFAULT : 0; \
> }
>
> #define RD8 readb_relaxed
> #define RD16 readw_relaxed
> #define RD32 readl_relaxed
>
> DEFINE_BLOCK_READ(8)
> DEFINE_BLOCK_READ(16)
> DEFINE_BLOCK_READ(32)
>
> #define DEFINE_BLOCK_WRITE(N) \
> static int write##N(void __user *src, void *buf, void __iomem *io, size_t len) \
> { \
> size_t i; u##N *temp = buf; \
> if (copy_from_user(temp, src, len)) return -EFAULT; \
> for (i = 0; i < len; i += N/8) WR##N(temp[i], io + i); \
> return 0; \
> }
>
> #define WR8 writeb_relaxed
> #define WR16 writew_relaxed
> #define WR32 writel_relaxed
>
> DEFINE_BLOCK_WRITE(8)
> DEFINE_BLOCK_WRITE(16)
> DEFINE_BLOCK_WRITE(32)
>
> #define TILE_SIZE (16u << 10)
> typedef int fun_t(void __user *ua, void *buf, void __iomem *io, size_t len);
>
> static int block_copy(void __user *ua, phys_addr_t pa, size_t bytes, fun_t *fun)
> {
> int err = 0;
> size_t pos = 0;
> void *buf = kmalloc(TILE_SIZE, GFP_KERNEL);
> if (buf == NULL) err = -ENOMEM;
>
> while (pos < bytes && !err)
> {
> size_t tile = min(bytes-pos, TILE_SIZE);
> void __iomem *va = ioremap(pa + pos, tile);
>
> err = va ? fun(ua + pos, buf, va, tile) : -EFAULT;
> iounmap(va);
> pos += tile;
> }
>
> kfree(buf);
> return err;
> }
>
>
> and then the ioctl dispatcher calls e.g.
> block_copy(user_addr, phys_addr, count*4, read32);
>
>
> IIUC, Arnd mentioned that there might be an issue using readl_relaxed
> on a memory region with a big-endian kernel.
I think you're confused there, or Arnd's comment was incorrect.
In any case, I'm even more pissed off with you. Let me quote from
your earlier emails:
> The problem is that the kernel module's API is already set
> in stone, and it requires block copies with specific access
> sizes, e.g. block_copy8, block_copy16, block_copy32.
and:
> 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)
However, you've done away with those prototypes, and instead come up
with something that looks like:
int block_copy(void __user *ua, phys_addr_t pa, size_t bytes, fun_t *fun)
?
So, let me get this straight. You demand that the API can't be changed,
and I provide you with a solution which results in very little change to
the API.
However, rather than testing the version I carefully created for you,
you've decided that you're not going to, instead coming up with your
own solution which breaks all your previous demands.
This is totally rediculous. Why should I waste any more time with _any_
of your questions?
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
next prev parent reply other threads:[~2016-01-27 10:48 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
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 [this message]
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=20160127104811.GX10826@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).