Date: Thu, 16 Mar 2017 17:20:12 +0900
From: Greg KH <gregkh@linuxfoundation.org>
To: Michal Sojka <sojkam1@fel.cvut.cz>
Cc: linux-kernel@vger.kernel.org, lisovy@gmail.com
Subject: Re: [PATCH 1/3] uio: Allow handling of non page-aligned memory
 regions
Message-ID: <20170316082012.GA23493@kroah.com>
References: <20170307140948.30099-1-sojkam1@fel.cvut.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170307140948.30099-1-sojkam1@fel.cvut.cz>
User-Agent: Mutt/1.8.0 (2017-02-23)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2975
Lines: 77

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@fel.cvut.cz>
> ---
>  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