[RFC v1 0/7] iommu/smmu-v3: Workaround for hisilicon 161010801 erratum(reserve HW MSI)

[RFC v1 0/7] iommu/smmu-v3: Workaround for hisilicon 161010801 erratum(reserve HW MSI)

[shameer]

On certain HiSilicon platforms (Hip06/Hip07) the GIC ITS and
PCIe RC deviates from the standard implementation and this breaks
PCIe MSI functionality when SMMU is enabled.

The HiSilicon erratum 161010801 describes this limitation of certain
HiSilicon platforms to support the SMMU mappings for MSI transactions.
On these platforms GICv3 ITS translator is presented with the deviceID
by extending the MSI payload data to 64 bits to include the deviceID.
Hence, the PCIe controller on this platforms has to differentiate the
MSI payload against other DMA payload and has to modify the MSI payload.
This basically makes it difficult for this platforms to have a SMMU
translation for MSI.

This patch implements a ACPI table based quirk to reserve the hw msi
regions in the smmu-v3 driver which means these address regions will
not be translated and will be excluded from iova alloactions.

To implement this quirk, the following changes are incorporated 
into smmu-v3:
1. Added a general erratum framework based on arm_arch_timer erratum
   framework. The intention is to have a common framework for dt and
   acpi based quirk implementations.
2. Replaced the existing hisilicon, broken_prefetch_cmd quirk using 
   the new erratum framework (erratum-161010701)
3. Introduced a ACPI based quirk for erratum-161010801.
4. ACPI CSRT vendor specific blobs are used to pass the reserve address
   region info on these platforms.

Also please note that this patchset is based on Robin's patch series
"acpica: iort: Update SMMU models for IORT rev. C".
https://lkml.org/lkml/2017/5/12/211


Thanks,
Shameer

shameer (7):
  iommu/arm-smmu-v3: Add erratum framework structures
  iommu/arm-smmu-v3: Add erratum framework functions
  iommu/arm-smmu-v3: Replace the device tree binding for hisilicon
    broken prefetch cmd with erratum id
  iommu/arm-smmu-v3: Enable HiSilicon erratum 161010701
  iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010701
  iommu/arm-smmu-v3: Rearrange msi resv alloc functions
  iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

 .../devicetree/bindings/iommu/arm,smmu-v3.txt      |   2 +-
 arch/arm64/Kconfig                                 |  20 +-
 drivers/iommu/arm-smmu-v3.c                        | 225 ++++++++++++++++++---
 3 files changed, 218 insertions(+), 29 deletions(-)

-- 
2.5.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC v1 0/7] iommu/smmu-v3: Workaround for hisilicon 161010801 erratum(reserve HW MSI)

[shameer]

On certain HiSilicon platforms (Hip06/Hip07) the GIC ITS and
PCIe RC deviates from the standard implementation and this breaks
PCIe MSI functionality when SMMU is enabled.

The HiSilicon erratum 161010801 describes this limitation of certain
HiSilicon platforms to support the SMMU mappings for MSI transactions.
On these platforms GICv3 ITS translator is presented with the deviceID
by extending the MSI payload data to 64 bits to include the deviceID.
Hence, the PCIe controller on this platforms has to differentiate the
MSI payload against other DMA payload and has to modify the MSI payload.
This basically makes it difficult for this platforms to have a SMMU
translation for MSI.

This patch implements a ACPI table based quirk to reserve the hw msi
regions in the smmu-v3 driver which means these address regions will
not be translated and will be excluded from iova alloactions.

To implement this quirk, the following changes are incorporated 
into smmu-v3:
1. Added a general erratum framework based on arm_arch_timer erratum
   framework. The intention is to have a common framework for dt and
   acpi based quirk implementations.
2. Replaced the existing hisilicon, broken_prefetch_cmd quirk using 
   the new erratum framework (erratum-161010701)
3. Introduced a ACPI based quirk for erratum-161010801.
4. ACPI CSRT vendor specific blobs are used to pass the reserve address
   region info on these platforms.

Also please note that this patchset is based on Robin's patch series
"acpica: iort: Update SMMU models for IORT rev. C".
https://lkml.org/lkml/2017/5/12/211


Thanks,
Shameer

shameer (7):
  iommu/arm-smmu-v3: Add erratum framework structures
  iommu/arm-smmu-v3: Add erratum framework functions
  iommu/arm-smmu-v3: Replace the device tree binding for hisilicon
    broken prefetch cmd with erratum id
  iommu/arm-smmu-v3: Enable HiSilicon erratum 161010701
  iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010701
  iommu/arm-smmu-v3: Rearrange msi resv alloc functions
  iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

 .../devicetree/bindings/iommu/arm,smmu-v3.txt      |   2 +-
 arch/arm64/Kconfig                                 |  20 +-
 drivers/iommu/arm-smmu-v3.c                        | 225 ++++++++++++++++++---
 3 files changed, 218 insertions(+), 29 deletions(-)

-- 
2.5.0

[RFC v1 1/7] iommu/arm-smmu-v3: Add erratum framework structures

[shameer]

This is to introduce an erratum framework to smmu driver similar
to the one in arm_arch_timer code.

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index bbd46ef..a166590 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -660,6 +660,20 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
 	{ 0, NULL},
 };
 
+enum smmu_erratum_match_type {
+	se_match_dt,
+};
+
+struct smmu_erratum_workaround {
+	enum smmu_erratum_match_type match_type;
+	const void *id;	/* Indicate the Erratum ID */
+	const char *desc_str;
+};
+
+static const struct smmu_erratum_workaround smmu_workarounds[] = {
+
+};
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct arm_smmu_domain, domain);
-- 
2.5.0

[RFC v1 1/7] iommu/arm-smmu-v3: Add erratum framework structures

[shameer]

This is to introduce an erratum framework to smmu driver similar
to the one in arm_arch_timer code.

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index bbd46ef..a166590 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -660,6 +660,20 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
 	{ 0, NULL},
 };
 
+enum smmu_erratum_match_type {
+	se_match_dt,
+};
+
+struct smmu_erratum_workaround {
+	enum smmu_erratum_match_type match_type;
+	const void *id;	/* Indicate the Erratum ID */
+	const char *desc_str;
+};
+
+static const struct smmu_erratum_workaround smmu_workarounds[] = {
+
+};
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct arm_smmu_domain, domain);
-- 
2.5.0

[RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework functions

[shameer]

This will provide a way to replace the existing skip_prefetch_cmd
erratum using the new framework.

Signed-off-by: shameer <shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 58 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index a166590..f20d5d5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -664,16 +664,72 @@ enum smmu_erratum_match_type {
 	se_match_dt,
 };
 
+void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
+{
+	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
+}
+
 struct smmu_erratum_workaround {
 	enum smmu_erratum_match_type match_type;
 	const void *id;	/* Indicate the Erratum ID */
 	const char *desc_str;
+	void (*enable)(struct arm_smmu_device *, void *);
 };
 
 static const struct smmu_erratum_workaround smmu_workarounds[] = {
 
 };
 
+typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround *,
+							  const void *);
+static
+bool smmu_check_dt_erratum(const struct smmu_erratum_workaround *wa,
+						   const void *arg)
+{
+	const struct device_node *np = arg;
+
+	return of_property_read_bool(np, wa->id);
+}
+
+static void smmu_enable_errata(struct arm_smmu_device *smmu,
+				enum smmu_erratum_match_type type,
+				se_match_fn_t match_fn,
+				void *arg)
+{
+	const struct smmu_erratum_workaround *wa = smmu_workarounds;
+
+	for (; wa->desc_str; wa++) {
+		if (wa->match_type != type)
+			continue;
+
+		if (match_fn(wa, arg)) {
+			if (wa->enable) {
+				wa->enable(smmu, arg);
+				dev_info(smmu->dev,
+					"Enabling workaround for %s\n",
+					 wa->desc_str);
+			}
+		}
+	}
+}
+
+
+static void smmu_check_workarounds(struct arm_smmu_device *smmu,
+				  enum smmu_erratum_match_type type,
+				  void *arg)
+{
+	se_match_fn_t match_fn = NULL;
+
+	switch (type) {
+	case se_match_dt:
+		match_fn = smmu_check_dt_erratum;
+		break;
+	}
+
+	smmu_enable_errata(smmu, type, match_fn, arg);
+
+}
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct arm_smmu_domain, domain);
@@ -2641,6 +2697,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 
 	parse_driver_options(smmu);
 
+	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
+
 	if (of_dma_is_coherent(dev->of_node))
 		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
 
-- 
2.5.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework functions

[shameer]

This will provide a way to replace the existing skip_prefetch_cmd
erratum using the new framework.

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 58 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index a166590..f20d5d5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -664,16 +664,72 @@ enum smmu_erratum_match_type {
 	se_match_dt,
 };
 
+void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
+{
+	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
+}
+
 struct smmu_erratum_workaround {
 	enum smmu_erratum_match_type match_type;
 	const void *id;	/* Indicate the Erratum ID */
 	const char *desc_str;
+	void (*enable)(struct arm_smmu_device *, void *);
 };
 
 static const struct smmu_erratum_workaround smmu_workarounds[] = {
 
 };
 
+typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround *,
+							  const void *);
+static
+bool smmu_check_dt_erratum(const struct smmu_erratum_workaround *wa,
+						   const void *arg)
+{
+	const struct device_node *np = arg;
+
+	return of_property_read_bool(np, wa->id);
+}
+
+static void smmu_enable_errata(struct arm_smmu_device *smmu,
+				enum smmu_erratum_match_type type,
+				se_match_fn_t match_fn,
+				void *arg)
+{
+	const struct smmu_erratum_workaround *wa = smmu_workarounds;
+
+	for (; wa->desc_str; wa++) {
+		if (wa->match_type != type)
+			continue;
+
+		if (match_fn(wa, arg)) {
+			if (wa->enable) {
+				wa->enable(smmu, arg);
+				dev_info(smmu->dev,
+					"Enabling workaround for %s\n",
+					 wa->desc_str);
+			}
+		}
+	}
+}
+
+
+static void smmu_check_workarounds(struct arm_smmu_device *smmu,
+				  enum smmu_erratum_match_type type,
+				  void *arg)
+{
+	se_match_fn_t match_fn = NULL;
+
+	switch (type) {
+	case se_match_dt:
+		match_fn = smmu_check_dt_erratum;
+		break;
+	}
+
+	smmu_enable_errata(smmu, type, match_fn, arg);
+
+}
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct arm_smmu_domain, domain);
@@ -2641,6 +2697,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 
 	parse_driver_options(smmu);
 
+	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
+
 	if (of_dma_is_coherent(dev->of_node))
 		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
 
-- 
2.5.0

