Re: [Xen-devel] [PATCH RFC 1/2] gnttab: add IOMMU entries for grant mappings on auto-translated guests

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.04.14 at 16:37, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -749,6 +749,22 @@ __gnttab_map_grant_ref(
>              goto undo_out;
>          }
>      }
> +    else if ( need_iommu(ld) )
> +    {
> +        int err;
> +
> +        BUG_ON(!paging_mode_translate(ld));
> +        err = iommu_map_page(ld, op->host_addr >> PAGE_SHIFT, frame,
> +                             op->flags & GNTMAP_readonly ?
> +                                IOMMUF_readable :
> +                                IOMMUF_readable|IOMMUF_writable);
> +        if ( err )
> +        {
> +            double_gt_unlock(lgt, rgt);
> +            rc = GNTST_general_error;
> +            goto undo_out;
> +        }
> +    }

As much of this as possible should be folded with the if() branch.
And looking at the PV code, I think it makes no sense for the
conditions whether to map the page r/o or r/w to be different
between PV and non-PV.

Plus - wouldn't it be better to have this taken care of via
create_grant_host_mapping(), by not blindly calling
iommu_unmap_page() on anything other than p2m_ram_rw in
ept_set_entry() and p2m_set_entry(), the more that this should
already be taken care of in the iommu_hap_pt_share case.

> @@ -947,6 +963,18 @@ __gnttab_unmap_common(
>              goto unmap_out;
>          }
>      }
> +    else if ( need_iommu(ld) )
> +    {
> +        int err;
> +
> +        BUG_ON(!paging_mode_translate(ld));
> +        err = iommu_unmap_page(ld, op->host_addr >> PAGE_SHIFT);
> +        if ( err )
> +        {
> +            rc = GNTST_general_error;
> +            goto unmap_out;
> +        }
> +    }

Same thing here obviously.

Jan


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