lkml.org 
[lkml]   [2008]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] [UIO] Add alignment warnings for uio-mem
Hello Hans,

On Thu, Sep 18, 2008 at 11:53:18PM +0200, Hans J. Koch wrote:

> With your approach, sysfs would report a base address of 0x12345000 and
> a size of 0x90. Both is a lie. We don't want to encourage the user to
> access addresses outside the 16 bytes range the driver author originally
> announced.

True. It's actually quite dangerous to be able to write outside that
region at all, but I guess this can't be helped due to the nature of
mmap.

> UIO drivers are often used in embedded devices, where developers usually
> know the physical addresses of their devices and have them hardcoded in
> a #define. It's confusing if sysfs suddenly reports something different.
>
> The userspace part of the driver expects a 16 bytes range but is told
> there are 0x90 bytes at his disposal. It has to guess where the devices
> registers are (or if this is a completely different device...). It might
> also check the physical base address and find that this is wrong, too.
>
> The situation becomes even worse if you have two such chips on your
> board, and each reports a different size. If their addresses were in the
> same page, both would have the same base address, but different sizes...

ACK.

> Let's leave it to userspace. All userspace needs is the "offset"
> information you calculate in your patch. If we add a new sysfs attribute
> for that, userspace can simply add it to the address returned by mmap().
> This way, the userspace part of the driver will always work, no matter
> if the memory is page aligned or not. The "addr" and "size" attributes
> still report the true physical base address and size. _Every_ userspace
> driver, existing or yet to be written, can (and should) simply do
> something like
>
> base_addr = address_returned_by_mmap + offset_from_sysfs;
>
> What do you think about this? If you think this might work for you,
> could you please test the patch below? I haven't got such a hardware
> handy at the moment, so my patch is just compile-tested (but it looks
> obvious).

I'm perfectly fine with this approach. I tested it and it worked as
expected. The only thing I'd like to add is that 'offset' could be
renamed to 'map_offset' or 'mmap_offset' to be a bit more precise what
this value is about.

> Thanks again for pointing this out,

You're welcome. Thanks for dealing with the issue.

All the best,

Wolfram

--
Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2008-09-19 10:51    [W:0.070 / U:0.948 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site