Re: [PATCH v3 1/8] public / x86: Introduce __HYPERCALL_dm_op...

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
[snip]
> > +
> > +struct xen_dm_op_buf {
> > +    XEN_GUEST_HANDLE_64(void) h;
> > +    uint32_t size;
> > +};
> 
> Sorry to quibble, but there is a problem here which has only just
> occurred to me.  This ABI isn't futureproof, and has padding at the end
> which affects how the array is layed out.
> 
> The userspace side should be
> 
> struct xen_dm_op_buf {
>     void *h;
>     size_t size;
> }
> 
> which will work sensibly for 32bit and 64bit userspace, and futureproof
> (for when 128bit turns up).  Its size is also a power of two which
> avoids alignment issues in the array.
> 
> The kernel already has to parse this structure anyway, and will know the
> bitness of its userspace process.  We could easily (at this point)
> require the kernel to turn it into the kernels bitness for forwarding on
> to Xen, which covers the 32bit userspace under a 64bit kernel problem,
> in a way which won't break the hypercall ABI when 128bit comes along.
> 

As discussed...

xen_dm_op_buf is currently used directly in the tools code because there is no explicit support for DMOPs in privcmd. When explicit support is added then this can change and we can use a void ptr and size_t as you say.

For the privcmd -> Xen interface that using XEN_GUEST_HANDLE_64 does indeed limit us and so using XEN_GUEST_HANDLE would indeed seem more sensible (and we won't need a 32-bit compat wrapper for DMOP because it's a tools-only hypercall).

Let's see what Jan thinks.

  Paul

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel