From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH 2/4] iommu/arm-smmu-v3: Link domains and devices Date: Thu, 4 Apr 2019 15:39:24 +0100 Message-ID: <20190404143924.GB27823@fuggles.cambridge.arm.com> References: <20190320173634.21895-1-jean-philippe.brucker@arm.com> <20190320173634.21895-3-jean-philippe.brucker@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20190320173634.21895-3-jean-philippe.brucker-5wv7dgnIgG8@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: Jean-Philippe Brucker Cc: zhongmiao-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org, okaya-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org, linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, sudeep.holla-5wv7dgnIgG8@public.gmane.org, robin.murphy-5wv7dgnIgG8@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org List-Id: linux-acpi@vger.kernel.org Hi Jean-Philippe, First off, thanks for posting this: it's definitely something that I'm keen to support, and getting bits in a piece at a time is probably a good idea. On Wed, Mar 20, 2019 at 05:36:32PM +0000, Jean-Philippe Brucker wrote: > When removing a mapping from a domain, we need to send an invalidation to > all devices that might have stored it in their Address Translation Cache > (ATC). In addition when updating the context descriptor of a live domain, > we'll need to send invalidations for all devices attached to it. > > Maintain a list of devices in each domain, protected by a spinlock. It is > updated every time we attach or detach devices to and from domains. > > It needs to be a spinlock because we'll invalidate ATC entries from > within hardirq-safe contexts, but it may be possible to relax the read > side with RCU later. > > Signed-off-by: Jean-Philippe Brucker > --- > drivers/iommu/arm-smmu-v3.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index d3880010c6cf..66a29c113dbc 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -594,6 +594,11 @@ struct arm_smmu_device { > struct arm_smmu_master_data { > struct arm_smmu_device *smmu; > struct arm_smmu_strtab_ent ste; > + > + struct arm_smmu_domain *domain; > + struct list_head domain_head; > + > + struct device *dev; > }; > > /* SMMU private data for an IOMMU domain */ > @@ -618,6 +623,9 @@ struct arm_smmu_domain { > }; > > struct iommu_domain domain; > + > + struct list_head devices; > + spinlock_t devices_lock; > }; > > struct arm_smmu_option_prop { > @@ -1493,6 +1501,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type) > } > > mutex_init(&smmu_domain->init_mutex); > + INIT_LIST_HEAD(&smmu_domain->devices); > + spin_lock_init(&smmu_domain->devices_lock); I'm wondering whether we can't take this a bit further and re-organise the data structures to make this a little simpler overall. Something along the lines of: struct arm_smmu_master_data { struct list_head list; // masters in the same domain struct arm_smmu_device *smmu; unsigned int num_sids; u32 *sids; // Points into fwspec struct arm_smmu_domain *domain; // NULL -> !assigned }; and then just add a list_head into struct arm_smmu_domain to track the masters that have been attached (if you're feeling brave, you could put this into the s1_cfg). The ATC invalidation logic would then be: - Detaching a device: walk over the sids from the master data - Unmapping a range from a domain: walk over the attached masters I think this would also allow us to remove struct arm_smmu_strtab_ent completely. Dunno: this is one of the those things where you really have to try it to figure out why it doesn't work... Will From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D713DC4360F for ; Thu, 4 Apr 2019 14:39:40 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A58C22147C for ; Thu, 4 Apr 2019 14:39:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="GrniUxS2" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A58C22147C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=BYbMszjL0mtRC0s6+zw0husAcW0S+q9pkI/In7hHAV4=; b=GrniUxS2JH+ImP ylR4pTN1cc3xJ+upljSZK9vTSMBAagEH3JsgHoRAPCZXZnaAJtBR5rnyHHs6Inwru6M/I9HcdslNO LzX2ue2v+wK/B8v2eL5em4Vn+73Kg96AFmDsO2mYEJpzNYfW+Q2Doa9CkGCKXjRT/CyVxRIWVhWrU k3ZABv8fJ9pZyrDOeL8ACn09YFOxTex5Nz67vdtb6rtBTikCgc/n2IoUFNdQUp/5NrYsSh67DGTgS 5M+l9HtfNH8wd/Usx1Re8X/9VAywtcQA9be8kiV1i0b2MW5qJMTUhOPj7YEGFT4VJHtXeDR6hpq7n xsyWkEPKkl0pJPd5ktdw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hC3Wf-0005Bn-Nx; Thu, 04 Apr 2019 14:39:33 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hC3Wc-0005BP-KJ for linux-arm-kernel@lists.infradead.org; Thu, 04 Apr 2019 14:39:31 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 15828169E; Thu, 4 Apr 2019 07:39:29 -0700 (PDT) Received: from fuggles.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D12443F59C; Thu, 4 Apr 2019 07:39:26 -0700 (PDT) Date: Thu, 4 Apr 2019 15:39:24 +0100 From: Will Deacon To: Jean-Philippe Brucker Subject: Re: [PATCH 2/4] iommu/arm-smmu-v3: Link domains and devices Message-ID: <20190404143924.GB27823@fuggles.cambridge.arm.com> References: <20190320173634.21895-1-jean-philippe.brucker@arm.com> <20190320173634.21895-3-jean-philippe.brucker@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190320173634.21895-3-jean-philippe.brucker@arm.com> User-Agent: Mutt/1.11.1+86 (6f28e57d73f2) () X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190404_073930_679395_1D3EFECA X-CRM114-Status: GOOD ( 21.96 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: lorenzo.pieralisi@arm.com, zhongmiao@hisilicon.com, okaya@kernel.org, joro@8bytes.org, rjw@rjwysocki.net, linux-acpi@vger.kernel.org, iommu@lists.linux-foundation.org, hanjun.guo@linaro.org, sudeep.holla@arm.com, robin.murphy@arm.com, linux-arm-kernel@lists.infradead.org, lenb@kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Jean-Philippe, First off, thanks for posting this: it's definitely something that I'm keen to support, and getting bits in a piece at a time is probably a good idea. On Wed, Mar 20, 2019 at 05:36:32PM +0000, Jean-Philippe Brucker wrote: > When removing a mapping from a domain, we need to send an invalidation to > all devices that might have stored it in their Address Translation Cache > (ATC). In addition when updating the context descriptor of a live domain, > we'll need to send invalidations for all devices attached to it. > > Maintain a list of devices in each domain, protected by a spinlock. It is > updated every time we attach or detach devices to and from domains. > > It needs to be a spinlock because we'll invalidate ATC entries from > within hardirq-safe contexts, but it may be possible to relax the read > side with RCU later. > > Signed-off-by: Jean-Philippe Brucker > --- > drivers/iommu/arm-smmu-v3.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index d3880010c6cf..66a29c113dbc 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -594,6 +594,11 @@ struct arm_smmu_device { > struct arm_smmu_master_data { > struct arm_smmu_device *smmu; > struct arm_smmu_strtab_ent ste; > + > + struct arm_smmu_domain *domain; > + struct list_head domain_head; > + > + struct device *dev; > }; > > /* SMMU private data for an IOMMU domain */ > @@ -618,6 +623,9 @@ struct arm_smmu_domain { > }; > > struct iommu_domain domain; > + > + struct list_head devices; > + spinlock_t devices_lock; > }; > > struct arm_smmu_option_prop { > @@ -1493,6 +1501,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type) > } > > mutex_init(&smmu_domain->init_mutex); > + INIT_LIST_HEAD(&smmu_domain->devices); > + spin_lock_init(&smmu_domain->devices_lock); I'm wondering whether we can't take this a bit further and re-organise the data structures to make this a little simpler overall. Something along the lines of: struct arm_smmu_master_data { struct list_head list; // masters in the same domain struct arm_smmu_device *smmu; unsigned int num_sids; u32 *sids; // Points into fwspec struct arm_smmu_domain *domain; // NULL -> !assigned }; and then just add a list_head into struct arm_smmu_domain to track the masters that have been attached (if you're feeling brave, you could put this into the s1_cfg). The ATC invalidation logic would then be: - Detaching a device: walk over the sids from the master data - Unmapping a range from a domain: walk over the attached masters I think this would also allow us to remove struct arm_smmu_strtab_ent completely. Dunno: this is one of the those things where you really have to try it to figure out why it doesn't work... Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel