On Thu, 2017-08-31 at 10:58 +0200, Filippo Sironi wrote: > Previously, we were invalidating context cache and IOTLB globally when > clearing one context entry. This is a tad too aggressive. > Invalidate the context cache and IOTLB for the interested device only. > > Signed-off-by: Filippo Sironi <sir...@amazon.de> > Cc: David Woodhouse <d...@amazon.co.uk> > Cc: David Woodhouse <dw...@infradead.org> > Cc: Joerg Roedel <j...@8bytes.org> > Cc: Jacob Pan <jacob.jun....@linux.intel.com> > Cc: iommu@lists.linux-foundation.org > Cc: linux-ker...@vger.kernel.org
Acked-by: David Woodhouse <d...@amazon.co.uk>
I'd actually quite like to see us tidy this up some more. If there are
aliases, we're doing the flush multiple times. It might be nicer to do
no flushing in domain_contact_clear_one(), which is invoked for each
alias, and just to update a (u16 *)did. If *did was empty, set it. If
it was set (i.e. if there are aliases), then set it to a magic value
like 0xFFFF.
Then we can do the actual flush — either device-specific, or global —
just once in domain_context_clear().
We'd probably just move domain_context_clear_one() into its only caller
domain_context_clear_one_cb() while we're at it.
But in the meantime I'm more than happy with this patch which turns
multiple global flushes into multiple domain-specific flushes, and does
the right thing in the common case.
> ---
v2: Changed subject, killed clear_context_table(). Anything else?
> drivers/iommu/intel-iommu.c | 42 ++++++++++++++++++++++++------------------
> 1 file changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 3e8636f1220e..1aa4ad7974b9 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -974,20 +974,6 @@ static int device_context_mapped(struct intel_iommu
> *iommu, u8 bus, u8 devfn)
> return ret;
> }
>
> -static void clear_context_table(struct intel_iommu *iommu, u8 bus, u8 devfn)
> -{
> - struct context_entry *context;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&iommu->lock, flags);
> - context = iommu_context_addr(iommu, bus, devfn, 0);
> - if (context) {
> - context_clear_entry(context);
> - __iommu_flush_cache(iommu, context, sizeof(*context));
> - }
> - spin_unlock_irqrestore(&iommu->lock, flags);
> -}
> -
> static void free_context_table(struct intel_iommu *iommu)
> {
> int i;
> @@ -2351,13 +2337,33 @@ static inline int domain_pfn_mapping(struct
> dmar_domain *domain, unsigned long i
>
> static void domain_context_clear_one(struct intel_iommu *iommu, u8 bus, u8
> devfn)
> {
> + unsigned long flags;
> + struct context_entry *context;
> + u16 did_old;
> +
> if (!iommu)
> return;
>
> - clear_context_table(iommu, bus, devfn);
> - iommu->flush.flush_context(iommu, 0, 0, 0,
> - DMA_CCMD_GLOBAL_INVL);
> - iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
> + spin_lock_irqsave(&iommu->lock, flags);
> + context = iommu_context_addr(iommu, bus, devfn, 0);
> + if (!context) {
> + spin_unlock_irqrestore(&iommu->lock, flags);
> + return;
> + }
> + did_old = context_domain_id(context);
> + context_clear_entry(context);
> + __iommu_flush_cache(iommu, context, sizeof(*context));
> + spin_unlock_irqrestore(&iommu->lock, flags);
> + iommu->flush.flush_context(iommu,
> + did_old,
> + (((u16)bus) << 8) | devfn,
Arguably we ought to be PCI_DEVID() there, but I feel bad pointing that
out when you're adding about the fifth instance of open-coding it... :)
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
Amazon Web Services UK Limited. Registered in England and Wales with registration number 08650665 and which has its registered office at 60 Holborn Viaduct, London EC1A 2FD, United Kingdom.
_______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
