Re: [Xen-devel] [PATCH v3 08/13] x86: add iommu_op to query reserved ranges

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> Sent: 19 July 2018 10:37
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Jan Beulich <jbeulich@xxxxxxxx>;
> Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Konrad
> Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>
> Subject: Re: [PATCH v3 08/13] x86: add iommu_op to query reserved ranges
> 
> On Tue, Jul 17, 2018 at 02:38:11PM +0100, Paul Durrant wrote:
> [...]
> >  int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
> >  {
> > -    compat_iommu_op_t cmp;
> > +    compat_iommu_op_t cmp = {};
> > +    size_t offset;
> > +    static const size_t op_size[] = {
> > +        [XEN_IOMMUOP_query_reserved] = sizeof(struct
> compat_iommu_op_query_reserved),
> > +    };
> > +    size_t size;
> >      xen_iommu_op_t nat;
> > +    unsigned int u;
> > +    int32_t status;
> >      int rc;
> >
> > -    if ( buf->size < sizeof(cmp) )
> > +    offset = offsetof(struct compat_iommu_op, u);
> > +
> > +    if ( buf->size < offset )
> >          return -EFAULT;
> >
> > -    if ( copy_from_compat((void *)&cmp, buf->h, sizeof(cmp)) )
> > +    if ( copy_from_compat((void *)&cmp, buf->h, offset) )
> >          return -EFAULT;
> >
> >      if ( cmp.pad )
> > @@ -119,12 +204,85 @@ int
> compat_one_iommu_op(compat_iommu_op_buf_t *buf)
> >      if ( rc )
> >          return rc;
> >
> > +    if ( cmp.op >= ARRAY_SIZE(op_size) )
> > +        return -EOPNOTSUPP;
> > +
> > +    size = op_size[array_index_nospec(cmp.op, ARRAY_SIZE(op_size))];
> > +    if ( buf->size < offset + size )
> > +        return -EFAULT;
> > +
> > +    if ( copy_from_compat_offset((void *)&cmp.u, buf->h, offset, size) )
> > +        return -EFAULT;
> > +
> > +    /*
> > +     * The xlat magic doesn't quite know how to handle the union so
> > +     * we need to fix things up here.
> > +     */
> > +#define XLAT_iommu_op_u_query_reserved
> XEN_IOMMUOP_query_reserved
> 
> Missing undef for this.
> 

Yeah, not really needed but best to be tidy.

> [...]
> > diff --git a/xen/include/public/iommu_op.h
> b/xen/include/public/iommu_op.h
> > index c3b68f665a..02213c12a4 100644
> > --- a/xen/include/public/iommu_op.h
> > +++ b/xen/include/public/iommu_op.h
> > @@ -25,11 +25,50 @@
> >
> >  #include "xen.h"
> >
> > +typedef unsigned long xen_bfn_t;
> > +
> 
> This means xen_bfn_t will have different size on 32 bit and 64 bit
> platform.
> 

Good point. I guess it doesn't need to be related to the OS native width, so 
uint64_t is probably better.

  Paul

> And obviously other unsigned ints in this commit and xen_pfn_t in later
> patches will indeed make compat entry point necessary.
> 
> Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel