From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751001AbdHaIor (ORCPT ); Thu, 31 Aug 2017 04:44:47 -0400 Received: from smtp-fw-9101.amazon.com ([207.171.184.25]:14770 "EHLO smtp-fw-9101.amazon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750791AbdHaIoo (ORCPT ); Thu, 31 Aug 2017 04:44:44 -0400 X-IronPort-AV: E=Sophos;i="5.41,451,1498521600"; d="scan'208";a="699145782" From: "Sironi, Filippo" To: Joerg Roedel CC: "iommu@lists.linux-foundation.org" , "linux-kernel@vger.kernel.org" , "Woodhouse, David" , David Woodhouse 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/aKc6G0AgAFBXoA= Date: Thu, 31 Aug 2017 08:41:25 +0000 Message-ID: <8B5D8830-9DB1-40D9-BB98-C6F277E64F21@amazon.de> References: <1503929789-26698-1-git-send-email-sironi@amazon.de> <20170830133112.btbsmgbm2clz4767@8bytes.org> In-Reply-To: <20170830133112.btbsmgbm2clz4767@8bytes.org> 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.166.75] Content-Type: text/plain; charset="us-ascii" Content-ID: <764F48467F75E245B4A727A8C9C48850@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 v7V8it5s009692 Hi Joerg, > On 30. Aug 2017, at 15:31, Joerg Roedel wrote: > > Hi Filippo, > > please change the subject to: > > iommu/vt-d: Don't be too aggressive when clearing one context entry > > to follow the convention used in the iommu-tree. Another comment below. Will do. > On Mon, Aug 28, 2017 at 04:16:29PM +0200, Filippo Sironi wrote: >> 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; >> + } >> + did_old = context_domain_id(context); >> + spin_unlock_irqrestore(&iommu->lock, flags); >> clear_context_table(iommu, bus, devfn); > > This function is the only caller of clear_context_table(), which does > similar things (like fetching the context-entry) as you are adding > above. > > So you can either make clear_context_table() return the old domain-id > so that you don't need to do it here, or you get rid of the function > entirely and add the context_clear_entry() and __iommu_flush_cache() > calls into this code-path. > > Regards, > > Joerg I went for merging domain_context_clear_one() with context_clear_one(). 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Sironi, Filippo via iommu" Subject: Re: [PATCH] intel-iommu: Don't be too aggressive when clearing one context entry Date: Thu, 31 Aug 2017 08:41:25 +0000 Message-ID: <8B5D8830-9DB1-40D9-BB98-C6F277E64F21@amazon.de> References: <1503929789-26698-1-git-send-email-sironi@amazon.de> <20170830133112.btbsmgbm2clz4767@8bytes.org> Reply-To: "Sironi, Filippo" Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170830133112.btbsmgbm2clz4767-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> Content-Language: en-US Content-ID: <764F48467F75E245B4A727A8C9C48850-vV1OtcyAfmbQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Joerg Roedel Cc: "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , David Woodhouse , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "Woodhouse, David" List-Id: iommu@lists.linux-foundation.org Hi Joerg, > On 30. Aug 2017, at 15:31, Joerg Roedel wrote: > > Hi Filippo, > > please change the subject to: > > iommu/vt-d: Don't be too aggressive when clearing one context entry > > to follow the convention used in the iommu-tree. Another comment below. Will do. > On Mon, Aug 28, 2017 at 04:16:29PM +0200, Filippo Sironi wrote: >> 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; >> + } >> + did_old = context_domain_id(context); >> + spin_unlock_irqrestore(&iommu->lock, flags); >> clear_context_table(iommu, bus, devfn); > > This function is the only caller of clear_context_table(), which does > similar things (like fetching the context-entry) as you are adding > above. > > So you can either make clear_context_table() return the old domain-id > so that you don't need to do it here, or you get rid of the function > entirely and add the context_clear_entry() and __iommu_flush_cache() > calls into this code-path. > > Regards, > > Joerg I went for merging domain_context_clear_one() with context_clear_one(). 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