2019-06-10 18:53:27

by Jean-Philippe Brucker

[permalink] [raw]
Subject: [PATCH 0/8] iommu: Add auxiliary domain and PASID support to Arm SMMUv3

Add substreams and PCI PASID support to the SMMUv3 driver. At the moment
the driver supports a single address space per device. PASID enables
multiple address spaces per device, up to a million in theory (1 << 20).

Two kernel features will make use of PASIDs, auxiliary domains (AUXD)
and Shared Virtual Addressing (SVA). Auxiliary domains allow to program
PASID contexts using IOMMU domains. SVA allows to bind process address
spaces to device contexts and relieve device drivers of DMA management.

Since SVA support for SMMUv3 has a lot more dependencies (new fault API,
ASID pinning, generic bind, PRI or stall support, and so on),
introducing PASID support to the SMMUv3 driver is easier with auxiliary
domains.

The AUXD API allows device drivers to easily test PASID support of their
devices, although they need to allocate IOVA and pages themselves
because the DMA API doesn't support AUXD for the moment:

iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_AUX);
domain = iommu_domain_alloc(dev->bus);
iommu_aux_attach_device(domain, dev);
iommu_map(domain, iova, phys_addr, size, prot);
pasid = iommu_aux_get_pasid(domain);
/* Then launch DMA with the PASID and IOVA */

Auxiliary domains also allow to split devices into multiple contexts
assignable to guest, with vfio-mdev.

Past discussions for these patches:
* Auxiliary domains (patch 6)
[RFC PATCH 0/6] Auxiliary IOMMU domains and Arm SMMUv3
https://www.spinics.net/lists/iommu/msg30637.html
* SSID support for the SMMU (patches 2, 3, 4, 5, 7 and 8)
[PATCH v2 00/40] Shared Virtual Addressing for the IOMMU
https://lists.linuxfoundation.org/pipermail/iommu/2018-May/027595.html
* I/O ASID (patch 1)
[PATCH v3 00/16] Shared virtual address IOMMU and VT-d support
https://lkml.kernel.org/lkml/[email protected]/

Jean-Philippe Brucker (8):
iommu: Add I/O ASID allocator
dt-bindings: document PASID property for IOMMU masters
iommu/arm-smmu-v3: Support platform SSID
iommu/arm-smmu-v3: Add support for Substream IDs
iommu/arm-smmu-v3: Add second level of context descriptor table
iommu/arm-smmu-v3: Support auxiliary domains
iommu/arm-smmu-v3: Improve add_device() error handling
iommu/arm-smmu-v3: Add support for PCI PASID

.../devicetree/bindings/iommu/iommu.txt | 6 +
drivers/iommu/Kconfig | 5 +
drivers/iommu/Makefile | 1 +
drivers/iommu/arm-smmu-v3.c | 714 ++++++++++++++++--
drivers/iommu/ioasid.c | 150 ++++
drivers/iommu/of_iommu.c | 6 +-
include/linux/ioasid.h | 49 ++
include/linux/iommu.h | 1 +
8 files changed, 865 insertions(+), 67 deletions(-)
create mode 100644 drivers/iommu/ioasid.c
create mode 100644 include/linux/ioasid.h

--
2.21.0


2019-06-10 18:53:33

by Jean-Philippe Brucker

[permalink] [raw]
Subject: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

For platform devices that support SubstreamID (SSID), firmware provides
the number of supported SSID bits. Restrict it to what the SMMU supports
and cache it into master->ssid_bits.

Signed-off-by: Jean-Philippe Brucker <[email protected]>
---
drivers/iommu/arm-smmu-v3.c | 11 +++++++++++
drivers/iommu/of_iommu.c | 6 +++++-
include/linux/iommu.h | 1 +
3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4d5a694f02c2..3254f473e681 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -604,6 +604,7 @@ struct arm_smmu_master {
struct list_head domain_head;
u32 *sids;
unsigned int num_sids;
+ unsigned int ssid_bits;
bool ats_enabled :1;
};

@@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
}
}

+ master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
+
+ /*
+ * If the SMMU doesn't support 2-stage CD, limit the linear
+ * tables to a reasonable number of contexts, let's say
+ * 64kB / sizeof(ctx_desc) = 1024 = 2^10
+ */
+ if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
+ master->ssid_bits = min(master->ssid_bits, 10U);
+
group = iommu_group_get_for_dev(dev);
if (!IS_ERR(group)) {
iommu_group_put(group);
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index f04a6df65eb8..04f4f6b95d82 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
if (err)
break;
}
- }

+ fwspec = dev_iommu_fwspec_get(dev);
+ if (!err && fwspec)
+ of_property_read_u32(master_np, "pasid-num-bits",
+ &fwspec->num_pasid_bits);
+ }

