[PATCH v3 0/4] Add system mmu support for Armada-806

[PATCH v3 0/4] Add system mmu support for Armada-806

[Tomasz Nowicki]

There were already two versions of series to support SMMU for AP806,
including workaround for accessing ARM SMMU 64bit registers.
First [1] by Hanna Hawa and second [2] by Gregory CLEMENT.
Since it got stuck this is yet another try. I incorporated the V2 comments,
mainly by moving workaround code to arm-smmu-impl.c as requested.

For the record, AP-806 can't access SMMU registers with 64bit width,
this patches split the readq/writeq into two 32bit accesses instead
and update DT bindings.

The series was successfully tested on a vanilla v5.8-rc3 kernel and
Intel e1000e PCIe NIC. The same for platform devices like SATA and USB.

[1]: https://lkml.org/lkml/2018/10/15/373
[2]: https://lkml.org/lkml/2019/7/11/426

Hanna Hawa (1):
  iommu/arm-smmu: Workaround for Marvell Armada-AP806 SoC erratum
    #582743

Marcin Wojtas (1):
  arm64: dts: marvell: add SMMU support

Tomasz Nowicki (2):
  iommu/arm-smmu: Add SMMU ID2 register fixup hook
  dt-bindings: arm-smmu: add compatible string for Marvell Armada-AP806
    SMMU-500

 Documentation/arm64/silicon-errata.rst        |  3 ++
 .../devicetree/bindings/iommu/arm,smmu.yaml   |  5 ++
 arch/arm64/boot/dts/marvell/armada-8040.dtsi  | 36 +++++++++++++
 arch/arm64/boot/dts/marvell/armada-ap80x.dtsi | 17 ++++++
 drivers/iommu/arm-smmu-impl.c                 | 52 +++++++++++++++++++
 drivers/iommu/arm-smmu.c                      |  3 ++
 drivers/iommu/arm-smmu.h                      |  1 +
 7 files changed, 117 insertions(+)

-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 1/4] iommu/arm-smmu: Add SMMU ID2 register fixup hook

[Tomasz Nowicki]

We already have 'cfg_probe' hook which meant to override and apply
workarounds while probing ID registers. However, 'cfg_probe' is called
at the very end and therefore for some cases fixing up things becomes complex
or requires exporting of SMMU driver structures. Hence, seems it is better and
cleaner to do ID fixup right away. In preparation for adding Marvell
errata add an extra ID2 fixup hook.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/iommu/arm-smmu.c | 3 +++
 drivers/iommu/arm-smmu.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 243bc4cb2705..17c92e319754 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1857,6 +1857,9 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
 
 	/* ID2 */
 	id = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_ID2);
+	if (smmu->impl && smmu->impl->cfg_id2_fixup)
+		id = smmu->impl->cfg_id2_fixup(id);
+
 	size = arm_smmu_id_size_to_bits(FIELD_GET(ARM_SMMU_ID2_IAS, id));
 	smmu->ipa_size = size;
 
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index d172c024be61..f4c8bd7d0b34 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -382,6 +382,7 @@ struct arm_smmu_impl {
 	void (*write_reg64)(struct arm_smmu_device *smmu, int page, int offset,
 			    u64 val);
 	int (*cfg_probe)(struct arm_smmu_device *smmu);
+	u32 (*cfg_id2_fixup)(u32 id);
 	int (*reset)(struct arm_smmu_device *smmu);
 	int (*init_context)(struct arm_smmu_domain *smmu_domain);
 	void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 2/4] iommu/arm-smmu: Workaround for Marvell Armada-AP806 SoC erratum #582743

[Tomasz Nowicki]

From: Hanna Hawa <hannah@marvell.com>

Due to erratum #582743, the Marvell Armada-AP806 can't access 64bit to
ARM SMMUv2 registers.

Provide implementation relevant hooks:
- split the writeq/readq to two accesses of writel/readl.
- mask the MMU_IDR2.PTFSv8 fields to not use AArch64 format (but
only AARCH32_L) since with AArch64 format 32 bits access is not supported.

Note that separate writes/reads to 2 is not problem regards to
atomicity, because the driver use the readq/writeq while initialize
the SMMU, report for SMMU fault, and use spinlock in one
case (iova_to_phys).

Signed-off-by: Hanna Hawa <hannah@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 Documentation/arm64/silicon-errata.rst |  3 ++
 drivers/iommu/arm-smmu-impl.c          | 52 ++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index 936cf2a59ca4..157214d3abe1 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -125,6 +125,9 @@ stable kernels.
 | Cavium         | ThunderX2 Core  | #219            | CAVIUM_TX2_ERRATUM_219      |
 +----------------+-----------------+-----------------+-----------------------------+
 +----------------+-----------------+-----------------+-----------------------------+
+| Marvell        | ARM-MMU-500     | #582743         | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
++----------------+-----------------+-----------------+-----------------------------+
 | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585         |
 +----------------+-----------------+-----------------+-----------------------------+
 +----------------+-----------------+-----------------+-----------------------------+
diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index c75b9d957b70..c1fc5e1b8193 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -147,6 +147,53 @@ static const struct arm_smmu_impl arm_mmu500_impl = {
 	.reset = arm_mmu500_reset,
 };
 
