Re: [Xen-devel] [PATCH v6 08/14] vtd: add lookup_page method to iommu_ops

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.08.18 at 11:47, <paul.durrant@xxxxxxxxxx> wrote:
> This patch adds a new method to the VT-d IOMMU implementation to find the
> MFN currently mapped by the specified BFN along with a wrapper function in
> generic IOMMU code to call the implementation if it exists.

For this to go in, I think the AMD side of it wants to also be implemented.

> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -305,6 +305,17 @@ int iommu_unmap_page(struct domain *d, bfn_t bfn)
>      return rc;
>  }
>  
> +int iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t *mfn,
> +                      unsigned int *flags)
> +{
> +    const struct domain_iommu *hd = dom_iommu(d);
> +
> +    if ( !iommu_enabled || !hd->platform_ops )
> +        return -EOPNOTSUPP;
> +
> +    return hd->platform_ops->lookup_page(d, bfn, mfn, flags);
> +}

Shouldn't this be restricted to PV guests? HVM ones aren't supposed
to know about MFNs.

> +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t *mfn,
> +                                   unsigned int *flags)
> +{
> +    struct domain_iommu *hd = dom_iommu(d);
> +    struct dma_pte *page = NULL, *pte = NULL, val;

Pointless initializers. I also question the usefulness of "pte":

> +    u64 pg_maddr;
> +
> +    spin_lock(&hd->arch.mapping_lock);
> +
> +    pg_maddr = addr_to_dma_page_maddr(d, bfn_to_baddr(bfn), 0);
> +    if ( pg_maddr == 0 )
> +    {
> +        spin_unlock(&hd->arch.mapping_lock);
> +        return -ENOMEM;
> +    }
> +
> +    page = map_vtd_domain_page(pg_maddr);
> +    pte = page + (bfn_x(bfn) & LEVEL_MASK);
> +    val = *pte;

    val = page[bfn_x(bfn) & LEVEL_MASK];

Jan



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