Re: [PATCH 2/3] vme_user: Update API to work in mixed environments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 31, 2013 at 06:47:54PM -0500, Aaron Sierra wrote:
> This patch updates the vme_master and vme_slave structures to use
> types with well defined size and to prevent the compiler from
> inserting padding (between enable and vme_addr for one).
> 
> The original vme_master and vme_slave structs would be different
> sizes and have different layouts depending on whether they were built
> for a 32-bit or 64-bit system.
> 
> On x86 it is possible to have a 32-bit userspace and a 64-bit kernel.
> In this type of environment, the userspace and kernel vme_user APIs
> would disagree and prevent ioctls from executing (based on ioctl
> signatures from _IOR and _IOW).
> 
> Signed-off-by: Aaron Sierra <asierra@xxxxxxxxxxx>
> ---
>  drivers/staging/vme/devices/vme_user.c |    1 +
>  drivers/staging/vme/devices/vme_user.h |   26 +++++++++++++-------------
>  2 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
> index da7f759..e69f0cd 100644
> --- a/drivers/staging/vme/devices/vme_user.c
> +++ b/drivers/staging/vme/devices/vme_user.c
> @@ -147,6 +147,7 @@ static const struct file_operations vme_user_fops = {
>  	.write = vme_user_write,
>  	.llseek = vme_user_llseek,
>  	.unlocked_ioctl = vme_user_unlocked_ioctl,
> +	.compat_ioctl = vme_user_unlocked_ioctl,
>  };
>  
>  
> diff --git a/drivers/staging/vme/devices/vme_user.h b/drivers/staging/vme/devices/vme_user.h
> index 7d24cd6..6726a42 100644
> --- a/drivers/staging/vme/devices/vme_user.h
> +++ b/drivers/staging/vme/devices/vme_user.h
> @@ -6,13 +6,13 @@
>  /*
>   * VMEbus Master Window Configuration Structure
>   */
> -struct vme_master {
> -	int enable;			/* State of Window */
> -	unsigned long long vme_addr;	/* Starting Address on the VMEbus */
> -	unsigned long long size;	/* Window Size */
> -	u32 aspace;			/* Address Space */
> -	u32 cycle;		/* Cycle properties */
> -	u32 dwidth;		/* Maximum Data Width */
> +struct __attribute__((__packed__)) vme_master {

The attribute goes at the end of the structure, not the beginning.

> +	u32 enable;	/* State of Window */
> +	u64 vme_addr;	/* Starting Address on the VMEbus */
> +	u64 size;	/* Window Size */
> +	u32 aspace;	/* Address Space */
> +	u32 cycle;	/* Cycle properties */
> +	u32 dwidth;	/* Maximum Data Width */

And that's really all that is needed?  Did you test this out properly?

I don't think this will work due to packing issues, please rearrange the
structure to not need "packed" and then it should be ok.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux