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

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

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 20 January 2017 16:18
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; 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 v4 1/8] public / x86: Introduce __HYPERCALL_dm_op...
> 
> >>> On 17.01.17 at 18:29, <paul.durrant@citrix.com> wrote:
> > +static bool copy_buf_from_guest(xen_dm_op_buf_t bufs[],
> > +                                unsigned int nr_bufs, void *dst,
> > +                                unsigned int idx, size_t dst_size)
> > +{
> > +    size_t size = min_t(size_t, dst_size, bufs[idx].size);
> > +
> > +    return !copy_from_guest(dst, bufs[idx].h, size);
> > +}
> > +
> > +static bool copy_buf_to_guest(xen_dm_op_buf_t bufs[],
> > +                              unsigned int nr_bufs, unsigned int idx,
> > +                              void *src, size_t src_size)
> > +{
> > +    size_t size = min_t(size_t, bufs[idx].size, src_size);
> > +
> > +    return !copy_to_guest(bufs[idx].h, src, size);
> > +}
> 
> Wouldn't it be better to require an exact input size here? The guest
> providing a different amount is likely to indicate some version
> mismatch, build issue, or what not.
> 

Ok.

> > +#ifndef __XEN_PUBLIC_HVM_DM_OP_H__
> > +#define __XEN_PUBLIC_HVM_DM_OP_H__
> > +
> > +#if defined(__XEN__) || defined(__XEN_TOOLS__)
> > +
> > +#include "../xen.h"
> > +
> > +#define XEN_DMOP_invalid 0
> 
> Do we actually need this, btw?
> 

Not really, I just prefer to have 0 be an invalid sub-op and #defining it this way makes that obvious.

> > +struct xen_dm_op {
> > +    uint32_t op;
> > +};
> > +
> > +struct xen_dm_op_buf {
> > +    XEN_GUEST_HANDLE(void) h;
> > +    unsigned long size;
> 
> xen_ulong_t?
> 

If that's preferable.

> > --- a/xen/include/xlat.lst
> > +++ b/xen/include/xlat.lst
> > @@ -129,3 +129,4 @@
> >  ?	flask_setenforce		xsm/flask_op.h
> >  !	flask_sid_context		xsm/flask_op.h
> >  ?	flask_transition		xsm/flask_op.h
> > +!	dm_op_buf			hvm/dm_op.h
> 
> Please don't break the (mostly) sorted sequence here (sorting is
> done by header name first, for - I hope - obvious reasons).
> 

Ok.

> Jan


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