Re: [Xen-devel] [PATCH v6 05/14] public / x86: introduce __HYPERCALL_iommu_op

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 04 September 2018 13:55
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; xen-
> devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Konrad Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
> Subject: RE: [PATCH v6 05/14] public / x86: introduce
> __HYPERCALL_iommu_op
> 
> >>> On 04.09.18 at 14:23, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: 04 September 2018 12:50
> >>
> >> >>> On 23.08.18 at 11:47, <paul.durrant@xxxxxxxxxx> wrote:
> >> > +int compat_one_iommu_op(compat_iommu_op_buf_t *buf)
> >> > +{
> >> > +    compat_iommu_op_t cmp;
> >> > +    xen_iommu_op_t nat;
> >> > +    int rc;
> >> > +
> >> > +    if ( buf->size < sizeof(cmp) )
> >> > +        return -EFAULT;
> >> > +
> >> > +    if ( copy_from_compat((void *)&cmp, buf->h, sizeof(cmp)) )
> >> > +        return -EFAULT;
> >> > +
> >> > +    if ( cmp.pad )
> >> > +        return -EINVAL;
> >> > +
> >> > +    rc = xsm_iommu_op(XSM_PRIV, current->domain, cmp.op);
> >> > +    if ( rc )
> >> > +        return rc;
> >> > +
> >> > +    XLAT_iommu_op(&nat, &cmp);
> >> > +
> >> > +    iommu_op(&nat);
> >> > +
> >> > +    XLAT_iommu_op(&cmp, &nat);
> >> > +
> >> > +    if ( __copy_field_to_compat(compat_handle_cast(buf->h,
> >> > +                                                   compat_iommu_op_t),
> >> > +                                &cmp, status) )
> >>
> >> Since you're only after the status field, perhaps better to avoid the
> >> full-blown reverse XLAT_iommu_op() and copy just that one field?
> >>
> >
> > I kind of like the fact that the two calls mirror each other so I'd prefer
> > to keep it.
> 
> Would you mind looking at the generated code (once you have a few
> sub-ops in place)? If the compiler manages to remove most of the
> cruft, I'd be fine keeping it as is. If, however, a whole lot of extra
> code gets generated, I'd really like to ask to use the shorter form.
> 

I checked... it's not wonderfully compact (because of the nested switches I 
guess), so I'll cherry-pick the status and add a comment.

  Paul

> Jan
> 


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