+static u64 mrvl_mmu500_readq(struct arm_smmu_device *smmu, int page, int off)
+{
+	u64 val;
+
+	/*
+	 * Marvell Armada-AP806 erratum #582743.
+	 * Split all the readq to double readl
+	 */
+	val = (u64)readl_relaxed(arm_smmu_page(smmu, page) + off + 4) << 32;
+	val |= readl_relaxed(arm_smmu_page(smmu, page) + off);
+
+	return val;
+}
+
+static void mrvl_mmu500_writeq(struct arm_smmu_device *smmu, int page, int off,
+			       u64 val)
+{
+	/*
+	 * Marvell Armada-AP806 erratum #582743.
+	 * Split all the writeq to double writel
+	 */
+	writel_relaxed(upper_32_bits(val), arm_smmu_page(smmu, page) + off + 4);
+	writel_relaxed(lower_32_bits(val), arm_smmu_page(smmu, page) + off);
+}
+
+static u32 mrvl_mmu500_cfg_id2_fixup(u32 id)
+{
+
+	/*
+	 * Armada-AP806 erratum #582743.
+	 * Hide the SMMU_IDR2.PTFSv8 fields to sidestep the AArch64
+	 * formats altogether and allow using 32 bits access on the
+	 * interconnect.
+	 */
+	id &= ~(ARM_SMMU_ID2_PTFS_4K | ARM_SMMU_ID2_PTFS_16K |
+		ARM_SMMU_ID2_PTFS_64K);
+
+	return id;
+}
+
+static const struct arm_smmu_impl mrvl_mmu500_impl = {
+	.read_reg64 = mrvl_mmu500_readq,
+	.write_reg64 = mrvl_mmu500_writeq,
+	.cfg_id2_fixup = mrvl_mmu500_cfg_id2_fixup,
+	.reset = arm_mmu500_reset,
+};
+
 
 struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
 {
@@ -160,6 +207,11 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
 	 */
 	switch (smmu->model) {
 	case ARM_MMU500:
+		if (of_device_is_compatible(smmu->dev->of_node,
+					    "marvell,ap806-smmu-500")) {
+			smmu->impl = &mrvl_mmu500_impl;
+			return smmu;
+		}
 		smmu->impl = &arm_mmu500_impl;
 		break;
 	case CAVIUM_SMMUV2:
-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 3/4] dt-bindings: arm-smmu: add compatible string for Marvell Armada-AP806 SMMU-500

[Tomasz Nowicki]

Add specific compatible string for Marvell usage due to errata of
accessing 64bits registers of ARM SMMU, in AP806.

AP806 SoC uses the generic ARM-MMU500, and there's no specific
implementation of Marvell, this compatible is used for errata only.

Signed-off-by: Hanna Hawa <hannah@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index d7ceb4c34423..7beca9c00b12 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -38,6 +38,11 @@ properties:
               - qcom,sc7180-smmu-500
               - qcom,sdm845-smmu-500
           - const: arm,mmu-500
+      - description: Marvell SoCs implementing "arm,mmu-500"
+        items:
+          - enum:
+              - marvell,ap806-smmu-500
+          - const: arm,mmu-500
       - items:
           - const: arm,mmu-500
           - const: arm,smmu-v2
-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 4/4] arm64: dts: marvell: add SMMU support

[Tomasz Nowicki]

From: Marcin Wojtas <mw@semihalf.com>

Add IOMMU node for Marvell AP806 based SoCs together with platform
and PCI device Stream ID mapping.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 arch/arm64/boot/dts/marvell/armada-8040.dtsi  | 36 +++++++++++++++++++
 arch/arm64/boot/dts/marvell/armada-ap80x.dtsi | 17 +++++++++
 2 files changed, 53 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-8040.dtsi b/arch/arm64/boot/dts/marvell/armada-8040.dtsi
index 7699b19224c2..25c1df709f72 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-8040.dtsi
@@ -23,3 +23,39 @@
 &cp0_rtc {
 	status = "disabled";
 };
+
+&cp0_usb3_0 {
+	iommus = <&smmu 0x440>;
+};
+
+&cp0_usb3_1 {
+	iommus = <&smmu 0x441>;
+};
+
+&cp0_sata0 {
+	iommus = <&smmu 0x444>;
+};
+
+&cp0_sdhci0 {
+	iommus = <&smmu 0x445>;
+};
+
+&cp1_sata0 {
+	iommus = <&smmu 0x454>;
+};
+
+&cp1_usb3_0 {
+	iommus = <&smmu 0x450>;
+};
+
+&cp1_usb3_1 {
+	iommus = <&smmu 0x451>;
+};
+
+&cp0_pcie0 {
+	iommu-map =
+		<0x0   &smmu 0x480 0x20>,
+		<0x100 &smmu 0x4a0 0x20>,
+		<0x200 &smmu 0x4c0 0x20>;
+	iommu-map-mask = <0x031f>;
+};
diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
index 7f9b9a647717..ded8b8082d79 100644
--- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
@@ -56,6 +56,23 @@
 			compatible = "simple-bus";
 			ranges = <0x0 0x0 0xf0000000 0x1000000>;
 
