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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 35694C433EF for ; Tue, 24 May 2022 14:36:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238275AbiEXOgY (ORCPT ); Tue, 24 May 2022 10:36:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39096 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236719AbiEXOgW (ORCPT ); Tue, 24 May 2022 10:36:22 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id C06E150E0B for ; Tue, 24 May 2022 07:36:20 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2FD9F1FB; Tue, 24 May 2022 07:36:20 -0700 (PDT) Received: from [10.57.82.55] (unknown [10.57.82.55]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1E7193F66F; Tue, 24 May 2022 07:36:17 -0700 (PDT) Message-ID: Date: Tue, 24 May 2022 15:36:11 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support Content-Language: en-GB To: Lu Baolu , Joerg Roedel , Jason Gunthorpe , Christoph Hellwig , Kevin Tian , Ashok Raj , Will Deacon , Jean-Philippe Brucker , Dave Jiang , Vinod Koul Cc: Jean-Philippe Brucker , linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, Jacob jun Pan References: <20220519072047.2996983-1-baolu.lu@linux.intel.com> <20220519072047.2996983-4-baolu.lu@linux.intel.com> From: Robin Murphy In-Reply-To: <20220519072047.2996983-4-baolu.lu@linux.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2022-05-19 08:20, Lu Baolu wrote: [...] > diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c > index 106506143896..210c376f6043 100644 > --- a/drivers/iommu/iommu-sva-lib.c > +++ b/drivers/iommu/iommu-sva-lib.c > @@ -69,3 +69,51 @@ struct mm_struct *iommu_sva_find(ioasid_t pasid) > return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero); > } > EXPORT_SYMBOL_GPL(iommu_sva_find); > + > +/* > + * IOMMU SVA driver-oriented interfaces > + */ > +struct iommu_domain * > +iommu_sva_alloc_domain(struct bus_type *bus, struct mm_struct *mm) Argh, please no new bus-based external interfaces! Domain allocation needs to resolve to the right IOMMU instance to solve a number of issues, and cleaning up existing users of iommu_domain_alloc() to prepare for that is already hard enough. This is arguably even more relevant here than for other domain types, since SVA support is more likely to depend on specific features that can vary between IOMMU instances even with the same driver. Please make the external interface take a struct device, then resolve the ops through dev->iommu. Further nit: the naming inconsistency bugs me a bit - iommu_sva_domain_alloc() seems more natural. Also I'd question the symmetry vs. usability dichotomy of whether we *really* want two different free functions for a struct iommu_domain pointer, where any caller which might mix SVA and non-SVA usage then has to remember how they allocated any particular domain :/ > +{ > + struct iommu_sva_domain *sva_domain; > + struct iommu_domain *domain; > + > + if (!bus->iommu_ops || !bus->iommu_ops->sva_domain_ops) > + return ERR_PTR(-ENODEV); > + > + sva_domain = kzalloc(sizeof(*sva_domain), GFP_KERNEL); > + if (!sva_domain) > + return ERR_PTR(-ENOMEM); > + > + mmgrab(mm); > + sva_domain->mm = mm; > + > + domain = &sva_domain->domain; > + domain->type = IOMMU_DOMAIN_SVA; > + domain->ops = bus->iommu_ops->sva_domain_ops; I'd have thought it would be logical to pass IOMMU_DOMAIN_SVA to the normal domain_alloc call, so that driver-internal stuff like context descriptors can be still be hung off the domain as usual (rather than all drivers having to implement some extra internal lookup mechanism to handle all the SVA domain ops), but that's something we're free to come back and change later. FWIW I'd just stick the mm pointer in struct iommu_domain, in a union with the fault handler stuff and/or iova_cookie - those are mutually exclusive with SVA, right? Cheers, Robin. > + > + return domain; > +} > + > +void iommu_sva_free_domain(struct iommu_domain *domain) > +{ > + struct iommu_sva_domain *sva_domain = to_sva_domain(domain); > + > + mmdrop(sva_domain->mm); > + kfree(sva_domain); > +} > + 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 Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 97622C433F5 for ; Tue, 24 May 2022 14:36:25 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id EB67F41B52; Tue, 24 May 2022 14:36:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id m4dEfzAp5DDP; Tue, 24 May 2022 14:36:23 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 73F1F41986; Tue, 24 May 2022 14:36:23 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 43333C0032; Tue, 24 May 2022 14:36:23 +0000 (UTC) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5E04BC002D for ; Tue, 24 May 2022 14:36:22 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 33D93400FE for ; Tue, 24 May 2022 14:36:22 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id SUZHHJ9nj1Lh for ; Tue, 24 May 2022 14:36:21 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp2.osuosl.org (Postfix) with ESMTP id 4498D400C1 for ; Tue, 24 May 2022 14:36:21 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2FD9F1FB; Tue, 24 May 2022 07:36:20 -0700 (PDT) Received: from [10.57.82.55] (unknown [10.57.82.55]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1E7193F66F; Tue, 24 May 2022 07:36:17 -0700 (PDT) Message-ID: Date: Tue, 24 May 2022 15:36:11 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support Content-Language: en-GB To: Lu Baolu , Joerg Roedel , Jason Gunthorpe , Christoph Hellwig , Kevin Tian , Ashok Raj , Will Deacon , Jean-Philippe Brucker , Dave Jiang , Vinod Koul References: <20220519072047.2996983-1-baolu.lu@linux.intel.com> <20220519072047.2996983-4-baolu.lu@linux.intel.com> From: Robin Murphy In-Reply-To: <20220519072047.2996983-4-baolu.lu@linux.intel.com> Cc: Jean-Philippe Brucker , iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Jacob jun Pan X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" On 2022-05-19 08:20, Lu Baolu wrote: [...] > diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c > index 106506143896..210c376f6043 100644 > --- a/drivers/iommu/iommu-sva-lib.c > +++ b/drivers/iommu/iommu-sva-lib.c > @@ -69,3 +69,51 @@ struct mm_struct *iommu_sva_find(ioasid_t pasid) > return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero); > } > EXPORT_SYMBOL_GPL(iommu_sva_find); > + > +/* > + * IOMMU SVA driver-oriented interfaces > + */ > +struct iommu_domain * > +iommu_sva_alloc_domain(struct bus_type *bus, struct mm_struct *mm) Argh, please no new bus-based external interfaces! Domain allocation needs to resolve to the right IOMMU instance to solve a number of issues, and cleaning up existing users of iommu_domain_alloc() to prepare for that is already hard enough. This is arguably even more relevant here than for other domain types, since SVA support is more likely to depend on specific features that can vary between IOMMU instances even with the same driver. Please make the external interface take a struct device, then resolve the ops through dev->iommu. Further nit: the naming inconsistency bugs me a bit - iommu_sva_domain_alloc() seems more natural. Also I'd question the symmetry vs. usability dichotomy of whether we *really* want two different free functions for a struct iommu_domain pointer, where any caller which might mix SVA and non-SVA usage then has to remember how they allocated any particular domain :/ > +{ > + struct iommu_sva_domain *sva_domain; > + struct iommu_domain *domain; > + > + if (!bus->iommu_ops || !bus->iommu_ops->sva_domain_ops) > + return ERR_PTR(-ENODEV); > + > + sva_domain = kzalloc(sizeof(*sva_domain), GFP_KERNEL); > + if (!sva_domain) > + return ERR_PTR(-ENOMEM); > + > + mmgrab(mm); > + sva_domain->mm = mm; > + > + domain = &sva_domain->domain; > + domain->type = IOMMU_DOMAIN_SVA; > + domain->ops = bus->iommu_ops->sva_domain_ops; I'd have thought it would be logical to pass IOMMU_DOMAIN_SVA to the normal domain_alloc call, so that driver-internal stuff like context descriptors can be still be hung off the domain as usual (rather than all drivers having to implement some extra internal lookup mechanism to handle all the SVA domain ops), but that's something we're free to come back and change later. FWIW I'd just stick the mm pointer in struct iommu_domain, in a union with the fault handler stuff and/or iova_cookie - those are mutually exclusive with SVA, right? Cheers, Robin. > + > + return domain; > +} > + > +void iommu_sva_free_domain(struct iommu_domain *domain) > +{ > + struct iommu_sva_domain *sva_domain = to_sva_domain(domain); > + > + mmdrop(sva_domain->mm); > + kfree(sva_domain); > +} > + _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu