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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan



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