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

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

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 12 January 2017 16:29
> To: Andrew Cooper <Andrew.Cooper3@citrix.com>; Paul Durrant
> <Paul.Durrant@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@citrix.com>; Jennifer Herbert
> <jennifer.herbert@citrix.com>; Wei Liu <wei.liu2@citrix.com>; xen-
> devel@lists.xenproject.org; Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Subject: RE: [PATCH v3 1/8] public / x86: Introduce __HYPERCALL_dm_op...
> 
> >>> On 12.01.17 at 17:10, <Paul.Durrant@citrix.com> wrote:
> >> > +
> >> > +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.
> 
> Yes, padding needs to be added.
> 
> >> 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.
> 
> But that won't cover a 32-bit kernel.
>

Do we need to care about a 32-bit kernel for a tools-only hypercall? I thought a toolstack already had to be (at least) 64-bit to match Xen.

  Paul
 
> And I'm not sure we really need to bother considering hypothetical
> 128-bit architectures at this point in time.
> 
> > 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).
> 
> Just like with other tools only interfaces, I think this should remain a
> 64-bit handle, to avoid (as you say, but wrongly in conjunction with a
> "normal" handle) the need for a compat wrapper.
>
> Jan


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