> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 03 December 2018 15:29 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: Brian Woods <brian.wo...@amd.com>; Suravee Suthikulpanit > <suravee.suthikulpa...@amd.com>; Julien Grall <julien.gr...@arm.com>; > Andrew Cooper <andrew.coop...@citrix.com>; Roger Pau Monne > <roger....@citrix.com>; Wei Liu <wei.l...@citrix.com>; Kevin Tian > <kevin.t...@intel.com>; Stefano Stabellini <sstabell...@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org> > Subject: RE: [Xen-devel] [PATCH 2/2] iommu: elide flushing for higher > order map/unmap operations > > >>> On 03.12.18 at 16:18, <paul.durr...@citrix.com> wrote: > >> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On > Behalf > >> Of Jan Beulich > >> Sent: 03 December 2018 15:03 > >> > >> >>> On 30.11.18 at 11:45, <paul.durr...@citrix.com> wrote: > >> > --- a/xen/drivers/passthrough/vtd/iommu.c > >> > +++ b/xen/drivers/passthrough/vtd/iommu.c > >> > @@ -631,11 +631,14 @@ static int __must_check > iommu_flush_iotlb(struct > >> domain *d, dfn_t dfn, > >> > return rc; > >> > } > >> > > >> > -static int __must_check iommu_flush_iotlb_pages(struct domain *d, > >> > - dfn_t dfn, > >> > - unsigned int > >> page_count) > >> > +static int __must_check iommu_flush_iotlb_pages( > >> > + struct domain *d, dfn_t dfn, unsigned int page_count, > >> > + enum iommu_flush_type flush_type) > >> > >> Is the re-flowing really needed? > >> > > > > Yes. The enum is long and won't fit within 80 chars otherwise. > > How about calling the parameter by a shorter name, e.g. ft? >
Well I'm going to passing flags around now, so I may as well use an unsigned int. > >> > @@ -674,9 +677,6 @@ static int __must_check dma_pte_clear_one(struct > >> domain *domain, u64 addr) > >> > spin_unlock(&hd->arch.mapping_lock); > >> > iommu_flush_cache_entry(pte, sizeof(struct dma_pte)); > >> > > >> > - if ( !this_cpu(iommu_dont_flush_iotlb) ) > >> > - rc = iommu_flush_iotlb_pages(domain, daddr_to_dfn(addr), 1); > >> > >> This code not getting replaced by another addition right in this > >> source file, and this function's only caller being > >> intel_iommu_unmap_page() makes me wonder why you don't > >> have the unmap functions similarly hand back a flush indicator. > > > > Well, the assumption is that unmap is always modifying an existing > entry. Is > > that assumption wrong? > > I could certainly see an unmap happening for an already > unmapped area. Ok, I'll generalise it then. Paul > > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
