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

From: Paul Durrant <Paul.Durrant@citrix.com>
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: [PATCH v6 08/14] vtd: add lookup_page method to iommu_ops
Date: Wed, 12 Sep 2018 08:45:17 +0000	[thread overview]
Message-ID: <77f7af1c0daf48c9ab66bbd77cc022ec@AMSPEX02CL03.citrite.net> (raw)
In-Reply-To: <5B98D1B702000078001E7A8F@prv1-mh.provo.novell.com>

> -----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.

  Paul

> Jan
> 

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