[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v6 08/14] vtd: add lookup_page method to iommu_ops
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > Sent: 12 September 2018 10:03 > 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:53, <Paul.Durrant@xxxxxxxxxx> wrote: > >> From: Paul Durrant > >> Sent: 12 September 2018 09:52 > >> > >> > From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On > >> Behalf > >> > Of Paul Durrant > >> > Sent: 12 September 2018 09:45 > >> > > >> > > From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > >> > > Sent: 12 September 2018 09:44 > >> > > > >> > > >>> 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. > >> > >> Sorry... I'm getting confused myself now. It won't fire because in my case > the > >> domain here will always by PV (because it is the not the domain owning > the > >> GFN). I still think this is the wrong level for such a check though, but > >> I'll put > in > >> the ASSERT. > > And what would guarantee the ASSERT() to not trigger? As said, I'm > fine with this being enforced at a different level, but it needs to be > enforced somewhere. > > > Actually, no I still don't think the ASSERT is correct. Why should we rule > > out HVM guests being able to use PV-IOMMU? > > A HVM guest using the PV IOMMU is quite fine, but it shouldn't talk to > it in terms of MFNs. > Well, it has to talk MFNs at some level, surely? The output of the IOMMU is not subject to EPT/NPT, right? Paul > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |