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:45, <Paul.Durrant@xxxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 12 September 2018 09:44
>> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
>> Cc: George Dunlap <George.Dunlap@xxxxxxxxxx>; Kevin Tian
>> <kevin.tian@xxxxxxxxx>; xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
>> Subject: RE: [PATCH v6 08/14] vtd: add lookup_page method to iommu_ops
>> 
>> >>> 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.
>> 
> 
> It will though. I'm going to need to use this function for HVM guests after 
> having done a GFN lookup.

That's the subject domain then, not the calling one.

Jan



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