Re: [RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework functions

[Robin Murphy]

On 13/05/17 10:47, shameer wrote:
> This will provide a way to replace the existing skip_prefetch_cmd
> erratum using the new framework.

Yikes, between this and patch 1 we're already pushing 70 lines of new
code, and it still doesn't actually do anything yet. Implementing the
SMMUv3 equivalent of SMMUv2's acpi_smmu_get_data() would probably be
about 10 lines; all you need to do is set some quirk flags based on a
compatible value. These quirks aren't really any different in principle
to the firmware COHACC overrides that we already process.

Sorry, I'm saying no to a massively overengineered "framework" for
something so relatively simple.

Robin.

> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 58 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index a166590..f20d5d5 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -664,16 +664,72 @@ enum smmu_erratum_match_type {
>  	se_match_dt,
>  };
>  
> +void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
> +{
> +	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> +}
> +
>  struct smmu_erratum_workaround {
>  	enum smmu_erratum_match_type match_type;
>  	const void *id;	/* Indicate the Erratum ID */
>  	const char *desc_str;
> +	void (*enable)(struct arm_smmu_device *, void *);
>  };
>  
>  static const struct smmu_erratum_workaround smmu_workarounds[] = {
>  
>  };
>  
> +typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround *,
> +							  const void *);
> +static
> +bool smmu_check_dt_erratum(const struct smmu_erratum_workaround *wa,
> +						   const void *arg)
> +{
> +	const struct device_node *np = arg;
> +
> +	return of_property_read_bool(np, wa->id);
> +}
> +
> +static void smmu_enable_errata(struct arm_smmu_device *smmu,
> +				enum smmu_erratum_match_type type,
> +				se_match_fn_t match_fn,
> +				void *arg)
> +{
> +	const struct smmu_erratum_workaround *wa = smmu_workarounds;
> +
> +	for (; wa->desc_str; wa++) {
> +		if (wa->match_type != type)
> +			continue;
> +
> +		if (match_fn(wa, arg)) {
> +			if (wa->enable) {
> +				wa->enable(smmu, arg);
> +				dev_info(smmu->dev,
> +					"Enabling workaround for %s\n",
> +					 wa->desc_str);
> +			}
> +		}
> +	}
> +}
> +
> +
> +static void smmu_check_workarounds(struct arm_smmu_device *smmu,
> +				  enum smmu_erratum_match_type type,
> +				  void *arg)
> +{
> +	se_match_fn_t match_fn = NULL;
> +
> +	switch (type) {
> +	case se_match_dt:
> +		match_fn = smmu_check_dt_erratum;
> +		break;
> +	}
> +
> +	smmu_enable_errata(smmu, type, match_fn, arg);
> +
> +}
> +
>  static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>  {
>  	return container_of(dom, struct arm_smmu_domain, domain);
> @@ -2641,6 +2697,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
>  
>  	parse_driver_options(smmu);
>  
> +	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
> +
>  	if (of_dma_is_coherent(dev->of_node))
>  		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
>  
> 

[RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework functions

[Robin Murphy]

On 13/05/17 10:47, shameer wrote:
> This will provide a way to replace the existing skip_prefetch_cmd
> erratum using the new framework.

Yikes, between this and patch 1 we're already pushing 70 lines of new
code, and it still doesn't actually do anything yet. Implementing the
SMMUv3 equivalent of SMMUv2's acpi_smmu_get_data() would probably be
about 10 lines; all you need to do is set some quirk flags based on a
compatible value. These quirks aren't really any different in principle
to the firmware COHACC overrides that we already process.

Sorry, I'm saying no to a massively overengineered "framework" for
something so relatively simple.

Robin.

> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 58 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index a166590..f20d5d5 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -664,16 +664,72 @@ enum smmu_erratum_match_type {
>  	se_match_dt,
>  };
>  
> +void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
> +{
> +	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> +}
> +
>  struct smmu_erratum_workaround {
>  	enum smmu_erratum_match_type match_type;
>  	const void *id;	/* Indicate the Erratum ID */
>  	const char *desc_str;
> +	void (*enable)(struct arm_smmu_device *, void *);
>  };
>  
>  static const struct smmu_erratum_workaround smmu_workarounds[] = {
>  
>  };
>  
> +typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround *,
> +							  const void *);
> +static
> +bool smmu_check_dt_erratum(const struct smmu_erratum_workaround *wa,
> +						   const void *arg)
> +{
> +	const struct device_node *np = arg;
> +
> +	return of_property_read_bool(np, wa->id);
> +}
> +
> +static void smmu_enable_errata(struct arm_smmu_device *smmu,
> +				enum smmu_erratum_match_type type,
> +				se_match_fn_t match_fn,
> +				void *arg)
> +{
> +	const struct smmu_erratum_workaround *wa = smmu_workarounds;
> +
> +	for (; wa->desc_str; wa++) {
> +		if (wa->match_type != type)
> +			continue;
> +
> +		if (match_fn(wa, arg)) {
> +			if (wa->enable) {
> +				wa->enable(smmu, arg);
> +				dev_info(smmu->dev,
> +					"Enabling workaround for %s\n",
> +					 wa->desc_str);
> +			}
> +		}
> +	}
> +}
> +
> +
> +static void smmu_check_workarounds(struct arm_smmu_device *smmu,
> +				  enum smmu_erratum_match_type type,
> +				  void *arg)
> +{
> +	se_match_fn_t match_fn = NULL;
> +
> +	switch (type) {
> +	case se_match_dt:
> +		match_fn = smmu_check_dt_erratum;
> +		break;
> +	}
> +
> +	smmu_enable_errata(smmu, type, match_fn, arg);
> +
> +}
> +
>  static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>  {
>  	return container_of(dom, struct arm_smmu_domain, domain);
> @@ -2641,6 +2697,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
>  
>  	parse_driver_options(smmu);
>  
> +	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
> +
>  	if (of_dma_is_coherent(dev->of_node))
>  		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
>  
> 

RE: [RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework functions

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: Tuesday, May 16, 2017 2:08 PM
> To: Shameerali Kolothum Thodi; will.deacon@arm.com;
> mark.rutland@arm.com; lorenzo.pieralisi@arm.com; hanjun.guo@linaro.org
> Cc: Gabriele Paoloni; John Garry; iommu@lists.linux-foundation.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> acpi@vger.kernel.org; devel@acpica.org; Linuxarm; Wangzhou (B);
> Guohanjun (Hanjun Guo)
> Subject: Re: [RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework
> functions
> 
> On 13/05/17 10:47, shameer wrote:
> > This will provide a way to replace the existing skip_prefetch_cmd
> > erratum using the new framework.
> 
> Yikes, between this and patch 1 we're already pushing 70 lines of new
> code, and it still doesn't actually do anything yet. Implementing the
> SMMUv3 equivalent of SMMUv2's acpi_smmu_get_data() would probably
> be
> about 10 lines; all you need to do is set some quirk flags based on a
> compatible value. These quirks aren't really any different in principle
> to the firmware COHACC overrides that we already process.
> 
> Sorry, I'm saying no to a massively overengineered "framework" for
> something so relatively simple.

Thanks Robin for going through patches. The "framework" was added thinking
it might be useful when multiple quirk implementations are added to the
SMMUv3 driver from different vendors. As you said it doesn't look like, 
adding any value at the moment. I will revise patch set based on SMMUv2s
way for ACPI and retain the broken-* for dtb. 

Thanks,
Shameer
 
> Robin.
> 
> > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  drivers/iommu/arm-smmu-v3.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index a166590..f20d5d5 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -664,16 +664,72 @@ enum smmu_erratum_match_type {
> >  	se_match_dt,
> >  };
> >
> > +void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void
> *arg)
> > +{
> > +	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> > +}
> > +
> >  struct smmu_erratum_workaround {
> >  	enum smmu_erratum_match_type match_type;
> >  	const void *id;	/* Indicate the Erratum ID */
> >  	const char *desc_str;
> > +	void (*enable)(struct arm_smmu_device *, void *);
> >  };
> >
> >  static const struct smmu_erratum_workaround smmu_workarounds[] = {
> >
> >  };
> >
> > +typedef bool (*se_match_fn_t)(const struct
> smmu_erratum_workaround *,
> > +							  const void *);
> > +static
> > +bool smmu_check_dt_erratum(const struct smmu_erratum_workaround
> *wa,
> > +						   const void *arg)
> > +{
> > +	const struct device_node *np = arg;
> > +
> > +	return of_property_read_bool(np, wa->id);
> > +}
> > +
> > +static void smmu_enable_errata(struct arm_smmu_device *smmu,
> > +				enum smmu_erratum_match_type type,
> > +				se_match_fn_t match_fn,
> > +				void *arg)
> > +{
> > +	const struct smmu_erratum_workaround *wa =
> smmu_workarounds;
> > +
> > +	for (; wa->desc_str; wa++) {
> > +		if (wa->match_type != type)
> > +			continue;
> > +
> > +		if (match_fn(wa, arg)) {
> > +			if (wa->enable) {
> > +				wa->enable(smmu, arg);
> > +				dev_info(smmu->dev,
> > +					"Enabling workaround for %s\n",
> > +					 wa->desc_str);
> > +			}
> > +		}
> > +	}
> > +}
> > +
> > +
> > +static void smmu_check_workarounds(struct arm_smmu_device *smmu,
> > +				  enum smmu_erratum_match_type type,
> > +				  void *arg)
> > +{
> > +	se_match_fn_t match_fn = NULL;
> > +
> > +	switch (type) {
> > +	case se_match_dt:
> > +		match_fn = smmu_check_dt_erratum;
> > +		break;
> > +	}
> > +
> > +	smmu_enable_errata(smmu, type, match_fn, arg);
> > +
> > +}
> > +
> >  static struct arm_smmu_domain *to_smmu_domain(struct
> iommu_domain *dom)
> >  {
> >  	return container_of(dom, struct arm_smmu_domain, domain);
> > @@ -2641,6 +2697,8 @@ static int arm_smmu_device_dt_probe(struct
> platform_device *pdev,
> >
> >  	parse_driver_options(smmu);
> >
> > +	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
> > +
> >  	if (of_dma_is_coherent(dev->of_node))
> >  		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
> >
> >

[RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework functions

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy at arm.com]
> Sent: Tuesday, May 16, 2017 2:08 PM
> To: Shameerali Kolothum Thodi; will.deacon at arm.com;
> mark.rutland at arm.com; lorenzo.pieralisi at arm.com; hanjun.guo at linaro.org
> Cc: Gabriele Paoloni; John Garry; iommu at lists.linux-foundation.org;
> devicetree at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
> acpi at vger.kernel.org; devel at acpica.org; Linuxarm; Wangzhou (B);
> Guohanjun (Hanjun Guo)
> Subject: Re: [RFC v1 2/7] iommu/arm-smmu-v3: Add erratum framework
> functions
> 
> On 13/05/17 10:47, shameer wrote:
> > This will provide a way to replace the existing skip_prefetch_cmd
> > erratum using the new framework.
> 
> Yikes, between this and patch 1 we're already pushing 70 lines of new
> code, and it still doesn't actually do anything yet. Implementing the
> SMMUv3 equivalent of SMMUv2's acpi_smmu_get_data() would probably
> be
> about 10 lines; all you need to do is set some quirk flags based on a
> compatible value. These quirks aren't really any different in principle
> to the firmware COHACC overrides that we already process.
> 
> Sorry, I'm saying no to a massively overengineered "framework" for
> something so relatively simple.

Thanks Robin for going through patches. The "framework" was added thinking
it might be useful when multiple quirk implementations are added to the
SMMUv3 driver from different vendors. As you said it doesn't look like, 
adding any value at the moment. I will revise patch set based on SMMUv2s
way for ACPI and retain the broken-* for dtb. 

Thanks,
Shameer
 
> Robin.
> 
> > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  drivers/iommu/arm-smmu-v3.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index a166590..f20d5d5 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -664,16 +664,72 @@ enum smmu_erratum_match_type {
> >  	se_match_dt,
> >  };
> >
> > +void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void
> *arg)
> > +{
> > +	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> > +}
> > +
> >  struct smmu_erratum_workaround {
> >  	enum smmu_erratum_match_type match_type;
> >  	const void *id;	/* Indicate the Erratum ID */
> >  	const char *desc_str;
> > +	void (*enable)(struct arm_smmu_device *, void *);
> >  };
> >
> >  static const struct smmu_erratum_workaround smmu_workarounds[] = {
> >
> >  };
> >
> > +typedef bool (*se_match_fn_t)(const struct
> smmu_erratum_workaround *,
> > +							  const void *);
> > +static
> > +bool smmu_check_dt_erratum(const struct smmu_erratum_workaround
> *wa,
> > +						   const void *arg)
> > +{
> > +	const struct device_node *np = arg;
> > +
> > +	return of_property_read_bool(np, wa->id);
> > +}
> > +
> > +static void smmu_enable_errata(struct arm_smmu_device *smmu,
> > +				enum smmu_erratum_match_type type,
> > +				se_match_fn_t match_fn,
> > +				void *arg)
> > +{
> > +	const struct smmu_erratum_workaround *wa =
> smmu_workarounds;
> > +
> > +	for (; wa->desc_str; wa++) {
> > +		if (wa->match_type != type)
> > +			continue;
> > +
> > +		if (match_fn(wa, arg)) {
> > +			if (wa->enable) {
> > +				wa->enable(smmu, arg);
> > +				dev_info(smmu->dev,
> > +					"Enabling workaround for %s\n",
> > +					 wa->desc_str);
> > +			}
> > +		}
> > +	}
> > +}
> > +
> > +
> > +static void smmu_check_workarounds(struct arm_smmu_device *smmu,
> > +				  enum smmu_erratum_match_type type,
> > +				  void *arg)
> > +{
> > +	se_match_fn_t match_fn = NULL;
> > +
> > +	switch (type) {
> > +	case se_match_dt:
> > +		match_fn = smmu_check_dt_erratum;
> > +		break;
> > +	}
> > +
> > +	smmu_enable_errata(smmu, type, match_fn, arg);
> > +
> > +}
> > +
> >  static struct arm_smmu_domain *to_smmu_domain(struct
> iommu_domain *dom)
> >  {
> >  	return container_of(dom, struct arm_smmu_domain, domain);
> > @@ -2641,6 +2697,8 @@ static int arm_smmu_device_dt_probe(struct
> platform_device *pdev,
> >
> >  	parse_driver_options(smmu);
> >
> > +	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
> > +
> >  	if (of_dma_is_coherent(dev->of_node))
> >  		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
> >
> >

[RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc functions

[shameer]

This moves the SW MSI reserve region allocation to probe fn.

Signed-off-by: shameer <shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 770cc9e..e7a8a50 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -619,6 +619,9 @@ struct arm_smmu_device {
 
 	/* IOMMU core code handle */
 	struct iommu_device		iommu;
+
+	/* MSI Reserve region */
+	struct iommu_resv_region        *msi_region;
 };
 
 /* SMMU private data for each master */
@@ -1960,15 +1963,12 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
 static void arm_smmu_get_resv_regions(struct device *dev,
 				      struct list_head *head)
 {
-	struct iommu_resv_region *region;
-	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
-
-	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
-					 prot, IOMMU_RESV_SW_MSI);
-	if (!region)
-		return;
+	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 
-	list_add_tail(&region->list, head);
+	if (smmu && smmu->msi_region)
+		list_add_tail(&smmu->msi_region->list, head);
 
 	iommu_dma_get_resv_regions(dev, head);
 }
@@ -1978,8 +1978,13 @@ static void arm_smmu_put_resv_regions(struct device *dev,
 {
 	struct iommu_resv_region *entry, *next;
 
-	list_for_each_entry_safe(entry, next, head, list)
+	list_for_each_entry_safe(entry, next, head, list) {
+		if (entry->type == IOMMU_RESV_SW_MSI ||
+				entry->type == IOMMU_RESV_MSI)
+			continue;
+
 		kfree(entry);
+	}
 }
 
 static struct iommu_ops arm_smmu_ops = {
@@ -2711,6 +2716,17 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 	return ret;
 }
 
+static struct iommu_resv_region *arm_smmu_alloc_msi_region(
+				struct arm_smmu_device *smmu)
+{
+	struct iommu_resv_region *region;
+	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+
+	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
+					prot, IOMMU_RESV_SW_MSI);
+	return region;
+}
+
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	int irq, ret;
@@ -2756,6 +2772,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	if (irq > 0)
 		smmu->gerr_irq = irq;
 
+	smmu->msi_region = arm_smmu_alloc_msi_region(smmu);
+
 	if (dev->of_node) {
 		ret = arm_smmu_device_dt_probe(pdev, smmu);
 	} else {
-- 
2.5.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc functions

[shameer]

This moves the SW MSI reserve region allocation to probe fn.

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 770cc9e..e7a8a50 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -619,6 +619,9 @@ struct arm_smmu_device {
 
 	/* IOMMU core code handle */
 	struct iommu_device		iommu;
+
+	/* MSI Reserve region */
+	struct iommu_resv_region        *msi_region;
 };
 
 /* SMMU private data for each master */
@@ -1960,15 +1963,12 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
 static void arm_smmu_get_resv_regions(struct device *dev,
 				      struct list_head *head)
 {
-	struct iommu_resv_region *region;
-	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
-
-	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
-					 prot, IOMMU_RESV_SW_MSI);
-	if (!region)
-		return;
+	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 
-	list_add_tail(&region->list, head);
+	if (smmu && smmu->msi_region)
+		list_add_tail(&smmu->msi_region->list, head);
 
 	iommu_dma_get_resv_regions(dev, head);
 }
@@ -1978,8 +1978,13 @@ static void arm_smmu_put_resv_regions(struct device *dev,
 {
 	struct iommu_resv_region *entry, *next;
 
-	list_for_each_entry_safe(entry, next, head, list)
+	list_for_each_entry_safe(entry, next, head, list) {
+		if (entry->type == IOMMU_RESV_SW_MSI ||
+				entry->type == IOMMU_RESV_MSI)
+			continue;
+
 		kfree(entry);
+	}
 }
 
 static struct iommu_ops arm_smmu_ops = {
@@ -2711,6 +2716,17 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 	return ret;
 }
 
+static struct iommu_resv_region *arm_smmu_alloc_msi_region(
+				struct arm_smmu_device *smmu)
+{
+	struct iommu_resv_region *region;
+	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+
+	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
+					prot, IOMMU_RESV_SW_MSI);
+	return region;
+}
+
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	int irq, ret;
@@ -2756,6 +2772,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	if (irq > 0)
 		smmu->gerr_irq = irq;
 
+	smmu->msi_region = arm_smmu_alloc_msi_region(smmu);
+
 	if (dev->of_node) {
 		ret = arm_smmu_device_dt_probe(pdev, smmu);
 	} else {
-- 
2.5.0

Re: [RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc functions

[Robin Murphy]

On 13/05/17 10:47, shameer wrote:
> This moves the SW MSI reserve region allocation to probe fn.

Why?

> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 770cc9e..e7a8a50 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -619,6 +619,9 @@ struct arm_smmu_device {
>  
>  	/* IOMMU core code handle */
>  	struct iommu_device		iommu;
> +
> +	/* MSI Reserve region */
> +	struct iommu_resv_region        *msi_region;
>  };
>  
>  /* SMMU private data for each master */
> @@ -1960,15 +1963,12 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
>  static void arm_smmu_get_resv_regions(struct device *dev,
>  				      struct list_head *head)
>  {
> -	struct iommu_resv_region *region;
> -	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> -
> -	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
> -					 prot, IOMMU_RESV_SW_MSI);
> -	if (!region)
> -		return;
> +	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> +	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> +	struct arm_smmu_device *smmu = smmu_domain->smmu;

There's never any need for the domain dance if you don't need the
domain, just pull the smmu out of the device's iommu_fwspec.

> -	list_add_tail(&region->list, head);
> +	if (smmu && smmu->msi_region)
> +		list_add_tail(&smmu->msi_region->list, head);

Have you considered what happens for the second and subsequent domains
allocated on this SMMU? I can't see that ending well.

Robin.

>  	iommu_dma_get_resv_regions(dev, head);
>  }
> @@ -1978,8 +1978,13 @@ static void arm_smmu_put_resv_regions(struct device *dev,
>  {
>  	struct iommu_resv_region *entry, *next;
>  
> -	list_for_each_entry_safe(entry, next, head, list)
> +	list_for_each_entry_safe(entry, next, head, list) {
> +		if (entry->type == IOMMU_RESV_SW_MSI ||
> +				entry->type == IOMMU_RESV_MSI)
> +			continue;
> +
>  		kfree(entry);
> +	}
>  }
>  
>  static struct iommu_ops arm_smmu_ops = {
> @@ -2711,6 +2716,17 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
>  	return ret;
>  }
>  
> +static struct iommu_resv_region *arm_smmu_alloc_msi_region(
> +				struct arm_smmu_device *smmu)
> +{
> +	struct iommu_resv_region *region;
> +	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> +
> +	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
> +					prot, IOMMU_RESV_SW_MSI);
> +	return region;
> +}
> +
>  static int arm_smmu_device_probe(struct platform_device *pdev)
>  {
>  	int irq, ret;
> @@ -2756,6 +2772,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	if (irq > 0)
>  		smmu->gerr_irq = irq;
>  
> +	smmu->msi_region = arm_smmu_alloc_msi_region(smmu);
> +
>  	if (dev->of_node) {
>  		ret = arm_smmu_device_dt_probe(pdev, smmu);
>  	} else {
> 

[RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc functions

[Robin Murphy]

On 13/05/17 10:47, shameer wrote:
> This moves the SW MSI reserve region allocation to probe fn.

Why?

> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 770cc9e..e7a8a50 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -619,6 +619,9 @@ struct arm_smmu_device {
>  
>  	/* IOMMU core code handle */
>  	struct iommu_device		iommu;
> +
> +	/* MSI Reserve region */
> +	struct iommu_resv_region        *msi_region;
>  };
>  
>  /* SMMU private data for each master */
> @@ -1960,15 +1963,12 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
>  static void arm_smmu_get_resv_regions(struct device *dev,
>  				      struct list_head *head)
>  {
> -	struct iommu_resv_region *region;
> -	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> -
> -	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
> -					 prot, IOMMU_RESV_SW_MSI);
> -	if (!region)
> -		return;
> +	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> +	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> +	struct arm_smmu_device *smmu = smmu_domain->smmu;

There's never any need for the domain dance if you don't need the
domain, just pull the smmu out of the device's iommu_fwspec.

> -	list_add_tail(&region->list, head);
> +	if (smmu && smmu->msi_region)
> +		list_add_tail(&smmu->msi_region->list, head);

Have you considered what happens for the second and subsequent domains
allocated on this SMMU? I can't see that ending well.

Robin.

>  	iommu_dma_get_resv_regions(dev, head);
>  }
> @@ -1978,8 +1978,13 @@ static void arm_smmu_put_resv_regions(struct device *dev,
>  {
>  	struct iommu_resv_region *entry, *next;
>  
> -	list_for_each_entry_safe(entry, next, head, list)
> +	list_for_each_entry_safe(entry, next, head, list) {
> +		if (entry->type == IOMMU_RESV_SW_MSI ||
> +				entry->type == IOMMU_RESV_MSI)
> +			continue;
> +
>  		kfree(entry);
> +	}
>  }
>  
>  static struct iommu_ops arm_smmu_ops = {
> @@ -2711,6 +2716,17 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
>  	return ret;
>  }
>  
> +static struct iommu_resv_region *arm_smmu_alloc_msi_region(
> +				struct arm_smmu_device *smmu)
> +{
> +	struct iommu_resv_region *region;
> +	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> +
> +	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
> +					prot, IOMMU_RESV_SW_MSI);
> +	return region;
> +}
> +
>  static int arm_smmu_device_probe(struct platform_device *pdev)
>  {
>  	int irq, ret;
> @@ -2756,6 +2772,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	if (irq > 0)
>  		smmu->gerr_irq = irq;
>  
> +	smmu->msi_region = arm_smmu_alloc_msi_region(smmu);
> +
>  	if (dev->of_node) {
>  		ret = arm_smmu_device_dt_probe(pdev, smmu);
>  	} else {
> 

RE: [RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc functions

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy-5wv7dgnIgG8@public.gmane.org]
> Sent: Tuesday, May 16, 2017 2:28 PM
> To: Shameerali Kolothum Thodi; will.deacon-5wv7dgnIgG8@public.gmane.org;
> mark.rutland-5wv7dgnIgG8@public.gmane.org; lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org; hanjun.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Gabriele Paoloni; John Garry; Linuxarm;
> linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Wangzhou
> (B); Guohanjun (Hanjun Guo); linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org;
> devel-E0kO6a4B6psdnm+yROfE0A@public.gmane.org
> Subject: Re: [RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc
> functions
> 
> On 13/05/17 10:47, shameer wrote:
> > This moves the SW MSI reserve region allocation to probe fn.
> 
> Why?

Sure, I will modify the commit message to mention about the next patch
where we will add the quirk to modify the default SW MSI region with a HW MSI
region on Hip06/07 platforms.

> > Signed-off-by: shameer <shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > ---
> >  drivers/iommu/arm-smmu-v3.c | 36 +++++++++++++++++++++++++++---
> ------
> >  1 file changed, 27 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index 770cc9e..e7a8a50 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -619,6 +619,9 @@ struct arm_smmu_device {
> >
> >  	/* IOMMU core code handle */
> >  	struct iommu_device		iommu;
> > +
> > +	/* MSI Reserve region */
> > +	struct iommu_resv_region        *msi_region;
> >  };
> >
> >  /* SMMU private data for each master */
> > @@ -1960,15 +1963,12 @@ static int arm_smmu_of_xlate(struct device
> *dev, struct of_phandle_args *args)
> >  static void arm_smmu_get_resv_regions(struct device *dev,
> >  				      struct list_head *head)
> >  {
> > -	struct iommu_resv_region *region;
> > -	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> > -
> > -	region = iommu_alloc_resv_region(MSI_IOVA_BASE,
> MSI_IOVA_LENGTH,
> > -					 prot, IOMMU_RESV_SW_MSI);
> > -	if (!region)
> > -		return;
> > +	struct iommu_domain *domain =
> iommu_get_domain_for_dev(dev);
> > +	struct arm_smmu_domain *smmu_domain =
> to_smmu_domain(domain);
> > +	struct arm_smmu_device *smmu = smmu_domain->smmu;
> 
> There's never any need for the domain dance if you don't need the
> domain, just pull the smmu out of the device's iommu_fwspec.

Ok. That makes sense. Thanks.

> > -	list_add_tail(&region->list, head);
> > +	if (smmu && smmu->msi_region)
> > +		list_add_tail(&smmu->msi_region->list, head);
> 
> Have you considered what happens for the second and subsequent domains
> allocated on this SMMU? I can't see that ending well.

Yes, I probably missed this part. I will recheck this.

Thanks,
Shameer
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc functions

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy at arm.com]
> Sent: Tuesday, May 16, 2017 2:28 PM
> To: Shameerali Kolothum Thodi; will.deacon at arm.com;
> mark.rutland at arm.com; lorenzo.pieralisi at arm.com; hanjun.guo at linaro.org
> Cc: devicetree at vger.kernel.org; Gabriele Paoloni; John Garry; Linuxarm;
> linux-acpi at vger.kernel.org; iommu at lists.linux-foundation.org; Wangzhou
> (B); Guohanjun (Hanjun Guo); linux-arm-kernel at lists.infradead.org;
> devel at acpica.org
> Subject: Re: [RFC v1 6/7] iommu/arm-smmu-v3: Rearrange msi resv alloc
> functions
> 
> On 13/05/17 10:47, shameer wrote:
> > This moves the SW MSI reserve region allocation to probe fn.
> 
> Why?

Sure, I will modify the commit message to mention about the next patch
where we will add the quirk to modify the default SW MSI region with a HW MSI
region on Hip06/07 platforms.

> > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  drivers/iommu/arm-smmu-v3.c | 36 +++++++++++++++++++++++++++---
> ------
> >  1 file changed, 27 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index 770cc9e..e7a8a50 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -619,6 +619,9 @@ struct arm_smmu_device {
> >
> >  	/* IOMMU core code handle */
> >  	struct iommu_device		iommu;
> > +
> > +	/* MSI Reserve region */
> > +	struct iommu_resv_region        *msi_region;
> >  };
> >
> >  /* SMMU private data for each master */
> > @@ -1960,15 +1963,12 @@ static int arm_smmu_of_xlate(struct device
> *dev, struct of_phandle_args *args)
> >  static void arm_smmu_get_resv_regions(struct device *dev,
> >  				      struct list_head *head)
> >  {
> > -	struct iommu_resv_region *region;
> > -	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> > -
> > -	region = iommu_alloc_resv_region(MSI_IOVA_BASE,
> MSI_IOVA_LENGTH,
> > -					 prot, IOMMU_RESV_SW_MSI);
> > -	if (!region)
> > -		return;
> > +	struct iommu_domain *domain =
> iommu_get_domain_for_dev(dev);
> > +	struct arm_smmu_domain *smmu_domain =
> to_smmu_domain(domain);
> > +	struct arm_smmu_device *smmu = smmu_domain->smmu;
> 
> There's never any need for the domain dance if you don't need the
> domain, just pull the smmu out of the device's iommu_fwspec.

Ok. That makes sense. Thanks.

> > -	list_add_tail(&region->list, head);
> > +	if (smmu && smmu->msi_region)
> > +		list_add_tail(&smmu->msi_region->list, head);
> 
> Have you considered what happens for the second and subsequent domains
> allocated on this SMMU? I can't see that ending well.

Yes, I probably missed this part. I will recheck this.

Thanks,
Shameer

[RFC v1 3/7] iommu/arm-smmu-v3: Replace the device tree binding for hisilicon broken prefetch cmd with erratum id

[shameer]

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
index be57550..bf48e92 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
@@ -46,7 +46,7 @@ the PCIe specification.
                         devicetree/bindings/interrupt-controller/msi.txt
                       for a description of the msi-parent property.
 
-- hisilicon,broken-prefetch-cmd
+- hisilicon,erratum-161010701
                     : Avoid sending CMD_PREFETCH_* commands to the SMMU.
 
 ** Example
-- 
2.5.0

[RFC v1 3/7] iommu/arm-smmu-v3: Replace the device tree binding for hisilicon broken prefetch cmd with erratum id

[shameer]

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
index be57550..bf48e92 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
@@ -46,7 +46,7 @@ the PCIe specification.
                         devicetree/bindings/interrupt-controller/msi.txt
                       for a description of the msi-parent property.
 
-- hisilicon,broken-prefetch-cmd
+- hisilicon,erratum-161010701
                     : Avoid sending CMD_PREFETCH_* commands to the SMMU.
 
 ** Example
-- 
2.5.0

Re: [RFC v1 3/7] iommu/arm-smmu-v3: Replace the device tree binding for hisilicon broken prefetch cmd with erratum id

[Rob Herring]

On Sat, May 13, 2017 at 4:47 AM, shameer
<shameerali.kolothum.thodi@huawei.com> wrote:
> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> index be57550..bf48e92 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> @@ -46,7 +46,7 @@ the PCIe specification.
>                          devicetree/bindings/interrupt-controller/msi.txt
>                        for a description of the msi-parent property.
>
> -- hisilicon,broken-prefetch-cmd
> +- hisilicon,erratum-161010701

Why? Your missing commit msg should answer that. This breaks
compatibility, so you are stuck with the old string.

I'd prefer that you start using the compatible string rather than
every vendor adding a property for every errata.

Rob

[RFC v1 3/7] iommu/arm-smmu-v3: Replace the device tree binding for hisilicon broken prefetch cmd with erratum id

[Rob Herring]

On Sat, May 13, 2017 at 4:47 AM, shameer
<shameerali.kolothum.thodi@huawei.com> wrote:
> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> index be57550..bf48e92 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> @@ -46,7 +46,7 @@ the PCIe specification.
>                          devicetree/bindings/interrupt-controller/msi.txt
>                        for a description of the msi-parent property.
>
> -- hisilicon,broken-prefetch-cmd
> +- hisilicon,erratum-161010701

Why? Your missing commit msg should answer that. This breaks
compatibility, so you are stuck with the old string.

I'd prefer that you start using the compatible string rather than
every vendor adding a property for every errata.

Rob

RE: [RFC v1 3/7] iommu/arm-smmu-v3: Replace the device tree binding for hisilicon broken prefetch cmd with erratum id

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Monday, May 15, 2017 4:24 PM
> To: Shameerali Kolothum Thodi
> Cc: Will Deacon; Robin Murphy; Mark Rutland; Lorenzo Pieralisi; Hanjun Guo;
> Gabriele Paoloni; John Garry; Linux IOMMU; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> devel@acpica.org; Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
> Subject: Re: [RFC v1 3/7] iommu/arm-smmu-v3: Replace the device tree
> binding for hisilicon broken prefetch cmd with erratum id
> 
> On Sat, May 13, 2017 at 4:47 AM, shameer
> <shameerali.kolothum.thodi@huawei.com> wrote:
> > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> > b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> > index be57550..bf48e92 100644
> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> > @@ -46,7 +46,7 @@ the PCIe specification.
> >                          devicetree/bindings/interrupt-controller/msi.txt
> >                        for a description of the msi-parent property.
> >
> > -- hisilicon,broken-prefetch-cmd
> > +- hisilicon,erratum-161010701
> 
> Why? Your missing commit msg should answer that. This breaks
> compatibility, so you are stuck with the old string.
> 
> I'd prefer that you start using the compatible string rather than every vendor
> adding a property for every errata.

Ok. I will wait for the review of other patches in this series and update it accordingly
in the next revision.

Thanks,
Shameer

[RFC v1 3/7] iommu/arm-smmu-v3: Replace the device tree binding for hisilicon broken prefetch cmd with erratum id

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Rob Herring [mailto:robh at kernel.org]
> Sent: Monday, May 15, 2017 4:24 PM
> To: Shameerali Kolothum Thodi
> Cc: Will Deacon; Robin Murphy; Mark Rutland; Lorenzo Pieralisi; Hanjun Guo;
> Gabriele Paoloni; John Garry; Linux IOMMU; devicetree at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org; linux-acpi at vger.kernel.org;
> devel at acpica.org; Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
> Subject: Re: [RFC v1 3/7] iommu/arm-smmu-v3: Replace the device tree
> binding for hisilicon broken prefetch cmd with erratum id
> 
> On Sat, May 13, 2017 at 4:47 AM, shameer
> <shameerali.kolothum.thodi@huawei.com> wrote:
> > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> > b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> > index be57550..bf48e92 100644
> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> > @@ -46,7 +46,7 @@ the PCIe specification.
> >                          devicetree/bindings/interrupt-controller/msi.txt
> >                        for a description of the msi-parent property.
> >
> > -- hisilicon,broken-prefetch-cmd
> > +- hisilicon,erratum-161010701
> 
> Why? Your missing commit msg should answer that. This breaks
> compatibility, so you are stuck with the old string.
> 
> I'd prefer that you start using the compatible string rather than every vendor
> adding a property for every errata.

Ok. I will wait for the review of other patches in this series and update it accordingly
in the next revision.

Thanks,
Shameer

[RFC v1 4/7] iommu/arm-smmu-v3: Enable HiSilicon erratum 161010701

[shameer]

This replaces the existing broken_prefetch_cmd quirk using
the new erratum framework.

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 arch/arm64/Kconfig          | 10 +++++++++-
 drivers/iommu/arm-smmu-v3.c | 36 ++++++++++--------------------------
 2 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a39029b..21d61ff 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -508,8 +508,16 @@ config QCOM_FALKOR_ERRATUM_1009
 
 	  If unsure, say Y.
 
-endmenu
+config HISILICON_ERRATUM_161010701
+	bool "HiSilicon erratum 161010701: Skip SMMU Prefetch Cmd"
+	default y
+	help
+	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 doesn't support the
+	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
 
+	  If unsure, say Y.
+
+endmenu
 
 choice
 	prompt "Page size"
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f20d5d5..14538cb 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -650,16 +650,6 @@ struct arm_smmu_domain {
 	struct iommu_domain		domain;
 };
 
-struct arm_smmu_option_prop {
-	u32 opt;
-	const char *prop;
-};
-
-static struct arm_smmu_option_prop arm_smmu_options[] = {
-	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
-	{ 0, NULL},
-};
-
 enum smmu_erratum_match_type {
 	se_match_dt,
 };
@@ -677,7 +667,17 @@ struct smmu_erratum_workaround {
 };
 
 static const struct smmu_erratum_workaround smmu_workarounds[] = {
+#ifdef CONFIG_HISILICON_ERRATUM_161010701
+	{
+		.match_type = se_match_dt,
+		.id = "hisilicon,erratum-161010701",
+		.desc_str = "HiSilicon erratum 161010701",
+		.enable = erratum_skip_prefetch_cmd,
+	},
+#endif
+	{
 
+	},
 };
 
 typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround *,
@@ -735,20 +735,6 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 	return container_of(dom, struct arm_smmu_domain, domain);
 }
 
-static void parse_driver_options(struct arm_smmu_device *smmu)
-{
-	int i = 0;
-
-	do {
-		if (of_property_read_bool(smmu->dev->of_node,
-						arm_smmu_options[i].prop)) {
-			smmu->options |= arm_smmu_options[i].opt;
-			dev_notice(smmu->dev, "option %s\n",
-				arm_smmu_options[i].prop);
-		}
-	} while (arm_smmu_options[++i].opt);
-}
-
 /* Low-level queue manipulation functions */
 static bool queue_full(struct arm_smmu_queue *q)
 {
@@ -2695,8 +2681,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 	else
 		ret = 0;
 
-	parse_driver_options(smmu);
-
 	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
 
 	if (of_dma_is_coherent(dev->of_node))
-- 
2.5.0

[RFC v1 4/7] iommu/arm-smmu-v3: Enable HiSilicon erratum 161010701

[shameer]

This replaces the existing broken_prefetch_cmd quirk using
the new erratum framework.

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 arch/arm64/Kconfig          | 10 +++++++++-
 drivers/iommu/arm-smmu-v3.c | 36 ++++++++++--------------------------
 2 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a39029b..21d61ff 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -508,8 +508,16 @@ config QCOM_FALKOR_ERRATUM_1009
 
 	  If unsure, say Y.
 
-endmenu
+config HISILICON_ERRATUM_161010701
+	bool "HiSilicon erratum 161010701: Skip SMMU Prefetch Cmd"
+	default y
+	help
+	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 doesn't support the
+	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
 
+	  If unsure, say Y.
+
+endmenu
 
 choice
 	prompt "Page size"
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f20d5d5..14538cb 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -650,16 +650,6 @@ struct arm_smmu_domain {
 	struct iommu_domain		domain;
 };
 
-struct arm_smmu_option_prop {
-	u32 opt;
-	const char *prop;
-};
-
-static struct arm_smmu_option_prop arm_smmu_options[] = {
-	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
-	{ 0, NULL},
-};
-
 enum smmu_erratum_match_type {
 	se_match_dt,
 };
@@ -677,7 +667,17 @@ struct smmu_erratum_workaround {
 };
 
 static const struct smmu_erratum_workaround smmu_workarounds[] = {
+#ifdef CONFIG_HISILICON_ERRATUM_161010701
+	{
+		.match_type = se_match_dt,
+		.id = "hisilicon,erratum-161010701",
+		.desc_str = "HiSilicon erratum 161010701",
+		.enable = erratum_skip_prefetch_cmd,
+	},
+#endif
+	{
 
+	},
 };
 
 typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround *,
@@ -735,20 +735,6 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 	return container_of(dom, struct arm_smmu_domain, domain);
 }
 
-static void parse_driver_options(struct arm_smmu_device *smmu)
-{
-	int i = 0;
-
-	do {
-		if (of_property_read_bool(smmu->dev->of_node,
-						arm_smmu_options[i].prop)) {
-			smmu->options |= arm_smmu_options[i].opt;
-			dev_notice(smmu->dev, "option %s\n",
-				arm_smmu_options[i].prop);
-		}
-	} while (arm_smmu_options[++i].opt);
-}
-
 /* Low-level queue manipulation functions */
 static bool queue_full(struct arm_smmu_queue *q)
 {
@@ -2695,8 +2681,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 	else
 		ret = 0;
 
-	parse_driver_options(smmu);
-
 	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
 
 	if (of_dma_is_coherent(dev->of_node))
-- 
2.5.0

Re: [RFC v1 4/7] iommu/arm-smmu-v3: Enable HiSilicon erratum 161010701

[Robin Murphy]

On 13/05/17 10:47, shameer wrote:
> This replaces the existing broken_prefetch_cmd quirk using
> the new erratum framework.
> 
> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  arch/arm64/Kconfig          | 10 +++++++++-
>  drivers/iommu/arm-smmu-v3.c | 36 ++++++++++--------------------------
>  2 files changed, 19 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index a39029b..21d61ff 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -508,8 +508,16 @@ config QCOM_FALKOR_ERRATUM_1009
>  
>  	  If unsure, say Y.
>  
> -endmenu
> +config HISILICON_ERRATUM_161010701
> +	bool "HiSilicon erratum 161010701: Skip SMMU Prefetch Cmd"
> +	default y
> +	help
> +	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 doesn't support the
> +	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
>  
> +	  If unsure, say Y.

We don't need a config. This feature (not to mention others like it) is
runtime-detected, and has effectively zero execution overhead and
negligible Image-size overhead either way; why would anyone ever turn it
off?

Robin.

> +
> +endmenu
>  
>  choice
>  	prompt "Page size"
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index f20d5d5..14538cb 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -650,16 +650,6 @@ struct arm_smmu_domain {
>  	struct iommu_domain		domain;
>  };
>  
> -struct arm_smmu_option_prop {
> -	u32 opt;
> -	const char *prop;
> -};
> -
> -static struct arm_smmu_option_prop arm_smmu_options[] = {
> -	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
> -	{ 0, NULL},
> -};
> -
>  enum smmu_erratum_match_type {
>  	se_match_dt,
>  };
> @@ -677,7 +667,17 @@ struct smmu_erratum_workaround {
>  };
>  
>  static const struct smmu_erratum_workaround smmu_workarounds[] = {
> +#ifdef CONFIG_HISILICON_ERRATUM_161010701
> +	{
> +		.match_type = se_match_dt,
> +		.id = "hisilicon,erratum-161010701",
> +		.desc_str = "HiSilicon erratum 161010701",
> +		.enable = erratum_skip_prefetch_cmd,
> +	},
> +#endif
> +	{
>  
> +	},
>  };
>  
>  typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround *,
> @@ -735,20 +735,6 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>  	return container_of(dom, struct arm_smmu_domain, domain);
>  }
>  
> -static void parse_driver_options(struct arm_smmu_device *smmu)
> -{
> -	int i = 0;
> -
> -	do {
> -		if (of_property_read_bool(smmu->dev->of_node,
> -						arm_smmu_options[i].prop)) {
> -			smmu->options |= arm_smmu_options[i].opt;
> -			dev_notice(smmu->dev, "option %s\n",
> -				arm_smmu_options[i].prop);
> -		}
> -	} while (arm_smmu_options[++i].opt);
> -}
> -
>  /* Low-level queue manipulation functions */
>  static bool queue_full(struct arm_smmu_queue *q)
>  {
> @@ -2695,8 +2681,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
>  	else
>  		ret = 0;
>  
> -	parse_driver_options(smmu);
> -
>  	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
>  
>  	if (of_dma_is_coherent(dev->of_node))
> 

[RFC v1 4/7] iommu/arm-smmu-v3: Enable HiSilicon erratum 161010701

[Robin Murphy]

On 13/05/17 10:47, shameer wrote:
> This replaces the existing broken_prefetch_cmd quirk using
> the new erratum framework.
> 
> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  arch/arm64/Kconfig          | 10 +++++++++-
>  drivers/iommu/arm-smmu-v3.c | 36 ++++++++++--------------------------
>  2 files changed, 19 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index a39029b..21d61ff 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -508,8 +508,16 @@ config QCOM_FALKOR_ERRATUM_1009
>  
>  	  If unsure, say Y.
>  
> -endmenu
> +config HISILICON_ERRATUM_161010701
> +	bool "HiSilicon erratum 161010701: Skip SMMU Prefetch Cmd"
> +	default y
> +	help
> +	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 doesn't support the
> +	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
>  
> +	  If unsure, say Y.

We don't need a config. This feature (not to mention others like it) is
runtime-detected, and has effectively zero execution overhead and
negligible Image-size overhead either way; why would anyone ever turn it
off?

Robin.

> +
> +endmenu
>  
>  choice
>  	prompt "Page size"
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index f20d5d5..14538cb 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -650,16 +650,6 @@ struct arm_smmu_domain {
>  	struct iommu_domain		domain;
>  };
>  
> -struct arm_smmu_option_prop {
> -	u32 opt;
> -	const char *prop;
> -};
> -
> -static struct arm_smmu_option_prop arm_smmu_options[] = {
> -	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
> -	{ 0, NULL},
> -};
> -
>  enum smmu_erratum_match_type {
>  	se_match_dt,
>  };
> @@ -677,7 +667,17 @@ struct smmu_erratum_workaround {
>  };
>  
>  static const struct smmu_erratum_workaround smmu_workarounds[] = {
> +#ifdef CONFIG_HISILICON_ERRATUM_161010701
> +	{
> +		.match_type = se_match_dt,
> +		.id = "hisilicon,erratum-161010701",
> +		.desc_str = "HiSilicon erratum 161010701",
> +		.enable = erratum_skip_prefetch_cmd,
> +	},
> +#endif
> +	{
>  
> +	},
>  };
>  
>  typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround *,
> @@ -735,20 +735,6 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>  	return container_of(dom, struct arm_smmu_domain, domain);
>  }
>  
> -static void parse_driver_options(struct arm_smmu_device *smmu)
> -{
> -	int i = 0;
> -
> -	do {
> -		if (of_property_read_bool(smmu->dev->of_node,
> -						arm_smmu_options[i].prop)) {
> -			smmu->options |= arm_smmu_options[i].opt;
> -			dev_notice(smmu->dev, "option %s\n",
> -				arm_smmu_options[i].prop);
> -		}
> -	} while (arm_smmu_options[++i].opt);
> -}
> -
>  /* Low-level queue manipulation functions */
>  static bool queue_full(struct arm_smmu_queue *q)
>  {
> @@ -2695,8 +2681,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
>  	else
>  		ret = 0;
>  
> -	parse_driver_options(smmu);
> -
>  	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
>  
>  	if (of_dma_is_coherent(dev->of_node))
> 

RE: [RFC v1 4/7] iommu/arm-smmu-v3: Enable HiSilicon erratum 161010701

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy-5wv7dgnIgG8@public.gmane.org]
> Sent: Tuesday, May 16, 2017 2:13 PM
> To: Shameerali Kolothum Thodi; will.deacon-5wv7dgnIgG8@public.gmane.org;
> mark.rutland-5wv7dgnIgG8@public.gmane.org; lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org; hanjun.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Gabriele Paoloni; John Garry; Linuxarm;
> linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Wangzhou
> (B); Guohanjun (Hanjun Guo); linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org;
> devel-E0kO6a4B6psdnm+yROfE0A@public.gmane.org
> Subject: Re: [RFC v1 4/7] iommu/arm-smmu-v3: Enable HiSilicon erratum
> 161010701
> 
> On 13/05/17 10:47, shameer wrote:
> > This replaces the existing broken_prefetch_cmd quirk using
> > the new erratum framework.
> >
> > Signed-off-by: shameer <shameerali.kolothum.thodi-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > ---
> >  arch/arm64/Kconfig          | 10 +++++++++-
> >  drivers/iommu/arm-smmu-v3.c | 36 ++++++++++--------------------------
> >  2 files changed, 19 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index a39029b..21d61ff 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -508,8 +508,16 @@ config QCOM_FALKOR_ERRATUM_1009
> >
> >  	  If unsure, say Y.
> >
> > -endmenu
> > +config HISILICON_ERRATUM_161010701
> > +	bool "HiSilicon erratum 161010701: Skip SMMU Prefetch Cmd"
> > +	default y
> > +	help
> > +	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 doesn't support
> the
> > +	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
> >
> > +	  If unsure, say Y.
> 
> We don't need a config. This feature (not to mention others like it) is
> runtime-detected, and has effectively zero execution overhead and
> negligible Image-size overhead either way; why would anyone ever turn it
> off?

This was the side effect of the "framework". Sure will update on next revision.

Thanks,
Shameer
 
> Robin.
> 
> > +
> > +endmenu
> >
> >  choice
> >  	prompt "Page size"
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index f20d5d5..14538cb 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -650,16 +650,6 @@ struct arm_smmu_domain {
> >  	struct iommu_domain		domain;
> >  };
> >
> > -struct arm_smmu_option_prop {
> > -	u32 opt;
> > -	const char *prop;
> > -};
> > -
> > -static struct arm_smmu_option_prop arm_smmu_options[] = {
> > -	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-
> cmd" },
> > -	{ 0, NULL},
> > -};
> > -
> >  enum smmu_erratum_match_type {
> >  	se_match_dt,
> >  };
> > @@ -677,7 +667,17 @@ struct smmu_erratum_workaround {
> >  };
> >
> >  static const struct smmu_erratum_workaround smmu_workarounds[] = {
> > +#ifdef CONFIG_HISILICON_ERRATUM_161010701
> > +	{
> > +		.match_type = se_match_dt,
> > +		.id = "hisilicon,erratum-161010701",
> > +		.desc_str = "HiSilicon erratum 161010701",
> > +		.enable = erratum_skip_prefetch_cmd,
> > +	},
> > +#endif
> > +	{
> >
> > +	},
> >  };
> >
> >  typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround
> *,
> > @@ -735,20 +735,6 @@ static struct arm_smmu_domain
> *to_smmu_domain(struct iommu_domain *dom)
> >  	return container_of(dom, struct arm_smmu_domain, domain);
> >  }
> >
> > -static void parse_driver_options(struct arm_smmu_device *smmu)
> > -{
> > -	int i = 0;
> > -
> > -	do {
> > -		if (of_property_read_bool(smmu->dev->of_node,
> > -						arm_smmu_options[i].prop))
> {
> > -			smmu->options |= arm_smmu_options[i].opt;
> > -			dev_notice(smmu->dev, "option %s\n",
> > -				arm_smmu_options[i].prop);
> > -		}
> > -	} while (arm_smmu_options[++i].opt);
> > -}
> > -
> >  /* Low-level queue manipulation functions */
> >  static bool queue_full(struct arm_smmu_queue *q)
> >  {
> > @@ -2695,8 +2681,6 @@ static int arm_smmu_device_dt_probe(struct
> platform_device *pdev,
> >  	else
> >  		ret = 0;
> >
> > -	parse_driver_options(smmu);
> > -
> >  	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
> >
> >  	if (of_dma_is_coherent(dev->of_node))
> >

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC v1 4/7] iommu/arm-smmu-v3: Enable HiSilicon erratum 161010701

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy at arm.com]
> Sent: Tuesday, May 16, 2017 2:13 PM
> To: Shameerali Kolothum Thodi; will.deacon at arm.com;
> mark.rutland at arm.com; lorenzo.pieralisi at arm.com; hanjun.guo at linaro.org
> Cc: devicetree at vger.kernel.org; Gabriele Paoloni; John Garry; Linuxarm;
> linux-acpi at vger.kernel.org; iommu at lists.linux-foundation.org; Wangzhou
> (B); Guohanjun (Hanjun Guo); linux-arm-kernel at lists.infradead.org;
> devel at acpica.org
> Subject: Re: [RFC v1 4/7] iommu/arm-smmu-v3: Enable HiSilicon erratum
> 161010701
> 
> On 13/05/17 10:47, shameer wrote:
> > This replaces the existing broken_prefetch_cmd quirk using
> > the new erratum framework.
> >
> > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  arch/arm64/Kconfig          | 10 +++++++++-
> >  drivers/iommu/arm-smmu-v3.c | 36 ++++++++++--------------------------
> >  2 files changed, 19 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index a39029b..21d61ff 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -508,8 +508,16 @@ config QCOM_FALKOR_ERRATUM_1009
> >
> >  	  If unsure, say Y.
> >
> > -endmenu
> > +config HISILICON_ERRATUM_161010701
> > +	bool "HiSilicon erratum 161010701: Skip SMMU Prefetch Cmd"
> > +	default y
> > +	help
> > +	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 doesn't support
> the
> > +	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
> >
> > +	  If unsure, say Y.
> 
> We don't need a config. This feature (not to mention others like it) is
> runtime-detected, and has effectively zero execution overhead and
> negligible Image-size overhead either way; why would anyone ever turn it
> off?

This was the side effect of the "framework". Sure will update on next revision.

Thanks,
Shameer
 
> Robin.
> 
> > +
> > +endmenu
> >
> >  choice
> >  	prompt "Page size"
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index f20d5d5..14538cb 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -650,16 +650,6 @@ struct arm_smmu_domain {
> >  	struct iommu_domain		domain;
> >  };
> >
> > -struct arm_smmu_option_prop {
> > -	u32 opt;
> > -	const char *prop;
> > -};
> > -
> > -static struct arm_smmu_option_prop arm_smmu_options[] = {
> > -	{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-
> cmd" },
> > -	{ 0, NULL},
> > -};
> > -
> >  enum smmu_erratum_match_type {
> >  	se_match_dt,
> >  };
> > @@ -677,7 +667,17 @@ struct smmu_erratum_workaround {
> >  };
> >
> >  static const struct smmu_erratum_workaround smmu_workarounds[] = {
> > +#ifdef CONFIG_HISILICON_ERRATUM_161010701
> > +	{
> > +		.match_type = se_match_dt,
> > +		.id = "hisilicon,erratum-161010701",
> > +		.desc_str = "HiSilicon erratum 161010701",
> > +		.enable = erratum_skip_prefetch_cmd,
> > +	},
> > +#endif
> > +	{
> >
> > +	},
> >  };
> >
> >  typedef bool (*se_match_fn_t)(const struct smmu_erratum_workaround
> *,
> > @@ -735,20 +735,6 @@ static struct arm_smmu_domain
> *to_smmu_domain(struct iommu_domain *dom)
> >  	return container_of(dom, struct arm_smmu_domain, domain);
> >  }
> >
> > -static void parse_driver_options(struct arm_smmu_device *smmu)
> > -{
> > -	int i = 0;
> > -
> > -	do {
> > -		if (of_property_read_bool(smmu->dev->of_node,
> > -						arm_smmu_options[i].prop))
> {
> > -			smmu->options |= arm_smmu_options[i].opt;
> > -			dev_notice(smmu->dev, "option %s\n",
> > -				arm_smmu_options[i].prop);
> > -		}
> > -	} while (arm_smmu_options[++i].opt);
> > -}
> > -
> >  /* Low-level queue manipulation functions */
> >  static bool queue_full(struct arm_smmu_queue *q)
> >  {
> > @@ -2695,8 +2681,6 @@ static int arm_smmu_device_dt_probe(struct
> platform_device *pdev,
> >  	else
> >  		ret = 0;
> >
> > -	parse_driver_options(smmu);
> > -
> >  	smmu_check_workarounds(smmu, se_match_dt, dev->of_node);
> >
> >  	if (of_dma_is_coherent(dev->of_node))
> >

[RFC v1 5/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010701

[shameer]

This uses the ACPI IORT model number to enable the erratum.

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 14538cb..770cc9e 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -652,6 +652,7 @@ struct arm_smmu_domain {
 
 enum smmu_erratum_match_type {
 	se_match_dt,
+	se_match_acpi_iort_model,
 };
 
 void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
@@ -674,6 +675,13 @@ static const struct smmu_erratum_workaround smmu_workarounds[] = {
 		.desc_str = "HiSilicon erratum 161010701",
 		.enable = erratum_skip_prefetch_cmd,
 	},
+	{
+		.match_type = se_match_acpi_iort_model,
+		.id = (void *)ACPI_IORT_SMMU_HISILICON_HI161X,
+		.desc_str = "HiSilicon erratum 161010701",
+		.enable = erratum_skip_prefetch_cmd,
+	},
+
 #endif
 	{
 
@@ -691,6 +699,15 @@ bool smmu_check_dt_erratum(const struct smmu_erratum_workaround *wa,
 	return of_property_read_bool(np, wa->id);
 }
 
+static
+bool smmu_check_acpi_iort_erratum(const struct smmu_erratum_workaround *wa,
+				const void *arg)
+{
+	const struct acpi_iort_smmu_v3 *iort_smmu = arg;
+
+	return (iort_smmu->model == *(u32 *)(&wa->id)) ? true : false;
+}
+
 static void smmu_enable_errata(struct arm_smmu_device *smmu,
 				enum smmu_erratum_match_type type,
 				se_match_fn_t match_fn,
@@ -724,6 +741,9 @@ static void smmu_check_workarounds(struct arm_smmu_device *smmu,
 	case se_match_dt:
 		match_fn = smmu_check_dt_erratum;
 		break;
+	case se_match_acpi_iort_model:
+		match_fn = smmu_check_acpi_iort_erratum;
+		break;
 	}
 
 	smmu_enable_errata(smmu, type, match_fn, arg);
@@ -2654,6 +2674,8 @@ static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
 	/* Retrieve SMMUv3 specific data */
 	iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
 
+	smmu_check_workarounds(smmu, se_match_acpi_iort_model, iort_smmu);
+
 	if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
 		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
 
-- 
2.5.0

[RFC v1 5/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010701

[shameer]

This uses the ACPI IORT model number to enable the erratum.

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm-smmu-v3.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 14538cb..770cc9e 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -652,6 +652,7 @@ struct arm_smmu_domain {
 
 enum smmu_erratum_match_type {
 	se_match_dt,
+	se_match_acpi_iort_model,
 };
 
 void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
@@ -674,6 +675,13 @@ static const struct smmu_erratum_workaround smmu_workarounds[] = {
 		.desc_str = "HiSilicon erratum 161010701",
 		.enable = erratum_skip_prefetch_cmd,
 	},
+	{
+		.match_type = se_match_acpi_iort_model,
+		.id = (void *)ACPI_IORT_SMMU_HISILICON_HI161X,
+		.desc_str = "HiSilicon erratum 161010701",
+		.enable = erratum_skip_prefetch_cmd,
+	},
+
 #endif
 	{
 
@@ -691,6 +699,15 @@ bool smmu_check_dt_erratum(const struct smmu_erratum_workaround *wa,
 	return of_property_read_bool(np, wa->id);
 }
 
+static
+bool smmu_check_acpi_iort_erratum(const struct smmu_erratum_workaround *wa,
+				const void *arg)
+{
+	const struct acpi_iort_smmu_v3 *iort_smmu = arg;
+
+	return (iort_smmu->model == *(u32 *)(&wa->id)) ? true : false;
+}
+
 static void smmu_enable_errata(struct arm_smmu_device *smmu,
 				enum smmu_erratum_match_type type,
 				se_match_fn_t match_fn,
@@ -724,6 +741,9 @@ static void smmu_check_workarounds(struct arm_smmu_device *smmu,
 	case se_match_dt:
 		match_fn = smmu_check_dt_erratum;
 		break;
+	case se_match_acpi_iort_model:
+		match_fn = smmu_check_acpi_iort_erratum;
+		break;
 	}
 
 	smmu_enable_errata(smmu, type, match_fn, arg);
@@ -2654,6 +2674,8 @@ static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
 	/* Retrieve SMMUv3 specific data */
 	iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
 
+	smmu_check_workarounds(smmu, se_match_acpi_iort_model, iort_smmu);
+
 	if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
 		smmu->features |= ARM_SMMU_FEAT_COHERENCY;
 
-- 
2.5.0

[RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

[shameer]

The HiSilicon erratum 161010801 describes the limitation of HiSilicon
platforms Hip06/Hip07 to support the SMMU mappings for MSI transactions.

On these platforms GICv3 ITS translator is presented with the deviceID
by extending the MSI payload data to 64 bits to include the deviceID.
Hence, the PCIe controller on this platforms has to differentiate the
MSI payload against other DMA payload and has to modify the MSI payload.
This basically makes it difficult for this platforms to have a SMMU
translation for MSI.

This patch implements a ACPI table based quirk to reserve the hw msi
regions in the smmu-v3 driver which means these address regions will
not be translated and will be excluded from iova allocations.

ACPI CSRT vendor specific blobs are used to pass the reserve address
region info on these platforms. HiSilicon CSRT table contains a resource
group whose device Id matches with the SMMU device model id defined
in the IORT spec. This SMMU group will not have the optional Resource
group shared info.

typedef struct {
  UINT32  Length;
  UINT32  VendorId;
  UINT32  SubvendorId;
  UINT16  DeviceId;      ---->Set to IORT SMMU Model number.
  UINT16  SubdeviceId;
  UINT16  Revision;
  UINT8   Reserved[2];
  UINT32  SharedInfoLength;  ---->Set to zero.
  ACPI_CSRT_RESOURCE_DESCRIPTOR RsrcDesc;
} ACPI_CSRT_RESOURCE_GROUP;

The resource descriptor associated with this group will have the vendor
specific section populated to represent the MSI region as below.

typedef struct {
  UINT32  Length;
  UINT16  ResourceType;
  UINT16  ResourceSubtype;
  UINT32  UID;
  /* Vendor defined info */
  UINT64  Base;        -->Corresponding SMMU node base address.
  UINT64  MSIResvStart; -----> HW MSI reserve start address.
  UINT32  MSIResvLen; -------> HW MSI reserve len.
} ACPI_CSRT_RESOURCE_DESCRIPTOR;

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 arch/arm64/Kconfig          | 10 ++++++
 drivers/iommu/arm-smmu-v3.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 21d61ff..e7ebd97 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -516,6 +516,16 @@ config HISILICON_ERRATUM_161010701
 	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
 
 	  If unsure, say Y.
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801: Reserve HW MSI regions"
+	default y
+	help
+	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 has to bypass the
+	  GIC ITS doorbel msi address regions. This will make sure that ITS
+	  doorbell address regions are reserved and excluded from iova
+	  allocations.
+
+	  If unsure, say Y.
 
 endmenu
 
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e7a8a50..a33e339 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -663,6 +663,73 @@ void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
 	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
 }
 
+#ifdef CONFIG_HISILICON_ERRATUM_161010801
+static void parse_hisi_csrt_msi(struct arm_smmu_device *smmu,
+		const struct acpi_iort_smmu_v3 *iort)
+
+{
+	struct acpi_csrt_group *grp, *end;
+	struct acpi_table_csrt *csrt;
+	acpi_status status;
+
+	status = acpi_get_table(ACPI_SIG_CSRT, 0,
+				(struct acpi_table_header **)&csrt);
+	if (ACPI_FAILURE(status)) {
+		dev_warn(smmu->dev, "HiSi CSRT table get failed: 0x%x\n",
+								status);
+		return;
+	}
+
+	grp = (struct acpi_csrt_group *)(csrt + 1);
+	end = (struct acpi_csrt_group *)((void *)csrt + csrt->header.length);
+
+	while (grp < end) {
+		if (grp->device_id == iort->model) {
+			/*
+			 * We don't have the optional shared info for this grp
+			 * and has only one resource descriptor with vendor
+			 * defined msi region for this group. Go straight to
+			 * vendor defined info.
+			 */
+			struct acpi_csrt_descriptor *desc =
+					(struct acpi_csrt_descriptor *)&grp[1];
+
+			/*
+			 * HiSilicon CSRT vendor info. First 8 bytes gives smmu
+			 * node base addr, next 8 bytes HW MSI reserve region
+			 * addr and the remaining 4 byte the len.
+			 */
+			void *vendor = &desc[1];
+			u64 base = (*(u64 *)vendor);
+
+			if (base == iort->base_address && smmu->msi_region) {
+				/* Replace the default SW msi with HW msi */
+
+				smmu->msi_region->start =
+						*((u64 *)((u64 *)vendor+1));
+				smmu->msi_region->length =
+						*((u32 *)((u64 *)vendor+2));
+				smmu->msi_region->type = IOMMU_RESV_MSI;
+				dev_info(smmu->dev,
+					"HiSi msi addr 0x%pa size 0x%zx\n",
+					&smmu->msi_region->start,
+					smmu->msi_region->length);
+				return;
+			}
+		}
+
+		grp = (struct acpi_csrt_group *)((void *)grp + grp->length);
+	}
+
+}
+void erratum_hisi_resv_hw_msi(struct arm_smmu_device *smmu, void *arg)
+{
+	const struct acpi_iort_smmu_v3 *iort_smmu = arg;
+
+	parse_hisi_csrt_msi(smmu, iort_smmu);
+}
+#endif
+
 struct smmu_erratum_workaround {
 	enum smmu_erratum_match_type match_type;
 	const void *id;	/* Indicate the Erratum ID */
@@ -686,6 +753,14 @@ static const struct smmu_erratum_workaround smmu_workarounds[] = {
 	},
 
 #endif
+#ifdef CONFIG_HISILICON_ERRATUM_161010801
+	{
+		.match_type = se_match_acpi_iort_model,
+		.id = (void *)ACPI_IORT_SMMU_HISILICON_HI161X,
+		.desc_str = "HiSilicon erratum 161010801",
+		.enable = erratum_hisi_resv_hw_msi,
+	},
+#endif
 	{
 
 	},
-- 
2.5.0

[RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

[shameer]

The HiSilicon erratum 161010801 describes the limitation of HiSilicon
platforms Hip06/Hip07 to support the SMMU mappings for MSI transactions.

On these platforms GICv3 ITS translator is presented with the deviceID
by extending the MSI payload data to 64 bits to include the deviceID.
Hence, the PCIe controller on this platforms has to differentiate the
MSI payload against other DMA payload and has to modify the MSI payload.
This basically makes it difficult for this platforms to have a SMMU
translation for MSI.

This patch implements a ACPI table based quirk to reserve the hw msi
regions in the smmu-v3 driver which means these address regions will
not be translated and will be excluded from iova allocations.

ACPI CSRT vendor specific blobs are used to pass the reserve address
region info on these platforms. HiSilicon CSRT table contains a resource
group whose device Id matches with the SMMU device model id defined
in the IORT spec. This SMMU group will not have the optional Resource
group shared info.

typedef struct {
  UINT32  Length;
  UINT32  VendorId;
  UINT32  SubvendorId;
  UINT16  DeviceId;      ---->Set to IORT SMMU Model number.
  UINT16  SubdeviceId;
  UINT16  Revision;
  UINT8   Reserved[2];
  UINT32  SharedInfoLength;  ---->Set to zero.
  ACPI_CSRT_RESOURCE_DESCRIPTOR RsrcDesc;
} ACPI_CSRT_RESOURCE_GROUP;

The resource descriptor associated with this group will have the vendor
specific section populated to represent the MSI region as below.

typedef struct {
  UINT32  Length;
  UINT16  ResourceType;
  UINT16  ResourceSubtype;
  UINT32  UID;
  /* Vendor defined info */
  UINT64  Base;        -->Corresponding SMMU node base address.
  UINT64  MSIResvStart; -----> HW MSI reserve start address.
  UINT32  MSIResvLen; -------> HW MSI reserve len.
} ACPI_CSRT_RESOURCE_DESCRIPTOR;

Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
---
 arch/arm64/Kconfig          | 10 ++++++
 drivers/iommu/arm-smmu-v3.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 21d61ff..e7ebd97 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -516,6 +516,16 @@ config HISILICON_ERRATUM_161010701
 	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
 
 	  If unsure, say Y.
+config HISILICON_ERRATUM_161010801
+	bool "HiSilicon erratum 161010801: Reserve HW MSI regions"
+	default y
+	help
+	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 has to bypass the
+	  GIC ITS doorbel msi address regions. This will make sure that ITS
+	  doorbell address regions are reserved and excluded from iova
+	  allocations.
+
+	  If unsure, say Y.
 
 endmenu
 
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e7a8a50..a33e339 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -663,6 +663,73 @@ void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
 	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
 }
 
+#ifdef CONFIG_HISILICON_ERRATUM_161010801
+static void parse_hisi_csrt_msi(struct arm_smmu_device *smmu,
+		const struct acpi_iort_smmu_v3 *iort)
+
+{
+	struct acpi_csrt_group *grp, *end;
+	struct acpi_table_csrt *csrt;
+	acpi_status status;
+
+	status = acpi_get_table(ACPI_SIG_CSRT, 0,
+				(struct acpi_table_header **)&csrt);
+	if (ACPI_FAILURE(status)) {
+		dev_warn(smmu->dev, "HiSi CSRT table get failed: 0x%x\n",
+								status);
+		return;
+	}
+
+	grp = (struct acpi_csrt_group *)(csrt + 1);
+	end = (struct acpi_csrt_group *)((void *)csrt + csrt->header.length);
+
+	while (grp < end) {
+		if (grp->device_id == iort->model) {
+			/*
+			 * We don't have the optional shared info for this grp
+			 * and has only one resource descriptor with vendor
+			 * defined msi region for this group. Go straight to
+			 * vendor defined info.
+			 */
+			struct acpi_csrt_descriptor *desc =
+					(struct acpi_csrt_descriptor *)&grp[1];
+
+			/*
+			 * HiSilicon CSRT vendor info. First 8 bytes gives smmu
+			 * node base addr, next 8 bytes HW MSI reserve region
+			 * addr and the remaining 4 byte the len.
+			 */
+			void *vendor = &desc[1];
+			u64 base = (*(u64 *)vendor);
+
+			if (base == iort->base_address && smmu->msi_region) {
+				/* Replace the default SW msi with HW msi */
+
+				smmu->msi_region->start =
+						*((u64 *)((u64 *)vendor+1));
+				smmu->msi_region->length =
+						*((u32 *)((u64 *)vendor+2));
+				smmu->msi_region->type = IOMMU_RESV_MSI;
+				dev_info(smmu->dev,
+					"HiSi msi addr 0x%pa size 0x%zx\n",
+					&smmu->msi_region->start,
+					smmu->msi_region->length);
+				return;
+			}
+		}
+
+		grp = (struct acpi_csrt_group *)((void *)grp + grp->length);
+	}
+
+}
+void erratum_hisi_resv_hw_msi(struct arm_smmu_device *smmu, void *arg)
+{
+	const struct acpi_iort_smmu_v3 *iort_smmu = arg;
+
+	parse_hisi_csrt_msi(smmu, iort_smmu);
+}
+#endif
+
 struct smmu_erratum_workaround {
 	enum smmu_erratum_match_type match_type;
 	const void *id;	/* Indicate the Erratum ID */
@@ -686,6 +753,14 @@ static const struct smmu_erratum_workaround smmu_workarounds[] = {
 	},
 
 #endif
+#ifdef CONFIG_HISILICON_ERRATUM_161010801
+	{
+		.match_type = se_match_acpi_iort_model,
+		.id = (void *)ACPI_IORT_SMMU_HISILICON_HI161X,
+		.desc_str = "HiSilicon erratum 161010801",
+		.enable = erratum_hisi_resv_hw_msi,
+	},
+#endif
 	{
 
 	},
-- 
2.5.0

Re: [RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

[Robin Murphy]

On 13/05/17 10:47, shameer wrote:
> The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> platforms Hip06/Hip07 to support the SMMU mappings for MSI transactions.
> 
> On these platforms GICv3 ITS translator is presented with the deviceID
> by extending the MSI payload data to 64 bits to include the deviceID.
> Hence, the PCIe controller on this platforms has to differentiate the
> MSI payload against other DMA payload and has to modify the MSI payload.
> This basically makes it difficult for this platforms to have a SMMU
> translation for MSI.
> 
> This patch implements a ACPI table based quirk to reserve the hw msi
> regions in the smmu-v3 driver which means these address regions will
> not be translated and will be excluded from iova allocations.
> 
> ACPI CSRT vendor specific blobs are used to pass the reserve address
> region info on these platforms. HiSilicon CSRT table contains a resource
> group whose device Id matches with the SMMU device model id defined
> in the IORT spec. This SMMU group will not have the optional Resource
> group shared info.

Lorenzo made a point that it might be relatively straightforward to just
follow the IORT mapping for the SMMU through to the ITS MADT entry and
pull the ITS geometry out of that. It would certainly be nicer to have
such a helper abstracted away in the IORT code than have to go parsing
vendor-specific tables directly in the SMMU driver. I reckon it might be
worth taking that idea a bit further to see how it looks.

> typedef struct {
>   UINT32  Length;
>   UINT32  VendorId;
>   UINT32  SubvendorId;
>   UINT16  DeviceId;      ---->Set to IORT SMMU Model number.
>   UINT16  SubdeviceId;
>   UINT16  Revision;
>   UINT8   Reserved[2];
>   UINT32  SharedInfoLength;  ---->Set to zero.
>   ACPI_CSRT_RESOURCE_DESCRIPTOR RsrcDesc;
> } ACPI_CSRT_RESOURCE_GROUP;
> 
> The resource descriptor associated with this group will have the vendor
> specific section populated to represent the MSI region as below.
> 
> typedef struct {
>   UINT32  Length;
>   UINT16  ResourceType;
>   UINT16  ResourceSubtype;
>   UINT32  UID;
>   /* Vendor defined info */
>   UINT64  Base;        -->Corresponding SMMU node base address.
>   UINT64  MSIResvStart; -----> HW MSI reserve start address.
>   UINT32  MSIResvLen; -------> HW MSI reserve len.
> } ACPI_CSRT_RESOURCE_DESCRIPTOR;
> 
> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  arch/arm64/Kconfig          | 10 ++++++
>  drivers/iommu/arm-smmu-v3.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 85 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 21d61ff..e7ebd97 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig

I think you misspelled Documentation/arm64/silicon-errata.txt there ;)

Robin.

> @@ -516,6 +516,16 @@ config HISILICON_ERRATUM_161010701
>  	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
>  
>  	  If unsure, say Y.
> +config HISILICON_ERRATUM_161010801
> +	bool "HiSilicon erratum 161010801: Reserve HW MSI regions"
> +	default y
> +	help
> +	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 has to bypass the
> +	  GIC ITS doorbel msi address regions. This will make sure that ITS
> +	  doorbell address regions are reserved and excluded from iova
> +	  allocations.
> +
> +	  If unsure, say Y.
>  
>  endmenu
>  
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index e7a8a50..a33e339 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -663,6 +663,73 @@ void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
>  	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
>  }
>  
> +#ifdef CONFIG_HISILICON_ERRATUM_161010801
> +static void parse_hisi_csrt_msi(struct arm_smmu_device *smmu,
> +		const struct acpi_iort_smmu_v3 *iort)
> +
> +{
> +	struct acpi_csrt_group *grp, *end;
> +	struct acpi_table_csrt *csrt;
> +	acpi_status status;
> +
> +	status = acpi_get_table(ACPI_SIG_CSRT, 0,
> +				(struct acpi_table_header **)&csrt);
> +	if (ACPI_FAILURE(status)) {
> +		dev_warn(smmu->dev, "HiSi CSRT table get failed: 0x%x\n",
> +								status);
> +		return;
> +	}
> +
> +	grp = (struct acpi_csrt_group *)(csrt + 1);
> +	end = (struct acpi_csrt_group *)((void *)csrt + csrt->header.length);
> +
> +	while (grp < end) {
> +		if (grp->device_id == iort->model) {
> +			/*
> +			 * We don't have the optional shared info for this grp
> +			 * and has only one resource descriptor with vendor
> +			 * defined msi region for this group. Go straight to
> +			 * vendor defined info.
> +			 */
> +			struct acpi_csrt_descriptor *desc =
> +					(struct acpi_csrt_descriptor *)&grp[1];
> +
> +			/*
> +			 * HiSilicon CSRT vendor info. First 8 bytes gives smmu
> +			 * node base addr, next 8 bytes HW MSI reserve region
> +			 * addr and the remaining 4 byte the len.
> +			 */
> +			void *vendor = &desc[1];
> +			u64 base = (*(u64 *)vendor);
> +
> +			if (base == iort->base_address && smmu->msi_region) {
> +				/* Replace the default SW msi with HW msi */
> +
> +				smmu->msi_region->start =
> +						*((u64 *)((u64 *)vendor+1));
> +				smmu->msi_region->length =
> +						*((u32 *)((u64 *)vendor+2));
> +				smmu->msi_region->type = IOMMU_RESV_MSI;
> +				dev_info(smmu->dev,
> +					"HiSi msi addr 0x%pa size 0x%zx\n",
> +					&smmu->msi_region->start,
> +					smmu->msi_region->length);
> +				return;
> +			}
> +		}
> +
> +		grp = (struct acpi_csrt_group *)((void *)grp + grp->length);
> +	}
> +
> +}
> +void erratum_hisi_resv_hw_msi(struct arm_smmu_device *smmu, void *arg)
> +{
> +	const struct acpi_iort_smmu_v3 *iort_smmu = arg;
> +
> +	parse_hisi_csrt_msi(smmu, iort_smmu);
> +}
> +#endif
> +
>  struct smmu_erratum_workaround {
>  	enum smmu_erratum_match_type match_type;
>  	const void *id;	/* Indicate the Erratum ID */
> @@ -686,6 +753,14 @@ static const struct smmu_erratum_workaround smmu_workarounds[] = {
>  	},
>  
>  #endif
> +#ifdef CONFIG_HISILICON_ERRATUM_161010801
> +	{
> +		.match_type = se_match_acpi_iort_model,
> +		.id = (void *)ACPI_IORT_SMMU_HISILICON_HI161X,
> +		.desc_str = "HiSilicon erratum 161010801",
> +		.enable = erratum_hisi_resv_hw_msi,
> +	},
> +#endif
>  	{
>  
>  	},
> 

[RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

[Robin Murphy]

On 13/05/17 10:47, shameer wrote:
> The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> platforms Hip06/Hip07 to support the SMMU mappings for MSI transactions.
> 
> On these platforms GICv3 ITS translator is presented with the deviceID
> by extending the MSI payload data to 64 bits to include the deviceID.
> Hence, the PCIe controller on this platforms has to differentiate the
> MSI payload against other DMA payload and has to modify the MSI payload.
> This basically makes it difficult for this platforms to have a SMMU
> translation for MSI.
> 
> This patch implements a ACPI table based quirk to reserve the hw msi
> regions in the smmu-v3 driver which means these address regions will
> not be translated and will be excluded from iova allocations.
> 
> ACPI CSRT vendor specific blobs are used to pass the reserve address
> region info on these platforms. HiSilicon CSRT table contains a resource
> group whose device Id matches with the SMMU device model id defined
> in the IORT spec. This SMMU group will not have the optional Resource
> group shared info.

Lorenzo made a point that it might be relatively straightforward to just
follow the IORT mapping for the SMMU through to the ITS MADT entry and
pull the ITS geometry out of that. It would certainly be nicer to have
such a helper abstracted away in the IORT code than have to go parsing
vendor-specific tables directly in the SMMU driver. I reckon it might be
worth taking that idea a bit further to see how it looks.

> typedef struct {
>   UINT32  Length;
>   UINT32  VendorId;
>   UINT32  SubvendorId;
>   UINT16  DeviceId;      ---->Set to IORT SMMU Model number.
>   UINT16  SubdeviceId;
>   UINT16  Revision;
>   UINT8   Reserved[2];
>   UINT32  SharedInfoLength;  ---->Set to zero.
>   ACPI_CSRT_RESOURCE_DESCRIPTOR RsrcDesc;
> } ACPI_CSRT_RESOURCE_GROUP;
> 
> The resource descriptor associated with this group will have the vendor
> specific section populated to represent the MSI region as below.
> 
> typedef struct {
>   UINT32  Length;
>   UINT16  ResourceType;
>   UINT16  ResourceSubtype;
>   UINT32  UID;
>   /* Vendor defined info */
>   UINT64  Base;        -->Corresponding SMMU node base address.
>   UINT64  MSIResvStart; -----> HW MSI reserve start address.
>   UINT32  MSIResvLen; -------> HW MSI reserve len.
> } ACPI_CSRT_RESOURCE_DESCRIPTOR;
> 
> Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> ---
>  arch/arm64/Kconfig          | 10 ++++++
>  drivers/iommu/arm-smmu-v3.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 85 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 21d61ff..e7ebd97 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig

I think you misspelled Documentation/arm64/silicon-errata.txt there ;)

Robin.

> @@ -516,6 +516,16 @@ config HISILICON_ERRATUM_161010701
>  	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
>  
>  	  If unsure, say Y.
> +config HISILICON_ERRATUM_161010801
> +	bool "HiSilicon erratum 161010801: Reserve HW MSI regions"
> +	default y
> +	help
> +	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 has to bypass the
> +	  GIC ITS doorbel msi address regions. This will make sure that ITS
> +	  doorbell address regions are reserved and excluded from iova
> +	  allocations.
> +
> +	  If unsure, say Y.
>  
>  endmenu
>  
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index e7a8a50..a33e339 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -663,6 +663,73 @@ void erratum_skip_prefetch_cmd(struct arm_smmu_device *smmu, void *arg)
>  	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
>  }
>  
> +#ifdef CONFIG_HISILICON_ERRATUM_161010801
> +static void parse_hisi_csrt_msi(struct arm_smmu_device *smmu,
> +		const struct acpi_iort_smmu_v3 *iort)
> +
> +{
> +	struct acpi_csrt_group *grp, *end;
> +	struct acpi_table_csrt *csrt;
> +	acpi_status status;
> +
> +	status = acpi_get_table(ACPI_SIG_CSRT, 0,
> +				(struct acpi_table_header **)&csrt);
> +	if (ACPI_FAILURE(status)) {
> +		dev_warn(smmu->dev, "HiSi CSRT table get failed: 0x%x\n",
> +								status);
> +		return;
> +	}
> +
> +	grp = (struct acpi_csrt_group *)(csrt + 1);
> +	end = (struct acpi_csrt_group *)((void *)csrt + csrt->header.length);
> +
> +	while (grp < end) {
> +		if (grp->device_id == iort->model) {
> +			/*
> +			 * We don't have the optional shared info for this grp
> +			 * and has only one resource descriptor with vendor
> +			 * defined msi region for this group. Go straight to
> +			 * vendor defined info.
> +			 */
> +			struct acpi_csrt_descriptor *desc =
> +					(struct acpi_csrt_descriptor *)&grp[1];
> +
> +			/*
> +			 * HiSilicon CSRT vendor info. First 8 bytes gives smmu
> +			 * node base addr, next 8 bytes HW MSI reserve region
> +			 * addr and the remaining 4 byte the len.
> +			 */
> +			void *vendor = &desc[1];
> +			u64 base = (*(u64 *)vendor);
> +
> +			if (base == iort->base_address && smmu->msi_region) {
> +				/* Replace the default SW msi with HW msi */
> +
> +				smmu->msi_region->start =
> +						*((u64 *)((u64 *)vendor+1));
> +				smmu->msi_region->length =
> +						*((u32 *)((u64 *)vendor+2));
> +				smmu->msi_region->type = IOMMU_RESV_MSI;
> +				dev_info(smmu->dev,
> +					"HiSi msi addr 0x%pa size 0x%zx\n",
> +					&smmu->msi_region->start,
> +					smmu->msi_region->length);
> +				return;
> +			}
> +		}
> +
> +		grp = (struct acpi_csrt_group *)((void *)grp + grp->length);
> +	}
> +
> +}
> +void erratum_hisi_resv_hw_msi(struct arm_smmu_device *smmu, void *arg)
> +{
> +	const struct acpi_iort_smmu_v3 *iort_smmu = arg;
> +
> +	parse_hisi_csrt_msi(smmu, iort_smmu);
> +}
> +#endif
> +
>  struct smmu_erratum_workaround {
>  	enum smmu_erratum_match_type match_type;
>  	const void *id;	/* Indicate the Erratum ID */
> @@ -686,6 +753,14 @@ static const struct smmu_erratum_workaround smmu_workarounds[] = {
>  	},
>  
>  #endif
> +#ifdef CONFIG_HISILICON_ERRATUM_161010801
> +	{
> +		.match_type = se_match_acpi_iort_model,
> +		.id = (void *)ACPI_IORT_SMMU_HISILICON_HI161X,
> +		.desc_str = "HiSilicon erratum 161010801",
> +		.enable = erratum_hisi_resv_hw_msi,
> +	},
> +#endif
>  	{
>  
>  	},
> 

RE: [RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: Tuesday, May 16, 2017 2:50 PM
> To: Shameerali Kolothum Thodi; will.deacon@arm.com;
> mark.rutland@arm.com; lorenzo.pieralisi@arm.com; hanjun.guo@linaro.org
> Cc: Gabriele Paoloni; John Garry; iommu@lists.linux-foundation.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> acpi@vger.kernel.org; devel@acpica.org; Linuxarm; Wangzhou (B);
> Guohanjun (Hanjun Guo)
> Subject: Re: [RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon
> erratum 161010801
> 
> On 13/05/17 10:47, shameer wrote:
> > The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> > platforms Hip06/Hip07 to support the SMMU mappings for MSI
> transactions.
> >
> > On these platforms GICv3 ITS translator is presented with the deviceID
> > by extending the MSI payload data to 64 bits to include the deviceID.
> > Hence, the PCIe controller on this platforms has to differentiate the
> > MSI payload against other DMA payload and has to modify the MSI
> payload.
> > This basically makes it difficult for this platforms to have a SMMU
> > translation for MSI.
> >
> > This patch implements a ACPI table based quirk to reserve the hw msi
> > regions in the smmu-v3 driver which means these address regions will
> > not be translated and will be excluded from iova allocations.
> >
> > ACPI CSRT vendor specific blobs are used to pass the reserve address
> > region info on these platforms. HiSilicon CSRT table contains a resource
> > group whose device Id matches with the SMMU device model id defined
> > in the IORT spec. This SMMU group will not have the optional Resource
> > group shared info.
> 
> Lorenzo made a point that it might be relatively straightforward to just
> follow the IORT mapping for the SMMU through to the ITS MADT entry and
> pull the ITS geometry out of that. It would certainly be nicer to have
> such a helper abstracted away in the IORT code than have to go parsing
> vendor-specific tables directly in the SMMU driver. I reckon it might be
> worth taking that idea a bit further to see how it looks.

Ok. John has already mentioned this idea in our off-list discussion and we 
have one implementation where we go through the IORT node ID mappings
array to find out the associated IORT ITS  node. It then iterates over the ACPI
MADT  GIC ITS table entries, , looking for a match for the GIC ITS id, and 
retrieves the base address of the matching ITS.

But as you said, it requires some helper from the IORT code , otherwise we 
will end up adding all the ACPI table parse code in SMMUv3. We will take
another look at this.

Thanks,
Shameer

> > typedef struct {
> >   UINT32  Length;
> >   UINT32  VendorId;
> >   UINT32  SubvendorId;
> >   UINT16  DeviceId;      ---->Set to IORT SMMU Model number.
> >   UINT16  SubdeviceId;
> >   UINT16  Revision;
> >   UINT8   Reserved[2];
> >   UINT32  SharedInfoLength;  ---->Set to zero.
> >   ACPI_CSRT_RESOURCE_DESCRIPTOR RsrcDesc;
> > } ACPI_CSRT_RESOURCE_GROUP;
> >
> > The resource descriptor associated with this group will have the vendor
> > specific section populated to represent the MSI region as below.
> >
> > typedef struct {
> >   UINT32  Length;
> >   UINT16  ResourceType;
> >   UINT16  ResourceSubtype;
> >   UINT32  UID;
> >   /* Vendor defined info */
> >   UINT64  Base;        -->Corresponding SMMU node base address.
> >   UINT64  MSIResvStart; -----> HW MSI reserve start address.
> >   UINT32  MSIResvLen; -------> HW MSI reserve len.
> > } ACPI_CSRT_RESOURCE_DESCRIPTOR;
> >
> > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  arch/arm64/Kconfig          | 10 ++++++
> >  drivers/iommu/arm-smmu-v3.c | 75
> +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 85 insertions(+)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 21d61ff..e7ebd97 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> 
> I think you misspelled Documentation/arm64/silicon-errata.txt there ;)
> 
> Robin.
> 
> > @@ -516,6 +516,16 @@ config HISILICON_ERRATUM_161010701
> >  	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
> >
> >  	  If unsure, say Y.
> > +config HISILICON_ERRATUM_161010801
> > +	bool "HiSilicon erratum 161010801: Reserve HW MSI regions"
> > +	default y
> > +	help
> > +	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 has to bypass the
> > +	  GIC ITS doorbel msi address regions. This will make sure that ITS
> > +	  doorbell address regions are reserved and excluded from iova
> > +	  allocations.
> > +
> > +	  If unsure, say Y.
> >
> >  endmenu
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index e7a8a50..a33e339 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -663,6 +663,73 @@ void erratum_skip_prefetch_cmd(struct
> arm_smmu_device *smmu, void *arg)
> >  	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> >  }
> >
> > +#ifdef CONFIG_HISILICON_ERRATUM_161010801
> > +static void parse_hisi_csrt_msi(struct arm_smmu_device *smmu,
> > +		const struct acpi_iort_smmu_v3 *iort)
> > +
> > +{
> > +	struct acpi_csrt_group *grp, *end;
> > +	struct acpi_table_csrt *csrt;
> > +	acpi_status status;
> > +
> > +	status = acpi_get_table(ACPI_SIG_CSRT, 0,
> > +				(struct acpi_table_header **)&csrt);
> > +	if (ACPI_FAILURE(status)) {
> > +		dev_warn(smmu->dev, "HiSi CSRT table get failed: 0x%x\n",
> > +								status);
> > +		return;
> > +	}
> > +
> > +	grp = (struct acpi_csrt_group *)(csrt + 1);
> > +	end = (struct acpi_csrt_group *)((void *)csrt + csrt->header.length);
> > +
> > +	while (grp < end) {
> > +		if (grp->device_id == iort->model) {
> > +			/*
> > +			 * We don't have the optional shared info for this grp
> > +			 * and has only one resource descriptor with vendor
> > +			 * defined msi region for this group. Go straight to
> > +			 * vendor defined info.
> > +			 */
> > +			struct acpi_csrt_descriptor *desc =
> > +					(struct acpi_csrt_descriptor
> *)&grp[1];
> > +
> > +			/*
> > +			 * HiSilicon CSRT vendor info. First 8 bytes gives smmu
> > +			 * node base addr, next 8 bytes HW MSI reserve
> region
> > +			 * addr and the remaining 4 byte the len.
> > +			 */
> > +			void *vendor = &desc[1];
> > +			u64 base = (*(u64 *)vendor);
> > +
> > +			if (base == iort->base_address && smmu-
> >msi_region) {
> > +				/* Replace the default SW msi with HW msi
> */
> > +
> > +				smmu->msi_region->start =
> > +						*((u64 *)((u64 *)vendor+1));
> > +				smmu->msi_region->length =
> > +						*((u32 *)((u64 *)vendor+2));
> > +				smmu->msi_region->type =
> IOMMU_RESV_MSI;
> > +				dev_info(smmu->dev,
> > +					"HiSi msi addr 0x%pa size 0x%zx\n",
> > +					&smmu->msi_region->start,
> > +					smmu->msi_region->length);
> > +				return;
> > +			}
> > +		}
> > +
> > +		grp = (struct acpi_csrt_group *)((void *)grp + grp->length);
> > +	}
> > +
> > +}
> > +void erratum_hisi_resv_hw_msi(struct arm_smmu_device *smmu, void
> *arg)
> > +{
> > +	const struct acpi_iort_smmu_v3 *iort_smmu = arg;
> > +
> > +	parse_hisi_csrt_msi(smmu, iort_smmu);
> > +}
> > +#endif
> > +
> >  struct smmu_erratum_workaround {
> >  	enum smmu_erratum_match_type match_type;
> >  	const void *id;	/* Indicate the Erratum ID */
> > @@ -686,6 +753,14 @@ static const struct smmu_erratum_workaround
> smmu_workarounds[] = {
> >  	},
> >
> >  #endif
> > +#ifdef CONFIG_HISILICON_ERRATUM_161010801
> > +	{
> > +		.match_type = se_match_acpi_iort_model,
> > +		.id = (void *)ACPI_IORT_SMMU_HISILICON_HI161X,
> > +		.desc_str = "HiSilicon erratum 161010801",
> > +		.enable = erratum_hisi_resv_hw_msi,
> > +	},
> > +#endif
> >  	{
> >
> >  	},
> >

[RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy at arm.com]
> Sent: Tuesday, May 16, 2017 2:50 PM
> To: Shameerali Kolothum Thodi; will.deacon at arm.com;
> mark.rutland at arm.com; lorenzo.pieralisi at arm.com; hanjun.guo at linaro.org
> Cc: Gabriele Paoloni; John Garry; iommu at lists.linux-foundation.org;
> devicetree at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
> acpi at vger.kernel.org; devel at acpica.org; Linuxarm; Wangzhou (B);
> Guohanjun (Hanjun Guo)
> Subject: Re: [RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon
> erratum 161010801
> 
> On 13/05/17 10:47, shameer wrote:
> > The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> > platforms Hip06/Hip07 to support the SMMU mappings for MSI
> transactions.
> >
> > On these platforms GICv3 ITS translator is presented with the deviceID
> > by extending the MSI payload data to 64 bits to include the deviceID.
> > Hence, the PCIe controller on this platforms has to differentiate the
> > MSI payload against other DMA payload and has to modify the MSI
> payload.
> > This basically makes it difficult for this platforms to have a SMMU
> > translation for MSI.
> >
> > This patch implements a ACPI table based quirk to reserve the hw msi
> > regions in the smmu-v3 driver which means these address regions will
> > not be translated and will be excluded from iova allocations.
> >
> > ACPI CSRT vendor specific blobs are used to pass the reserve address
> > region info on these platforms. HiSilicon CSRT table contains a resource
> > group whose device Id matches with the SMMU device model id defined
> > in the IORT spec. This SMMU group will not have the optional Resource
> > group shared info.
> 
> Lorenzo made a point that it might be relatively straightforward to just
> follow the IORT mapping for the SMMU through to the ITS MADT entry and
> pull the ITS geometry out of that. It would certainly be nicer to have
> such a helper abstracted away in the IORT code than have to go parsing
> vendor-specific tables directly in the SMMU driver. I reckon it might be
> worth taking that idea a bit further to see how it looks.

Ok. John has already mentioned this idea in our off-list discussion and we 
have one implementation where we go through the IORT node ID mappings
array to find out the associated IORT ITS  node. It then iterates over the ACPI
MADT  GIC ITS table entries, , looking for a match for the GIC ITS id, and 
retrieves the base address of the matching ITS.

But as you said, it requires some helper from the IORT code , otherwise we 
will end up adding all the ACPI table parse code in SMMUv3. We will take
another look at this.

Thanks,
Shameer

> > typedef struct {
> >   UINT32  Length;
> >   UINT32  VendorId;
> >   UINT32  SubvendorId;
> >   UINT16  DeviceId;      ---->Set to IORT SMMU Model number.
> >   UINT16  SubdeviceId;
> >   UINT16  Revision;
> >   UINT8   Reserved[2];
> >   UINT32  SharedInfoLength;  ---->Set to zero.
> >   ACPI_CSRT_RESOURCE_DESCRIPTOR RsrcDesc;
> > } ACPI_CSRT_RESOURCE_GROUP;
> >
> > The resource descriptor associated with this group will have the vendor
> > specific section populated to represent the MSI region as below.
> >
> > typedef struct {
> >   UINT32  Length;
> >   UINT16  ResourceType;
> >   UINT16  ResourceSubtype;
> >   UINT32  UID;
> >   /* Vendor defined info */
> >   UINT64  Base;        -->Corresponding SMMU node base address.
> >   UINT64  MSIResvStart; -----> HW MSI reserve start address.
> >   UINT32  MSIResvLen; -------> HW MSI reserve len.
> > } ACPI_CSRT_RESOURCE_DESCRIPTOR;
> >
> > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  arch/arm64/Kconfig          | 10 ++++++
> >  drivers/iommu/arm-smmu-v3.c | 75
> +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 85 insertions(+)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 21d61ff..e7ebd97 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> 
> I think you misspelled Documentation/arm64/silicon-errata.txt there ;)
> 
> Robin.
> 
> > @@ -516,6 +516,16 @@ config HISILICON_ERRATUM_161010701
> >  	  CMD_PREFETCH_CFG. This will skip the prefetch cmd usage.
> >
> >  	  If unsure, say Y.
> > +config HISILICON_ERRATUM_161010801
> > +	bool "HiSilicon erratum 161010801: Reserve HW MSI regions"
> > +	default y
> > +	help
> > +	  On HiSilicon Hip06/Hip07 platforms, the SMMU v3 has to bypass the
> > +	  GIC ITS doorbel msi address regions. This will make sure that ITS
> > +	  doorbell address regions are reserved and excluded from iova
> > +	  allocations.
> > +
> > +	  If unsure, say Y.
> >
> >  endmenu
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index e7a8a50..a33e339 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -663,6 +663,73 @@ void erratum_skip_prefetch_cmd(struct
> arm_smmu_device *smmu, void *arg)
> >  	smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> >  }
> >
> > +#ifdef CONFIG_HISILICON_ERRATUM_161010801
> > +static void parse_hisi_csrt_msi(struct arm_smmu_device *smmu,
> > +		const struct acpi_iort_smmu_v3 *iort)
> > +
> > +{
> > +	struct acpi_csrt_group *grp, *end;
> > +	struct acpi_table_csrt *csrt;
> > +	acpi_status status;
> > +
> > +	status = acpi_get_table(ACPI_SIG_CSRT, 0,
> > +				(struct acpi_table_header **)&csrt);
> > +	if (ACPI_FAILURE(status)) {
> > +		dev_warn(smmu->dev, "HiSi CSRT table get failed: 0x%x\n",
> > +								status);
> > +		return;
> > +	}
> > +
> > +	grp = (struct acpi_csrt_group *)(csrt + 1);
> > +	end = (struct acpi_csrt_group *)((void *)csrt + csrt->header.length);
> > +
> > +	while (grp < end) {
> > +		if (grp->device_id == iort->model) {
> > +			/*
> > +			 * We don't have the optional shared info for this grp
> > +			 * and has only one resource descriptor with vendor
> > +			 * defined msi region for this group. Go straight to
> > +			 * vendor defined info.
> > +			 */
> > +			struct acpi_csrt_descriptor *desc =
> > +					(struct acpi_csrt_descriptor
> *)&grp[1];
> > +
> > +			/*
> > +			 * HiSilicon CSRT vendor info. First 8 bytes gives smmu
> > +			 * node base addr, next 8 bytes HW MSI reserve
> region
> > +			 * addr and the remaining 4 byte the len.
> > +			 */
> > +			void *vendor = &desc[1];
> > +			u64 base = (*(u64 *)vendor);
> > +
> > +			if (base == iort->base_address && smmu-
> >msi_region) {
> > +				/* Replace the default SW msi with HW msi
> */
> > +
> > +				smmu->msi_region->start =
> > +						*((u64 *)((u64 *)vendor+1));
> > +				smmu->msi_region->length =
> > +						*((u32 *)((u64 *)vendor+2));
> > +				smmu->msi_region->type =
> IOMMU_RESV_MSI;
> > +				dev_info(smmu->dev,
> > +					"HiSi msi addr 0x%pa size 0x%zx\n",
> > +					&smmu->msi_region->start,
> > +					smmu->msi_region->length);
> > +				return;
> > +			}
> > +		}
> > +
> > +		grp = (struct acpi_csrt_group *)((void *)grp + grp->length);
> > +	}
> > +
> > +}
> > +void erratum_hisi_resv_hw_msi(struct arm_smmu_device *smmu, void
> *arg)
> > +{
> > +	const struct acpi_iort_smmu_v3 *iort_smmu = arg;
> > +
> > +	parse_hisi_csrt_msi(smmu, iort_smmu);
> > +}
> > +#endif
> > +
> >  struct smmu_erratum_workaround {
> >  	enum smmu_erratum_match_type match_type;
> >  	const void *id;	/* Indicate the Erratum ID */
> > @@ -686,6 +753,14 @@ static const struct smmu_erratum_workaround
> smmu_workarounds[] = {
> >  	},
> >
> >  #endif
> > +#ifdef CONFIG_HISILICON_ERRATUM_161010801
> > +	{
> > +		.match_type = se_match_acpi_iort_model,
> > +		.id = (void *)ACPI_IORT_SMMU_HISILICON_HI161X,
> > +		.desc_str = "HiSilicon erratum 161010801",
> > +		.enable = erratum_hisi_resv_hw_msi,
> > +	},
> > +#endif
> >  	{
> >
> >  	},
> >

Re: [RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

[John Garry]

On 16/05/2017 15:03, Shameerali Kolothum Thodi wrote:
>> > Lorenzo made a point that it might be relatively straightforward to just
>> > follow the IORT mapping for the SMMU through to the ITS MADT entry and
>> > pull the ITS geometry out of that. It would certainly be nicer to have
>> > such a helper abstracted away in the IORT code than have to go parsing
>> > vendor-specific tables directly in the SMMU driver. I reckon it might be
>> > worth taking that idea a bit further to see how it looks.
> Ok. John has already mentioned this idea in our off-list discussion and we
> have one implementation where we go through the IORT node ID mappings
> array to find out the associated IORT ITS  node. It then iterates over the ACPI
> MADT  GIC ITS table entries, , looking for a match for the GIC ITS id, and
> retrieves the base address of the matching ITS.
>
> But as you said, it requires some helper from the IORT code , otherwise we
> will end up adding all the ACPI table parse code in SMMUv3. We will take
> another look at this.
>
> Thanks,
> Shameer
>

It could also be worth considering hard-coding the reserved address in 
the SMMUv3 driver for this model. This would not involve (more) reliance 
on CSRT or IORT driver. However, note that hip07 has multiple SMMUs, and 
we only need to enable the quirk for the SMMU in front the PCIe host 
controller.

John

BTW, FYI, hi161x is alias for hip07/06


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC v1 7/7] iommu/arm-smmu-v3: Enable ACPI based HiSilicon erratum 161010801

[John Garry]

On 16/05/2017 15:03, Shameerali Kolothum Thodi wrote:
>> > Lorenzo made a point that it might be relatively straightforward to just
>> > follow the IORT mapping for the SMMU through to the ITS MADT entry and
>> > pull the ITS geometry out of that. It would certainly be nicer to have
>> > such a helper abstracted away in the IORT code than have to go parsing
>> > vendor-specific tables directly in the SMMU driver. I reckon it might be
>> > worth taking that idea a bit further to see how it looks.
> Ok. John has already mentioned this idea in our off-list discussion and we
> have one implementation where we go through the IORT node ID mappings
> array to find out the associated IORT ITS  node. It then iterates over the ACPI
> MADT  GIC ITS table entries, , looking for a match for the GIC ITS id, and
> retrieves the base address of the matching ITS.
>
> But as you said, it requires some helper from the IORT code , otherwise we
> will end up adding all the ACPI table parse code in SMMUv3. We will take
> another look at this.
>
> Thanks,
> Shameer
>

It could also be worth considering hard-coding the reserved address in 
the SMMUv3 driver for this model. This would not involve (more) reliance 
on CSRT or IORT driver. However, note that hip07 has multiple SMMUs, and 
we only need to enable the quirk for the SMMU in front the PCIe host 
controller.

John

BTW, FYI, hi161x is alias for hip07/06