Re: [PATCH v2 3/4] iommu: elide flushing for higher order map/unmap operations

From: "Jan Beulich" <JBeulich@suse.com>
To: Paul Durrant <paul.durrant@citrix.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Tim Deegan <tim@xen.org>,
	george.dunlap@citrix.com, Julien Grall <julien.grall@arm.com>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Ian Jackson <Ian.Jackson@citrix.com>,
	Brian Woods <brian.woods@amd.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v2 3/4] iommu: elide flushing for higher order map/unmap operations
Date: Tue, 04 Dec 2018 10:20:18 -0700	[thread overview]
Message-ID: <5C06B7520200007800202D7B@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <ad36b7431b004ce89a6de1700a399a75@AMSPEX02CL03.citrite.net>

>>> On 04.12.18 at 17:53, <Paul.Durrant@citrix.com> wrote:
>> From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf
>> Of Jan Beulich
>> Sent: 04 December 2018 16:02
>> 
>> >>> On 04.12.18 at 16:36, <Paul.Durrant@citrix.com> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> Sent: 04 December 2018 15:17
>> >>
>> >> >>> On 03.12.18 at 18:40, <paul.durrant@citrix.com> wrote:
>> >> > --- a/xen/drivers/passthrough/vtd/iommu.c
>> >> > +++ b/xen/drivers/passthrough/vtd/iommu.c
>> >> > @@ -633,11 +633,14 @@ static int __must_check
>> iommu_flush_iotlb(struct
>> >> domain *d, dfn_t dfn,
>> >> >
>> >> >  static int __must_check iommu_flush_iotlb_pages(struct domain *d,
>> >> >                                                  dfn_t dfn,
>> >> > -                                                unsigned int
>> >> page_count)
>> >> > +                                                unsigned int
>> >> page_count,
>> >> > +                                                unsigned int
>> >> flush_flags)
>> >> >  {
>> >> >      ASSERT(page_count && !dfn_eq(dfn, INVALID_DFN));
>> >> > +    ASSERT(flush_flags);
>> >> >
>> >> > -    return iommu_flush_iotlb(d, dfn, 1, page_count);
>> >> > +    return iommu_flush_iotlb(d, dfn, flush_flags &
>> >> IOMMU_FLUSHF_modified,
>> >> > +                             page_count);
>> >>
>> >> Why the restriction to "modified"?
>> >
>> > The parameter is a bool which should be true if an existing PTE was
>> modified
>> > or false otherwise. I can make this !!(flush_flags &
>> IOMMU_FLUSHF_modified) is
>> > you prefer.
>> 
>> No, that wasn't my point. The question is why this isn't just
>> "flush_flags", without any masking. Iirc there are precautions
>> in the VT-d code to deal with hardware which may cache
>> non-present entries. In that case "added" requires flushing too.
>> 
> 
> I don't understand. iommu_flush_iotlb()'s third argument is 
> 'dma_old_pte_present' so that should be true iff IOMMU_FLUSHF_modified is 
> set. IOMMU_FLUSHF_added is irrelevant to the implementation.

Oh, you're right. I'm sorry for the noise then.

Jan

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