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

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Paul Durrant
> Sent: 12 September 2018 09:52
> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich'
> <JBeulich@suse.com>
> Cc: xen-devel <xen-devel@lists.xenproject.org>; Kevin Tian
> <kevin.tian@intel.com>; George Dunlap <George.Dunlap@citrix.com>
> Subject: RE: [PATCH v6 08/14] vtd: add lookup_page method to iommu_ops
> 
> > -----Original Message-----
> > From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On
> Behalf
> > Of Paul Durrant
> > Sent: 12 September 2018 09:45
> > To: 'Jan Beulich' <JBeulich@suse.com>
> > Cc: xen-devel <xen-devel@lists.xenproject.org>; Kevin Tian
> > <kevin.tian@intel.com>; George Dunlap <George.Dunlap@citrix.com>
> > Subject: Re: [Xen-devel] [PATCH v6 08/14] vtd: add lookup_page method
> to
> > iommu_ops
> >
> > > -----Original Message-----
> > > From: Jan Beulich [mailto:JBeulich@suse.com]
> > > Sent: 12 September 2018 09:44
> > > To: Paul Durrant <Paul.Durrant@citrix.com>
> > > Cc: George Dunlap <George.Dunlap@citrix.com>; Kevin Tian
> > > <kevin.tian@intel.com>; xen-devel <xen-devel@lists.xenproject.org>
> > > Subject: RE: [PATCH v6 08/14] vtd: add lookup_page method to
> > iommu_ops
> > >
> > > >>> On 12.09.18 at 10:31, <Paul.Durrant@citrix.com> wrote:
> > > >> From: Jan Beulich [mailto:JBeulich@suse.com]
> > > >> Sent: 07 September 2018 12:11
> > > >>
> > > >> >>> On 23.08.18 at 11:47, <paul.durrant@citrix.com> 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.
> 

Actually, no I still don't think the ASSERT is correct. Why should we rule out HVM guests being able to use PV-IOMMU?

  Paul

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