[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 09/13] vtd: add lookup_page method to iommu_ops
> -----Original Message----- > From: dunlapg@xxxxxxxxx [mailto:dunlapg@xxxxxxxxx] On Behalf Of > George Dunlap > Sent: 11 July 2018 11:52 > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx> > Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Kevin Tian > <kevin.tian@xxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx> > Subject: Re: [Xen-devel] [PATCH v2 09/13] vtd: add lookup_page method to > iommu_ops > > On Sat, Jul 7, 2018 at 12:05 PM, Paul Durrant <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. > > > > This functionality will be used by a subsequent patch. > > > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx> > > --- > > Cc: Kevin Tian <kevin.tian@xxxxxxxxx> > > Cc: Jan Beulich <jbeulich@xxxxxxxx> > > > > v2: > > - Addressed some comments from Jan. > > --- > > xen/drivers/passthrough/iommu.c | 11 ++++++++++ > > xen/drivers/passthrough/vtd/iommu.c | 40 > +++++++++++++++++++++++++++++++++++++ > > xen/drivers/passthrough/vtd/iommu.h | 1 + > > xen/include/xen/iommu.h | 4 ++++ > > 4 files changed, 56 insertions(+) > > > > diff --git a/xen/drivers/passthrough/iommu.c > b/xen/drivers/passthrough/iommu.c > > index 3fbd3ebaf6..f25aa3f1d6 100644 > > --- a/xen/drivers/passthrough/iommu.c > > +++ b/xen/drivers/passthrough/iommu.c > > @@ -306,6 +306,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 7cd3813b9f..438bef670d 100644 > > --- a/xen/drivers/passthrough/vtd/iommu.c > > +++ b/xen/drivers/passthrough/vtd/iommu.c > > @@ -1831,6 +1831,45 @@ 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) > > +{ > > + struct domain_iommu *hd = dom_iommu(d); > > + struct dma_pte *page = NULL, *pte = NULL, val; > > + u64 pg_maddr; > > + > > + spin_lock(&hd->arch.mapping_lock); > > + > > + pg_maddr = addr_to_dma_page_maddr(d, bfn_to_baddr(bfn), 0); > > + if ( pg_maddr == 0 ) > > + { > > + spin_unlock(&hd->arch.mapping_lock); > > + return -ENOMEM; > > + } > > + > > + page = map_vtd_domain_page(pg_maddr); > > + pte = page + (bfn_x(bfn) & LEVEL_MASK); > > + val = *pte; > > + if ( !dma_pte_present(val) ) > > + { > > + unmap_vtd_domain_page(page); > > + spin_unlock(&hd->arch.mapping_lock); > > + return -ENOMEM; > > Should this be -EEXIST? Or maybe return MFN_INVALID? > Do you mean ENOENT? EEXIST implies it exists but it shouldn't, right? > Also, could you do the unmap / unlock first and then do the check, > rather than duplicating things? > Sure. > > + } > > + > > + unmap_vtd_domain_page(page); > > + spin_unlock(&hd->arch.mapping_lock); > > + > > + *mfn = maddr_to_mfn(dma_pte_addr(val)); > > + *flags = 0; > > + if ( dma_pte_prot(val) & DMA_PTE_READ ) > > + *flags |= IOMMUF_readable; > > + if ( dma_pte_prot(val) & DMA_PTE_WRITE ) > > + *flags |= IOMMUF_writable; > > This is a bit strange, since all dma_pte_prot() does is return val & > DMA_PTE_READ|DMA_PTE_WRITE. Would it make sense to implement > dma_pte_read() / dma_pte_write() instead (like dma_pte_present())? > Yes, I can do that. > Everything else looks good to me. > Thanks, Paul > -George _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |