* [PATCH v3 01/13] iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-13 16:45 ` Jonathan Cameron
2019-12-17 10:59 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 02/13] dt-bindings: document PASID property for IOMMU masters Jean-Philippe Brucker
` (12 subsequent siblings)
13 siblings, 2 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
Since commit 518a2f1925c3 ("dma-mapping: zero memory returned from
dma_alloc_*"), dma_alloc_* always initializes memory to zero, so there
is no need to use dma_zalloc_* or pass the __GFP_ZERO flag anymore.
The flag was introduced by commit 04fa26c71be5 ("iommu/arm-smmu: Convert
DMA buffer allocations to the managed API"), since the managed API
didn't provide a dmam_zalloc_coherent() function.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index effe72eb89e7..d4e8b7f8d9f4 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1675,7 +1675,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
desc->span = STRTAB_SPLIT + 1;
desc->l2ptr = dmam_alloc_coherent(smmu->dev, size, &desc->l2ptr_dma,
- GFP_KERNEL | __GFP_ZERO);
+ GFP_KERNEL);
if (!desc->l2ptr) {
dev_err(smmu->dev,
"failed to allocate l2 stream table for SID %u\n",
@@ -2161,8 +2161,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
return asid;
cfg->cdptr = dmam_alloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
- &cfg->cdptr_dma,
- GFP_KERNEL | __GFP_ZERO);
+ &cfg->cdptr_dma, GFP_KERNEL);
if (!cfg->cdptr) {
dev_warn(smmu->dev, "failed to allocate context descriptor\n");
ret = -ENOMEM;
@@ -2883,7 +2882,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
strtab = dmam_alloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
- GFP_KERNEL | __GFP_ZERO);
+ GFP_KERNEL);
if (!strtab) {
dev_err(smmu->dev,
"failed to allocate l1 stream table (%u bytes)\n",
@@ -2910,7 +2909,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
size = (1 << smmu->sid_bits) * (STRTAB_STE_DWORDS << 3);
strtab = dmam_alloc_coherent(smmu->dev, size, &cfg->strtab_dma,
- GFP_KERNEL | __GFP_ZERO);
+ GFP_KERNEL);
if (!strtab) {
dev_err(smmu->dev,
"failed to allocate linear stream table (%u bytes)\n",
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 01/13] iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation
2019-12-09 18:05 ` [PATCH v3 01/13] iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation Jean-Philippe Brucker
@ 2019-12-13 16:45 ` Jonathan Cameron
2019-12-17 10:59 ` Auger Eric
1 sibling, 0 replies; 40+ messages in thread
From: Jonathan Cameron @ 2019-12-13 16:45 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, zhangfei.gao
On Mon, 9 Dec 2019 19:05:02 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> Since commit 518a2f1925c3 ("dma-mapping: zero memory returned from
> dma_alloc_*"), dma_alloc_* always initializes memory to zero, so there
> is no need to use dma_zalloc_* or pass the __GFP_ZERO flag anymore.
>
> The flag was introduced by commit 04fa26c71be5 ("iommu/arm-smmu: Convert
> DMA buffer allocations to the managed API"), since the managed API
> didn't provide a dmam_zalloc_coherent() function.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Good to tidy these up whilst we are here.
Jonathan
> ---
> drivers/iommu/arm-smmu-v3.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index effe72eb89e7..d4e8b7f8d9f4 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1675,7 +1675,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
>
> desc->span = STRTAB_SPLIT + 1;
> desc->l2ptr = dmam_alloc_coherent(smmu->dev, size, &desc->l2ptr_dma,
> - GFP_KERNEL | __GFP_ZERO);
> + GFP_KERNEL);
> if (!desc->l2ptr) {
> dev_err(smmu->dev,
> "failed to allocate l2 stream table for SID %u\n",
> @@ -2161,8 +2161,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
> return asid;
>
> cfg->cdptr = dmam_alloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
> - &cfg->cdptr_dma,
> - GFP_KERNEL | __GFP_ZERO);
> + &cfg->cdptr_dma, GFP_KERNEL);
> if (!cfg->cdptr) {
> dev_warn(smmu->dev, "failed to allocate context descriptor\n");
> ret = -ENOMEM;
> @@ -2883,7 +2882,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>
> l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
> strtab = dmam_alloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
> - GFP_KERNEL | __GFP_ZERO);
> + GFP_KERNEL);
> if (!strtab) {
> dev_err(smmu->dev,
> "failed to allocate l1 stream table (%u bytes)\n",
> @@ -2910,7 +2909,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
>
> size = (1 << smmu->sid_bits) * (STRTAB_STE_DWORDS << 3);
> strtab = dmam_alloc_coherent(smmu->dev, size, &cfg->strtab_dma,
> - GFP_KERNEL | __GFP_ZERO);
> + GFP_KERNEL);
> if (!strtab) {
> dev_err(smmu->dev,
> "failed to allocate linear stream table (%u bytes)\n",
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 01/13] iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation
2019-12-09 18:05 ` [PATCH v3 01/13] iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation Jean-Philippe Brucker
2019-12-13 16:45 ` Jonathan Cameron
@ 2019-12-17 10:59 ` Auger Eric
1 sibling, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-17 10:59 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> Since commit 518a2f1925c3 ("dma-mapping: zero memory returned from
> dma_alloc_*"), dma_alloc_* always initializes memory to zero, so there
> is no need to use dma_zalloc_* or pass the __GFP_ZERO flag anymore.
>
> The flag was introduced by commit 04fa26c71be5 ("iommu/arm-smmu: Convert
> DMA buffer allocations to the managed API"), since the managed API
> didn't provide a dmam_zalloc_coherent() function.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> drivers/iommu/arm-smmu-v3.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index effe72eb89e7..d4e8b7f8d9f4 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1675,7 +1675,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
>
> desc->span = STRTAB_SPLIT + 1;
> desc->l2ptr = dmam_alloc_coherent(smmu->dev, size, &desc->l2ptr_dma,
> - GFP_KERNEL | __GFP_ZERO);
> + GFP_KERNEL);
> if (!desc->l2ptr) {
> dev_err(smmu->dev,
> "failed to allocate l2 stream table for SID %u\n",
> @@ -2161,8 +2161,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
> return asid;
>
> cfg->cdptr = dmam_alloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
> - &cfg->cdptr_dma,
> - GFP_KERNEL | __GFP_ZERO);
> + &cfg->cdptr_dma, GFP_KERNEL);
> if (!cfg->cdptr) {
> dev_warn(smmu->dev, "failed to allocate context descriptor\n");
> ret = -ENOMEM;
> @@ -2883,7 +2882,7 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
>
> l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
> strtab = dmam_alloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
> - GFP_KERNEL | __GFP_ZERO);
> + GFP_KERNEL);
> if (!strtab) {
> dev_err(smmu->dev,
> "failed to allocate l1 stream table (%u bytes)\n",
> @@ -2910,7 +2909,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
>
> size = (1 << smmu->sid_bits) * (STRTAB_STE_DWORDS << 3);
> strtab = dmam_alloc_coherent(smmu->dev, size, &cfg->strtab_dma,
> - GFP_KERNEL | __GFP_ZERO);
> + GFP_KERNEL);
> if (!strtab) {
> dev_err(smmu->dev,
> "failed to allocate linear stream table (%u bytes)\n",
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 02/13] dt-bindings: document PASID property for IOMMU masters
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 01/13] iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID Jean-Philippe Brucker
` (11 subsequent siblings)
13 siblings, 0 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
On Arm systems, some platform devices behind an SMMU may support the PASID
feature, which offers multiple address space. Let the firmware tell us
when a device supports PASID.
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
Documentation/devicetree/bindings/iommu/iommu.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt b/Documentation/devicetree/bindings/iommu/iommu.txt
index 5a8b4624defc..3c36334e4f94 100644
--- a/Documentation/devicetree/bindings/iommu/iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/iommu.txt
@@ -86,6 +86,12 @@ have a means to turn off translation. But it is invalid in such cases to
disable the IOMMU's device tree node in the first place because it would
prevent any driver from properly setting up the translations.
+Optional properties:
+--------------------
+- pasid-num-bits: Some masters support multiple address spaces for DMA, by
+ tagging DMA transactions with an address space identifier. By default,
+ this is 0, which means that the device only has one address space.
+
Notes:
======
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 01/13] iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 02/13] dt-bindings: document PASID property for IOMMU masters Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-17 11:05 ` Auger Eric
2019-12-18 10:17 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 04/13] ACPI/IORT: Support PASID for platform devices Jean-Philippe Brucker
` (10 subsequent siblings)
13 siblings, 2 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
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, which will also be used for PCI
PASID.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 13 +++++++++++++
drivers/iommu/of_iommu.c | 6 +++++-
include/linux/iommu.h | 2 ++
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d4e8b7f8d9f4..837b4283b4dc 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -292,6 +292,12 @@
#define CTXDESC_CD_1_TTB0_MASK GENMASK_ULL(51, 4)
+/*
+ * When the SMMU only supports linear context descriptor tables, pick a
+ * reasonable size limit (64kB).
+ */
+#define CTXDESC_LINEAR_CDMAX ilog2(SZ_64K / (CTXDESC_CD_DWORDS << 3))
+
/* Convert between AArch64 (CPU) TCR format and SMMU CD format */
#define ARM_SMMU_TCR2CD(tcr, fld) FIELD_PREP(CTXDESC_CD_0_TCR_##fld, \
FIELD_GET(ARM64_TCR_##fld, tcr))
@@ -638,6 +644,7 @@ struct arm_smmu_master {
u32 *sids;
unsigned int num_sids;
bool ats_enabled;
+ unsigned int ssid_bits;
};
/* SMMU private data for an IOMMU domain */
@@ -2571,6 +2578,12 @@ static int arm_smmu_add_device(struct device *dev)
}
}
+ master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
+
+ if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
+ master->ssid_bits = min_t(u8, master->ssid_bits,
+ CTXDESC_LINEAR_CDMAX);
+
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 026ad2b29dcd..b3ccb2f7f1c7 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -196,8 +196,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 3a113c5d7394..bd46775c3329 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -581,6 +581,7 @@ struct iommu_group *fsl_mc_device_group(struct device *dev);
* @ops: ops for this device's IOMMU
* @iommu_fwnode: firmware handle for this device's IOMMU
* @iommu_priv: IOMMU driver private data for this device
+ * @num_pasid_bits: number of PASID bits supported by this device
* @num_ids: number of associated device IDs
* @ids: IDs which this device may present to the IOMMU
*/
@@ -589,6 +590,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.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID
2019-12-09 18:05 ` [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID Jean-Philippe Brucker
@ 2019-12-17 11:05 ` Auger Eric
2019-12-17 15:21 ` Jean-Philippe Brucker
2019-12-18 10:17 ` Auger Eric
1 sibling, 1 reply; 40+ messages in thread
From: Auger Eric @ 2019-12-17 11:05 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 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, which will also be used for PCI
> PASID.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> drivers/iommu/arm-smmu-v3.c | 13 +++++++++++++
> drivers/iommu/of_iommu.c | 6 +++++-
> include/linux/iommu.h | 2 ++
> 3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index d4e8b7f8d9f4..837b4283b4dc 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -292,6 +292,12 @@
>
> #define CTXDESC_CD_1_TTB0_MASK GENMASK_ULL(51, 4)
>
> +/*
> + * When the SMMU only supports linear context descriptor tables, pick a
> + * reasonable size limit (64kB).
> + */
> +#define CTXDESC_LINEAR_CDMAX ilog2(SZ_64K / (CTXDESC_CD_DWORDS << 3))
> +
> /* Convert between AArch64 (CPU) TCR format and SMMU CD format */
> #define ARM_SMMU_TCR2CD(tcr, fld) FIELD_PREP(CTXDESC_CD_0_TCR_##fld, \
> FIELD_GET(ARM64_TCR_##fld, tcr))
> @@ -638,6 +644,7 @@ struct arm_smmu_master {
> u32 *sids;
> unsigned int num_sids;
> bool ats_enabled;
> + unsigned int ssid_bits;
> };
>
> /* SMMU private data for an IOMMU domain */
> @@ -2571,6 +2578,12 @@ static int arm_smmu_add_device(struct device *dev)
> }
> }
>
> + master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
> +
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> + master->ssid_bits = min_t(u8, master->ssid_bits,
> + CTXDESC_LINEAR_CDMAX);
> +
> 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 026ad2b29dcd..b3ccb2f7f1c7 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -196,8 +196,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);
> + }
This patch dedicates to platform devices however I fail to understand,
at that stage, how/when do you retrieve/store the same max capability
for PCI devices?
>
> /*
> * Two success conditions can be represented by non-negative err here:
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 3a113c5d7394..bd46775c3329 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -581,6 +581,7 @@ struct iommu_group *fsl_mc_device_group(struct device *dev);
> * @ops: ops for this device's IOMMU
> * @iommu_fwnode: firmware handle for this device's IOMMU
> * @iommu_priv: IOMMU driver private data for this device
> + * @num_pasid_bits: number of PASID bits supported by this device
> * @num_ids: number of associated device IDs
> * @ids: IDs which this device may present to the IOMMU
> */
> @@ -589,6 +590,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];
> };
>
Besides,
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID
2019-12-17 11:05 ` Auger Eric
@ 2019-12-17 15:21 ` Jean-Philippe Brucker
2019-12-17 16:46 ` Auger Eric
0 siblings, 1 reply; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-17 15:21 UTC (permalink / raw)
To: Auger Eric
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Eric,
On Tue, Dec 17, 2019 at 12:05:18PM +0100, Auger Eric wrote:
> > + fwspec = dev_iommu_fwspec_get(dev);
> > + if (!err && fwspec)
> > + of_property_read_u32(master_np, "pasid-num-bits",
> > + &fwspec->num_pasid_bits);
> > + }
> This patch dedicates to platform devices however I fail to understand,
> at that stage, how/when do you retrieve/store the same max capability
> for PCI devices?
For PCI devices, the max capability is only described by the PCIe PASID
capability, not by firmware. Patch 13 deals with PCI by setting
masted->ssid_bits from the PASID capability directly, ignoring
fwspec->num_pasid_bits.
> Besides,
>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks!
Jean
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID
2019-12-17 15:21 ` Jean-Philippe Brucker
@ 2019-12-17 16:46 ` Auger Eric
0 siblings, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-17 16:46 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: mark.rutland, devicetree, lorenzo.pieralisi, linux-pci, joro,
sudeep.holla, rjw, robin.murphy, linux-acpi, iommu, robh+dt,
jonathan.cameron, guohanjun, bhelgaas, zhangfei.gao, will,
linux-arm-kernel, lenb
Hi Jean,
On 12/17/19 4:21 PM, Jean-Philippe Brucker wrote:
> Hi Eric,
>
> On Tue, Dec 17, 2019 at 12:05:18PM +0100, Auger Eric wrote:
>>> + fwspec = dev_iommu_fwspec_get(dev);
>>> + if (!err && fwspec)
>>> + of_property_read_u32(master_np, "pasid-num-bits",
>>> + &fwspec->num_pasid_bits);
>>> + }
>> This patch dedicates to platform devices however I fail to understand,
>> at that stage, how/when do you retrieve/store the same max capability
>> for PCI devices?
>
> For PCI devices, the max capability is only described by the PCIe PASID
> capability, not by firmware. Patch 13 deals with PCI by setting
> masted->ssid_bits from the PASID capability directly, ignoring
> fwspec->num_pasid_bits.
Get it now. thank you for the explanation.
Eric
>
>> Besides,
>>
>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>
> Thanks!
> Jean
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID
2019-12-09 18:05 ` [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID Jean-Philippe Brucker
2019-12-17 11:05 ` Auger Eric
@ 2019-12-18 10:17 ` Auger Eric
2019-12-18 16:06 ` Jean-Philippe Brucker
1 sibling, 1 reply; 40+ messages in thread
From: Auger Eric @ 2019-12-18 10:17 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 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, which will also be used for PCI
> PASID.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
the title of the patch does not really explain what it actually does. At
this stage we are far from supporting SSIDs ;-) Same for 04?
Thanks
Eric
> ---
> drivers/iommu/arm-smmu-v3.c | 13 +++++++++++++
> drivers/iommu/of_iommu.c | 6 +++++-
> include/linux/iommu.h | 2 ++
> 3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index d4e8b7f8d9f4..837b4283b4dc 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -292,6 +292,12 @@
>
> #define CTXDESC_CD_1_TTB0_MASK GENMASK_ULL(51, 4)
>
> +/*
> + * When the SMMU only supports linear context descriptor tables, pick a
> + * reasonable size limit (64kB).
> + */
> +#define CTXDESC_LINEAR_CDMAX ilog2(SZ_64K / (CTXDESC_CD_DWORDS << 3))
> +
> /* Convert between AArch64 (CPU) TCR format and SMMU CD format */
> #define ARM_SMMU_TCR2CD(tcr, fld) FIELD_PREP(CTXDESC_CD_0_TCR_##fld, \
> FIELD_GET(ARM64_TCR_##fld, tcr))
> @@ -638,6 +644,7 @@ struct arm_smmu_master {
> u32 *sids;
> unsigned int num_sids;
> bool ats_enabled;
> + unsigned int ssid_bits;
> };
>
> /* SMMU private data for an IOMMU domain */
> @@ -2571,6 +2578,12 @@ static int arm_smmu_add_device(struct device *dev)
> }
> }
>
> + master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
> +
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> + master->ssid_bits = min_t(u8, master->ssid_bits,
> + CTXDESC_LINEAR_CDMAX);
> +
> 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 026ad2b29dcd..b3ccb2f7f1c7 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -196,8 +196,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 3a113c5d7394..bd46775c3329 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -581,6 +581,7 @@ struct iommu_group *fsl_mc_device_group(struct device *dev);
> * @ops: ops for this device's IOMMU
> * @iommu_fwnode: firmware handle for this device's IOMMU
> * @iommu_priv: IOMMU driver private data for this device
> + * @num_pasid_bits: number of PASID bits supported by this device
> * @num_ids: number of associated device IDs
> * @ids: IDs which this device may present to the IOMMU
> */
> @@ -589,6 +590,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];
> };
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID
2019-12-18 10:17 ` Auger Eric
@ 2019-12-18 16:06 ` Jean-Philippe Brucker
0 siblings, 0 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-18 16:06 UTC (permalink / raw)
To: Auger Eric
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
On Wed, Dec 18, 2019 at 11:17:40AM +0100, Auger Eric wrote:
> Hi Jean,
>
> On 12/9/19 7:05 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, which will also be used for PCI
> > PASID.
> >
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> the title of the patch does not really explain what it actually does. At
> this stage we are far from supporting SSIDs ;-) Same for 04?
Alright I'll make this "Parse PASID devicetree property of platform
devices" and "Parse SSID property of named component node"
Thanks,
Jean
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 04/13] ACPI/IORT: Support PASID for platform devices
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (2 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-17 13:27 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 05/13] iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support Jean-Philippe Brucker
` (9 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
Named component nodes in the IORT tables describe the number of
Substream ID bits (aka. PASID) supported by the device. Propagate this
value to the fwspec structure in order to enable PASID for platform
devices.
Acked-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/acpi/arm64/iort.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 33f71983e001..39f389214ecf 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -11,6 +11,7 @@
#define pr_fmt(fmt) "ACPI: IORT: " fmt
#include <linux/acpi_iort.h>
+#include <linux/bitfield.h>
#include <linux/iommu.h>
#include <linux/kernel.h>
#include <linux/list.h>
@@ -924,6 +925,20 @@ static int iort_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
return iort_iommu_xlate(info->dev, parent, streamid);
}
+static void iort_named_component_init(struct device *dev,
+ struct acpi_iort_node *node)
+{
+ struct acpi_iort_named_component *nc;
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+
+ if (!fwspec)
+ return;
+
+ nc = (struct acpi_iort_named_component *)node->node_data;
+ fwspec->num_pasid_bits = FIELD_GET(ACPI_IORT_NC_PASID_BITS,
+ nc->node_flags);
+}
+
/**
* iort_iommu_configure - Set-up IOMMU configuration for a device.
*
@@ -978,6 +993,9 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
if (parent)
err = iort_iommu_xlate(dev, parent, streamid);
} while (parent && !err);
+
+ if (!err)
+ iort_named_component_init(dev, node);
}
/*
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 04/13] ACPI/IORT: Support PASID for platform devices
2019-12-09 18:05 ` [PATCH v3 04/13] ACPI/IORT: Support PASID for platform devices Jean-Philippe Brucker
@ 2019-12-17 13:27 ` Auger Eric
0 siblings, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-17 13:27 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> Named component nodes in the IORT tables describe the number of
> Substream ID bits (aka. PASID) supported by the device. Propagate this
> value to the fwspec structure in order to enable PASID for platform
> devices.
>
> Acked-by: Hanjun Guo <guohanjun@huawei.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> drivers/acpi/arm64/iort.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 33f71983e001..39f389214ecf 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -11,6 +11,7 @@
> #define pr_fmt(fmt) "ACPI: IORT: " fmt
>
> #include <linux/acpi_iort.h>
> +#include <linux/bitfield.h>> #include <linux/iommu.h>
> #include <linux/kernel.h>
> #include <linux/list.h>
> @@ -924,6 +925,20 @@ static int iort_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
> return iort_iommu_xlate(info->dev, parent, streamid);
> }
>
> +static void iort_named_component_init(struct device *dev,
> + struct acpi_iort_node *node)
> +{
> + struct acpi_iort_named_component *nc;
> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> + if (!fwspec)
> + return;
> +
> + nc = (struct acpi_iort_named_component *)node->node_data;
> + fwspec->num_pasid_bits = FIELD_GET(ACPI_IORT_NC_PASID_BITS,
> + nc->node_flags);
> +}
> +
> /**
> * iort_iommu_configure - Set-up IOMMU configuration for a device.
> *
> @@ -978,6 +993,9 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
> if (parent)
> err = iort_iommu_xlate(dev, parent, streamid);
> } while (parent && !err);
> +
> + if (!err)
> + iort_named_component_init(dev, node);
> }
>
> /*
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 05/13] iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (3 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 04/13] ACPI/IORT: Support PASID for platform devices Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-17 13:27 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 06/13] iommu/arm-smmu-v3: Add context descriptor tables allocators Jean-Philippe Brucker
` (8 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
When adding SSID support to the SMMUv3 driver, we'll need to manipulate
leaf pasid tables and context descriptors. Extract the context
descriptor structure and introduce a new table structure.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 44 +++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 837b4283b4dc..b287e303b1d7 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -553,16 +553,21 @@ struct arm_smmu_strtab_l1_desc {
dma_addr_t l2ptr_dma;
};
+struct arm_smmu_ctx_desc {
+ u16 asid;
+ u64 ttbr;
+ u64 tcr;
+ u64 mair;
+};
+
+struct arm_smmu_cd_table {
+ __le64 *ptr;
+ dma_addr_t ptr_dma;
+};
+
struct arm_smmu_s1_cfg {
- __le64 *cdptr;
- dma_addr_t cdptr_dma;
-
- struct arm_smmu_ctx_desc {
- u16 asid;
- u64 ttbr;
- u64 tcr;
- u64 mair;
- } cd;
+ struct arm_smmu_cd_table table;
+ struct arm_smmu_ctx_desc cd;
};
struct arm_smmu_s2_cfg {
@@ -1471,6 +1476,7 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
struct arm_smmu_s1_cfg *cfg)
{
u64 val;
+ __le64 *cdptr = cfg->table.ptr;
/*
* We don't need to issue any invalidation here, as we'll invalidate
@@ -1488,12 +1494,12 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
val |= CTXDESC_CD_0_S;
- cfg->cdptr[0] = cpu_to_le64(val);
+ cdptr[0] = cpu_to_le64(val);
val = cfg->cd.ttbr & CTXDESC_CD_1_TTB0_MASK;
- cfg->cdptr[1] = cpu_to_le64(val);
+ cdptr[1] = cpu_to_le64(val);
- cfg->cdptr[3] = cpu_to_le64(cfg->cd.mair);
+ cdptr[3] = cpu_to_le64(cfg->cd.mair);
}
/* Stream table manipulation functions */
@@ -1624,7 +1630,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
!(smmu->features & ARM_SMMU_FEAT_STALL_FORCE))
dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
- val |= (s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
+ val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
}
@@ -2138,11 +2144,11 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
- if (cfg->cdptr) {
+ if (cfg->table.ptr) {
dmam_free_coherent(smmu_domain->smmu->dev,
CTXDESC_CD_DWORDS << 3,
- cfg->cdptr,
- cfg->cdptr_dma);
+ cfg->table.ptr,
+ cfg->table.ptr_dma);
arm_smmu_bitmap_free(smmu->asid_map, cfg->cd.asid);
}
@@ -2167,9 +2173,9 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
if (asid < 0)
return asid;
- cfg->cdptr = dmam_alloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
- &cfg->cdptr_dma, GFP_KERNEL);
- if (!cfg->cdptr) {
+ cfg->table.ptr = dmam_alloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
+ &cfg->table.ptr_dma, GFP_KERNEL);
+ if (!cfg->table.ptr) {
dev_warn(smmu->dev, "failed to allocate context descriptor\n");
ret = -ENOMEM;
goto out_free_asid;
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 05/13] iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support
2019-12-09 18:05 ` [PATCH v3 05/13] iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support Jean-Philippe Brucker
@ 2019-12-17 13:27 ` Auger Eric
0 siblings, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-17 13:27 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> When adding SSID support to the SMMUv3 driver, we'll need to manipulate
> leaf pasid tables and context descriptors. Extract the context
> descriptor structure and introduce a new table structure.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> drivers/iommu/arm-smmu-v3.c | 44 +++++++++++++++++++++----------------
> 1 file changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 837b4283b4dc..b287e303b1d7 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -553,16 +553,21 @@ struct arm_smmu_strtab_l1_desc {
> dma_addr_t l2ptr_dma;
> };
>
> +struct arm_smmu_ctx_desc {
> + u16 asid;
> + u64 ttbr;
> + u64 tcr;
> + u64 mair;
> +};
> +
> +struct arm_smmu_cd_table {
> + __le64 *ptr;
> + dma_addr_t ptr_dma;
> +};
> +
> struct arm_smmu_s1_cfg {
> - __le64 *cdptr;
> - dma_addr_t cdptr_dma;
> -
> - struct arm_smmu_ctx_desc {
> - u16 asid;
> - u64 ttbr;
> - u64 tcr;
> - u64 mair;
> - } cd;
> + struct arm_smmu_cd_table table;
> + struct arm_smmu_ctx_desc cd;
> };
>
> struct arm_smmu_s2_cfg {
> @@ -1471,6 +1476,7 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
> struct arm_smmu_s1_cfg *cfg)
> {
> u64 val;
> + __le64 *cdptr = cfg->table.ptr;
>
> /*
> * We don't need to issue any invalidation here, as we'll invalidate
> @@ -1488,12 +1494,12 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
> if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> val |= CTXDESC_CD_0_S;
>
> - cfg->cdptr[0] = cpu_to_le64(val);
> + cdptr[0] = cpu_to_le64(val);
>
> val = cfg->cd.ttbr & CTXDESC_CD_1_TTB0_MASK;
> - cfg->cdptr[1] = cpu_to_le64(val);
> + cdptr[1] = cpu_to_le64(val);
>
> - cfg->cdptr[3] = cpu_to_le64(cfg->cd.mair);
> + cdptr[3] = cpu_to_le64(cfg->cd.mair);
> }
>
> /* Stream table manipulation functions */
> @@ -1624,7 +1630,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> !(smmu->features & ARM_SMMU_FEAT_STALL_FORCE))
> dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
>
> - val |= (s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> + val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
> }
>
> @@ -2138,11 +2144,11 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>
> - if (cfg->cdptr) {
> + if (cfg->table.ptr) {
> dmam_free_coherent(smmu_domain->smmu->dev,
> CTXDESC_CD_DWORDS << 3,
> - cfg->cdptr,
> - cfg->cdptr_dma);
> + cfg->table.ptr,
> + cfg->table.ptr_dma);
>
> arm_smmu_bitmap_free(smmu->asid_map, cfg->cd.asid);
> }
> @@ -2167,9 +2173,9 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
> if (asid < 0)
> return asid;
>
> - cfg->cdptr = dmam_alloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
> - &cfg->cdptr_dma, GFP_KERNEL);
> - if (!cfg->cdptr) {
> + cfg->table.ptr = dmam_alloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
> + &cfg->table.ptr_dma, GFP_KERNEL);
> + if (!cfg->table.ptr) {
> dev_warn(smmu->dev, "failed to allocate context descriptor\n");
> ret = -ENOMEM;
> goto out_free_asid;
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 06/13] iommu/arm-smmu-v3: Add context descriptor tables allocators
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (4 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 05/13] iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-17 13:36 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs Jean-Philippe Brucker
` (7 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
Support for SSID will require allocating context descriptor tables. Move
the context descriptor allocation to separate functions.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 57 ++++++++++++++++++++++++++++++-------
1 file changed, 46 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index b287e303b1d7..43d6a7ded6e4 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -568,6 +568,7 @@ struct arm_smmu_cd_table {
struct arm_smmu_s1_cfg {
struct arm_smmu_cd_table table;
struct arm_smmu_ctx_desc cd;
+ u8 s1cdmax;
};
struct arm_smmu_s2_cfg {
@@ -1455,6 +1456,31 @@ static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
}
/* Context descriptor manipulation functions */
+static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu,
+ struct arm_smmu_cd_table *table,
+ size_t num_entries)
+{
+ size_t size = num_entries * (CTXDESC_CD_DWORDS << 3);
+
+ table->ptr = dmam_alloc_coherent(smmu->dev, size, &table->ptr_dma,
+ GFP_KERNEL);
+ if (!table->ptr) {
+ dev_warn(smmu->dev,
+ "failed to allocate context descriptor table\n");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+static void arm_smmu_free_cd_leaf_table(struct arm_smmu_device *smmu,
+ struct arm_smmu_cd_table *table,
+ size_t num_entries)
+{
+ size_t size = num_entries * (CTXDESC_CD_DWORDS << 3);
+
+ dmam_free_coherent(smmu->dev, size, table->ptr, table->ptr_dma);
+}
+
static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
{
u64 val = 0;
@@ -1502,6 +1528,23 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
cdptr[3] = cpu_to_le64(cfg->cd.mair);
}
+static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
+{
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+
+ return arm_smmu_alloc_cd_leaf_table(smmu, &cfg->table,
+ 1 << cfg->s1cdmax);
+}
+
+static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
+{
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+
+ arm_smmu_free_cd_leaf_table(smmu, &cfg->table, 1 << cfg->s1cdmax);
+}
+
/* Stream table manipulation functions */
static void
arm_smmu_write_strtab_l1_desc(__le64 *dst, struct arm_smmu_strtab_l1_desc *desc)
@@ -2145,11 +2188,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
if (cfg->table.ptr) {
- dmam_free_coherent(smmu_domain->smmu->dev,
- CTXDESC_CD_DWORDS << 3,
- cfg->table.ptr,
- cfg->table.ptr_dma);
-
+ arm_smmu_free_cd_tables(smmu_domain);
arm_smmu_bitmap_free(smmu->asid_map, cfg->cd.asid);
}
} else {
@@ -2173,13 +2212,9 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
if (asid < 0)
return asid;
- cfg->table.ptr = dmam_alloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
- &cfg->table.ptr_dma, GFP_KERNEL);
- if (!cfg->table.ptr) {
- dev_warn(smmu->dev, "failed to allocate context descriptor\n");
- ret = -ENOMEM;
+ ret = arm_smmu_alloc_cd_tables(smmu_domain);
+ if (ret)
goto out_free_asid;
- }
cfg->cd.asid = (u16)asid;
cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 06/13] iommu/arm-smmu-v3: Add context descriptor tables allocators
2019-12-09 18:05 ` [PATCH v3 06/13] iommu/arm-smmu-v3: Add context descriptor tables allocators Jean-Philippe Brucker
@ 2019-12-17 13:36 ` Auger Eric
0 siblings, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-17 13:36 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> Support for SSID will require allocating context descriptor tables. Move
> the context descriptor allocation to separate functions.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> drivers/iommu/arm-smmu-v3.c | 57 ++++++++++++++++++++++++++++++-------
> 1 file changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index b287e303b1d7..43d6a7ded6e4 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -568,6 +568,7 @@ struct arm_smmu_cd_table {
> struct arm_smmu_s1_cfg {
> struct arm_smmu_cd_table table;
> struct arm_smmu_ctx_desc cd;
> + u8 s1cdmax;
> };
>
> struct arm_smmu_s2_cfg {
> @@ -1455,6 +1456,31 @@ static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
> }
>
> /* Context descriptor manipulation functions */
> +static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu,
> + struct arm_smmu_cd_table *table,
> + size_t num_entries)
> +{
> + size_t size = num_entries * (CTXDESC_CD_DWORDS << 3);
> +
> + table->ptr = dmam_alloc_coherent(smmu->dev, size, &table->ptr_dma,
> + GFP_KERNEL);
> + if (!table->ptr) {
> + dev_warn(smmu->dev,
> + "failed to allocate context descriptor table\n");
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +
> +static void arm_smmu_free_cd_leaf_table(struct arm_smmu_device *smmu,
> + struct arm_smmu_cd_table *table,
> + size_t num_entries)
> +{
> + size_t size = num_entries * (CTXDESC_CD_DWORDS << 3);
> +
> + dmam_free_coherent(smmu->dev, size, table->ptr, table->ptr_dma);
> +}
> +
> static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
> {
> u64 val = 0;
> @@ -1502,6 +1528,23 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
> cdptr[3] = cpu_to_le64(cfg->cd.mair);
> }
>
> +static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
> +{
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> +
> + return arm_smmu_alloc_cd_leaf_table(smmu, &cfg->table,
> + 1 << cfg->s1cdmax);
> +}
> +
> +static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
> +{
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> +
> + arm_smmu_free_cd_leaf_table(smmu, &cfg->table, 1 << cfg->s1cdmax);
> +}
> +
> /* Stream table manipulation functions */
> static void
> arm_smmu_write_strtab_l1_desc(__le64 *dst, struct arm_smmu_strtab_l1_desc *desc)
> @@ -2145,11 +2188,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>
> if (cfg->table.ptr) {
> - dmam_free_coherent(smmu_domain->smmu->dev,
> - CTXDESC_CD_DWORDS << 3,
> - cfg->table.ptr,
> - cfg->table.ptr_dma);
> -
> + arm_smmu_free_cd_tables(smmu_domain);
> arm_smmu_bitmap_free(smmu->asid_map, cfg->cd.asid);
> }
> } else {
> @@ -2173,13 +2212,9 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
> if (asid < 0)
> return asid;
>
> - cfg->table.ptr = dmam_alloc_coherent(smmu->dev, CTXDESC_CD_DWORDS << 3,
> - &cfg->table.ptr_dma, GFP_KERNEL);
> - if (!cfg->table.ptr) {
> - dev_warn(smmu->dev, "failed to allocate context descriptor\n");
> - ret = -ENOMEM;
> + ret = arm_smmu_alloc_cd_tables(smmu_domain);
> + if (ret)
> goto out_free_asid;
> - }
>
> cfg->cd.asid = (u16)asid;
> cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (5 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 06/13] iommu/arm-smmu-v3: Add context descriptor tables allocators Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-17 16:43 ` Auger Eric
2019-12-18 17:03 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 08/13] iommu/arm-smmu-v3: Propate ssid_bits Jean-Philippe Brucker
` (6 subsequent siblings)
13 siblings, 2 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
At the moment, the SMMUv3 driver implements only one stage-1 or stage-2
page directory per device. However SMMUv3 allows more than one address
space for some devices, by providing multiple stage-1 page directories. In
addition to the Stream ID (SID), that identifies a device, we can now have
Substream IDs (SSID) identifying an address space. In PCIe, SID is called
Requester ID (RID) and SSID is called Process Address-Space ID (PASID).
A complete stage-1 walk goes through the context descriptor table:
Stream tables Ctx. Desc. tables Page tables
+--------+ ,------->+-------+ ,------->+-------+
: : | : : | : :
+--------+ | +-------+ | +-------+
SID->| STE |---' SSID->| CD |---' IOVA->| PTE |--> IPA
+--------+ +-------+ +-------+
: : : : : :
+--------+ +-------+ +-------+
Rewrite arm_smmu_write_ctx_desc() to modify context descriptor table
entries. To keep things simple we only implement one level of context
descriptor tables here, but as with stream and page tables, an SSID can
be split to index multiple levels of tables.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 125 +++++++++++++++++++++++++++++-------
1 file changed, 102 insertions(+), 23 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 43d6a7ded6e4..a01071123c34 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -227,6 +227,11 @@
#define STRTAB_STE_0_S1CTXPTR_MASK GENMASK_ULL(51, 6)
#define STRTAB_STE_0_S1CDMAX GENMASK_ULL(63, 59)
+#define STRTAB_STE_1_S1DSS GENMASK_ULL(1, 0)
+#define STRTAB_STE_1_S1DSS_TERMINATE 0x0
+#define STRTAB_STE_1_S1DSS_BYPASS 0x1
+#define STRTAB_STE_1_S1DSS_SSID0 0x2
+
#define STRTAB_STE_1_S1C_CACHE_NC 0UL
#define STRTAB_STE_1_S1C_CACHE_WBRA 1UL
#define STRTAB_STE_1_S1C_CACHE_WT 2UL
@@ -329,6 +334,7 @@
#define CMDQ_PREFETCH_1_SIZE GENMASK_ULL(4, 0)
#define CMDQ_PREFETCH_1_ADDR_MASK GENMASK_ULL(63, 12)
+#define CMDQ_CFGI_0_SSID GENMASK_ULL(31, 12)
#define CMDQ_CFGI_0_SID GENMASK_ULL(63, 32)
#define CMDQ_CFGI_1_LEAF (1UL << 0)
#define CMDQ_CFGI_1_RANGE GENMASK_ULL(4, 0)
@@ -446,8 +452,11 @@ struct arm_smmu_cmdq_ent {
#define CMDQ_OP_CFGI_STE 0x3
#define CMDQ_OP_CFGI_ALL 0x4
+ #define CMDQ_OP_CFGI_CD 0x5
+ #define CMDQ_OP_CFGI_CD_ALL 0x6
struct {
u32 sid;
+ u32 ssid;
union {
bool leaf;
u8 span;
@@ -568,6 +577,7 @@ struct arm_smmu_cd_table {
struct arm_smmu_s1_cfg {
struct arm_smmu_cd_table table;
struct arm_smmu_ctx_desc cd;
+ u8 s1fmt;
u8 s1cdmax;
};
@@ -860,10 +870,16 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
cmd[1] |= FIELD_PREP(CMDQ_PREFETCH_1_SIZE, ent->prefetch.size);
cmd[1] |= ent->prefetch.addr & CMDQ_PREFETCH_1_ADDR_MASK;
break;
+ case CMDQ_OP_CFGI_CD:
+ cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SSID, ent->cfgi.ssid);
+ /* Fallthrough */
case CMDQ_OP_CFGI_STE:
cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SID, ent->cfgi.sid);
cmd[1] |= FIELD_PREP(CMDQ_CFGI_1_LEAF, ent->cfgi.leaf);
break;
+ case CMDQ_OP_CFGI_CD_ALL:
+ cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SID, ent->cfgi.sid);
+ break;
case CMDQ_OP_CFGI_ALL:
/* Cover the entire SID range */
cmd[1] |= FIELD_PREP(CMDQ_CFGI_1_RANGE, 31);
@@ -1456,6 +1472,33 @@ static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
}
/* Context descriptor manipulation functions */
+static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
+ int ssid, bool leaf)
+{
+ size_t i;
+ unsigned long flags;
+ struct arm_smmu_master *master;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_cmdq_ent cmd = {
+ .opcode = CMDQ_OP_CFGI_CD,
+ .cfgi = {
+ .ssid = ssid,
+ .leaf = leaf,
+ },
+ };
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(master, &smmu_domain->devices, domain_head) {
+ for (i = 0; i < master->num_sids; i++) {
+ cmd.cfgi.sid = master->sids[i];
+ arm_smmu_cmdq_issue_cmd(smmu, &cmd);
+ }
+ }
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+
+ arm_smmu_cmdq_issue_sync(smmu);
+}
+
static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu,
struct arm_smmu_cd_table *table,
size_t num_entries)
@@ -1498,34 +1541,65 @@ static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
return val;
}
-static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
- struct arm_smmu_s1_cfg *cfg)
+static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
+ int ssid, struct arm_smmu_ctx_desc *cd)
{
- u64 val;
- __le64 *cdptr = cfg->table.ptr;
-
/*
- * We don't need to issue any invalidation here, as we'll invalidate
- * the STE when installing the new entry anyway.
+ * This function handles the following cases:
+ *
+ * (1) Install primary CD, for normal DMA traffic (SSID = 0).
+ * (2) Install a secondary CD, for SID+SSID traffic.
+ * (3) Update ASID of a CD. Atomically write the first 64 bits of the
+ * CD, then invalidate the old entry and mappings.
+ * (4) Remove a secondary CD.
*/
- val = arm_smmu_cpu_tcr_to_cd(cfg->cd.tcr) |
-#ifdef __BIG_ENDIAN
- CTXDESC_CD_0_ENDI |
-#endif
- CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
- CTXDESC_CD_0_AA64 | FIELD_PREP(CTXDESC_CD_0_ASID, cfg->cd.asid) |
- CTXDESC_CD_0_V;
+ u64 val;
+ bool cd_live;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
+ CTXDESC_CD_DWORDS;
- /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
- if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
- val |= CTXDESC_CD_0_S;
+ val = le64_to_cpu(cdptr[0]);
+ cd_live = !!(val & CTXDESC_CD_0_V);
- cdptr[0] = cpu_to_le64(val);
+ if (!cd) { /* (4) */
+ val = 0;
+ } else if (cd_live) { /* (3) */
+ val &= ~CTXDESC_CD_0_ASID;
+ val |= FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid);
+ /*
+ * Until CD+TLB invalidation, both ASIDs may be used for tagging
+ * this substream's traffic
+ */
+ } else { /* (1) and (2) */
+ cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
+ cdptr[2] = 0;
+ cdptr[3] = cpu_to_le64(cd->mair);
+
+ /*
+ * STE is live, and the SMMU might fetch this CD at any
+ * time. Ensure that it observes the rest of the CD before we
+ * enable it.
+ */
+ arm_smmu_sync_cd(smmu_domain, ssid, true);
- val = cfg->cd.ttbr & CTXDESC_CD_1_TTB0_MASK;
- cdptr[1] = cpu_to_le64(val);
+ val = arm_smmu_cpu_tcr_to_cd(cd->tcr) |
+#ifdef __BIG_ENDIAN
+ CTXDESC_CD_0_ENDI |
+#endif
+ CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
+ CTXDESC_CD_0_AA64 |
+ FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
+ CTXDESC_CD_0_V;
- cdptr[3] = cpu_to_le64(cfg->cd.mair);
+ /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
+ if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
+ val |= CTXDESC_CD_0_S;
+ }
+
+ WRITE_ONCE(cdptr[0], cpu_to_le64(val));
+ arm_smmu_sync_cd(smmu_domain, ssid, true);
+ return 0;
}
static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
@@ -1533,6 +1607,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+ cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
return arm_smmu_alloc_cd_leaf_table(smmu, &cfg->table,
1 << cfg->s1cdmax);
}
@@ -1664,6 +1739,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
if (s1_cfg) {
BUG_ON(ste_live);
dst[1] = cpu_to_le64(
+ FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
@@ -1674,7 +1750,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
- FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
+ FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
+ FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
+ FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
}
if (s2_cfg) {
@@ -2479,7 +2557,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
master->ats_enabled = arm_smmu_ats_supported(master);
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
- arm_smmu_write_ctx_desc(smmu, &smmu_domain->s1_cfg);
+ arm_smmu_write_ctx_desc(smmu_domain, 0,
+ &smmu_domain->s1_cfg.cd);
arm_smmu_install_ste_for_dev(master);
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs
2019-12-09 18:05 ` [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs Jean-Philippe Brucker
@ 2019-12-17 16:43 ` Auger Eric
2019-12-18 16:07 ` Jean-Philippe Brucker
2019-12-18 17:03 ` Auger Eric
1 sibling, 1 reply; 40+ messages in thread
From: Auger Eric @ 2019-12-17 16:43 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> At the moment, the SMMUv3 driver implements only one stage-1 or stage-2
> page directory per device. However SMMUv3 allows more than one address
> space for some devices, by providing multiple stage-1 page directories. In
> addition to the Stream ID (SID), that identifies a device, we can now have
> Substream IDs (SSID) identifying an address space. In PCIe, SID is called
> Requester ID (RID) and SSID is called Process Address-Space ID (PASID).
> A complete stage-1 walk goes through the context descriptor table:
>
> Stream tables Ctx. Desc. tables Page tables
> +--------+ ,------->+-------+ ,------->+-------+
> : : | : : | : :
> +--------+ | +-------+ | +-------+
> SID->| STE |---' SSID->| CD |---' IOVA->| PTE |--> IPA
> +--------+ +-------+ +-------+
> : : : : : :
> +--------+ +-------+ +-------+
>
> Rewrite arm_smmu_write_ctx_desc() to modify context descriptor table
> entries. To keep things simple we only implement one level of context
> descriptor tables here, but as with stream and page tables, an SSID can
> be split to index multiple levels of tables.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> drivers/iommu/arm-smmu-v3.c | 125 +++++++++++++++++++++++++++++-------
> 1 file changed, 102 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 43d6a7ded6e4..a01071123c34 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -227,6 +227,11 @@
> #define STRTAB_STE_0_S1CTXPTR_MASK GENMASK_ULL(51, 6)
> #define STRTAB_STE_0_S1CDMAX GENMASK_ULL(63, 59)
>
> +#define STRTAB_STE_1_S1DSS GENMASK_ULL(1, 0)
> +#define STRTAB_STE_1_S1DSS_TERMINATE 0x0
> +#define STRTAB_STE_1_S1DSS_BYPASS 0x1
> +#define STRTAB_STE_1_S1DSS_SSID0 0x2
> +
> #define STRTAB_STE_1_S1C_CACHE_NC 0UL
> #define STRTAB_STE_1_S1C_CACHE_WBRA 1UL
> #define STRTAB_STE_1_S1C_CACHE_WT 2UL
> @@ -329,6 +334,7 @@
> #define CMDQ_PREFETCH_1_SIZE GENMASK_ULL(4, 0)
> #define CMDQ_PREFETCH_1_ADDR_MASK GENMASK_ULL(63, 12)
>
> +#define CMDQ_CFGI_0_SSID GENMASK_ULL(31, 12)
> #define CMDQ_CFGI_0_SID GENMASK_ULL(63, 32)
> #define CMDQ_CFGI_1_LEAF (1UL << 0)
> #define CMDQ_CFGI_1_RANGE GENMASK_ULL(4, 0)
> @@ -446,8 +452,11 @@ struct arm_smmu_cmdq_ent {
>
> #define CMDQ_OP_CFGI_STE 0x3
> #define CMDQ_OP_CFGI_ALL 0x4
> + #define CMDQ_OP_CFGI_CD 0x5
> + #define CMDQ_OP_CFGI_CD_ALL 0x6
> struct {
> u32 sid;
> + u32 ssid;
> union {
> bool leaf;
> u8 span;
> @@ -568,6 +577,7 @@ struct arm_smmu_cd_table {
> struct arm_smmu_s1_cfg {
> struct arm_smmu_cd_table table;
> struct arm_smmu_ctx_desc cd;
> + u8 s1fmt;
> u8 s1cdmax;
> };
>
> @@ -860,10 +870,16 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
> cmd[1] |= FIELD_PREP(CMDQ_PREFETCH_1_SIZE, ent->prefetch.size);
> cmd[1] |= ent->prefetch.addr & CMDQ_PREFETCH_1_ADDR_MASK;
> break;
> + case CMDQ_OP_CFGI_CD:
> + cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SSID, ent->cfgi.ssid);
> + /* Fallthrough */
> case CMDQ_OP_CFGI_STE:
> cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SID, ent->cfgi.sid);
> cmd[1] |= FIELD_PREP(CMDQ_CFGI_1_LEAF, ent->cfgi.leaf);
> break;
> + case CMDQ_OP_CFGI_CD_ALL:
> + cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SID, ent->cfgi.sid);
> + break;
> case CMDQ_OP_CFGI_ALL:
> /* Cover the entire SID range */
> cmd[1] |= FIELD_PREP(CMDQ_CFGI_1_RANGE, 31);
> @@ -1456,6 +1472,33 @@ static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
> }
>
> /* Context descriptor manipulation functions */
> +static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
> + int ssid, bool leaf)
> +{
> + size_t i;
> + unsigned long flags;
> + struct arm_smmu_master *master;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_cmdq_ent cmd = {
> + .opcode = CMDQ_OP_CFGI_CD,
> + .cfgi = {
> + .ssid = ssid,
> + .leaf = leaf,
> + },
> + };
> +
> + spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> + list_for_each_entry(master, &smmu_domain->devices, domain_head) {
> + for (i = 0; i < master->num_sids; i++) {
> + cmd.cfgi.sid = master->sids[i];
> + arm_smmu_cmdq_issue_cmd(smmu, &cmd);
> + }
> + }
> + spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
> +
> + arm_smmu_cmdq_issue_sync(smmu);
> +}
> +
> static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu,
> struct arm_smmu_cd_table *table,
> size_t num_entries)
> @@ -1498,34 +1541,65 @@ static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
> return val;
> }
>
> -static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
> - struct arm_smmu_s1_cfg *cfg)
> +static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
> + int ssid, struct arm_smmu_ctx_desc *cd)
> {
> - u64 val;
> - __le64 *cdptr = cfg->table.ptr;
> -
> /*
> - * We don't need to issue any invalidation here, as we'll invalidate
> - * the STE when installing the new entry anyway.
> + * This function handles the following cases:
> + *
> + * (1) Install primary CD, for normal DMA traffic (SSID = 0).
> + * (2) Install a secondary CD, for SID+SSID traffic.
> + * (3) Update ASID of a CD. Atomically write the first 64 bits of the
> + * CD, then invalidate the old entry and mappings.
> + * (4) Remove a secondary CD.
I see arm_smmu_write_ctx_desc getting called with non null cd in
arm_smmu_attach_dev but I do not see it removed in the detach path?
> */
> - val = arm_smmu_cpu_tcr_to_cd(cfg->cd.tcr) |
> -#ifdef __BIG_ENDIAN
> - CTXDESC_CD_0_ENDI |
> -#endif
> - CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
> - CTXDESC_CD_0_AA64 | FIELD_PREP(CTXDESC_CD_0_ASID, cfg->cd.asid) |
> - CTXDESC_CD_0_V;
> + u64 val;
> + bool cd_live;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
> + CTXDESC_CD_DWORDS;>
> - /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
> - if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> - val |= CTXDESC_CD_0_S;
> + val = le64_to_cpu(cdptr[0]);
> + cd_live = !!(val & CTXDESC_CD_0_V);
>
> - cdptr[0] = cpu_to_le64(val);
> + if (!cd) { /* (4) */
> + val = 0;
> + } else if (cd_live) { /* (3) */
> + val &= ~CTXDESC_CD_0_ASID;
> + val |= FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid);
> + /*
> + * Until CD+TLB invalidation, both ASIDs may be used for tagging
> + * this substream's traffic
> + */
> + } else { /* (1) and (2) */
> + cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
> + cdptr[2] = 0;
> + cdptr[3] = cpu_to_le64(cd->mair);
> +
> + /*
> + * STE is live, and the SMMU might fetch this CD at any
> + * time. Ensure that it observes the rest of the CD before we
> + * enable it.
Mostly, on't you want the invalid state to be seen?
> + */
> + arm_smmu_sync_cd(smmu_domain, ssid, true);
>
> - val = cfg->cd.ttbr & CTXDESC_CD_1_TTB0_MASK;
> - cdptr[1] = cpu_to_le64(val);
> + val = arm_smmu_cpu_tcr_to_cd(cd->tcr) |
> +#ifdef __BIG_ENDIAN
> + CTXDESC_CD_0_ENDI |
> +#endif
> + CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
> + CTXDESC_CD_0_AA64 |
> + FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
> + CTXDESC_CD_0_V;
>
> - cdptr[3] = cpu_to_le64(cfg->cd.mair);
> + /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
> + if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> + val |= CTXDESC_CD_0_S;
> + }
> +
> + WRITE_ONCE(cdptr[0], cpu_to_le64(val));
> + arm_smmu_sync_cd(smmu_domain, ssid, true);
> + return 0;
> }
>
> static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
> @@ -1533,6 +1607,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>
> + cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
> return arm_smmu_alloc_cd_leaf_table(smmu, &cfg->table,
> 1 << cfg->s1cdmax);
> }
> @@ -1664,6 +1739,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> if (s1_cfg) {
> BUG_ON(ste_live);
> dst[1] = cpu_to_le64(
> + FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
> FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
> FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
> FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
> @@ -1674,7 +1750,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
>
> val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> - FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
> + FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
> + FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
> + FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
Do I understand correctly that each time we update the CD, we also
update the STE. This is not wrong but theoretically this is not needed,
right?
This also explains why you do not need the CMD_PREFETCH_CONFIG with ssid
because you invalidate the config per sid?
> }
>
> if (s2_cfg) {
> @@ -2479,7 +2557,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> master->ats_enabled = arm_smmu_ats_supported(master);
>
> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
> - arm_smmu_write_ctx_desc(smmu, &smmu_domain->s1_cfg);
> + arm_smmu_write_ctx_desc(smmu_domain, 0,
> + &smmu_domain->s1_cfg.cd);
>
> arm_smmu_install_ste_for_dev(master);
>
>
Thanks
Eric
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs
2019-12-17 16:43 ` Auger Eric
@ 2019-12-18 16:07 ` Jean-Philippe Brucker
2019-12-18 16:59 ` Auger Eric
0 siblings, 1 reply; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-18 16:07 UTC (permalink / raw)
To: Auger Eric
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
On Tue, Dec 17, 2019 at 05:43:59PM +0100, Auger Eric wrote:
> > -static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
> > - struct arm_smmu_s1_cfg *cfg)
> > +static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
> > + int ssid, struct arm_smmu_ctx_desc *cd)
> > {
> > - u64 val;
> > - __le64 *cdptr = cfg->table.ptr;
> > -
> > /*
> > - * We don't need to issue any invalidation here, as we'll invalidate
> > - * the STE when installing the new entry anyway.
> > + * This function handles the following cases:
> > + *
> > + * (1) Install primary CD, for normal DMA traffic (SSID = 0).
> > + * (2) Install a secondary CD, for SID+SSID traffic.
> > + * (3) Update ASID of a CD. Atomically write the first 64 bits of the
> > + * CD, then invalidate the old entry and mappings.
> > + * (4) Remove a secondary CD.
> I see arm_smmu_write_ctx_desc getting called with non null cd in
> arm_smmu_attach_dev but I do not see it removed in the detach path?
No we don't have to remove the primary CD, since detach clears the STE and
frees the CD tables.
> > */
> > - val = arm_smmu_cpu_tcr_to_cd(cfg->cd.tcr) |
> > -#ifdef __BIG_ENDIAN
> > - CTXDESC_CD_0_ENDI |
> > -#endif
> > - CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
> > - CTXDESC_CD_0_AA64 | FIELD_PREP(CTXDESC_CD_0_ASID, cfg->cd.asid) |
> > - CTXDESC_CD_0_V;
> > + u64 val;
> > + bool cd_live;
> > + struct arm_smmu_device *smmu = smmu_domain->smmu;
> > + __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
> > + CTXDESC_CD_DWORDS;>
> > - /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
> > - if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> > - val |= CTXDESC_CD_0_S;
> > + val = le64_to_cpu(cdptr[0]);
> > + cd_live = !!(val & CTXDESC_CD_0_V);
> >
> > - cdptr[0] = cpu_to_le64(val);
> > + if (!cd) { /* (4) */
> > + val = 0;
> > + } else if (cd_live) { /* (3) */
> > + val &= ~CTXDESC_CD_0_ASID;
> > + val |= FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid);
> > + /*
> > + * Until CD+TLB invalidation, both ASIDs may be used for tagging
> > + * this substream's traffic
> > + */
> > + } else { /* (1) and (2) */
> > + cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
> > + cdptr[2] = 0;
> > + cdptr[3] = cpu_to_le64(cd->mair);
> > +
> > + /*
> > + * STE is live, and the SMMU might fetch this CD at any
> > + * time. Ensure that it observes the rest of the CD before we
> > + * enable it.
> Mostly, on't you want the invalid state to be seen?
Sorry I didn't get your question. Without the sync, the SMMU could read an
invalid address in CD[1], read V=1 in CD[0] and proceed with dereferencing
the bogus pointer before seeing our update to CD[1].
To prevent this we follow the update procedure described in IHI0070Ca
3.21.3.1 (Configuration structure update procedure):
Because the SMMU can read any reachable structure at any time, and is not
required to read the double-words of the structure in order, Arm
recommends that the following procedure is used to initialize structures:
1. Structure starts invalid, having V == 0.
2. Fill in all fields, leaving V == 0, then perform a DSB operation to
ensure written data is observable from the SMMU.
3. Issue a CMD_CFGI_<STRUCT>, as appropriate.
4. Issue a CMD_SYNC, and wait for completion.
5. Set V to 1, then perform a DSB operation to ensure write is
observable by the SMMU.
6. Issue CMD_CFGI_<STRUCT>, as appropriate.
7. Optionally issue a CMD_SYNC, and wait for completion. This must be
done if a subsequent software operation, such as enabling device
DMA, depends on the SMMU using the new structure.
> > @@ -1664,6 +1739,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> > if (s1_cfg) {
> > BUG_ON(ste_live);
> > dst[1] = cpu_to_le64(
> > + FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
> > FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
> > FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
> > FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
> > @@ -1674,7 +1750,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> > dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
> >
> > val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> > - FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
> > + FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
> > + FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
> > + FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
> Do I understand correctly that each time we update the CD, we also
> update the STE. This is not wrong but theoretically this is not needed,
> right?
Only the primary CD. Once we add SVA (or auxiliary domains) we need to
update secondary CDs while the STE is live. This series is just
preparation for SVA support. I didn't include the full support because it
requires another 25+ patches.
> This also explains why you do not need the CMD_PREFETCH_CONFIG with ssid
> because you invalidate the config per sid?
Hm, I didn't add a CMD_PREFETCH_CONFIG to CDs because I don't know if it's
useful. Maybe as future optimization.
Thanks,
Jean
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs
2019-12-18 16:07 ` Jean-Philippe Brucker
@ 2019-12-18 16:59 ` Auger Eric
0 siblings, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-18 16:59 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/18/19 5:07 PM, Jean-Philippe Brucker wrote:
> On Tue, Dec 17, 2019 at 05:43:59PM +0100, Auger Eric wrote:
>>> -static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
>>> - struct arm_smmu_s1_cfg *cfg)
>>> +static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
>>> + int ssid, struct arm_smmu_ctx_desc *cd)
>>> {
>>> - u64 val;
>>> - __le64 *cdptr = cfg->table.ptr;
>>> -
>>> /*
>>> - * We don't need to issue any invalidation here, as we'll invalidate
>>> - * the STE when installing the new entry anyway.
>>> + * This function handles the following cases:
>>> + *
>>> + * (1) Install primary CD, for normal DMA traffic (SSID = 0).
>>> + * (2) Install a secondary CD, for SID+SSID traffic.
>>> + * (3) Update ASID of a CD. Atomically write the first 64 bits of the
>>> + * CD, then invalidate the old entry and mappings.
>>> + * (4) Remove a secondary CD.
>> I see arm_smmu_write_ctx_desc getting called with non null cd in
>> arm_smmu_attach_dev but I do not see it removed in the detach path?
>
> No we don't have to remove the primary CD, since detach clears the STE and
> frees the CD tables.
OK
>
>>> */
>>> - val = arm_smmu_cpu_tcr_to_cd(cfg->cd.tcr) |
>>> -#ifdef __BIG_ENDIAN
>>> - CTXDESC_CD_0_ENDI |
>>> -#endif
>>> - CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
>>> - CTXDESC_CD_0_AA64 | FIELD_PREP(CTXDESC_CD_0_ASID, cfg->cd.asid) |
>>> - CTXDESC_CD_0_V;
>>> + u64 val;
>>> + bool cd_live;
>>> + struct arm_smmu_device *smmu = smmu_domain->smmu;
>>> + __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
>>> + CTXDESC_CD_DWORDS;>
>>> - /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
>>> - if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
>>> - val |= CTXDESC_CD_0_S;
>>> + val = le64_to_cpu(cdptr[0]);
>>> + cd_live = !!(val & CTXDESC_CD_0_V);
>>>
>>> - cdptr[0] = cpu_to_le64(val);
>>> + if (!cd) { /* (4) */
>>> + val = 0;
>>> + } else if (cd_live) { /* (3) */
>>> + val &= ~CTXDESC_CD_0_ASID;
>>> + val |= FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid);
>>> + /*
>>> + * Until CD+TLB invalidation, both ASIDs may be used for tagging
>>> + * this substream's traffic
>>> + */
>>> + } else { /* (1) and (2) */
>>> + cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
>>> + cdptr[2] = 0;
>>> + cdptr[3] = cpu_to_le64(cd->mair);
>>> +
>>> + /*
>>> + * STE is live, and the SMMU might fetch this CD at any
>>> + * time. Ensure that it observes the rest of the CD before we
>>> + * enable it.
>> Mostly, on't you want the invalid state to be seen?
Sorry I wanted to say you mostly want to enforce the INVALID bit (what I
understand from your below explanations). I was confused by the wording
"Ensure that it observes the rest of the CD before we enable it".
>
> Sorry I didn't get your question. Without the sync, the SMMU could read an
> invalid address in CD[1], read V=1 in CD[0] and proceed with dereferencing
> the bogus pointer before seeing our update to CD[1].
>
> To prevent this we follow the update procedure described in IHI0070Ca
> 3.21.3.1 (Configuration structure update procedure):
>
> Because the SMMU can read any reachable structure at any time, and is not
> required to read the double-words of the structure in order, Arm
> recommends that the following procedure is used to initialize structures:
> 1. Structure starts invalid, having V == 0.
> 2. Fill in all fields, leaving V == 0, then perform a DSB operation to
> ensure written data is observable from the SMMU.
> 3. Issue a CMD_CFGI_<STRUCT>, as appropriate.
> 4. Issue a CMD_SYNC, and wait for completion.
> 5. Set V to 1, then perform a DSB operation to ensure write is
> observable by the SMMU.
> 6. Issue CMD_CFGI_<STRUCT>, as appropriate.
> 7. Optionally issue a CMD_SYNC, and wait for completion. This must be
> done if a subsequent software operation, such as enabling device
> DMA, depends on the SMMU using the new structure.
>
>>> @@ -1664,6 +1739,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
>>> if (s1_cfg) {
>>> BUG_ON(ste_live);
>>> dst[1] = cpu_to_le64(
>>> + FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
>>> FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
>>> FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
>>> FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
>>> @@ -1674,7 +1750,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
>>> dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
>>>
>>> val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
>>> - FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
>>> + FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
>>> + FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
>>> + FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
>> Do I understand correctly that each time we update the CD, we also
>> update the STE. This is not wrong but theoretically this is not needed,
>> right?
>
> Only the primary CD. Once we add SVA (or auxiliary domains) we need to
> update secondary CDs while the STE is live. This series is just
> preparation for SVA support. I didn't include the full support because it
> requires another 25+ patches.
OK
>
>> This also explains why you do not need the CMD_PREFETCH_CONFIG with ssid
>> because you invalidate the config per sid?
>
> Hm, I didn't add a CMD_PREFETCH_CONFIG to CDs because I don't know if it's
> useful. Maybe as future optimization.
OK
Thanks
Eric
>
> Thanks,
> Jean
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs
2019-12-09 18:05 ` [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs Jean-Philippe Brucker
2019-12-17 16:43 ` Auger Eric
@ 2019-12-18 17:03 ` Auger Eric
1 sibling, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-18 17:03 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> At the moment, the SMMUv3 driver implements only one stage-1 or stage-2
> page directory per device. However SMMUv3 allows more than one address
> space for some devices, by providing multiple stage-1 page directories. In
> addition to the Stream ID (SID), that identifies a device, we can now have
> Substream IDs (SSID) identifying an address space. In PCIe, SID is called
> Requester ID (RID) and SSID is called Process Address-Space ID (PASID).
> A complete stage-1 walk goes through the context descriptor table:
>
> Stream tables Ctx. Desc. tables Page tables
> +--------+ ,------->+-------+ ,------->+-------+
> : : | : : | : :
> +--------+ | +-------+ | +-------+
> SID->| STE |---' SSID->| CD |---' IOVA->| PTE |--> IPA
> +--------+ +-------+ +-------+
> : : : : : :
> +--------+ +-------+ +-------+
>
> Rewrite arm_smmu_write_ctx_desc() to modify context descriptor table
> entries. To keep things simple we only implement one level of context
> descriptor tables here, but as with stream and page tables, an SSID can
> be split to index multiple levels of tables.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> drivers/iommu/arm-smmu-v3.c | 125 +++++++++++++++++++++++++++++-------
> 1 file changed, 102 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 43d6a7ded6e4..a01071123c34 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -227,6 +227,11 @@
> #define STRTAB_STE_0_S1CTXPTR_MASK GENMASK_ULL(51, 6)
> #define STRTAB_STE_0_S1CDMAX GENMASK_ULL(63, 59)
>
> +#define STRTAB_STE_1_S1DSS GENMASK_ULL(1, 0)
> +#define STRTAB_STE_1_S1DSS_TERMINATE 0x0
> +#define STRTAB_STE_1_S1DSS_BYPASS 0x1
> +#define STRTAB_STE_1_S1DSS_SSID0 0x2
> +
> #define STRTAB_STE_1_S1C_CACHE_NC 0UL
> #define STRTAB_STE_1_S1C_CACHE_WBRA 1UL
> #define STRTAB_STE_1_S1C_CACHE_WT 2UL
> @@ -329,6 +334,7 @@
> #define CMDQ_PREFETCH_1_SIZE GENMASK_ULL(4, 0)
> #define CMDQ_PREFETCH_1_ADDR_MASK GENMASK_ULL(63, 12)
>
> +#define CMDQ_CFGI_0_SSID GENMASK_ULL(31, 12)
> #define CMDQ_CFGI_0_SID GENMASK_ULL(63, 32)
> #define CMDQ_CFGI_1_LEAF (1UL << 0)
> #define CMDQ_CFGI_1_RANGE GENMASK_ULL(4, 0)
> @@ -446,8 +452,11 @@ struct arm_smmu_cmdq_ent {
>
> #define CMDQ_OP_CFGI_STE 0x3
> #define CMDQ_OP_CFGI_ALL 0x4
> + #define CMDQ_OP_CFGI_CD 0x5
> + #define CMDQ_OP_CFGI_CD_ALL 0x6
> struct {
> u32 sid;
> + u32 ssid;
> union {
> bool leaf;
> u8 span;
> @@ -568,6 +577,7 @@ struct arm_smmu_cd_table {
> struct arm_smmu_s1_cfg {
> struct arm_smmu_cd_table table;
> struct arm_smmu_ctx_desc cd;
> + u8 s1fmt;
> u8 s1cdmax;
> };
>
> @@ -860,10 +870,16 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
> cmd[1] |= FIELD_PREP(CMDQ_PREFETCH_1_SIZE, ent->prefetch.size);
> cmd[1] |= ent->prefetch.addr & CMDQ_PREFETCH_1_ADDR_MASK;
> break;
> + case CMDQ_OP_CFGI_CD:
> + cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SSID, ent->cfgi.ssid);
> + /* Fallthrough */
> case CMDQ_OP_CFGI_STE:
> cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SID, ent->cfgi.sid);
> cmd[1] |= FIELD_PREP(CMDQ_CFGI_1_LEAF, ent->cfgi.leaf);
> break;
> + case CMDQ_OP_CFGI_CD_ALL:
> + cmd[0] |= FIELD_PREP(CMDQ_CFGI_0_SID, ent->cfgi.sid);
> + break;
> case CMDQ_OP_CFGI_ALL:
> /* Cover the entire SID range */
> cmd[1] |= FIELD_PREP(CMDQ_CFGI_1_RANGE, 31);
> @@ -1456,6 +1472,33 @@ static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
> }
>
> /* Context descriptor manipulation functions */
> +static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
> + int ssid, bool leaf)
> +{
> + size_t i;
> + unsigned long flags;
> + struct arm_smmu_master *master;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_cmdq_ent cmd = {
> + .opcode = CMDQ_OP_CFGI_CD,
> + .cfgi = {
> + .ssid = ssid,
> + .leaf = leaf,
> + },
> + };
> +
> + spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> + list_for_each_entry(master, &smmu_domain->devices, domain_head) {
> + for (i = 0; i < master->num_sids; i++) {
> + cmd.cfgi.sid = master->sids[i];
> + arm_smmu_cmdq_issue_cmd(smmu, &cmd);
> + }
> + }
> + spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
> +
> + arm_smmu_cmdq_issue_sync(smmu);
> +}
> +
> static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu,
> struct arm_smmu_cd_table *table,
> size_t num_entries)
> @@ -1498,34 +1541,65 @@ static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
> return val;
> }
>
> -static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
> - struct arm_smmu_s1_cfg *cfg)
> +static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
> + int ssid, struct arm_smmu_ctx_desc *cd)
> {
> - u64 val;
> - __le64 *cdptr = cfg->table.ptr;
> -
> /*
> - * We don't need to issue any invalidation here, as we'll invalidate
> - * the STE when installing the new entry anyway.
> + * This function handles the following cases:
> + *
> + * (1) Install primary CD, for normal DMA traffic (SSID = 0).
> + * (2) Install a secondary CD, for SID+SSID traffic.
> + * (3) Update ASID of a CD. Atomically write the first 64 bits of the
> + * CD, then invalidate the old entry and mappings.
> + * (4) Remove a secondary CD.
> */
> - val = arm_smmu_cpu_tcr_to_cd(cfg->cd.tcr) |
> -#ifdef __BIG_ENDIAN
> - CTXDESC_CD_0_ENDI |
> -#endif
> - CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
> - CTXDESC_CD_0_AA64 | FIELD_PREP(CTXDESC_CD_0_ASID, cfg->cd.asid) |
> - CTXDESC_CD_0_V;
> + u64 val;
> + bool cd_live;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
> + CTXDESC_CD_DWORDS;
>
> - /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
> - if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> - val |= CTXDESC_CD_0_S;
> + val = le64_to_cpu(cdptr[0]);
> + cd_live = !!(val & CTXDESC_CD_0_V);
>
> - cdptr[0] = cpu_to_le64(val);
> + if (!cd) { /* (4) */
> + val = 0;
> + } else if (cd_live) { /* (3) */
> + val &= ~CTXDESC_CD_0_ASID;
> + val |= FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid);
> + /*
> + * Until CD+TLB invalidation, both ASIDs may be used for tagging
> + * this substream's traffic
> + */
> + } else { /* (1) and (2) */
> + cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
> + cdptr[2] = 0;
> + cdptr[3] = cpu_to_le64(cd->mair);
> +
> + /*
> + * STE is live, and the SMMU might fetch this CD at any
> + * time. Ensure that it observes the rest of the CD before we
> + * enable it.
> + */
> + arm_smmu_sync_cd(smmu_domain, ssid, true);
>
> - val = cfg->cd.ttbr & CTXDESC_CD_1_TTB0_MASK;
> - cdptr[1] = cpu_to_le64(val);
> + val = arm_smmu_cpu_tcr_to_cd(cd->tcr) |
> +#ifdef __BIG_ENDIAN
> + CTXDESC_CD_0_ENDI |
> +#endif
> + CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
> + CTXDESC_CD_0_AA64 |
> + FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
> + CTXDESC_CD_0_V;
>
> - cdptr[3] = cpu_to_le64(cfg->cd.mair);
> + /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
> + if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> + val |= CTXDESC_CD_0_S;
> + }
> +
> + WRITE_ONCE(cdptr[0], cpu_to_le64(val));
> + arm_smmu_sync_cd(smmu_domain, ssid, true);
> + return 0;
> }
>
> static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
> @@ -1533,6 +1607,7 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>
> + cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
> return arm_smmu_alloc_cd_leaf_table(smmu, &cfg->table,
> 1 << cfg->s1cdmax);
> }
> @@ -1664,6 +1739,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> if (s1_cfg) {
> BUG_ON(ste_live);
> dst[1] = cpu_to_le64(
> + FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
> FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
> FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
> FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
> @@ -1674,7 +1750,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
>
> val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> - FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
> + FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
> + FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
> + FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
> }
>
> if (s2_cfg) {
> @@ -2479,7 +2557,8 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> master->ats_enabled = arm_smmu_ats_supported(master);
>
> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
> - arm_smmu_write_ctx_desc(smmu, &smmu_domain->s1_cfg);
> + arm_smmu_write_ctx_desc(smmu_domain, 0,
> + &smmu_domain->s1_cfg.cd);
>
> arm_smmu_install_ste_for_dev(master);
>
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 08/13] iommu/arm-smmu-v3: Propate ssid_bits
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (6 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-17 17:07 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 09/13] iommu/arm-smmu-v3: Handle failure of arm_smmu_write_ctx_desc() Jean-Philippe Brucker
` (5 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
Now that we support substream IDs, initialize s1cdmax with the number of
SSID bits supported by a master and the SMMU.
Context descriptor tables are allocated once for the first master
attached to a domain. Therefore attaching multiple devices with
different SSID sizes is tricky, and we currently don't support it.
As a future improvement it would be nice to at least support attaching a
SSID-capable device to a domain that isn't using SSID, by reallocating
the SSID table. This would allow supporting a SSID-capable device that
is in the same IOMMU group as a bridge, for example. Varying SSID size
is less of a concern, since the PCIe specification "highly recommends"
that devices supporting PASID implement all 20 bits of it.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index a01071123c34..f260abadde6d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2279,6 +2279,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
}
static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
+ struct arm_smmu_master *master,
struct io_pgtable_cfg *pgtbl_cfg)
{
int ret;
@@ -2290,6 +2291,8 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
if (asid < 0)
return asid;
+ cfg->s1cdmax = master->ssid_bits;
+
ret = arm_smmu_alloc_cd_tables(smmu_domain);
if (ret)
goto out_free_asid;
@@ -2306,6 +2309,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
}
static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
+ struct arm_smmu_master *master,
struct io_pgtable_cfg *pgtbl_cfg)
{
int vmid;
@@ -2322,7 +2326,8 @@ static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
return 0;
}
-static int arm_smmu_domain_finalise(struct iommu_domain *domain)
+static int arm_smmu_domain_finalise(struct iommu_domain *domain,
+ struct arm_smmu_master *master)
{
int ret;
unsigned long ias, oas;
@@ -2330,6 +2335,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
struct io_pgtable_cfg pgtbl_cfg;
struct io_pgtable_ops *pgtbl_ops;
int (*finalise_stage_fn)(struct arm_smmu_domain *,
+ struct arm_smmu_master *,
struct io_pgtable_cfg *);
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain->smmu;
@@ -2384,7 +2390,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
domain->geometry.aperture_end = (1UL << pgtbl_cfg.ias) - 1;
domain->geometry.force_aperture = true;
- ret = finalise_stage_fn(smmu_domain, &pgtbl_cfg);
+ ret = finalise_stage_fn(smmu_domain, master, &pgtbl_cfg);
if (ret < 0) {
free_io_pgtable_ops(pgtbl_ops);
return ret;
@@ -2537,7 +2543,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
if (!smmu_domain->smmu) {
smmu_domain->smmu = smmu;
- ret = arm_smmu_domain_finalise(domain);
+ ret = arm_smmu_domain_finalise(domain, master);
if (ret) {
smmu_domain->smmu = NULL;
goto out_unlock;
@@ -2549,6 +2555,13 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
dev_name(smmu->dev));
ret = -ENXIO;
goto out_unlock;
+ } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
+ master->ssid_bits != smmu_domain->s1_cfg.s1cdmax) {
+ dev_err(dev,
+ "cannot attach to incompatible domain (%u SSID bits != %u)\n",
+ smmu_domain->s1_cfg.s1cdmax, master->ssid_bits);
+ ret = -EINVAL;
+ goto out_unlock;
}
master->domain = smmu_domain;
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 08/13] iommu/arm-smmu-v3: Propate ssid_bits
2019-12-09 18:05 ` [PATCH v3 08/13] iommu/arm-smmu-v3: Propate ssid_bits Jean-Philippe Brucker
@ 2019-12-17 17:07 ` Auger Eric
2019-12-18 16:08 ` Jean-Philippe Brucker
0 siblings, 1 reply; 40+ messages in thread
From: Auger Eric @ 2019-12-17 17:07 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
s/Propate/Propagate in the commit title.
> Now that we support substream IDs, initialize s1cdmax with the number of
> SSID bits supported by a master and the SMMU.
>
> Context descriptor tables are allocated once for the first master
> attached to a domain. Therefore attaching multiple devices with
> different SSID sizes is tricky, and we currently don't support it.
>
> As a future improvement it would be nice to at least support attaching a
> SSID-capable device to a domain that isn't using SSID, by reallocating
> the SSID table.
Isn't that use case relevant (I mean using both devices in a non SSID
use case). For platform devices you can work this around with FW but for
PCI devices?
This would allow supporting a SSID-capable device that
> is in the same IOMMU group as a bridge, for example. Varying SSID size
> is less of a concern, since the PCIe specification "highly recommends"
> that devices supporting PASID implement all 20 bits of it.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Besides, Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> drivers/iommu/arm-smmu-v3.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index a01071123c34..f260abadde6d 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2279,6 +2279,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
> }
>
> static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
> + struct arm_smmu_master *master,
> struct io_pgtable_cfg *pgtbl_cfg)
> {
> int ret;
> @@ -2290,6 +2291,8 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
> if (asid < 0)
> return asid;
>
> + cfg->s1cdmax = master->ssid_bits;
> +
> ret = arm_smmu_alloc_cd_tables(smmu_domain);
> if (ret)
> goto out_free_asid;
> @@ -2306,6 +2309,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
> }
>
> static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
> + struct arm_smmu_master *master,
> struct io_pgtable_cfg *pgtbl_cfg)
> {
> int vmid;
> @@ -2322,7 +2326,8 @@ static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
> return 0;
> }
>
> -static int arm_smmu_domain_finalise(struct iommu_domain *domain)
> +static int arm_smmu_domain_finalise(struct iommu_domain *domain,
> + struct arm_smmu_master *master)
> {
> int ret;
> unsigned long ias, oas;
> @@ -2330,6 +2335,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
> struct io_pgtable_cfg pgtbl_cfg;
> struct io_pgtable_ops *pgtbl_ops;
> int (*finalise_stage_fn)(struct arm_smmu_domain *,
> + struct arm_smmu_master *,
> struct io_pgtable_cfg *);
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> @@ -2384,7 +2390,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
> domain->geometry.aperture_end = (1UL << pgtbl_cfg.ias) - 1;
> domain->geometry.force_aperture = true;
>
> - ret = finalise_stage_fn(smmu_domain, &pgtbl_cfg);
> + ret = finalise_stage_fn(smmu_domain, master, &pgtbl_cfg);
> if (ret < 0) {
> free_io_pgtable_ops(pgtbl_ops);
> return ret;
> @@ -2537,7 +2543,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>
> if (!smmu_domain->smmu) {
> smmu_domain->smmu = smmu;
> - ret = arm_smmu_domain_finalise(domain);
> + ret = arm_smmu_domain_finalise(domain, master);
> if (ret) {
> smmu_domain->smmu = NULL;
> goto out_unlock;
> @@ -2549,6 +2555,13 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> dev_name(smmu->dev));
> ret = -ENXIO;
> goto out_unlock;
> + } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
> + master->ssid_bits != smmu_domain->s1_cfg.s1cdmax) {
> + dev_err(dev,
> + "cannot attach to incompatible domain (%u SSID bits != %u)\n",
> + smmu_domain->s1_cfg.s1cdmax, master->ssid_bits);
> + ret = -EINVAL;
> + goto out_unlock;
> }
>
> master->domain = smmu_domain;
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 08/13] iommu/arm-smmu-v3: Propate ssid_bits
2019-12-17 17:07 ` Auger Eric
@ 2019-12-18 16:08 ` Jean-Philippe Brucker
2019-12-18 17:00 ` Auger Eric
0 siblings, 1 reply; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-18 16:08 UTC (permalink / raw)
To: Auger Eric
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
On Tue, Dec 17, 2019 at 06:07:26PM +0100, Auger Eric wrote:
> Hi Jean,
>
> On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
>
> s/Propate/Propagate in the commit title.
> > Now that we support substream IDs, initialize s1cdmax with the number of
> > SSID bits supported by a master and the SMMU.
> >
> > Context descriptor tables are allocated once for the first master
> > attached to a domain. Therefore attaching multiple devices with
> > different SSID sizes is tricky, and we currently don't support it.
> >
> > As a future improvement it would be nice to at least support attaching a
> > SSID-capable device to a domain that isn't using SSID, by reallocating
> > the SSID table.
> Isn't that use case relevant (I mean using both devices in a non SSID
> use case). For platform devices you can work this around with FW but for
> PCI devices?
Normally each device gets its own domain. Especially since PASID is a PCI
Express capability, I expect them to be properly isolated with ACS, each
with its own IOMMU group. So I don't think this is too relevant for the
moment, it would be a quirk for a broken system.
Thanks,
Jean
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 08/13] iommu/arm-smmu-v3: Propate ssid_bits
2019-12-18 16:08 ` Jean-Philippe Brucker
@ 2019-12-18 17:00 ` Auger Eric
0 siblings, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-18 17:00 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/18/19 5:08 PM, Jean-Philippe Brucker wrote:
> On Tue, Dec 17, 2019 at 06:07:26PM +0100, Auger Eric wrote:
>> Hi Jean,
>>
>> On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
>>
>> s/Propate/Propagate in the commit title.
>>> Now that we support substream IDs, initialize s1cdmax with the number of
>>> SSID bits supported by a master and the SMMU.
>>>
>>> Context descriptor tables are allocated once for the first master
>>> attached to a domain. Therefore attaching multiple devices with
>>> different SSID sizes is tricky, and we currently don't support it.
>>>
>>> As a future improvement it would be nice to at least support attaching a
>>> SSID-capable device to a domain that isn't using SSID, by reallocating
>>> the SSID table.
>> Isn't that use case relevant (I mean using both devices in a non SSID
>> use case). For platform devices you can work this around with FW but for
>> PCI devices?
>
> Normally each device gets its own domain. Especially since PASID is a PCI
> Express capability, I expect them to be properly isolated with ACS, each
> with its own IOMMU group. So I don't think this is too relevant for the
> moment, it would be a quirk for a broken system.
OK
Eric
>
> Thanks,
> Jean
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 09/13] iommu/arm-smmu-v3: Handle failure of arm_smmu_write_ctx_desc()
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (7 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 08/13] iommu/arm-smmu-v3: Propate ssid_bits Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-17 17:24 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 10/13] iommu/arm-smmu-v3: Add second level of context descriptor table Jean-Philippe Brucker
` (4 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
Second-level context descriptor tables will be allocated lazily in
arm_smmu_write_ctx_desc(). Help with handling allocation failure by
moving the CD write into arm_smmu_domain_finalise_s1().
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f260abadde6d..fc5119f34187 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2301,8 +2301,15 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
cfg->cd.tcr = pgtbl_cfg->arm_lpae_s1_cfg.tcr;
cfg->cd.mair = pgtbl_cfg->arm_lpae_s1_cfg.mair;
+
+ ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd);
+ if (ret)
+ goto out_free_tables;
+
return 0;
+out_free_tables:
+ arm_smmu_free_cd_tables(smmu_domain);
out_free_asid:
arm_smmu_bitmap_free(smmu->asid_map, asid);
return ret;
@@ -2569,10 +2576,6 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
if (smmu_domain->stage != ARM_SMMU_DOMAIN_BYPASS)
master->ats_enabled = arm_smmu_ats_supported(master);
- if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
- arm_smmu_write_ctx_desc(smmu_domain, 0,
- &smmu_domain->s1_cfg.cd);
-
arm_smmu_install_ste_for_dev(master);
spin_lock_irqsave(&smmu_domain->devices_lock, flags);
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 09/13] iommu/arm-smmu-v3: Handle failure of arm_smmu_write_ctx_desc()
2019-12-09 18:05 ` [PATCH v3 09/13] iommu/arm-smmu-v3: Handle failure of arm_smmu_write_ctx_desc() Jean-Philippe Brucker
@ 2019-12-17 17:24 ` Auger Eric
0 siblings, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-17 17:24 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> Second-level context descriptor tables will be allocated lazily in
> arm_smmu_write_ctx_desc(). Help with handling allocation failure by
> moving the CD write into arm_smmu_domain_finalise_s1().
nit: would rather change the title to something like "Prepare for
arm_smmu_write_ctx_desc() failure"
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> drivers/iommu/arm-smmu-v3.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index f260abadde6d..fc5119f34187 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2301,8 +2301,15 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
> cfg->cd.ttbr = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
> cfg->cd.tcr = pgtbl_cfg->arm_lpae_s1_cfg.tcr;
> cfg->cd.mair = pgtbl_cfg->arm_lpae_s1_cfg.mair;
> +
> + ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd);
> + if (ret)
> + goto out_free_tables;
> +
> return 0;
>
> +out_free_tables:
> + arm_smmu_free_cd_tables(smmu_domain);
> out_free_asid:
> arm_smmu_bitmap_free(smmu->asid_map, asid);
> return ret;
> @@ -2569,10 +2576,6 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> if (smmu_domain->stage != ARM_SMMU_DOMAIN_BYPASS)
> master->ats_enabled = arm_smmu_ats_supported(master);
>
> - if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
> - arm_smmu_write_ctx_desc(smmu_domain, 0,
> - &smmu_domain->s1_cfg.cd);
> -
> arm_smmu_install_ste_for_dev(master);
>
> spin_lock_irqsave(&smmu_domain->devices_lock, flags);
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 10/13] iommu/arm-smmu-v3: Add second level of context descriptor table
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (8 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 09/13] iommu/arm-smmu-v3: Handle failure of arm_smmu_write_ctx_desc() Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-13 17:07 ` Jonathan Cameron
2019-12-18 9:59 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 11/13] iommu/arm-smmu-v3: Improve add_device() error handling Jean-Philippe Brucker
` (3 subsequent siblings)
13 siblings, 2 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
The SMMU can support up to 20 bits of SSID. Add a second level of page
tables to accommodate this. Devices that support more than 1024 SSIDs now
have a table of 1024 L1 entries (8kB), pointing to tables of 1024 context
descriptors (64kB), allocated on demand.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 153 +++++++++++++++++++++++++++++++++---
1 file changed, 143 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index fc5119f34187..52adcdfda58b 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -224,6 +224,7 @@
#define STRTAB_STE_0_S1FMT GENMASK_ULL(5, 4)
#define STRTAB_STE_0_S1FMT_LINEAR 0
+#define STRTAB_STE_0_S1FMT_64K_L2 2
#define STRTAB_STE_0_S1CTXPTR_MASK GENMASK_ULL(51, 6)
#define STRTAB_STE_0_S1CDMAX GENMASK_ULL(63, 59)
@@ -263,7 +264,20 @@
#define STRTAB_STE_3_S2TTB_MASK GENMASK_ULL(51, 4)
-/* Context descriptor (stage-1 only) */
+/*
+ * Context descriptors.
+ *
+ * Linear: when less than 1024 SSIDs are supported
+ * 2lvl: at most 1024 L1 entries,
+ * 1024 lazy entries per table.
+ */
+#define CTXDESC_SPLIT 10
+#define CTXDESC_L2_ENTRIES (1 << CTXDESC_SPLIT)
+
+#define CTXDESC_L1_DESC_DWORDS 1
+#define CTXDESC_L1_DESC_VALID 1
+#define CTXDESC_L1_DESC_L2PTR_MASK GENMASK_ULL(51, 12)
+
#define CTXDESC_CD_DWORDS 8
#define CTXDESC_CD_0_TCR_T0SZ GENMASK_ULL(5, 0)
#define ARM64_TCR_T0SZ GENMASK_ULL(5, 0)
@@ -575,7 +589,10 @@ struct arm_smmu_cd_table {
};
struct arm_smmu_s1_cfg {
- struct arm_smmu_cd_table table;
+ struct arm_smmu_cd_table *tables;
+ size_t num_tables;
+ __le64 *l1ptr;
+ dma_addr_t l1ptr_dma;
struct arm_smmu_ctx_desc cd;
u8 s1fmt;
u8 s1cdmax;
@@ -1521,9 +1538,53 @@ static void arm_smmu_free_cd_leaf_table(struct arm_smmu_device *smmu,
{
size_t size = num_entries * (CTXDESC_CD_DWORDS << 3);
+ if (!table->ptr)
+ return;
dmam_free_coherent(smmu->dev, size, table->ptr, table->ptr_dma);
}
+static void arm_smmu_write_cd_l1_desc(__le64 *dst,
+ struct arm_smmu_cd_table *table)
+{
+ u64 val = (table->ptr_dma & CTXDESC_L1_DESC_L2PTR_MASK) |
+ CTXDESC_L1_DESC_VALID;
+
+ WRITE_ONCE(*dst, cpu_to_le64(val));
+}
+
+static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
+ u32 ssid)
+{
+ __le64 *l1ptr;
+ unsigned int idx;
+ struct arm_smmu_cd_table *table;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+
+ if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) {
+ table = &cfg->tables[0];
+ idx = ssid;
+ } else {
+ idx = ssid >> CTXDESC_SPLIT;
+ if (idx >= cfg->num_tables)
+ return NULL;
+
+ table = &cfg->tables[idx];
+ if (!table->ptr) {
+ if (arm_smmu_alloc_cd_leaf_table(smmu, table,
+ CTXDESC_L2_ENTRIES))
+ return NULL;
+
+ l1ptr = cfg->l1ptr + idx * CTXDESC_L1_DESC_DWORDS;
+ arm_smmu_write_cd_l1_desc(l1ptr, table);
+ /* An invalid L1CD can be cached */
+ arm_smmu_sync_cd(smmu_domain, ssid, false);
+ }
+ idx = ssid & (CTXDESC_L2_ENTRIES - 1);
+ }
+ return table->ptr + idx * CTXDESC_CD_DWORDS;
+}
+
static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
{
u64 val = 0;
@@ -1556,8 +1617,10 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
u64 val;
bool cd_live;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
- CTXDESC_CD_DWORDS;
+ __le64 *cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
+
+ if (!cdptr)
+ return -ENOMEM;
val = le64_to_cpu(cdptr[0]);
cd_live = !!(val & CTXDESC_CD_0_V);
@@ -1604,20 +1667,87 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
{
+ int ret;
+ size_t size = 0;
+ size_t max_contexts;
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
- cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
- return arm_smmu_alloc_cd_leaf_table(smmu, &cfg->table,
- 1 << cfg->s1cdmax);
+ max_contexts = 1 << cfg->s1cdmax;
+
+ if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB) ||
+ max_contexts <= CTXDESC_L2_ENTRIES) {
+ cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
+ cfg->num_tables = 1;
+ } else {
+ cfg->s1fmt = STRTAB_STE_0_S1FMT_64K_L2;
+ cfg->num_tables = DIV_ROUND_UP(max_contexts,
+ CTXDESC_L2_ENTRIES);
+
+ size = cfg->num_tables * (CTXDESC_L1_DESC_DWORDS << 3);
+ cfg->l1ptr = dmam_alloc_coherent(smmu->dev, size,
+ &cfg->l1ptr_dma,
+ GFP_KERNEL);
+ if (!cfg->l1ptr) {
+ dev_warn(smmu->dev,
+ "failed to allocate L1 context table\n");
+ return -ENOMEM;
+ }
+ }
+
+ cfg->tables = devm_kzalloc(smmu->dev, sizeof(struct arm_smmu_cd_table) *
+ cfg->num_tables, GFP_KERNEL);
+ if (!cfg->tables) {
+ ret = -ENOMEM;
+ goto err_free_l1;
+ }
+
+ /*
+ * Only allocate a leaf table for linear case. With two levels, leaf
+ * tables are allocated lazily.
+ */
+ if (!cfg->l1ptr) {
+ ret = arm_smmu_alloc_cd_leaf_table(smmu, &cfg->tables[0],
+ max_contexts);
+ if (ret)
+ goto err_free_tables;
+ }
+
+ return 0;
+
+err_free_tables:
+ devm_kfree(smmu->dev, cfg->tables);
+ cfg->tables = NULL;
+err_free_l1:
+ if (cfg->l1ptr) {
+ dmam_free_coherent(smmu->dev, size, cfg->l1ptr, cfg->l1ptr_dma);
+ cfg->l1ptr = NULL;
+ cfg->l1ptr_dma = 0;
+ }
+ return ret;
}
static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
{
+ int i;
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+ size_t num_leaf_entries = 1 << cfg->s1cdmax;
+ struct arm_smmu_cd_table *table = cfg->tables;
+
+ if (cfg->l1ptr) {
+ size_t size = cfg->num_tables * (CTXDESC_L1_DESC_DWORDS << 3);
- arm_smmu_free_cd_leaf_table(smmu, &cfg->table, 1 << cfg->s1cdmax);
+ dmam_free_coherent(smmu->dev, size, cfg->l1ptr, cfg->l1ptr_dma);
+ cfg->l1ptr = NULL;
+ cfg->l1ptr_dma = 0;
+ num_leaf_entries = CTXDESC_L2_ENTRIES;
+ }
+
+ for (i = 0; i < cfg->num_tables; i++, table++)
+ arm_smmu_free_cd_leaf_table(smmu, table, num_leaf_entries);
+ devm_kfree(smmu->dev, cfg->tables);
+ cfg->tables = NULL;
}
/* Stream table manipulation functions */
@@ -1737,6 +1867,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
}
if (s1_cfg) {
+ dma_addr_t ptr_dma = s1_cfg->l1ptr ? s1_cfg->l1ptr_dma :
+ s1_cfg->tables[0].ptr_dma;
+
BUG_ON(ste_live);
dst[1] = cpu_to_le64(
FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
@@ -1749,7 +1882,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
!(smmu->features & ARM_SMMU_FEAT_STALL_FORCE))
dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
- val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
+ val |= (ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
@@ -2265,7 +2398,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
- if (cfg->table.ptr) {
+ if (cfg->tables) {
arm_smmu_free_cd_tables(smmu_domain);
arm_smmu_bitmap_free(smmu->asid_map, cfg->cd.asid);
}
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 10/13] iommu/arm-smmu-v3: Add second level of context descriptor table
2019-12-09 18:05 ` [PATCH v3 10/13] iommu/arm-smmu-v3: Add second level of context descriptor table Jean-Philippe Brucker
@ 2019-12-13 17:07 ` Jonathan Cameron
2019-12-18 9:59 ` Auger Eric
1 sibling, 0 replies; 40+ messages in thread
From: Jonathan Cameron @ 2019-12-13 17:07 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, zhangfei.gao
On Mon, 9 Dec 2019 19:05:11 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> The SMMU can support up to 20 bits of SSID. Add a second level of page
> tables to accommodate this. Devices that support more than 1024 SSIDs now
> have a table of 1024 L1 entries (8kB), pointing to tables of 1024 context
> descriptors (64kB), allocated on demand.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
One tiny little comment inline. I really don't mind if you ignore it ;)
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> drivers/iommu/arm-smmu-v3.c | 153 +++++++++++++++++++++++++++++++++---
> 1 file changed, 143 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index fc5119f34187..52adcdfda58b 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -224,6 +224,7 @@
>
> #define STRTAB_STE_0_S1FMT GENMASK_ULL(5, 4)
> #define STRTAB_STE_0_S1FMT_LINEAR 0
> +#define STRTAB_STE_0_S1FMT_64K_L2 2
> #define STRTAB_STE_0_S1CTXPTR_MASK GENMASK_ULL(51, 6)
> #define STRTAB_STE_0_S1CDMAX GENMASK_ULL(63, 59)
>
> @@ -263,7 +264,20 @@
>
> #define STRTAB_STE_3_S2TTB_MASK GENMASK_ULL(51, 4)
>
> -/* Context descriptor (stage-1 only) */
> +/*
> + * Context descriptors.
> + *
> + * Linear: when less than 1024 SSIDs are supported
> + * 2lvl: at most 1024 L1 entries,
> + * 1024 lazy entries per table.
> + */
> +#define CTXDESC_SPLIT 10
> +#define CTXDESC_L2_ENTRIES (1 << CTXDESC_SPLIT)
> +
> +#define CTXDESC_L1_DESC_DWORDS 1
> +#define CTXDESC_L1_DESC_VALID 1
> +#define CTXDESC_L1_DESC_L2PTR_MASK GENMASK_ULL(51, 12)
> +
> #define CTXDESC_CD_DWORDS 8
> #define CTXDESC_CD_0_TCR_T0SZ GENMASK_ULL(5, 0)
> #define ARM64_TCR_T0SZ GENMASK_ULL(5, 0)
> @@ -575,7 +589,10 @@ struct arm_smmu_cd_table {
> };
>
> struct arm_smmu_s1_cfg {
> - struct arm_smmu_cd_table table;
> + struct arm_smmu_cd_table *tables;
> + size_t num_tables;
> + __le64 *l1ptr;
> + dma_addr_t l1ptr_dma;
> struct arm_smmu_ctx_desc cd;
> u8 s1fmt;
> u8 s1cdmax;
> @@ -1521,9 +1538,53 @@ static void arm_smmu_free_cd_leaf_table(struct arm_smmu_device *smmu,
> {
> size_t size = num_entries * (CTXDESC_CD_DWORDS << 3);
>
> + if (!table->ptr)
> + return;
> dmam_free_coherent(smmu->dev, size, table->ptr, table->ptr_dma);
> }
>
> +static void arm_smmu_write_cd_l1_desc(__le64 *dst,
> + struct arm_smmu_cd_table *table)
> +{
> + u64 val = (table->ptr_dma & CTXDESC_L1_DESC_L2PTR_MASK) |
> + CTXDESC_L1_DESC_VALID;
> +
> + WRITE_ONCE(*dst, cpu_to_le64(val));
> +}
> +
> +static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
> + u32 ssid)
> +{
> + __le64 *l1ptr;
> + unsigned int idx;
> + struct arm_smmu_cd_table *table;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> +
> + if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) {
> + table = &cfg->tables[0];
> + idx = ssid;
> + } else {
> + idx = ssid >> CTXDESC_SPLIT;
> + if (idx >= cfg->num_tables)
> + return NULL;
> +
> + table = &cfg->tables[idx];
> + if (!table->ptr) {
> + if (arm_smmu_alloc_cd_leaf_table(smmu, table,
> + CTXDESC_L2_ENTRIES))
> + return NULL;
> +
> + l1ptr = cfg->l1ptr + idx * CTXDESC_L1_DESC_DWORDS;
> + arm_smmu_write_cd_l1_desc(l1ptr, table);
> + /* An invalid L1CD can be cached */
> + arm_smmu_sync_cd(smmu_domain, ssid, false);
> + }
> + idx = ssid & (CTXDESC_L2_ENTRIES - 1);
> + }
> + return table->ptr + idx * CTXDESC_CD_DWORDS;
> +}
> +
> static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
> {
> u64 val = 0;
> @@ -1556,8 +1617,10 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
> u64 val;
> bool cd_live;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> - __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
> - CTXDESC_CD_DWORDS;
> + __le64 *cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
> +
> + if (!cdptr)
> + return -ENOMEM;
>
> val = le64_to_cpu(cdptr[0]);
> cd_live = !!(val & CTXDESC_CD_0_V);
> @@ -1604,20 +1667,87 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
>
> static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
> {
> + int ret;
> + size_t size = 0;
> + size_t max_contexts;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>
> - cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
> - return arm_smmu_alloc_cd_leaf_table(smmu, &cfg->table,
> - 1 << cfg->s1cdmax);
> + max_contexts = 1 << cfg->s1cdmax;
> +
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB) ||
> + max_contexts <= CTXDESC_L2_ENTRIES) {
> + cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
> + cfg->num_tables = 1;
> + } else {
> + cfg->s1fmt = STRTAB_STE_0_S1FMT_64K_L2;
> + cfg->num_tables = DIV_ROUND_UP(max_contexts,
> + CTXDESC_L2_ENTRIES);
> +
> + size = cfg->num_tables * (CTXDESC_L1_DESC_DWORDS << 3);
> + cfg->l1ptr = dmam_alloc_coherent(smmu->dev, size,
> + &cfg->l1ptr_dma,
> + GFP_KERNEL);
> + if (!cfg->l1ptr) {
> + dev_warn(smmu->dev,
> + "failed to allocate L1 context table\n");
> + return -ENOMEM;
> + }
> + }
> +
> + cfg->tables = devm_kzalloc(smmu->dev, sizeof(struct arm_smmu_cd_table) *
> + cfg->num_tables, GFP_KERNEL);
> + if (!cfg->tables) {
> + ret = -ENOMEM;
> + goto err_free_l1;
> + }
> +
> + /*
> + * Only allocate a leaf table for linear case. With two levels, leaf
> + * tables are allocated lazily.
> + */
> + if (!cfg->l1ptr) {
This might have been the tiniest bit clearer if the check matched the description
by checking it's a linear table rather than what is kind of a side effect of it
not being non linear.
if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR)
> + ret = arm_smmu_alloc_cd_leaf_table(smmu, &cfg->tables[0],
> + max_contexts);
> + if (ret)
> + goto err_free_tables;
> + }
> +
> + return 0;
> +
> +err_free_tables:
> + devm_kfree(smmu->dev, cfg->tables);
> + cfg->tables = NULL;
> +err_free_l1:
> + if (cfg->l1ptr) {
> + dmam_free_coherent(smmu->dev, size, cfg->l1ptr, cfg->l1ptr_dma);
> + cfg->l1ptr = NULL;
> + cfg->l1ptr_dma = 0;
> + }
> + return ret;
> }
>
> static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
> {
> + int i;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> + size_t num_leaf_entries = 1 << cfg->s1cdmax;
> + struct arm_smmu_cd_table *table = cfg->tables;
> +
> + if (cfg->l1ptr) {
> + size_t size = cfg->num_tables * (CTXDESC_L1_DESC_DWORDS << 3);
>
> - arm_smmu_free_cd_leaf_table(smmu, &cfg->table, 1 << cfg->s1cdmax);
> + dmam_free_coherent(smmu->dev, size, cfg->l1ptr, cfg->l1ptr_dma);
> + cfg->l1ptr = NULL;
> + cfg->l1ptr_dma = 0;
> + num_leaf_entries = CTXDESC_L2_ENTRIES;
> + }
> +
> + for (i = 0; i < cfg->num_tables; i++, table++)
> + arm_smmu_free_cd_leaf_table(smmu, table, num_leaf_entries);
> + devm_kfree(smmu->dev, cfg->tables);
> + cfg->tables = NULL;
> }
>
> /* Stream table manipulation functions */
> @@ -1737,6 +1867,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> }
>
> if (s1_cfg) {
> + dma_addr_t ptr_dma = s1_cfg->l1ptr ? s1_cfg->l1ptr_dma :
> + s1_cfg->tables[0].ptr_dma;
> +
> BUG_ON(ste_live);
> dst[1] = cpu_to_le64(
> FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
> @@ -1749,7 +1882,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> !(smmu->features & ARM_SMMU_FEAT_STALL_FORCE))
> dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
>
> - val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> + val |= (ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
> FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
> FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
> @@ -2265,7 +2398,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>
> - if (cfg->table.ptr) {
> + if (cfg->tables) {
> arm_smmu_free_cd_tables(smmu_domain);
> arm_smmu_bitmap_free(smmu->asid_map, cfg->cd.asid);
> }
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 10/13] iommu/arm-smmu-v3: Add second level of context descriptor table
2019-12-09 18:05 ` [PATCH v3 10/13] iommu/arm-smmu-v3: Add second level of context descriptor table Jean-Philippe Brucker
2019-12-13 17:07 ` Jonathan Cameron
@ 2019-12-18 9:59 ` Auger Eric
2019-12-18 16:10 ` Jean-Philippe Brucker
1 sibling, 1 reply; 40+ messages in thread
From: Auger Eric @ 2019-12-18 9:59 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> The SMMU can support up to 20 bits of SSID. Add a second level of page
> tables to accommodate this. Devices that support more than 1024 SSIDs now
> have a table of 1024 L1 entries (8kB), pointing to tables of 1024 context
> descriptors (64kB), allocated on demand.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> drivers/iommu/arm-smmu-v3.c | 153 +++++++++++++++++++++++++++++++++---
> 1 file changed, 143 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index fc5119f34187..52adcdfda58b 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -224,6 +224,7 @@
>
> #define STRTAB_STE_0_S1FMT GENMASK_ULL(5, 4)
> #define STRTAB_STE_0_S1FMT_LINEAR 0
> +#define STRTAB_STE_0_S1FMT_64K_L2 2
> #define STRTAB_STE_0_S1CTXPTR_MASK GENMASK_ULL(51, 6)
> #define STRTAB_STE_0_S1CDMAX GENMASK_ULL(63, 59)
>
> @@ -263,7 +264,20 @@
>
> #define STRTAB_STE_3_S2TTB_MASK GENMASK_ULL(51, 4)
>
> -/* Context descriptor (stage-1 only) */
> +/*
> + * Context descriptors.
> + *
> + * Linear: when less than 1024 SSIDs are supported
> + * 2lvl: at most 1024 L1 entries,
> + * 1024 lazy entries per table.
> + */
> +#define CTXDESC_SPLIT 10
> +#define CTXDESC_L2_ENTRIES (1 << CTXDESC_SPLIT)
> +
> +#define CTXDESC_L1_DESC_DWORDS 1
> +#define CTXDESC_L1_DESC_VALID 1
> +#define CTXDESC_L1_DESC_L2PTR_MASK GENMASK_ULL(51, 12)
> +
> #define CTXDESC_CD_DWORDS 8
> #define CTXDESC_CD_0_TCR_T0SZ GENMASK_ULL(5, 0)
> #define ARM64_TCR_T0SZ GENMASK_ULL(5, 0)
> @@ -575,7 +589,10 @@ struct arm_smmu_cd_table {
> };
>
> struct arm_smmu_s1_cfg {
> - struct arm_smmu_cd_table table;
> + struct arm_smmu_cd_table *tables;
> + size_t num_tables;
> + __le64 *l1ptr;
you may add a comment saying that l1ptr and l1ptr_dma are only set/used
in non linear case and one comment saying that "tables" represent leaf
tables.
> + dma_addr_t l1ptr_dma;
> struct arm_smmu_ctx_desc cd;
> u8 s1fmt;
> u8 s1cdmax;
> @@ -1521,9 +1538,53 @@ static void arm_smmu_free_cd_leaf_table(struct arm_smmu_device *smmu,
> {
> size_t size = num_entries * (CTXDESC_CD_DWORDS << 3);
>
> + if (!table->ptr)
> + return;
> dmam_free_coherent(smmu->dev, size, table->ptr, table->ptr_dma);
> }
>
> +static void arm_smmu_write_cd_l1_desc(__le64 *dst,
> + struct arm_smmu_cd_table *table)
> +{
> + u64 val = (table->ptr_dma & CTXDESC_L1_DESC_L2PTR_MASK) |
> + CTXDESC_L1_DESC_VALID;
> +
> + WRITE_ONCE(*dst, cpu_to_le64(val));
> +}
> +
> +static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
> + u32 ssid)
> +{
> + __le64 *l1ptr;
> + unsigned int idx;
> + struct arm_smmu_cd_table *table;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> +
> + if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) {
> + table = &cfg->tables[0];
> + idx = ssid;
> + } else {
nit: you may avoid this extra indent by either returning above or go to
a label.
> + idx = ssid >> CTXDESC_SPLIT;
> + if (idx >= cfg->num_tables)
> + return NULL;
> +
> + table = &cfg->tables[idx];
> + if (!table->ptr) {
> + if (arm_smmu_alloc_cd_leaf_table(smmu, table,
> + CTXDESC_L2_ENTRIES))
> + return NULL;
> +
> + l1ptr = cfg->l1ptr + idx * CTXDESC_L1_DESC_DWORDS;
> + arm_smmu_write_cd_l1_desc(l1ptr, table);
> + /* An invalid L1CD can be cached */
> + arm_smmu_sync_cd(smmu_domain, ssid, false);
> + }
> + idx = ssid & (CTXDESC_L2_ENTRIES - 1);
> + }
> + return table->ptr + idx * CTXDESC_CD_DWORDS;> +}
> +
> static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
> {
> u64 val = 0;
> @@ -1556,8 +1617,10 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
> u64 val;
> bool cd_live;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> - __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
> - CTXDESC_CD_DWORDS;
> + __le64 *cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
> +
> + if (!cdptr)
> + return -ENOMEM;
-ENOMEM does not fit well with (idx >= cfg->num_tables) case
Besides the idx is checked against the max table capacity only in non
linear mode. Can't you check the ssid against cfg->s1cdmax earlier?
>
> val = le64_to_cpu(cdptr[0]);
> cd_live = !!(val & CTXDESC_CD_0_V);
> @@ -1604,20 +1667,87 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
>
> static int arm_smmu_alloc_cd_tables(struct arm_smmu_domain *smmu_domain)
> {
> + int ret;
> + size_t size = 0;
> + size_t max_contexts;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>
> - cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
> - return arm_smmu_alloc_cd_leaf_table(smmu, &cfg->table,
> - 1 << cfg->s1cdmax);
> + max_contexts = 1 << cfg->s1cdmax;
> +
> + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB) ||
> + max_contexts <= CTXDESC_L2_ENTRIES) {
> + cfg->s1fmt = STRTAB_STE_0_S1FMT_LINEAR;
> + cfg->num_tables = 1;
> + } else {
> + cfg->s1fmt = STRTAB_STE_0_S1FMT_64K_L2;
> + cfg->num_tables = DIV_ROUND_UP(max_contexts,
> + CTXDESC_L2_ENTRIES);
> +
> + size = cfg->num_tables * (CTXDESC_L1_DESC_DWORDS << 3);
> + cfg->l1ptr = dmam_alloc_coherent(smmu->dev, size,
> + &cfg->l1ptr_dma,
> + GFP_KERNEL);
> + if (!cfg->l1ptr) {
> + dev_warn(smmu->dev,
> + "failed to allocate L1 context table\n");
> + return -ENOMEM;
> + }
> + }
> +
> + cfg->tables = devm_kzalloc(smmu->dev, sizeof(struct arm_smmu_cd_table) *
> + cfg->num_tables, GFP_KERNEL);
> + if (!cfg->tables) {
> + ret = -ENOMEM;
> + goto err_free_l1;
> + }
> +
> + /*
> + * Only allocate a leaf table for linear case. With two levels, leaf
> + * tables are allocated lazily.
> + */
> + if (!cfg->l1ptr) {
> + ret = arm_smmu_alloc_cd_leaf_table(smmu, &cfg->tables[0],
> + max_contexts);
> + if (ret)
> + goto err_free_tables;
> + }
> +
> + return 0;
> +
> +err_free_tables:
> + devm_kfree(smmu->dev, cfg->tables);
> + cfg->tables = NULL;
> +err_free_l1:
> + if (cfg->l1ptr) {
> + dmam_free_coherent(smmu->dev, size, cfg->l1ptr, cfg->l1ptr_dma);
> + cfg->l1ptr = NULL;
> + cfg->l1ptr_dma = 0;
> + }
> + return ret;
> }
>
> static void arm_smmu_free_cd_tables(struct arm_smmu_domain *smmu_domain)
> {
> + int i;
> struct arm_smmu_device *smmu = smmu_domain->smmu;
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> + size_t num_leaf_entries = 1 << cfg->s1cdmax;
> + struct arm_smmu_cd_table *table = cfg->tables;
> +
> + if (cfg->l1ptr) {
> + size_t size = cfg->num_tables * (CTXDESC_L1_DESC_DWORDS << 3);
>
> - arm_smmu_free_cd_leaf_table(smmu, &cfg->table, 1 << cfg->s1cdmax);
> + dmam_free_coherent(smmu->dev, size, cfg->l1ptr, cfg->l1ptr_dma);
> + cfg->l1ptr = NULL;
> + cfg->l1ptr_dma = 0;
> + num_leaf_entries = CTXDESC_L2_ENTRIES;
> + }
> +
> + for (i = 0; i < cfg->num_tables; i++, table++)
> + arm_smmu_free_cd_leaf_table(smmu, table, num_leaf_entries);
> + devm_kfree(smmu->dev, cfg->tables);
> + cfg->tables = NULL;
> }
>
> /* Stream table manipulation functions */
> @@ -1737,6 +1867,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> }
>
> if (s1_cfg) {
> + dma_addr_t ptr_dma = s1_cfg->l1ptr ? s1_cfg->l1ptr_dma :
> + s1_cfg->tables[0].ptr_dma;
> +
> BUG_ON(ste_live);
> dst[1] = cpu_to_le64(
> FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
> @@ -1749,7 +1882,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> !(smmu->features & ARM_SMMU_FEAT_STALL_FORCE))
> dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
>
> - val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> + val |= (ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
> FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
> FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
> @@ -2265,7 +2398,7 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
> if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
>
> - if (cfg->table.ptr) {
> + if (cfg->tables) {
> arm_smmu_free_cd_tables(smmu_domain);
> arm_smmu_bitmap_free(smmu->asid_map, cfg->cd.asid);
> }
>
Otherwise looks good to me.
Thanks
Eric
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 10/13] iommu/arm-smmu-v3: Add second level of context descriptor table
2019-12-18 9:59 ` Auger Eric
@ 2019-12-18 16:10 ` Jean-Philippe Brucker
0 siblings, 0 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-18 16:10 UTC (permalink / raw)
To: Auger Eric
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
On Wed, Dec 18, 2019 at 10:59:36AM +0100, Auger Eric wrote:
> > struct arm_smmu_s1_cfg {
> > - struct arm_smmu_cd_table table;
> > + struct arm_smmu_cd_table *tables;
> > + size_t num_tables;
> > + __le64 *l1ptr;
> you may add a comment saying that l1ptr and l1ptr_dma are only set/used
> in non linear case and one comment saying that "tables" represent leaf
> tables.
I now have
/* Leaf tables or linear table */
and
/* First level tables, when two level are used */
but I'm not entirely convinced it adds value
> > + dma_addr_t l1ptr_dma;
> > struct arm_smmu_ctx_desc cd;
> > u8 s1fmt;
> > u8 s1cdmax;
> > @@ -1521,9 +1538,53 @@ static void arm_smmu_free_cd_leaf_table(struct arm_smmu_device *smmu,
> > {
> > size_t size = num_entries * (CTXDESC_CD_DWORDS << 3);
> >
> > + if (!table->ptr)
> > + return;
> > dmam_free_coherent(smmu->dev, size, table->ptr, table->ptr_dma);
> > }
> >
> > +static void arm_smmu_write_cd_l1_desc(__le64 *dst,
> > + struct arm_smmu_cd_table *table)
> > +{
> > + u64 val = (table->ptr_dma & CTXDESC_L1_DESC_L2PTR_MASK) |
> > + CTXDESC_L1_DESC_VALID;
> > +
> > + WRITE_ONCE(*dst, cpu_to_le64(val));
> > +}
> > +
> > +static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
> > + u32 ssid)
> > +{
> > + __le64 *l1ptr;
> > + unsigned int idx;
> > + struct arm_smmu_cd_table *table;
> > + struct arm_smmu_device *smmu = smmu_domain->smmu;
> > + struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
> > +
> > + if (cfg->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) {
> > + table = &cfg->tables[0];
> > + idx = ssid;
> > + } else {
> nit: you may avoid this extra indent by either returning above or go to
> a label.
> > + idx = ssid >> CTXDESC_SPLIT;
> > + if (idx >= cfg->num_tables)
> > + return NULL;
> > +
> > + table = &cfg->tables[idx];
> > + if (!table->ptr) {
> > + if (arm_smmu_alloc_cd_leaf_table(smmu, table,
> > + CTXDESC_L2_ENTRIES))
> > + return NULL;
> > +
> > + l1ptr = cfg->l1ptr + idx * CTXDESC_L1_DESC_DWORDS;
> > + arm_smmu_write_cd_l1_desc(l1ptr, table);
> > + /* An invalid L1CD can be cached */
> > + arm_smmu_sync_cd(smmu_domain, ssid, false);
> > + }
> > + idx = ssid & (CTXDESC_L2_ENTRIES - 1);
> > + }
> > + return table->ptr + idx * CTXDESC_CD_DWORDS;> +}
> > +
> > static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
> > {
> > u64 val = 0;
> > @@ -1556,8 +1617,10 @@ static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
> > u64 val;
> > bool cd_live;
> > struct arm_smmu_device *smmu = smmu_domain->smmu;
> > - __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
> > - CTXDESC_CD_DWORDS;
> > + __le64 *cdptr = arm_smmu_get_cd_ptr(smmu_domain, ssid);
> > +
> > + if (!cdptr)
> > + return -ENOMEM;
> -ENOMEM does not fit well with (idx >= cfg->num_tables) case
> Besides the idx is checked against the max table capacity only in non
> linear mode. Can't you check the ssid against cfg->s1cdmax earlier?
Ok, I'll move that check here
Thanks,
Jean
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 11/13] iommu/arm-smmu-v3: Improve add_device() error handling
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (9 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 10/13] iommu/arm-smmu-v3: Add second level of context descriptor table Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 12/13] PCI/ATS: Add PASID stubs Jean-Philippe Brucker
` (2 subsequent siblings)
13 siblings, 0 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
Let add_device() clean up after itself. The iommu_bus_init() function
does call remove_device() on error, but other sites (e.g. of_iommu) do
not.
Don't free level-2 stream tables because we'd have to track if we
allocated each of them or if they are used by other endpoints. It's not
worth the hassle since they are managed resources.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 52adcdfda58b..d20a79108f8a 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2836,14 +2836,16 @@ static int arm_smmu_add_device(struct device *dev)
for (i = 0; i < master->num_sids; i++) {
u32 sid = master->sids[i];
- if (!arm_smmu_sid_in_range(smmu, sid))
- return -ERANGE;
+ if (!arm_smmu_sid_in_range(smmu, sid)) {
+ ret = -ERANGE;
+ goto err_free_master;
+ }
/* Ensure l2 strtab is initialised */
if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
ret = arm_smmu_init_l2_strtab(smmu, sid);
if (ret)
- return ret;
+ goto err_free_master;
}
}
@@ -2853,13 +2855,25 @@ static int arm_smmu_add_device(struct device *dev)
master->ssid_bits = min_t(u8, master->ssid_bits,
CTXDESC_LINEAR_CDMAX);
+ ret = iommu_device_link(&smmu->iommu, dev);
+ if (ret)
+ goto err_free_master;
+
group = iommu_group_get_for_dev(dev);
- if (!IS_ERR(group)) {
- iommu_group_put(group);
- iommu_device_link(&smmu->iommu, dev);
+ if (IS_ERR(group)) {
+ ret = PTR_ERR(group);
+ goto err_unlink;
}
- return PTR_ERR_OR_ZERO(group);
+ iommu_group_put(group);
+ return 0;
+
+err_unlink:
+ iommu_device_unlink(&smmu->iommu, dev);
+err_free_master:
+ kfree(master);
+ fwspec->iommu_priv = NULL;
+ return ret;
}
static void arm_smmu_remove_device(struct device *dev)
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH v3 12/13] PCI/ATS: Add PASID stubs
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (10 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 11/13] iommu/arm-smmu-v3: Improve add_device() error handling Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-10 21:07 ` Bjorn Helgaas
2019-12-18 10:02 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 13/13] iommu/arm-smmu-v3: Add support for PCI PASID Jean-Philippe Brucker
2019-12-13 17:10 ` [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jonathan Cameron
13 siblings, 2 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
The SMMUv3 driver, which may be built without CONFIG_PCI, will soon gain
PASID support. Partially revert commit c6e9aefbf9db ("PCI/ATS: Remove
unused PRI and PASID stubs") to re-introduce the PASID stubs, and avoid
adding more #ifdefs to the SMMU driver.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
include/linux/pci-ats.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
index 5d62e78946a3..d08f0869f121 100644
--- a/include/linux/pci-ats.h
+++ b/include/linux/pci-ats.h
@@ -33,6 +33,9 @@ void pci_disable_pasid(struct pci_dev *pdev);
int pci_pasid_features(struct pci_dev *pdev);
int pci_max_pasids(struct pci_dev *pdev);
#else /* CONFIG_PCI_PASID */
+static inline int pci_enable_pasid(struct pci_dev *pdev, int features)
+{ return -EINVAL; }
+static inline void pci_disable_pasid(struct pci_dev *pdev) { }
static inline int pci_pasid_features(struct pci_dev *pdev)
{ return -EINVAL; }
static inline int pci_max_pasids(struct pci_dev *pdev)
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 12/13] PCI/ATS: Add PASID stubs
2019-12-09 18:05 ` [PATCH v3 12/13] PCI/ATS: Add PASID stubs Jean-Philippe Brucker
@ 2019-12-10 21:07 ` Bjorn Helgaas
2019-12-18 10:02 ` Auger Eric
1 sibling, 0 replies; 40+ messages in thread
From: Bjorn Helgaas @ 2019-12-10 21:07 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, eric.auger,
jonathan.cameron, zhangfei.gao
On Mon, Dec 09, 2019 at 07:05:13PM +0100, Jean-Philippe Brucker wrote:
> The SMMUv3 driver, which may be built without CONFIG_PCI, will soon gain
> PASID support. Partially revert commit c6e9aefbf9db ("PCI/ATS: Remove
> unused PRI and PASID stubs") to re-introduce the PASID stubs, and avoid
> adding more #ifdefs to the SMMU driver.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> include/linux/pci-ats.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
> index 5d62e78946a3..d08f0869f121 100644
> --- a/include/linux/pci-ats.h
> +++ b/include/linux/pci-ats.h
> @@ -33,6 +33,9 @@ void pci_disable_pasid(struct pci_dev *pdev);
> int pci_pasid_features(struct pci_dev *pdev);
> int pci_max_pasids(struct pci_dev *pdev);
> #else /* CONFIG_PCI_PASID */
> +static inline int pci_enable_pasid(struct pci_dev *pdev, int features)
> +{ return -EINVAL; }
> +static inline void pci_disable_pasid(struct pci_dev *pdev) { }
> static inline int pci_pasid_features(struct pci_dev *pdev)
> { return -EINVAL; }
> static inline int pci_max_pasids(struct pci_dev *pdev)
> --
> 2.24.0
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 12/13] PCI/ATS: Add PASID stubs
2019-12-09 18:05 ` [PATCH v3 12/13] PCI/ATS: Add PASID stubs Jean-Philippe Brucker
2019-12-10 21:07 ` Bjorn Helgaas
@ 2019-12-18 10:02 ` Auger Eric
1 sibling, 0 replies; 40+ messages in thread
From: Auger Eric @ 2019-12-18 10:02 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> The SMMUv3 driver, which may be built without CONFIG_PCI, will soon gain
> PASID support. Partially revert commit c6e9aefbf9db ("PCI/ATS: Remove
> unused PRI and PASID stubs") to re-introduce the PASID stubs, and avoid
> adding more #ifdefs to the SMMU driver.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> include/linux/pci-ats.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
> index 5d62e78946a3..d08f0869f121 100644
> --- a/include/linux/pci-ats.h
> +++ b/include/linux/pci-ats.h
> @@ -33,6 +33,9 @@ void pci_disable_pasid(struct pci_dev *pdev);
> int pci_pasid_features(struct pci_dev *pdev);
> int pci_max_pasids(struct pci_dev *pdev);
> #else /* CONFIG_PCI_PASID */
> +static inline int pci_enable_pasid(struct pci_dev *pdev, int features)
> +{ return -EINVAL; }
> +static inline void pci_disable_pasid(struct pci_dev *pdev) { }
> static inline int pci_pasid_features(struct pci_dev *pdev)
> { return -EINVAL; }
> static inline int pci_max_pasids(struct pci_dev *pdev)
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH v3 13/13] iommu/arm-smmu-v3: Add support for PCI PASID
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (11 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 12/13] PCI/ATS: Add PASID stubs Jean-Philippe Brucker
@ 2019-12-09 18:05 ` Jean-Philippe Brucker
2019-12-18 10:17 ` Auger Eric
2019-12-13 17:10 ` [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jonathan Cameron
13 siblings, 1 reply; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-09 18:05 UTC (permalink / raw)
To: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
eric.auger, jonathan.cameron, zhangfei.gao
Enable PASID for PCI devices that support it. Since the SSID tables are
allocated by arm_smmu_attach_dev(), PASID has to be enabled early enough.
arm_smmu_dev_feature_enable() would be too late, since by that time the
main DMA domain has already been attached. Do it in add_device() instead.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
drivers/iommu/arm-smmu-v3.c | 51 ++++++++++++++++++++++++++++++++++++-
1 file changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d20a79108f8a..cde7af39681c 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2643,6 +2643,49 @@ static void arm_smmu_disable_ats(struct arm_smmu_master *master)
atomic_dec(&smmu_domain->nr_ats_masters);
}
+static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
+{
+ int ret;
+ int features;
+ int num_pasids;
+ struct pci_dev *pdev;
+
+ if (!dev_is_pci(master->dev))
+ return -ENODEV;
+
+ pdev = to_pci_dev(master->dev);
+
+ features = pci_pasid_features(pdev);
+ if (features < 0)
+ return -ENODEV;
+
+ num_pasids = pci_max_pasids(pdev);
+ if (num_pasids <= 0)
+ return -ENODEV;
+
+ ret = pci_enable_pasid(pdev, features);
+ if (!ret)
+ master->ssid_bits = min_t(u8, ilog2(num_pasids),
+ master->smmu->ssid_bits);
+ return ret;
+}
+
+static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
+{
+ struct pci_dev *pdev;
+
+ if (!dev_is_pci(master->dev))
+ return;
+
+ pdev = to_pci_dev(master->dev);
+
+ if (!pdev->pasid_enabled)
+ return;
+
+ master->ssid_bits = 0;
+ pci_disable_pasid(pdev);
+}
+
static void arm_smmu_detach_dev(struct arm_smmu_master *master)
{
unsigned long flags;
@@ -2851,13 +2894,16 @@ static int arm_smmu_add_device(struct device *dev)
master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
+ /* Note that PASID must be enabled before, and disabled after ATS */
+ arm_smmu_enable_pasid(master);
+
if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
master->ssid_bits = min_t(u8, master->ssid_bits,
CTXDESC_LINEAR_CDMAX);
ret = iommu_device_link(&smmu->iommu, dev);
if (ret)
- goto err_free_master;
+ goto err_disable_pasid;
group = iommu_group_get_for_dev(dev);
if (IS_ERR(group)) {
@@ -2870,6 +2916,8 @@ static int arm_smmu_add_device(struct device *dev)
err_unlink:
iommu_device_unlink(&smmu->iommu, dev);
+err_disable_pasid:
+ arm_smmu_disable_pasid(master);
err_free_master:
kfree(master);
fwspec->iommu_priv = NULL;
@@ -2890,6 +2938,7 @@ static void arm_smmu_remove_device(struct device *dev)
arm_smmu_detach_dev(master);
iommu_group_remove_device(dev);
iommu_device_unlink(&smmu->iommu, dev);
+ arm_smmu_disable_pasid(master);
kfree(master);
iommu_fwspec_free(dev);
}
--
2.24.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH v3 13/13] iommu/arm-smmu-v3: Add support for PCI PASID
2019-12-09 18:05 ` [PATCH v3 13/13] iommu/arm-smmu-v3: Add support for PCI PASID Jean-Philippe Brucker
@ 2019-12-18 10:17 ` Auger Eric
2019-12-18 16:13 ` Jean-Philippe Brucker
0 siblings, 1 reply; 40+ messages in thread
From: Auger Eric @ 2019-12-18 10:17 UTC (permalink / raw)
To: Jean-Philippe Brucker, linux-pci, linux-arm-kernel, linux-acpi,
devicetree, iommu
Cc: joro, robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
Hi Jean,
On 12/9/19 7:05 PM, Jean-Philippe Brucker wrote:
> Enable PASID for PCI devices that support it. Since the SSID tables are
> allocated by arm_smmu_attach_dev(), PASID has to be enabled early enough.
> arm_smmu_dev_feature_enable() would be too late, since by that time the
> main DMA domain has already been attached. Do it in add_device() instead.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> drivers/iommu/arm-smmu-v3.c | 51 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index d20a79108f8a..cde7af39681c 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2643,6 +2643,49 @@ static void arm_smmu_disable_ats(struct arm_smmu_master *master)
> atomic_dec(&smmu_domain->nr_ats_masters);
> }
>
> +static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
> +{
> + int ret;
> + int features;
> + int num_pasids;
> + struct pci_dev *pdev;
> +
> + if (!dev_is_pci(master->dev))
> + return -ENODEV;
> +
> + pdev = to_pci_dev(master->dev);
> +
> + features = pci_pasid_features(pdev);
> + if (features < 0)
> + return -ENODEV;
why -ENODEV?
> +
> + num_pasids = pci_max_pasids(pdev);
> + if (num_pasids <= 0)
> + return -ENODEV;
> +
> + ret = pci_enable_pasid(pdev, features);
> + if (!ret)
> + master->ssid_bits = min_t(u8, ilog2(num_pasids),
> + master->smmu->ssid_bits);
so here we are ;-)
> + return ret;
> +}
> +
> +static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
> +{
> + struct pci_dev *pdev;
> +
> + if (!dev_is_pci(master->dev))
> + return;
> +
> + pdev = to_pci_dev(master->dev);
> +
> + if (!pdev->pasid_enabled)
> + return;
> +
> + master->ssid_bits = 0;
> + pci_disable_pasid(pdev);
> +}
> +
> static void arm_smmu_detach_dev(struct arm_smmu_master *master)
> {
> unsigned long flags;
> @@ -2851,13 +2894,16 @@ static int arm_smmu_add_device(struct device *dev)
>
> master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
>
> + /* Note that PASID must be enabled before, and disabled after ATS */
> + arm_smmu_enable_pasid(master);
No error handling?
> +
> if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
> master->ssid_bits = min_t(u8, master->ssid_bits,
> CTXDESC_LINEAR_CDMAX);
>
> ret = iommu_device_link(&smmu->iommu, dev);
> if (ret)
> - goto err_free_master;
> + goto err_disable_pasid;
>
> group = iommu_group_get_for_dev(dev);
> if (IS_ERR(group)) {
> @@ -2870,6 +2916,8 @@ static int arm_smmu_add_device(struct device *dev)
>
> err_unlink:
> iommu_device_unlink(&smmu->iommu, dev);
> +err_disable_pasid:
> + arm_smmu_disable_pasid(master);
> err_free_master:
> kfree(master);
> fwspec->iommu_priv = NULL;
> @@ -2890,6 +2938,7 @@ static void arm_smmu_remove_device(struct device *dev)
> arm_smmu_detach_dev(master);
> iommu_group_remove_device(dev);
> iommu_device_unlink(&smmu->iommu, dev);
> + arm_smmu_disable_pasid(master);
> kfree(master);
> iommu_fwspec_free(dev);
> }
>
Thanks
Eric
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH v3 13/13] iommu/arm-smmu-v3: Add support for PCI PASID
2019-12-18 10:17 ` Auger Eric
@ 2019-12-18 16:13 ` Jean-Philippe Brucker
0 siblings, 0 replies; 40+ messages in thread
From: Jean-Philippe Brucker @ 2019-12-18 16:13 UTC (permalink / raw)
To: Auger Eric
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, lenb, will, robin.murphy, bhelgaas,
jonathan.cameron, zhangfei.gao
On Wed, Dec 18, 2019 at 11:17:55AM +0100, Auger Eric wrote:
> > +static int arm_smmu_enable_pasid(struct arm_smmu_master *master)
> > +{
> > + int ret;
> > + int features;
> > + int num_pasids;
> > + struct pci_dev *pdev;
> > +
> > + if (!dev_is_pci(master->dev))
> > + return -ENODEV;
> > +
> > + pdev = to_pci_dev(master->dev);
> > +
> > + features = pci_pasid_features(pdev);
> > + if (features < 0)
> > + return -ENODEV;
> why -ENODEV?
Right that should return features. The below should return num_pasids.
> > +
> > + num_pasids = pci_max_pasids(pdev);
> > + if (num_pasids <= 0)
> > + return -ENODEV;
> > +
> > + ret = pci_enable_pasid(pdev, features);
> > + if (!ret)
> > + master->ssid_bits = min_t(u8, ilog2(num_pasids),
> > + master->smmu->ssid_bits);
> so here we are ;-)
> > + return ret;
> > +}
> > +
> > +static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
> > +{
> > + struct pci_dev *pdev;
> > +
> > + if (!dev_is_pci(master->dev))
> > + return;
> > +
> > + pdev = to_pci_dev(master->dev);
> > +
> > + if (!pdev->pasid_enabled)
> > + return;
> > +
> > + master->ssid_bits = 0;
> > + pci_disable_pasid(pdev);
> > +}
> > +
> > static void arm_smmu_detach_dev(struct arm_smmu_master *master)
> > {
> > unsigned long flags;
> > @@ -2851,13 +2894,16 @@ static int arm_smmu_add_device(struct device *dev)
> >
> > master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits);
> >
> > + /* Note that PASID must be enabled before, and disabled after ATS */
> > + arm_smmu_enable_pasid(master);
> No error handling?
The device still works if PASID isn't supported or cannot be enabled, it
just won't have some capabilities (IOMMU_DEV_FEAT_AUX and
IOMMU_DEV_FEAT_SVA), so I don't think add_device should return an error.
But it's a good point, I think at least printing an error like
arm_smmu_enable_ats() does would be better.
Thanks,
Jean
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
` (12 preceding siblings ...)
2019-12-09 18:05 ` [PATCH v3 13/13] iommu/arm-smmu-v3: Add support for PCI PASID Jean-Philippe Brucker
@ 2019-12-13 17:10 ` Jonathan Cameron
13 siblings, 0 replies; 40+ messages in thread
From: Jonathan Cameron @ 2019-12-13 17:10 UTC (permalink / raw)
To: Jean-Philippe Brucker, lenb
Cc: linux-pci, linux-arm-kernel, linux-acpi, devicetree, iommu, joro,
robh+dt, mark.rutland, lorenzo.pieralisi, guohanjun,
sudeep.holla, rjw, will, robin.murphy, bhelgaas, eric.auger,
zhangfei.gao
On Mon, 9 Dec 2019 19:05:01 +0100
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote:
> Add support for Substream ID and PASIDs to the SMMUv3 driver.
> Changes since v2 [1]:
>
> * Split preparatory work into patches 5, 6, 8 and 9.
>
> * Added patch 1. Not strictly relevant, but since we're moving the DMA
> allocations and adding a new one, we might as well clean the flags
> first.
>
> * Fixed a double free reported by Jonathan, and other small
> issues.
>
> * Added patch 12. Upstream commit c6e9aefbf9db ("PCI/ATS: Remove unused
> PRI and PASID stubs") removed the unused PASID stubs. Since the SMMU
> driver can be built without PCI, the stubs are now needed.
>
> [1] https://lore.kernel.org/linux-iommu/20191108152508.4039168-1-jean-philippe@linaro.org/
Hi Jean-Philippe,
Series looks great to me. FWIW
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
for the patches I didn't comment on in this version as I couldn't find anything
to comment about ;)
Thanks
Jonathan
>
> Jean-Philippe Brucker (13):
> iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation
> dt-bindings: document PASID property for IOMMU masters
> iommu/arm-smmu-v3: Support platform SSID
> ACPI/IORT: Support PASID for platform devices
> iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support
> iommu/arm-smmu-v3: Add context descriptor tables allocators
> iommu/arm-smmu-v3: Add support for Substream IDs
> iommu/arm-smmu-v3: Propate ssid_bits
> iommu/arm-smmu-v3: Handle failure of arm_smmu_write_ctx_desc()
> iommu/arm-smmu-v3: Add second level of context descriptor table
> iommu/arm-smmu-v3: Improve add_device() error handling
> PCI/ATS: Add PASID stubs
> iommu/arm-smmu-v3: Add support for PCI PASID
>
> .../devicetree/bindings/iommu/iommu.txt | 6 +
> drivers/acpi/arm64/iort.c | 18 +
> drivers/iommu/arm-smmu-v3.c | 462 +++++++++++++++---
> drivers/iommu/of_iommu.c | 6 +-
> include/linux/iommu.h | 2 +
> include/linux/pci-ats.h | 3 +
> 6 files changed, 437 insertions(+), 60 deletions(-)
>
^ permalink raw reply [flat|nested] 40+ messages in thread