iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Lu Baolu <baolu.lu@linux.intel.com>,
	Joerg Roedel <joro@8bytes.org>, Jason Gunthorpe <jgg@nvidia.com>,
	Christoph Hellwig <hch@infradead.org>,
	Kevin Tian <kevin.tian@intel.com>,
	Ashok Raj <ashok.raj@intel.com>, Will Deacon <will@kernel.org>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	Dave Jiang <dave.jiang@intel.com>, Vinod Koul <vkoul@kernel.org>
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>,
	iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	Jacob jun Pan <jacob.jun.pan@intel.com>
Subject: Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support
Date: Tue, 24 May 2022 15:36:11 +0100	[thread overview]
Message-ID: <f3170016-4d7f-e78e-db48-68305f683349@arm.com> (raw)
In-Reply-To: <20220519072047.2996983-4-baolu.lu@linux.intel.com>

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

  parent reply	other threads:[~2022-05-24 14:36 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-19  7:20 [PATCH v7 00/10] iommu: SVA and IOPF refactoring Lu Baolu
2022-05-19  7:20 ` [PATCH v7 01/10] iommu: Add pasids field in struct iommu_device Lu Baolu
2022-05-19 10:37   ` Jean-Philippe Brucker
2022-05-19 11:55     ` Baolu Lu
2022-05-24  9:24   ` Tian, Kevin
2022-05-25  2:03     ` Baolu Lu
2022-05-25  2:13       ` Baolu Lu
2022-05-19  7:20 ` [PATCH v7 02/10] iommu: Remove SVM_FLAG_SUPERVISOR_MODE support Lu Baolu
2022-05-19 16:22   ` Jean-Philippe Brucker
2022-05-24  9:27   ` Tian, Kevin
2022-05-19  7:20 ` [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support Lu Baolu
2022-05-19 16:33   ` Jean-Philippe Brucker
2022-05-20  4:55     ` Baolu Lu
2022-05-23  7:12   ` Baolu Lu
2022-05-24  9:44     ` Tian, Kevin
2022-05-25  2:18       ` Baolu Lu
2022-05-24  9:39   ` Tian, Kevin
2022-05-24 13:38     ` Jason Gunthorpe via iommu
2022-05-25  0:44       ` Tian, Kevin
2022-05-25  2:38         ` Baolu Lu
2022-05-25  4:50     ` Baolu Lu
2022-05-24 13:44   ` Jason Gunthorpe via iommu
2022-05-25  5:19     ` Baolu Lu
2022-05-25 15:25       ` Jason Gunthorpe via iommu
2022-05-26  1:03         ` Baolu Lu
2022-05-25  5:33     ` Baolu Lu
2022-05-24 14:36   ` Robin Murphy [this message]
2022-05-25  6:20     ` Baolu Lu
2022-05-25 10:07       ` Robin Murphy
2022-05-25 11:06         ` Jean-Philippe Brucker
2022-05-25 13:11           ` Baolu Lu
2022-05-19  7:20 ` [PATCH v7 04/10] iommu/vt-d: Add SVA domain support Lu Baolu
2022-05-19  7:20 ` [PATCH v7 05/10] arm-smmu-v3/sva: " Lu Baolu
2022-05-19 16:37   ` Jean-Philippe Brucker
2022-05-19  7:20 ` [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device() Lu Baolu
2022-05-19 16:39   ` Jean-Philippe Brucker
2022-05-20  6:38     ` Baolu Lu
2022-05-20 11:28       ` Jean-Philippe Brucker
2022-05-23  3:07         ` Baolu Lu
2022-05-24 10:22   ` Tian, Kevin
2022-05-24 10:57     ` Jean-Philippe Brucker
2022-05-25  2:04       ` Tian, Kevin
2022-05-25  7:29         ` Jean-Philippe Brucker
2022-06-02  6:46           ` Tian, Kevin
2022-05-19  7:20 ` [PATCH v7 07/10] iommu: Remove SVA related callbacks from iommu ops Lu Baolu
2022-05-24 10:23   ` Tian, Kevin
2022-05-19  7:20 ` [PATCH v7 08/10] iommu: Prepare IOMMU domain for IOPF Lu Baolu
2022-05-19 16:40   ` Jean-Philippe Brucker
2022-05-19  7:20 ` [PATCH v7 09/10] iommu: Per-domain I/O page fault handling Lu Baolu
2022-05-19  7:20 ` [PATCH v7 10/10] iommu: Rename iommu-sva-lib.{c,h} Lu Baolu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f3170016-4d7f-e78e-db48-68305f683349@arm.com \
    --to=robin.murphy@arm.com \
    --cc=ashok.raj@intel.com \
    --cc=baolu.lu@linux.intel.com \
    --cc=dave.jiang@intel.com \
    --cc=hch@infradead.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jacob.jun.pan@intel.com \
    --cc=jean-philippe@linaro.com \
    --cc=jean-philippe@linaro.org \
    --cc=jgg@nvidia.com \
    --cc=joro@8bytes.org \
    --cc=kevin.tian@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vkoul@kernel.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).