Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Paul Durrant [mailto:Paul.Durrant@xxxxxxxxxx]
> Sent: Tuesday, August 7, 2018 4:22 PM
> 
> > -----Original Message-----
> > From: Tian, Kevin [mailto:kevin.tian@xxxxxxxxx]
> > Sent: 07 August 2018 04:25
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>; George Dunlap
> > <George.Dunlap@xxxxxxxxxx>
> > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to
> iommu_ops
> >
> > > From: Paul Durrant [mailto:paul.durrant@xxxxxxxxxx]
> > > Sent: Saturday, August 4, 2018 1:22 AM
> > >
> > > 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.
> > >
> > > This functionality will be used by a subsequent patch.
> > >
> > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > > Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > ---
> > > Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> > > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
> > >
> > > v3:
> > >  - Addressed comments from George.
> > >
> > > v2:
> > >  - Addressed some comments from Jan.
> > > ---
> > >  xen/drivers/passthrough/iommu.c     | 11 +++++++++++
> > >  xen/drivers/passthrough/vtd/iommu.c | 34
> > > ++++++++++++++++++++++++++++++++++
> > >  xen/drivers/passthrough/vtd/iommu.h |  3 +++
> > >  xen/include/xen/iommu.h             |  4 ++++
> > >  4 files changed, 52 insertions(+)
> > >
> > > diff --git a/xen/drivers/passthrough/iommu.c
> > > b/xen/drivers/passthrough/iommu.c
> > > index b10a37e5d7..9b7baca93f 100644
> > > --- 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);
> > > +}
> > > +
> > >  static void iommu_free_pagetables(unsigned long unused)
> > >  {
> > >      do {
> > > diff --git a/xen/drivers/passthrough/vtd/iommu.c
> > > b/xen/drivers/passthrough/vtd/iommu.c
> > > index 282e227414..8cd3b59aa0 100644
> > > --- a/xen/drivers/passthrough/vtd/iommu.c
> > > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > > @@ -1830,6 +1830,39 @@ static int __must_check
> > > intel_iommu_unmap_page(struct domain *d,
> > >      return dma_pte_clear_one(d, bfn_to_baddr(bfn));
> > >  }
> > >
> > > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn,
> mfn_t
> > > *mfn,
> > > +                                   unsigned int *flags)
> >
> > Not looking at later patches yet... but in concept bfn address
> > space is per device instead of per domain.
> 
> Not in this case. Xen has always maintained a single IOMMU address per
> virtual machine. That is what BFN refers to.
> 
>   Paul
> 
> > In default situation
> > (w/o pvIOMMU exposed), all devices assigned to dom0 share
> > the same address space (bfn=pfn) which is currently linked
> > from domain structure. Then with pvIOMMU exposed, dom0
> > starts to manage individual pfn address space (called IOVA
> > address space within dom0) per assigned device. In that case
> > lookup should accept a bdf number and then find the right
> > page table...
> >
> 
> No. That is over-complicating things and would probably involve re-writing
> most of the IOMMU code in Xen AFAICT.
> 

it's not over-complicating. it's about correctness.

think about the role of IOMMU. Now you expose it to dom0. Then
dom0 wants to use it to isolate devices. What is the matter having
an IOMMU which supports only one global address space cross all
devices, from dom0's p.o.v? If that is the case, why does dom0 ever
want to change bfn globally on all devices?

Thanks
Kevin

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