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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.09.18 at 10:31, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 07 September 2018 12:11
>> 
>> >>> 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.
> 
> Agreed, but I think this is the wrong level to be applying such a check: 
> iommu_map_page() is supplied an MFN regardless of whether the domain is PV or 
> HVM, so I think it is reasonable for a lookup function to work in terms of 
> MFNs.

I don't mind much where the check sits, but ASSERT(!is_hvm_domain()),
if placed here, should not trigger.

Jan



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