+			smmu: iommu@5000000 {
+				compatible = "marvell,ap806-smmu-500", "arm,mmu-500";
+				reg = <0x100000 0x100000>;
+				dma-coherent;
+				#iommu-cells = <1>;
+				#global-interrupts = <1>;
+				interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
 			gic: interrupt-controller@210000 {
 				compatible = "arm,gic-400";
 				#interrupt-cells = <3>;
-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 1/4] iommu/arm-smmu: Add SMMU ID2 register fixup hook

[Robin Murphy]

On 2020-07-02 21:16, Tomasz Nowicki wrote:
> We already have 'cfg_probe' hook which meant to override and apply
> workarounds while probing ID registers. However, 'cfg_probe' is called
> at the very end and therefore for some cases fixing up things becomes complex
> or requires exporting of SMMU driver structures. Hence, seems it is better and
> cleaner to do ID fixup right away. In preparation for adding Marvell
> errata add an extra ID2 fixup hook.

Hmm, the intent of ->cfg_probe was very much to give impl a chance to 
adjust the detected features before we start consuming them, with this 
exact case in mind. Since the Cavium quirk isn't actually doing that - 
it just needs to run *anywhere* in the whole probe process - I'm under 
no illusion that I put the hook in exactly the right place first time 
around ;)

The diff below should be more on the mark...

Robin.

----->8-----
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 243bc4cb2705..884ffca5b1eb 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1891,6 +1891,9 @@ static int arm_smmu_device_cfg_probe(struct 
arm_smmu_device *smmu)
  			smmu->features |= ARM_SMMU_FEAT_FMT_AARCH64_64K;
  	}

+	if (smmu->impl && smmu->impl->cfg_probe)
+		return smmu->impl->cfg_probe(smmu);
+
  	/* Now we've corralled the various formats, what'll it do? */
  	if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_S)
  		smmu->pgsize_bitmap |= SZ_4K | SZ_64K | SZ_1M | SZ_16M;
@@ -1909,7 +1912,6 @@ static int arm_smmu_device_cfg_probe(struct 
arm_smmu_device *smmu)
  	dev_notice(smmu->dev, "\tSupported page sizes: 0x%08lx\n",
  		   smmu->pgsize_bitmap);

-
  	if (smmu->features & ARM_SMMU_FEAT_TRANS_S1)
  		dev_notice(smmu->dev, "\tStage-1: %lu-bit VA -> %lu-bit IPA\n",
  			   smmu->va_size, smmu->ipa_size);
@@ -1918,9 +1920,6 @@ static int arm_smmu_device_cfg_probe(struct 
arm_smmu_device *smmu)
  		dev_notice(smmu->dev, "\tStage-2: %lu-bit IPA -> %lu-bit PA\n",
  			   smmu->ipa_size, smmu->pa_size);

-	if (smmu->impl && smmu->impl->cfg_probe)
-		return smmu->impl->cfg_probe(smmu);
-
  	return 0;
  }

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 2/4] iommu/arm-smmu: Workaround for Marvell Armada-AP806 SoC erratum #582743

[Robin Murphy]

On 2020-07-02 21:16, Tomasz Nowicki wrote:
> From: Hanna Hawa <hannah@marvell.com>
> 
> Due to erratum #582743, the Marvell Armada-AP806 can't access 64bit to
> ARM SMMUv2 registers.
> 
> Provide implementation relevant hooks:
> - split the writeq/readq to two accesses of writel/readl.
> - mask the MMU_IDR2.PTFSv8 fields to not use AArch64 format (but
> only AARCH32_L) since with AArch64 format 32 bits access is not supported.
> 
> Note that separate writes/reads to 2 is not problem regards to
> atomicity, because the driver use the readq/writeq while initialize
> the SMMU, report for SMMU fault, and use spinlock in one
> case (iova_to_phys).

The comment about the spinlock seems to be out of date, and TBH that 
whole sentence is a bit unclear - how about something like:

"Note that most 64-bit registers like TTBRn can be accessed as two 
32-bit halves without issue, and AArch32 format ensures that the 
register writes which must be atomic (for TLBI etc.) need only be 32-bit."

