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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.12.18 at 16:18, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf
>> Of Jan Beulich
>> Sent: 03 December 2018 15:03
>> 
>> >>> On 30.11.18 at 11:45, <paul.durrant@xxxxxxxxxx> 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?

>> > @@ -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.

Jan



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