Re: [PATCH 1/3] uio: Allow handling of non page-aligned memory regions

From: Greg KH
Date: Thu Mar 16 2017 - 04:28:49 EST


On Tue, Mar 07, 2017 at 03:09:46PM +0100, Michal Sojka wrote:
> Since commit b65502879556 ("uio: we cannot mmap unaligned page
> contents") addresses and sizes of UIO memory regions must be
> page-aligned. If the address in the BAR register is not page-aligned,
> the mentioned commit forces the UIO driver to round the address down
> to the page size. Then, there is no easy way for user-space to learn
> the offset of the actual memory region within the page, because the
> offset seen in the sysfs is calculated from the rounded address and
> thus it is always zero.
>
> Fix that problem by including the offset in struct uio_mem. UIO
> drivers can set this field and its value is reported via sysfs.

It is, where?

>
> Drivers for hardware with page-aligned BARs need not to be modified
> provided that they initialize struct uio_info with zeros.
>
> Signed-off-by: Michal Sojka <sojkam1@xxxxxxxxxxx>
> ---
> drivers/uio/uio.c | 2 +-
> include/linux/uio_driver.h | 13 ++++++++-----
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index fba021f5736a..27c329131350 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -66,7 +66,7 @@ static ssize_t map_size_show(struct uio_mem *mem, char *buf)
>
> static ssize_t map_offset_show(struct uio_mem *mem, char *buf)
> {
> - return sprintf(buf, "0x%llx\n", (unsigned long long)mem->addr & ~PAGE_MASK);
> + return sprintf(buf, "0x%llx\n", (unsigned long long)mem->offs);

All you changed was this value that sysfs shows.

> }
>
> struct map_sysfs_entry {
> diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
> index 32c0e83d6239..89b53094f127 100644
> --- a/include/linux/uio_driver.h
> +++ b/include/linux/uio_driver.h
> @@ -23,11 +23,13 @@ struct uio_map;
> /**
> * struct uio_mem - description of a UIO memory region
> * @name: name of the memory region for identification
> - * @addr: address of the device's memory (phys_addr is used since
> - * addr can be logical, virtual, or physical & phys_addr_t
> - * should always be large enough to handle any of the
> - * address types)
> - * @size: size of IO
> + * @addr: address of the device's memory rounded to page
> + * size (phys_addr is used since addr can be
> + * logical, virtual, or physical & phys_addr_t
> + * should always be large enough to handle any of
> + * the address types)
> + * @offs: offset of device memory within the page
> + * @size: size of IO (multiple of page size)
> * @memtype: type of memory addr points to
> * @internal_addr: ioremap-ped version of addr, for driver internal use
> * @map: for use by the UIO core only.
> @@ -35,6 +37,7 @@ struct uio_map;
> struct uio_mem {
> const char *name;
> phys_addr_t addr;
> + unsigned long offs;;

Did you really test this patch? Why the two ;;? And who sets this?

I think you broke things :(

thanks,

greg k-h