> Signed-off-by: Hanna Hawa <hannah@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>   Documentation/arm64/silicon-errata.rst |  3 ++
>   drivers/iommu/arm-smmu-impl.c          | 52 ++++++++++++++++++++++++++
>   2 files changed, 55 insertions(+)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index 936cf2a59ca4..157214d3abe1 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -125,6 +125,9 @@ stable kernels.
>   | Cavium         | ThunderX2 Core  | #219            | CAVIUM_TX2_ERRATUM_219      |
>   +----------------+-----------------+-----------------+-----------------------------+
>   +----------------+-----------------+-----------------+-----------------------------+
> +| Marvell        | ARM-MMU-500     | #582743         | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
> ++----------------+-----------------+-----------------+-----------------------------+
>   | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585         |
>   +----------------+-----------------+-----------------+-----------------------------+
>   +----------------+-----------------+-----------------+-----------------------------+
> diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
> index c75b9d957b70..c1fc5e1b8193 100644
> --- a/drivers/iommu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm-smmu-impl.c
> @@ -147,6 +147,53 @@ static const struct arm_smmu_impl arm_mmu500_impl = {
>   	.reset = arm_mmu500_reset,
>   };
>   
> +static u64 mrvl_mmu500_readq(struct arm_smmu_device *smmu, int page, int off)
> +{
> +	u64 val;
> +
> +	/*
> +	 * Marvell Armada-AP806 erratum #582743.
> +	 * Split all the readq to double readl
> +	 */
> +	val = (u64)readl_relaxed(arm_smmu_page(smmu, page) + off + 4) << 32;
> +	val |= readl_relaxed(arm_smmu_page(smmu, page) + off);

Even though io-64-nonatomic-hi-lo.h doesn't override readq() etc. for 
64-bit builds, you can still use hi_lo_readq_relaxed() directly.

> +
> +	return val;
> +}
> +
> +static void mrvl_mmu500_writeq(struct arm_smmu_device *smmu, int page, int off,
> +			       u64 val)
> +{
> +	/*
> +	 * Marvell Armada-AP806 erratum #582743.
> +	 * Split all the writeq to double writel
> +	 */
> +	writel_relaxed(upper_32_bits(val), arm_smmu_page(smmu, page) + off + 4);
> +	writel_relaxed(lower_32_bits(val), arm_smmu_page(smmu, page) + off);

Similarly, hi_lo_writeq_relaxed().

> +}
> +
> +static u32 mrvl_mmu500_cfg_id2_fixup(u32 id)
> +{
> +
> +	/*
> +	 * Armada-AP806 erratum #582743.
> +	 * Hide the SMMU_IDR2.PTFSv8 fields to sidestep the AArch64
> +	 * formats altogether and allow using 32 bits access on the
> +	 * interconnect.
> +	 */
> +	id &= ~(ARM_SMMU_ID2_PTFS_4K | ARM_SMMU_ID2_PTFS_16K |
> +		ARM_SMMU_ID2_PTFS_64K);
> +
> +	return id;
> +}
> +
> +static const struct arm_smmu_impl mrvl_mmu500_impl = {
> +	.read_reg64 = mrvl_mmu500_readq,
> +	.write_reg64 = mrvl_mmu500_writeq,
> +	.cfg_id2_fixup = mrvl_mmu500_cfg_id2_fixup,
> +	.reset = arm_mmu500_reset,
> +};
> +
>   
>   struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
>   {
> @@ -160,6 +207,11 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
>   	 */
>   	switch (smmu->model) {
>   	case ARM_MMU500:
> +		if (of_device_is_compatible(smmu->dev->of_node,

Nit: there's a local "np" variable now.

> +					    "marvell,ap806-smmu-500")) {
> +			smmu->impl = &mrvl_mmu500_impl;
> +			return smmu;
> +		}

Please put this with the other integration checks below the switch 
statement. Yes, it means we'll end up assigning smmu->impl twice for 
this particular case, but that's the intended pattern.

Robin.

>   		smmu->impl = &arm_mmu500_impl;
>   		break;
>   	case CAVIUM_SMMUV2:
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 3/4] dt-bindings: arm-smmu: add compatible string for Marvell Armada-AP806 SMMU-500

[Robin Murphy]

On 2020-07-02 21:16, Tomasz Nowicki wrote:
> Add specific compatible string for Marvell usage due to errata of
> accessing 64bits registers of ARM SMMU, in AP806.
> 
> AP806 SoC uses the generic ARM-MMU500, and there's no specific
> implementation of Marvell, this compatible is used for errata only.
> 
> Signed-off-by: Hanna Hawa <hannah@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>   Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> index d7ceb4c34423..7beca9c00b12 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> @@ -38,6 +38,11 @@ properties:
>                 - qcom,sc7180-smmu-500
>                 - qcom,sdm845-smmu-500
>             - const: arm,mmu-500
> +      - description: Marvell SoCs implementing "arm,mmu-500"
> +        items:
> +          - enum:
> +              - marvell,ap806-smmu-500

Isn't a single-valued enum just a constant? :P

Robin.

> +          - const: arm,mmu-500
>         - items:
>             - const: arm,mmu-500
>             - const: arm,smmu-v2
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 4/4] arm64: dts: marvell: add SMMU support

[Robin Murphy]

On 2020-07-02 21:16, Tomasz Nowicki wrote:
> From: Marcin Wojtas <mw@semihalf.com>
> 
> Add IOMMU node for Marvell AP806 based SoCs together with platform
> and PCI device Stream ID mapping.
> 
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>   arch/arm64/boot/dts/marvell/armada-8040.dtsi  | 36 +++++++++++++++++++
>   arch/arm64/boot/dts/marvell/armada-ap80x.dtsi | 17 +++++++++
>   2 files changed, 53 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040.dtsi b/arch/arm64/boot/dts/marvell/armada-8040.dtsi
> index 7699b19224c2..25c1df709f72 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-8040.dtsi
> @@ -23,3 +23,39 @@
>   &cp0_rtc {
>   	status = "disabled";
>   };
> +
> +&cp0_usb3_0 {
> +	iommus = <&smmu 0x440>;
> +};
> +
> +&cp0_usb3_1 {
> +	iommus = <&smmu 0x441>;
> +};
> +
> +&cp0_sata0 {
> +	iommus = <&smmu 0x444>;
> +};
> +
> +&cp0_sdhci0 {
> +	iommus = <&smmu 0x445>;
> +};
> +
> +&cp1_sata0 {
> +	iommus = <&smmu 0x454>;
> +};
> +
> +&cp1_usb3_0 {
> +	iommus = <&smmu 0x450>;
> +};
> +
> +&cp1_usb3_1 {
> +	iommus = <&smmu 0x451>;
> +};
> +
> +&cp0_pcie0 {
> +	iommu-map =
> +		<0x0   &smmu 0x480 0x20>,
> +		<0x100 &smmu 0x4a0 0x20>,
> +		<0x200 &smmu 0x4c0 0x20>;
> +	iommu-map-mask = <0x031f>;

Nice! I do like a good compressed mapping :D

> +};
> diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> index 7f9b9a647717..ded8b8082d79 100644
> --- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> @@ -56,6 +56,23 @@
>   			compatible = "simple-bus";
>   			ranges = <0x0 0x0 0xf0000000 0x1000000>;
>   
> +			smmu: iommu@5000000 {
> +				compatible = "marvell,ap806-smmu-500", "arm,mmu-500";
> +				reg = <0x100000 0x100000>;
> +				dma-coherent;
> +				#iommu-cells = <1>;
> +				#global-interrupts = <1>;
> +				interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> +					     <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;

I'd recommend you have the node disabled by default here, then 
explicitly enable it in armada-8040.dtsi where you add the Stream IDs. 
Otherwise it will also end up enabled for 8020, 70x0, etc. where 
disable_bypass will then catastrophically break everything.

Robin.

> +			};
> +
>   			gic: interrupt-controller@210000 {
>   				compatible = "arm,gic-400";
>   				#interrupt-cells = <3>;
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 1/4] iommu/arm-smmu: Add SMMU ID2 register fixup hook