/*
* Two success conditions can be represented by non-negative err here:
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 519e40fb23ce..b91df613385f 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -536,6 +536,7 @@ struct iommu_fwspec {
struct fwnode_handle *iommu_fwnode;
void *iommu_priv;
u32 flags;
+ u32 num_pasid_bits;
unsigned int num_ids;
u32 ids[1];
};
--
2.21.0

2019-06-11 09:53:41

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

On Mon, 10 Jun 2019 19:47:09 +0100
Jean-Philippe Brucker <[email protected]> wrote:

> For platform devices that support SubstreamID (SSID), firmware provides
> the number of supported SSID bits. Restrict it to what the SMMU supports
> and cache it into master->ssid_bits.
>
> Signed-off-by: Jean-Philippe Brucker <[email protected]>

Missing kernel-doc.

Thanks,

Jonathan

> ---
> drivers/iommu/arm-smmu-v3.c | 11 +++++++++++
> drivers/iommu/of_iommu.c | 6 +++++-
> include/linux/iommu.h | 1 +
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 4d5a694f02c2..3254f473e681 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -604,6 +604,7 @@ struct arm_smmu_master {
> struct list_head domain_head;
> u32 *sids;
> unsigned int num_sids;
> + unsigned int ssid_bits;
> bool ats_enabled :1;
> };
>
> @@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
> }
> }
>
> + master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
> +
> + /*
> + * If the SMMU doesn't support 2-stage CD, limit the linear
> + * tables to a reasonable number of contexts, let's say
> + * 64kB / sizeof(ctx_desc) = 1024 = 2^10
> + */
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> + master->ssid_bits = min(master->ssid_bits, 10U);
> +
> group = iommu_group_get_for_dev(dev);
> if (!IS_ERR(group)) {
> iommu_group_put(group);
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index f04a6df65eb8..04f4f6b95d82 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> if (err)
> break;
> }
> - }
>
> + fwspec = dev_iommu_fwspec_get(dev);
> + if (!err && fwspec)
> + of_property_read_u32(master_np, "pasid-num-bits",
> + &fwspec->num_pasid_bits);
> + }
>
> /*
> * Two success conditions can be represented by non-negative err here:
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 519e40fb23ce..b91df613385f 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -536,6 +536,7 @@ struct iommu_fwspec {
> struct fwnode_handle *iommu_fwnode;
> void *iommu_priv;
> u32 flags;
> + u32 num_pasid_bits;

This structure has kernel doc so you need to add something for this.

> unsigned int num_ids;
> u32 ids[1];
> };


2019-06-11 14:36:19

by Jean-Philippe Brucker

[permalink] [raw]
Subject: Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

On 11/06/2019 10:42, Jonathan Cameron wrote:
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 519e40fb23ce..b91df613385f 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -536,6 +536,7 @@ struct iommu_fwspec {
>> struct fwnode_handle *iommu_fwnode;
>> void *iommu_priv;
>> u32 flags;
>> + u32 num_pasid_bits;
>
> This structure has kernel doc so you need to add something for this.

Good catch

Thanks,
Jean

2019-06-18 18:09:19

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

On Mon, Jun 10, 2019 at 07:47:09PM +0100, Jean-Philippe Brucker wrote:
> For platform devices that support SubstreamID (SSID), firmware provides
> the number of supported SSID bits. Restrict it to what the SMMU supports
> and cache it into master->ssid_bits.
>
> Signed-off-by: Jean-Philippe Brucker <[email protected]>
> ---
> drivers/iommu/arm-smmu-v3.c | 11 +++++++++++
> drivers/iommu/of_iommu.c | 6 +++++-
> include/linux/iommu.h | 1 +
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 4d5a694f02c2..3254f473e681 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -604,6 +604,7 @@ struct arm_smmu_master {
> struct list_head domain_head;
> u32 *sids;
> unsigned int num_sids;
> + unsigned int ssid_bits;
> bool ats_enabled :1;
> };
>
> @@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
> }
> }
>
> + master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
> +
> + /*
> + * If the SMMU doesn't support 2-stage CD, limit the linear
> + * tables to a reasonable number of contexts, let's say
> + * 64kB / sizeof(ctx_desc) = 1024 = 2^10
> + */
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> + master->ssid_bits = min(master->ssid_bits, 10U);

Please introduce a #define for the 10, so that it is computed in the way
you describe in the comment (a bit like we do for things like queue sizes).

> +
> group = iommu_group_get_for_dev(dev);
> if (!IS_ERR(group)) {
> iommu_group_put(group);
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index f04a6df65eb8..04f4f6b95d82 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> if (err)
> break;
> }
> - }
>
> + fwspec = dev_iommu_fwspec_get(dev);
> + if (!err && fwspec)
> + of_property_read_u32(master_np, "pasid-num-bits",
> + &fwspec->num_pasid_bits);
> + }

Hmm. Do you know if there's anything in ACPI for this?

Otherwise, patch looks fine. Thanks.

Will

2019-06-19 11:54:48

by Jean-Philippe Brucker

[permalink] [raw]
Subject: Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

