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

[Aaron Sierra]

----- Original Message -----
> From: "Greg Kroah-Hartman" <gregkh at linuxfoundation.org>
> Sent: Tuesday, December 3, 2013 1:07:51 PM
> 

[snip]

> >  
> > 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.

Ok, that's easy to fix. Just curious, is this more than just a style
convention? The MPC512x DMA driver puts the __packed__ attribute at the same
location as I did. More importantly, GCC did the right thing with the
structure from my perspective.

> 
> > +	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?

The strong types and __packed__ attribute provided consistent results for
sizeof(struct vme_master) and sizeof(struct vme_slave). I performed testing
in the following environments:

x86: 32-bit userspace, 32-bit kernel
x86: 32-bit userspace, 64-bit kernel
x86: 64-bit userspace, 64-bit kernel
ppc: 32-bit userspace, 32-bit kernel

> 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.

I considered rearranging the structure to place 64-bit values first, but I
thought including the __packed__ attribute would help remind developers that
there are compiler issues that need to be considered when dealing with the
userspace API and at the same time only require developers to respect the
need for strong types.

-Aaron