[Tomasz Nowicki]

On 03.07.2020 10:24, Robin Murphy wrote:
> On 2020-07-02 21:16, Tomasz Nowicki wrote:
>> We already have 'cfg_probe' hook which meant to override and apply
>> workarounds while probing ID registers. However, 'cfg_probe' is called
>> at the very end and therefore for some cases fixing up things becomes 
>> complex
>> or requires exporting of SMMU driver structures. Hence, seems it is 
>> better and
>> cleaner to do ID fixup right away. In preparation for adding Marvell
>> errata add an extra ID2 fixup hook.
> 
> Hmm, the intent of ->cfg_probe was very much to give impl a chance to 
> adjust the detected features before we start consuming them, with this 
> exact case in mind. Since the Cavium quirk isn't actually doing that - 
> it just needs to run *anywhere* in the whole probe process - I'm under 
> no illusion that I put the hook in exactly the right place first time 
> around ;)
> 
> The diff below should be more on the mark...
> 
> Robin.
> 
> ----->8-----
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 243bc4cb2705..884ffca5b1eb 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1891,6 +1891,9 @@ static int arm_smmu_device_cfg_probe(struct 
> arm_smmu_device *smmu)
>               smmu->features |= ARM_SMMU_FEAT_FMT_AARCH64_64K;
>       }
> 
> +    if (smmu->impl && smmu->impl->cfg_probe)
> +        return smmu->impl->cfg_probe(smmu);
> +
>       /* Now we've corralled the various formats, what'll it do? */
>       if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_S)
>           smmu->pgsize_bitmap |= SZ_4K | SZ_64K | SZ_1M | SZ_16M;
> @@ -1909,7 +1912,6 @@ static int arm_smmu_device_cfg_probe(struct 
> arm_smmu_device *smmu)
>       dev_notice(smmu->dev, "\tSupported page sizes: 0x%08lx\n",
>              smmu->pgsize_bitmap);
> 
> -
>       if (smmu->features & ARM_SMMU_FEAT_TRANS_S1)
>           dev_notice(smmu->dev, "\tStage-1: %lu-bit VA -> %lu-bit IPA\n",
>                  smmu->va_size, smmu->ipa_size);
> @@ -1918,9 +1920,6 @@ static int arm_smmu_device_cfg_probe(struct 
> arm_smmu_device *smmu)
>           dev_notice(smmu->dev, "\tStage-2: %lu-bit IPA -> %lu-bit PA\n",
>                  smmu->ipa_size, smmu->pa_size);
> 
> -    if (smmu->impl && smmu->impl->cfg_probe)
> -        return smmu->impl->cfg_probe(smmu);
> -
>       return 0;
>   }
> 

I was under impression that ->cfg_probe was meant for Cavium alike 
errata (position independent). Then I will move ->cfg_probe as you 
suggested. I prefer not to add yet another impl hook too :)

Thanks,
Tomasz
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 3/4] dt-bindings: arm-smmu: add compatible string for Marvell Armada-AP806 SMMU-500

[Tomasz Nowicki]

On 03.07.2020 11:05, Robin Murphy wrote:
> On 2020-07-02 21:16, Tomasz Nowicki wrote:
>> Add specific compatible string for Marvell usage due to errata of
>> accessing 64bits registers of ARM SMMU, in AP806.
>>
>> AP806 SoC uses the generic ARM-MMU500, and there's no specific
>> implementation of Marvell, this compatible is used for errata only.
>>
>> Signed-off-by: Hanna Hawa <hannah@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>   Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml 
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> index d7ceb4c34423..7beca9c00b12 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> @@ -38,6 +38,11 @@ properties:
>>                 - qcom,sc7180-smmu-500
>>                 - qcom,sdm845-smmu-500
>>             - const: arm,mmu-500
>> +      - description: Marvell SoCs implementing "arm,mmu-500"
>> +        items:
>> +          - enum:
>> +              - marvell,ap806-smmu-500
> 
> Isn't a single-valued enum just a constant? :P

That's how copy-paste engineering ends up :)

Thanks,
Tomasz
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 4/4] arm64: dts: marvell: add SMMU support

[Tomasz Nowicki]