On 18/06/2019 19:08, Will Deacon wrote:
>> + /*
>> + * If the SMMU doesn't support 2-stage CD, limit the linear
>> + * tables to a reasonable number of contexts, let's say
>> + * 64kB / sizeof(ctx_desc) = 1024 = 2^10
>> + */
>> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
>> + master->ssid_bits = min(master->ssid_bits, 10U);
>
> Please introduce a #define for the 10, so that it is computed in the way
> you describe in the comment (a bit like we do for things like queue sizes).

Ok

>> +
>> group = iommu_group_get_for_dev(dev);
>> if (!IS_ERR(group)) {
>> iommu_group_put(group);
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index f04a6df65eb8..04f4f6b95d82 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
>> if (err)
>> break;
>> }
>> - }
>>
>> + fwspec = dev_iommu_fwspec_get(dev);
>> + if (!err && fwspec)
>> + of_property_read_u32(master_np, "pasid-num-bits",
>> + &fwspec->num_pasid_bits);
>> + }
>
> Hmm. Do you know if there's anything in ACPI for this?

Yes, IORT version D introduced a "substream width" field for the Named
component node (platform device). I don't think it existed last time I
checked, so I'll see about supporting it.

Thanks,
Jean

2019-07-08 10:36:15

by Eric Auger

[permalink] [raw]
Subject: Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

Hi Jean,

On 6/10/19 8:47 PM, Jean-Philippe Brucker wrote:
> For platform devices that support SubstreamID (SSID), firmware provides
> the number of supported SSID bits. Restrict it to what the SMMU supports
> and cache it into master->ssid_bits.
The commit message may give the impression the master's ssid_bits field
only is used for platform devices.
>
> Signed-off-by: Jean-Philippe Brucker <[email protected]>
> ---
> drivers/iommu/arm-smmu-v3.c | 11 +++++++++++
> drivers/iommu/of_iommu.c | 6 +++++-
> include/linux/iommu.h | 1 +
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 4d5a694f02c2..3254f473e681 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -604,6 +604,7 @@ struct arm_smmu_master {
> struct list_head domain_head;
> u32 *sids;
> unsigned int num_sids;
> + unsigned int ssid_bits;
> bool ats_enabled :1;
> };
>
> @@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
> }
> }
>
> + master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
In case the device is a PCI device, what is the value taken by
fwspec->num_pasid_bits?
> +
> + /*
> + * If the SMMU doesn't support 2-stage CD, limit the linear
> + * tables to a reasonable number of contexts, let's say
> + * 64kB / sizeof(ctx_desc) = 1024 = 2^10
ctx_desc is 26B so 11bits would be OK
> + */
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> + master->ssid_bits = min(master->ssid_bits, 10U);
> +
> group = iommu_group_get_for_dev(dev);
> if (!IS_ERR(group)) {
> iommu_group_put(group);
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index f04a6df65eb8..04f4f6b95d82 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -206,8 +206,12 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> if (err)
> break;
> }
> - }
>
> + fwspec = dev_iommu_fwspec_get(dev);
> + if (!err && fwspec)
> + of_property_read_u32(master_np, "pasid-num-bits",
> + &fwspec->num_pasid_bits);
> + }
>
> /*
> * Two success conditions can be represented by non-negative err here:
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 519e40fb23ce..b91df613385f 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -536,6 +536,7 @@ struct iommu_fwspec {
> struct fwnode_handle *iommu_fwnode;
> void *iommu_priv;
> u32 flags;
> + u32 num_pasid_bits;
> unsigned int num_ids;
> u32 ids[1];
> };
>
Thanks

Eric

2019-09-19 20:49:41

by Jean-Philippe Brucker

[permalink] [raw]
Subject: Re: [PATCH 3/8] iommu/arm-smmu-v3: Support platform SSID

Hi Eric,

Sorry for the delay. I'll see if I can resend this for v5.5, although I
can't do much testing at the moment.

On Mon, Jul 08, 2019 at 09:58:22AM +0200, Auger Eric wrote:
> Hi Jean,
>
> On 6/10/19 8:47 PM, Jean-Philippe Brucker wrote:
> > For platform devices that support SubstreamID (SSID), firmware provides
> > the number of supported SSID bits. Restrict it to what the SMMU supports
> > and cache it into master->ssid_bits.
> The commit message may give the impression the master's ssid_bits field
> only is used for platform devices.

Ok maybe I should add that this field will be used for PCI PASID as
well.

> > @@ -2097,6 +2098,16 @@ static int arm_smmu_add_device(struct device *dev)
> > }
> > }
> >
> > + master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
> In case the device is a PCI device, what is the value taken by
> fwspec->num_pasid_bits?

It would be zero, as firmware only specifies a value for platform
devices. For a PCI device, patch 8/8 fills master->ssid_bits from the
PCIe PASID capability.

> > + /*
> > + * If the SMMU doesn't support 2-stage CD, limit the linear
> > + * tables to a reasonable number of contexts, let's say
> > + * 64kB / sizeof(ctx_desc) = 1024 = 2^10
> ctx_desc is 26B so 11bits would be OK

This refers to the size of the hardware context descriptor, not struct
arm_smmu_ctx_desc. Next version moves this to a define and makes it
clearer.

Thanks,
Jean