From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751349AbdHaIr6 (ORCPT ); Thu, 31 Aug 2017 04:47:58 -0400 Received: from smtp-fw-9102.amazon.com ([207.171.184.29]:41350 "EHLO smtp-fw-9102.amazon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750733AbdHaIr4 (ORCPT ); Thu, 31 Aug 2017 04:47:56 -0400 X-IronPort-AV: E=Sophos;i="5.41,451,1498521600"; d="scan'208";a="563525547" From: "Sironi, Filippo" To: Jacob Pan CC: Filippo Sironi via iommu , "linux-kernel@vger.kernel.org" , "David Woodhouse" , "Woodhouse, David" Subject: Re: [PATCH] intel-iommu: Don't be too aggressive when clearing one context entry Thread-Topic: [PATCH] intel-iommu: Don't be too aggressive when clearing one context entry Thread-Index: AQHTIAhIl49pMhlev0+w0q7oOR1y/aKdD1OAgAEcC4A= Date: Thu, 31 Aug 2017 08:47:03 +0000 Message-ID: <77293ECA-2B4C-496C-9DDD-A5A2618F00A1@amazon.de> References: <1503929789-26698-1-git-send-email-sironi@amazon.de> <20170830085025.7027c9f8@jacob-builder> In-Reply-To: <20170830085025.7027c9f8@jacob-builder> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-messagesentrepresentingtype: 1 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.43.165.42] Content-Type: text/plain; charset="us-ascii" Content-ID: <7A78EDE5B774D64BBA7786D39150A1DC@amazon.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v7V8m5uo013082 Hi Jacob, > On 30. Aug 2017, at 17:50, Jacob Pan wrote: > > On Mon, 28 Aug 2017 16:16:29 +0200 > Filippo Sironi via iommu 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 >> Cc: David Woodhouse >> Cc: David Woodhouse >> Cc: Joerg Roedel >> Cc: iommu@lists.linux-foundation.org >> Cc: linux-kernel@vger.kernel.org >> --- >> drivers/iommu/intel-iommu.c | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c >> index 3e8636f1220e..4bf3e59b0929 100644 >> --- a/drivers/iommu/intel-iommu.c >> +++ b/drivers/iommu/intel-iommu.c >> @@ -2351,13 +2351,32 @@ 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; >> >> + spin_lock_irqsave(&iommu->lock, flags); >> + context = iommu_context_addr(iommu, bus, devfn, 0); >> + if (!context) { >> + spin_unlock_irqrestore(&iommu->lock, flags); >> + return; >> + } > perhaps check with device_context_mapped()? Using device_context_mapped() wouldn't simplify the code since it would just tell me that at the time of check there was a context. I would still need to lock, get the context, check if the context is valid, do the work, and unlock. Modifying device_context_mapped() to return the context isn't going to work either because it may go away in the meantime since I wouldn't hold the lock. >> + did_old = context_domain_id(context); >> + spin_unlock_irqrestore(&iommu->lock, flags); >> 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); >> + iommu->flush.flush_context(iommu, >> + did_old, >> + (((u16)bus) << 8) | devfn, >> + DMA_CCMD_MASK_NOBIT, >> + DMA_CCMD_DEVICE_INVL); >> + iommu->flush.flush_iotlb(iommu, >> + did_old, >> + 0, >> + 0, >> + DMA_TLB_DSI_FLUSH); >> } >> >> static inline void unlink_domain_info(struct device_domain_info >> *info) > > [Jacob Pan] Regards, Filippo Amazon Development Center Germany GmbH Berlin - Dresden - Aachen main office: Krausenstr. 38, 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B