On 03.07.2020 11:16, Robin Murphy wrote:
> On 2020-07-02 21:16, Tomasz Nowicki wrote:
>> From: Marcin Wojtas <mw@semihalf.com>
>>
>> Add IOMMU node for Marvell AP806 based SoCs together with platform
>> and PCI device Stream ID mapping.
>>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>   arch/arm64/boot/dts/marvell/armada-8040.dtsi  | 36 +++++++++++++++++++
>>   arch/arm64/boot/dts/marvell/armada-ap80x.dtsi | 17 +++++++++
>>   2 files changed, 53 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/marvell/armada-8040.dtsi 
>> b/arch/arm64/boot/dts/marvell/armada-8040.dtsi
>> index 7699b19224c2..25c1df709f72 100644
>> --- a/arch/arm64/boot/dts/marvell/armada-8040.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/armada-8040.dtsi
>> @@ -23,3 +23,39 @@
>>   &cp0_rtc {
>>       status = "disabled";
>>   };
>> +
>> +&cp0_usb3_0 {
>> +    iommus = <&smmu 0x440>;
>> +};
>> +
>> +&cp0_usb3_1 {
>> +    iommus = <&smmu 0x441>;
>> +};
>> +
>> +&cp0_sata0 {
>> +    iommus = <&smmu 0x444>;
>> +};
>> +
>> +&cp0_sdhci0 {
>> +    iommus = <&smmu 0x445>;
>> +};
>> +
>> +&cp1_sata0 {
>> +    iommus = <&smmu 0x454>;
>> +};
>> +
>> +&cp1_usb3_0 {
>> +    iommus = <&smmu 0x450>;
>> +};
>> +
>> +&cp1_usb3_1 {
>> +    iommus = <&smmu 0x451>;
>> +};
>> +
>> +&cp0_pcie0 {
>> +    iommu-map =
>> +        <0x0   &smmu 0x480 0x20>,
>> +        <0x100 &smmu 0x4a0 0x20>,
>> +        <0x200 &smmu 0x4c0 0x20>;
>> +    iommu-map-mask = <0x031f>;
> 
> Nice! I do like a good compressed mapping :D
> 
>> +};
>> diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi 
>> b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
>> index 7f9b9a647717..ded8b8082d79 100644
>> --- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
>> @@ -56,6 +56,23 @@
>>               compatible = "simple-bus";
>>               ranges = <0x0 0x0 0xf0000000 0x1000000>;
>> +            smmu: iommu@5000000 {
>> +                compatible = "marvell,ap806-smmu-500", "arm,mmu-500";
>> +                reg = <0x100000 0x100000>;
>> +                dma-coherent;
>> +                #iommu-cells = <1>;
>> +                #global-interrupts = <1>;
>> +                interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
>> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
> 
> I'd recommend you have the node disabled by default here, then 
> explicitly enable it in armada-8040.dtsi where you add the Stream IDs. 
> Otherwise it will also end up enabled for 8020, 70x0, etc. where 
> disable_bypass will then catastrophically break everything.
> 

Good point! I will fix this.

Thanks,
Tomasz
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 4/4] arm64: dts: marvell: add SMMU support

[Marcin Wojtas]

Hi Tomasz,


pt., 3 lip 2020 o 11:33 Tomasz Nowicki <tn@semihalf.com> napisał(a):
>
> On 03.07.2020 11:16, Robin Murphy wrote:
> > On 2020-07-02 21:16, Tomasz Nowicki wrote:
> >> From: Marcin Wojtas <mw@semihalf.com>
> >>
> >> Add IOMMU node for Marvell AP806 based SoCs together with platform
> >> and PCI device Stream ID mapping.
> >>
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> >> ---
> >>   arch/arm64/boot/dts/marvell/armada-8040.dtsi  | 36 +++++++++++++++++++
> >>   arch/arm64/boot/dts/marvell/armada-ap80x.dtsi | 17 +++++++++
> >>   2 files changed, 53 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/marvell/armada-8040.dtsi
> >> b/arch/arm64/boot/dts/marvell/armada-8040.dtsi
> >> index 7699b19224c2..25c1df709f72 100644
> >> --- a/arch/arm64/boot/dts/marvell/armada-8040.dtsi
> >> +++ b/arch/arm64/boot/dts/marvell/armada-8040.dtsi
> >> @@ -23,3 +23,39 @@
> >>   &cp0_rtc {
> >>       status = "disabled";
> >>   };
> >> +
> >> +&cp0_usb3_0 {
> >> +    iommus = <&smmu 0x440>;
> >> +};
> >> +
> >> +&cp0_usb3_1 {
> >> +    iommus = <&smmu 0x441>;
> >> +};
> >> +
> >> +&cp0_sata0 {
> >> +    iommus = <&smmu 0x444>;
> >> +};
> >> +
> >> +&cp0_sdhci0 {
> >> +    iommus = <&smmu 0x445>;
> >> +};
> >> +
> >> +&cp1_sata0 {
> >> +    iommus = <&smmu 0x454>;
> >> +};
> >> +
> >> +&cp1_usb3_0 {
> >> +    iommus = <&smmu 0x450>;
> >> +};
> >> +
> >> +&cp1_usb3_1 {
> >> +    iommus = <&smmu 0x451>;
> >> +};
> >> +
> >> +&cp0_pcie0 {
> >> +    iommu-map =
> >> +        <0x0   &smmu 0x480 0x20>,
> >> +        <0x100 &smmu 0x4a0 0x20>,
> >> +        <0x200 &smmu 0x4c0 0x20>;
> >> +    iommu-map-mask = <0x031f>;
> >
> > Nice! I do like a good compressed mapping :D
> >
> >> +};
> >> diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> >> b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> >> index 7f9b9a647717..ded8b8082d79 100644
> >> --- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> >> +++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> >> @@ -56,6 +56,23 @@
> >>               compatible = "simple-bus";
> >>               ranges = <0x0 0x0 0xf0000000 0x1000000>;
> >> +            smmu: iommu@5000000 {
> >> +                compatible = "marvell,ap806-smmu-500", "arm,mmu-500";
> >> +                reg = <0x100000 0x100000>;
> >> +                dma-coherent;
> >> +                #iommu-cells = <1>;
> >> +                #global-interrupts = <1>;
> >> +                interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> >> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> >> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> >> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> >> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> >> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> >> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> >> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
> >> +                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
> >
> > I'd recommend you have the node disabled by default here, then
> > explicitly enable it in armada-8040.dtsi where you add the Stream IDs.
> > Otherwise it will also end up enabled for 8020, 70x0, etc. where
> > disable_bypass will then catastrophically break everything.
> >
>
> Good point! I will fix this.
>

In addition to above, I think it is worth defining the stream ID's for
Armada 7040 and CN913x SoCs.

Best regards,
Marcin
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 2/4] iommu/arm-smmu: Workaround for Marvell Armada-AP806 SoC erratum #582743

