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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.12.18 at 17:53, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf
>> Of Jan Beulich
>> Sent: 04 December 2018 16:02
>> 
>> >>> On 04.12.18 at 16:36, <Paul.Durrant@xxxxxxxxxx> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >> Sent: 04 December 2018 15:17
>> >>
>> >> >>> On 03.12.18 at 18:40, <paul.durrant@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel