Re: [PATCH 15/17] iommu: remove DOMAIN_ATTR_NESTING
On Thu, Mar 25, 2021 at 06:12:37AM +, Tian, Kevin wrote: > Agree. The vSVA series is still undergoing a refactor according to Jason's > comment thus won't be ready in short term. It's better to let this one > go in first. Would be great to get a few more reviews while we're at it :)
RE: [PATCH 15/17] iommu: remove DOMAIN_ATTR_NESTING
> From: Auger Eric > Sent: Monday, March 15, 2021 3:52 PM > To: Christoph Hellwig > Cc: k...@vger.kernel.org; Will Deacon ; linuxppc- > d...@lists.ozlabs.org; dri-de...@lists.freedesktop.org; Li Yang > ; io...@lists.linux-foundation.org; > > Hi Christoph, > > On 3/14/21 4:58 PM, Christoph Hellwig wrote: > > On Sun, Mar 14, 2021 at 11:44:52AM +0100, Auger Eric wrote: > >> As mentionned by Robin, there are series planning to use > >> DOMAIN_ATTR_NESTING to get info about the nested caps of the iommu > (ARM > >> and Intel): > >> > >> [Patch v8 00/10] vfio: expose virtual Shared Virtual Addressing to VMs > >> patches 1, 2, 3 > >> > >> Is the plan to introduce a new domain_get_nesting_info ops then? > > > > The plan as usual would be to add it the series adding that support. > > Not sure what the merge plans are - if the series is ready to be > > merged I could rebase on top of it, otherwise that series will need > > to add the method. > OK I think your series may be upstreamed first. > Agree. The vSVA series is still undergoing a refactor according to Jason's comment thus won't be ready in short term. It's better to let this one go in first. Thanks Kevin
Re: [PATCH 15/17] iommu: remove DOMAIN_ATTR_NESTING
Hi Christoph, On 3/14/21 4:58 PM, Christoph Hellwig wrote: > On Sun, Mar 14, 2021 at 11:44:52AM +0100, Auger Eric wrote: >> As mentionned by Robin, there are series planning to use >> DOMAIN_ATTR_NESTING to get info about the nested caps of the iommu (ARM >> and Intel): >> >> [Patch v8 00/10] vfio: expose virtual Shared Virtual Addressing to VMs >> patches 1, 2, 3 >> >> Is the plan to introduce a new domain_get_nesting_info ops then? > > The plan as usual would be to add it the series adding that support. > Not sure what the merge plans are - if the series is ready to be > merged I could rebase on top of it, otherwise that series will need > to add the method. OK I think your series may be upstreamed first. Thanks Eric >
Re: [PATCH 15/17] iommu: remove DOMAIN_ATTR_NESTING
On Sun, Mar 14, 2021 at 11:44:52AM +0100, Auger Eric wrote: > As mentionned by Robin, there are series planning to use > DOMAIN_ATTR_NESTING to get info about the nested caps of the iommu (ARM > and Intel): > > [Patch v8 00/10] vfio: expose virtual Shared Virtual Addressing to VMs > patches 1, 2, 3 > > Is the plan to introduce a new domain_get_nesting_info ops then? The plan as usual would be to add it the series adding that support. Not sure what the merge plans are - if the series is ready to be merged I could rebase on top of it, otherwise that series will need to add the method.
Re: [PATCH 15/17] iommu: remove DOMAIN_ATTR_NESTING
Hi Christoph,
On 3/1/21 9:42 AM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 40 ++---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 30 ++--
> drivers/iommu/intel/iommu.c | 28 +--
> drivers/iommu/iommu.c | 8 +
> drivers/vfio/vfio_iommu_type1.c | 5 +--
> include/linux/iommu.h | 4 ++-
> 6 files changed, 50 insertions(+), 65 deletions(-)
As mentionned by Robin, there are series planning to use
DOMAIN_ATTR_NESTING to get info about the nested caps of the iommu (ARM
and Intel):
[Patch v8 00/10] vfio: expose virtual Shared Virtual Addressing to VMs
patches 1, 2, 3
Is the plan to introduce a new domain_get_nesting_info ops then?
Thanks
Eric
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index bf96172e8c1f71..8e6fee3ea454d3 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2466,41 +2466,21 @@ static void arm_smmu_dma_enable_flush_queue(struct
> iommu_domain *domain)
> to_smmu_domain(domain)->non_strict = true;
> }
>
> -static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
> - enum iommu_attr attr, void *data)
> +static int arm_smmu_domain_enable_nesting(struct iommu_domain *domain)
> {
> - int ret = 0;
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> + int ret = -EPERM;
>
> - mutex_lock(_domain->init_mutex);
> + if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> + return -EINVAL;
>
> - switch (domain->type) {
> - case IOMMU_DOMAIN_UNMANAGED:
> - switch (attr) {
> - case DOMAIN_ATTR_NESTING:
> - if (smmu_domain->smmu) {
> - ret = -EPERM;
> - goto out_unlock;
> - }
> -
> - if (*(int *)data)
> - smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
> - else
> - smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
> - break;
> - default:
> - ret = -ENODEV;
> - }
> - break;
> - case IOMMU_DOMAIN_DMA:
> - ret = -ENODEV;
> - break;
> - default:
> - ret = -EINVAL;
> + mutex_lock(_domain->init_mutex);
> + if (!smmu_domain->smmu) {
> + smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
> + ret = 0;
> }
> -
> -out_unlock:
> mutex_unlock(_domain->init_mutex);
> +
> return ret;
> }
>
> @@ -2603,7 +2583,7 @@ static struct iommu_ops arm_smmu_ops = {
> .device_group = arm_smmu_device_group,
> .dma_use_flush_queue= arm_smmu_dma_use_flush_queue,
> .dma_enable_flush_queue = arm_smmu_dma_enable_flush_queue,
> - .domain_set_attr= arm_smmu_domain_set_attr,
> + .domain_enable_nesting = arm_smmu_domain_enable_nesting,
> .of_xlate = arm_smmu_of_xlate,
> .get_resv_regions = arm_smmu_get_resv_regions,
> .put_resv_regions = generic_iommu_put_resv_regions,
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index e7893e96f5177a..2e17d990d04481 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -1497,6 +1497,24 @@ static void arm_smmu_dma_enable_flush_queue(struct
> iommu_domain *domain)
> to_smmu_domain(domain)->pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
> }
>
> +static int arm_smmu_domain_enable_nesting(struct iommu_domain *domain)
> +{
> + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> + int ret = -EPERM;
> +
> + if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> + return -EINVAL;
> +
> + mutex_lock(_domain->init_mutex);
> + if (!smmu_domain->smmu) {
> + smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
> + ret = 0;
> + }
> + mutex_unlock(_domain->init_mutex);
> +
> + return ret;
> +}
> +
> static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
> enum iommu_attr attr, void *data)
> {
> @@ -1508,17 +1526,6 @@ static int arm_smmu_domain_set_attr(struct
> iommu_domain *domain,
> switch(domain->type) {
> case IOMMU_DOMAIN_UNMANAGED:
> switch (attr) {
> - case DOMAIN_ATTR_NESTING:
> - if (smmu_domain->smmu) {
> - ret = -EPERM;
> - goto out_unlock;
> - }
> -
> - if (*(int *)data)
> - smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
> -
[PATCH 15/17] iommu: remove DOMAIN_ATTR_NESTING
Signed-off-by: Christoph Hellwig
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 40 ++---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 30 ++--
drivers/iommu/intel/iommu.c | 28 +--
drivers/iommu/iommu.c | 8 +
drivers/vfio/vfio_iommu_type1.c | 5 +--
include/linux/iommu.h | 4 ++-
6 files changed, 50 insertions(+), 65 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index bf96172e8c1f71..8e6fee3ea454d3 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2466,41 +2466,21 @@ static void arm_smmu_dma_enable_flush_queue(struct
iommu_domain *domain)
to_smmu_domain(domain)->non_strict = true;
}
-static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
+static int arm_smmu_domain_enable_nesting(struct iommu_domain *domain)
{
- int ret = 0;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ int ret = -EPERM;
- mutex_lock(_domain->init_mutex);
+ if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+ return -EINVAL;
- switch (domain->type) {
- case IOMMU_DOMAIN_UNMANAGED:
- switch (attr) {
- case DOMAIN_ATTR_NESTING:
- if (smmu_domain->smmu) {
- ret = -EPERM;
- goto out_unlock;
- }
-
- if (*(int *)data)
- smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
- else
- smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
- break;
- default:
- ret = -ENODEV;
- }
- break;
- case IOMMU_DOMAIN_DMA:
- ret = -ENODEV;
- break;
- default:
- ret = -EINVAL;
+ mutex_lock(_domain->init_mutex);
+ if (!smmu_domain->smmu) {
+ smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
+ ret = 0;
}
-
-out_unlock:
mutex_unlock(_domain->init_mutex);
+
return ret;
}
@@ -2603,7 +2583,7 @@ static struct iommu_ops arm_smmu_ops = {
.device_group = arm_smmu_device_group,
.dma_use_flush_queue= arm_smmu_dma_use_flush_queue,
.dma_enable_flush_queue = arm_smmu_dma_enable_flush_queue,
- .domain_set_attr= arm_smmu_domain_set_attr,
+ .domain_enable_nesting = arm_smmu_domain_enable_nesting,
.of_xlate = arm_smmu_of_xlate,
.get_resv_regions = arm_smmu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index e7893e96f5177a..2e17d990d04481 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1497,6 +1497,24 @@ static void arm_smmu_dma_enable_flush_queue(struct
iommu_domain *domain)
to_smmu_domain(domain)->pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
}
+static int arm_smmu_domain_enable_nesting(struct iommu_domain *domain)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ int ret = -EPERM;
+
+ if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+ return -EINVAL;
+
+ mutex_lock(_domain->init_mutex);
+ if (!smmu_domain->smmu) {
+ smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
+ ret = 0;
+ }
+ mutex_unlock(_domain->init_mutex);
+
+ return ret;
+}
+
static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
enum iommu_attr attr, void *data)
{
@@ -1508,17 +1526,6 @@ static int arm_smmu_domain_set_attr(struct iommu_domain
*domain,
switch(domain->type) {
case IOMMU_DOMAIN_UNMANAGED:
switch (attr) {
- case DOMAIN_ATTR_NESTING:
- if (smmu_domain->smmu) {
- ret = -EPERM;
- goto out_unlock;
- }
-
- if (*(int *)data)
- smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
- else
- smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
- break;
case DOMAIN_ATTR_IO_PGTABLE_CFG: {
struct io_pgtable_domain_attr *pgtbl_cfg = data;
@@ -1603,6 +1610,7 @@ static struct iommu_ops arm_smmu_ops = {
.dma_use_flush_queue= arm_smmu_dma_use_flush_queue,
.dma_enable_flush_queue = arm_smmu_dma_enable_flush_queue,
.domain_set_attr=