[Tomasz Nowicki]

On 03.07.2020 11:03, Robin Murphy wrote:
> On 2020-07-02 21:16, Tomasz Nowicki wrote:
>> From: Hanna Hawa <hannah@marvell.com>
>>
>> Due to erratum #582743, the Marvell Armada-AP806 can't access 64bit to
>> ARM SMMUv2 registers.
>>
>> Provide implementation relevant hooks:
>> - split the writeq/readq to two accesses of writel/readl.
>> - mask the MMU_IDR2.PTFSv8 fields to not use AArch64 format (but
>> only AARCH32_L) since with AArch64 format 32 bits access is not 
>> supported.
>>
>> Note that separate writes/reads to 2 is not problem regards to
>> atomicity, because the driver use the readq/writeq while initialize
>> the SMMU, report for SMMU fault, and use spinlock in one
>> case (iova_to_phys).
> 
> The comment about the spinlock seems to be out of date, and TBH that 
> whole sentence is a bit unclear - how about something like:
> 
> "Note that most 64-bit registers like TTBRn can be accessed as two 
> 32-bit halves without issue, and AArch32 format ensures that the 
> register writes which must be atomic (for TLBI etc.) need only be 32-bit."
> 
>> Signed-off-by: Hanna Hawa <hannah@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>   Documentation/arm64/silicon-errata.rst |  3 ++
>>   drivers/iommu/arm-smmu-impl.c          | 52 ++++++++++++++++++++++++++
>>   2 files changed, 55 insertions(+)
>>
>> diff --git a/Documentation/arm64/silicon-errata.rst 
>> b/Documentation/arm64/silicon-errata.rst
>> index 936cf2a59ca4..157214d3abe1 100644
>> --- a/Documentation/arm64/silicon-errata.rst
>> +++ b/Documentation/arm64/silicon-errata.rst
>> @@ -125,6 +125,9 @@ stable kernels.
>>   | Cavium         | ThunderX2 Core  | #219            | 
>> CAVIUM_TX2_ERRATUM_219      |
>>   
>> +----------------+-----------------+-----------------+-----------------------------+ 
>>
>>   
>> +----------------+-----------------+-----------------+-----------------------------+ 
>>
>> +| Marvell        | ARM-MMU-500     | #582743         | 
>> N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+ 
>>
>> ++----------------+-----------------+-----------------+-----------------------------+ 
>>
>>   | Freescale/NXP  | LS2080A/LS1043A | A-008585        | 
>> FSL_ERRATUM_A008585         |
>>   
>> +----------------+-----------------+-----------------+-----------------------------+ 
>>
>>   
>> +----------------+-----------------+-----------------+-----------------------------+ 
>>
>> diff --git a/drivers/iommu/arm-smmu-impl.c 
>> b/drivers/iommu/arm-smmu-impl.c
>> index c75b9d957b70..c1fc5e1b8193 100644
>> --- a/drivers/iommu/arm-smmu-impl.c
>> +++ b/drivers/iommu/arm-smmu-impl.c
>> @@ -147,6 +147,53 @@ static const struct arm_smmu_impl arm_mmu500_impl 
>> = {
>>       .reset = arm_mmu500_reset,
>>   };
>> +static u64 mrvl_mmu500_readq(struct arm_smmu_device *smmu, int page, 
>> int off)
>> +{
>> +    u64 val;
>> +
>> +    /*
>> +     * Marvell Armada-AP806 erratum #582743.
>> +     * Split all the readq to double readl
>> +     */
>> +    val = (u64)readl_relaxed(arm_smmu_page(smmu, page) + off + 4) << 32;
>> +    val |= readl_relaxed(arm_smmu_page(smmu, page) + off);
> 
> Even though io-64-nonatomic-hi-lo.h doesn't override readq() etc. for 
> 64-bit builds, you can still use hi_lo_readq_relaxed() directly.
> 
>> +
>> +    return val;
>> +}
>> +
>> +static void mrvl_mmu500_writeq(struct arm_smmu_device *smmu, int 
>> page, int off,
>> +                   u64 val)
>> +{
>> +    /*
>> +     * Marvell Armada-AP806 erratum #582743.
>> +     * Split all the writeq to double writel
>> +     */
>> +    writel_relaxed(upper_32_bits(val), arm_smmu_page(smmu, page) + 
>> off + 4);
>> +    writel_relaxed(lower_32_bits(val), arm_smmu_page(smmu, page) + off);
> 
> Similarly, hi_lo_writeq_relaxed().
> 
>> +}
>> +
>> +static u32 mrvl_mmu500_cfg_id2_fixup(u32 id)
>> +{
>> +
>> +    /*
>> +     * Armada-AP806 erratum #582743.
>> +     * Hide the SMMU_IDR2.PTFSv8 fields to sidestep the AArch64
>> +     * formats altogether and allow using 32 bits access on the
>> +     * interconnect.
>> +     */
>> +    id &= ~(ARM_SMMU_ID2_PTFS_4K | ARM_SMMU_ID2_PTFS_16K |
>> +        ARM_SMMU_ID2_PTFS_64K);
>> +
>> +    return id;
>> +}
>> +
>> +static const struct arm_smmu_impl mrvl_mmu500_impl = {
>> +    .read_reg64 = mrvl_mmu500_readq,
>> +    .write_reg64 = mrvl_mmu500_writeq,
>> +    .cfg_id2_fixup = mrvl_mmu500_cfg_id2_fixup,
>> +    .reset = arm_mmu500_reset,
>> +};
>> +
>>   struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device 
>> *smmu)
>>   {
>> @@ -160,6 +207,11 @@ struct arm_smmu_device *arm_smmu_impl_init(struct 
>> arm_smmu_device *smmu)
>>        */
>>       switch (smmu->model) {
>>       case ARM_MMU500:
>> +        if (of_device_is_compatible(smmu->dev->of_node,
> 
> Nit: there's a local "np" variable now.
> 
>> +                        "marvell,ap806-smmu-500")) {
>> +            smmu->impl = &mrvl_mmu500_impl;
>> +            return smmu;
>> +        }
> 
> Please put this with the other integration checks below the switch 
> statement. Yes, it means we'll end up assigning smmu->impl twice for 
> this particular case, but that's the intended pattern.
> 

Thanks, all above comments do make sense and will be fixed in next spin.

Thanks,
Tomasz
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 3/4] dt-bindings: arm-smmu: add compatible string for Marvell Armada-AP806 SMMU-500

[Rob Herring]

On Fri, Jul 03, 2020 at 11:26:32AM +0200, Tomasz Nowicki wrote:
> On 03.07.2020 11:05, Robin Murphy wrote:
> > On 2020-07-02 21:16, Tomasz Nowicki wrote:
> > > Add specific compatible string for Marvell usage due to errata of
> > > accessing 64bits registers of ARM SMMU, in AP806.
> > > 
> > > AP806 SoC uses the generic ARM-MMU500, and there's no specific
> > > implementation of Marvell, this compatible is used for errata only.
> > > 
> > > Signed-off-by: Hanna Hawa <hannah@marvell.com>
> > > Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> > > Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> > > ---
> > >   Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > index d7ceb4c34423..7beca9c00b12 100644
> > > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > @@ -38,6 +38,11 @@ properties:
> > >                 - qcom,sc7180-smmu-500
> > >                 - qcom,sdm845-smmu-500
> > >             - const: arm,mmu-500
> > > +      - description: Marvell SoCs implementing "arm,mmu-500"
> > > +        items:
> > > +          - enum:
> > > +              - marvell,ap806-smmu-500
> > 
> > Isn't a single-valued enum just a constant? :P
> 
> That's how copy-paste engineering ends up :)

It's fine like this if you expect more SoCs to be added.

Either way,

Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/4] Add system mmu support for Armada-806

[Will Deacon]

Hi Tomasz,

On Thu, Jul 02, 2020 at 10:16:29PM +0200, Tomasz Nowicki wrote:
> There were already two versions of series to support SMMU for AP806,
> including workaround for accessing ARM SMMU 64bit registers.
> First [1] by Hanna Hawa and second [2] by Gregory CLEMENT.
> Since it got stuck this is yet another try. I incorporated the V2 comments,
> mainly by moving workaround code to arm-smmu-impl.c as requested.
> 
> For the record, AP-806 can't access SMMU registers with 64bit width,
> this patches split the readq/writeq into two 32bit accesses instead
> and update DT bindings.
> 
> The series was successfully tested on a vanilla v5.8-rc3 kernel and
> Intel e1000e PCIe NIC. The same for platform devices like SATA and USB.
> 
> [1]: https://lkml.org/lkml/2018/10/15/373
> [2]: https://lkml.org/lkml/2019/7/11/426

Do you have a v4 of this series? It looks like there were a few comments
left to address, but with that I can pick it up for 5.9.

Cheers,

Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/4] Add system mmu support for Armada-806

[Tomasz Nowicki]

Hi Will,

On 14.07.2020 10:19, Will Deacon wrote:
> Hi Tomasz,
> 
> On Thu, Jul 02, 2020 at 10:16:29PM +0200, Tomasz Nowicki wrote:
>> There were already two versions of series to support SMMU for AP806,
>> including workaround for accessing ARM SMMU 64bit registers.
>> First [1] by Hanna Hawa and second [2] by Gregory CLEMENT.
>> Since it got stuck this is yet another try. I incorporated the V2 comments,
>> mainly by moving workaround code to arm-smmu-impl.c as requested.
>>
>> For the record, AP-806 can't access SMMU registers with 64bit width,
>> this patches split the readq/writeq into two 32bit accesses instead
>> and update DT bindings.
>>
>> The series was successfully tested on a vanilla v5.8-rc3 kernel and
>> Intel e1000e PCIe NIC. The same for platform devices like SATA and USB.
>>
>> [1]: https://lkml.org/lkml/2018/10/15/373
>> [2]: https://lkml.org/lkml/2019/7/11/426
> 
> Do you have a v4 of this series? It looks like there were a few comments
> left to address, but with that I can pick it up for 5.9.

Yes, I will send it out today.

Thanks,
Tomasz
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu