Skip to contentSkip to conversation

ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

PATCH New Peter Chen 3 participants 9 messages Jun 7, 2016 – Jun 16, 2016

Subsystems (2)
lore
Share & tools
lore

Peter Chen

author
#0 lore raw

PATCH 1/1 ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

The imx6 SMP system has the same DMA memory coherency issue [1] with pl310 L2 controller. With this shared override bit set, the customer reports the DMA coherency issue is gone. Besides, I have tested the performance using USB ethernet with/without this bit, it shows no difference.

[1] http://patchwork.ozlabs.org/patch/469362/

---
 arch/arm/boot/dts/imx6qdl.dtsi | 1 +
 1 file changed, 1 insertion(+)
arch/arm/boot/dts/imx6qdl.dtsi +1 -0
index ed613eb..30e21ee 100644
@@ -185,6 +185,7 @@
 			cache-level = <2>;
 			arm,tag-latency = <4 2 3>;
 			arm,data-latency = <4 2 3>;
+			arm,shared-override;
 		};
 
 		pcie: pcie at 0x01000000 {
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

+ Lucas

Quoted 18 lines · Peter Chen

The imx6 SMP system has the same DMA memory coherency issue [1] with pl310 L2 controller. With this shared override bit set, the customer reports the DMA coherency issue is gone. Besides, I have tested the performance using USB ethernet with/without this bit, it shows no difference.

[1] http://patchwork.ozlabs.org/patch/469362/

---
 arch/arm/boot/dts/imx6qdl.dtsi | 1 +
 1 file changed, 1 insertion(+)
arch/arm/boot/dts/imx6qdl.dtsi +0 -0
index ed613eb..30e21ee 100644

arch/arm/boot/dts/imx6qdl.dtsi line 188

Quoted 4 lines
arch/arm/boot/dts/imx6qdl.dtsi +1 -0
@@ -185,6 +185,7 @@
 			cache-level = <2>;
 			arm,tag-latency = <4 2 3>;
 			arm,data-latency = <4 2 3>;
+			arm,shared-override;

Lucas had an objection to the change [1], considering the case that kernel is booted as non-secure.

Shawn

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/471026

Quoted 11 lines
arch/arm/boot/dts/imx6qdl.dtsi +0 -0
 		};
 
 		pcie: pcie at 0x01000000 {
Signature · 7 lines
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
author
#2 lore raw

PATCH 1/1 ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

Quoted 33 lines

Subject: Re: [PATCH 1/1] ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

+ Lucas

Quoted 18 lines · Peter Chen

The imx6 SMP system has the same DMA memory coherency issue [1] with pl310 L2 controller. With this shared override bit set, the customer reports the DMA coherency issue is gone. Besides, I have tested the performance using USB ethernet with/without this bit, it shows no difference.

[1] http://patchwork.ozlabs.org/patch/469362/

---
 arch/arm/boot/dts/imx6qdl.dtsi | 1 +
 1 file changed, 1 insertion(+)
(quoted diff) +0 -0

b/arch/arm/boot/dts/imx6qdl.dtsi index ed613eb..30e21ee 100644

arch/arm/boot/dts/imx6qdl.dtsi line 188

Quoted 4 lines
arch/arm/boot/dts/imx6qdl.dtsi +1 -0
@@ -185,6 +185,7 @@
 			cache-level = <2>;
 			arm,tag-latency = <4 2 3>;
 			arm,data-latency = <4 2 3>;
+			arm,shared-override;

Lucas had an objection to the change [1], considering the case that kernel is booted as non-secure.

Quoted 4 lines

Shawn

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/471026

Add another Lucas in case he changed his email.

Shawn, do you know what his mean? Why other pl310 parameters can be changed in kernel?

Peter

#3 lore raw

PATCH 1/1 ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

In reply to Shawn Guo

Am Samstag, den 11.06.2016, 19:57 +0800 schrieb Shawn Guo:

Quoted 16 lines

+ Lucas

Quoted 13 lines · Peter Chen

The imx6 SMP system has the same DMA memory coherency issue [1] with pl310 L2 controller. With this shared override bit set, the customer reports the DMA coherency issue is gone. Besides, I have tested the performance using USB ethernet with/without this bit, it shows no difference.

[1] http://patchwork.ozlabs.org/patch/469362/

---
 arch/arm/boot/dts/imx6qdl.dtsi | 1 +
 1 file changed, 1 insertion(+)
Quoted 12 lines

arch/arm/boot/dts/imx6qdl.dtsi line 188

Quoted 9 lines
arch/arm/boot/dts/imx6qdl.dtsi +1 -0
index ed613eb..30e21ee 100644
@@ -185,6 +185,7 @@
 			cache-level = <2>;
 			arm,tag-latency = <4 2 3>;
 			arm,data-latency = <4 2 3>;
+			arm,shared-override;

Lucas had an objection to the change [1], considering the case that kernel is booted as non-secure.

My objection to this change still stands. Configuring the L2C to be compliant to the ARMv7 ARM is at the same level as the CPU workarounds that can not be applied in secure mode. Those must be done in the firmware, if your firmware doesn't do it it's plain broken.

Regards,
Lucas

author
#4 lore raw

PATCH 1/1 ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

Quoted 31 lines

Am Samstag, den 11.06.2016, 19:57 +0800 schrieb Shawn Guo:

Quoted 16 lines

+ Lucas

Quoted 13 lines · Peter Chen

The imx6 SMP system has the same DMA memory coherency issue [1] with pl310 L2 controller. With this shared override bit set, the customer reports the DMA coherency issue is gone. Besides, I have tested the performance using USB ethernet with/without this bit, it shows no difference.

[1] http://patchwork.ozlabs.org/patch/469362/

---
 arch/arm/boot/dts/imx6qdl.dtsi | 1 +
 1 file changed, 1 insertion(+)
Quoted 12 lines

arch/arm/boot/dts/imx6qdl.dtsi line 188

Quoted 9 lines
(quoted diff) +0 -0

b/arch/arm/boot/dts/imx6qdl.dtsi index ed613eb..30e21ee 100644

arch/arm/boot/dts/imx6qdl.dtsi +1 -0
@@ -185,6 +185,7 @@
 			cache-level = <2>;
 			arm,tag-latency = <4 2 3>;
 			arm,data-latency = <4 2 3>;
+			arm,shared-override;

Lucas had an objection to the change [1], considering the case that kernel is booted as non-secure.

Quoted 5 lines

My objection to this change still stands. Configuring the L2C to be compliant to the ARMv7 ARM is at the same level as the CPU workarounds that can not be applied in secure mode. Those must be done in the firmware, if your firmware doesn't do it it's plain broken.

Sorry, I not understand what's your mean. We only changes L2 configuration when the cache is disabled, what problem will be?

Peter

#5 lore raw

PATCH 1/1 ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

Am Montag, den 13.06.2016, 09:42 +0000 schrieb Peter Chen:

Quoted 31 lines
Quoted 31 lines

Am Samstag, den 11.06.2016, 19:57 +0800 schrieb Shawn Guo:

Quoted 28 lines

+ Lucas

Quoted 18 lines · Peter Chen

The imx6 SMP system has the same DMA memory coherency issue [1] with pl310 L2 controller. With this shared override bit set, the customer reports the DMA coherency issue is gone. Besides, I have tested the performance using USB ethernet with/without this bit, it shows no difference.

[1] http://patchwork.ozlabs.org/patch/469362/

---
 arch/arm/boot/dts/imx6qdl.dtsi | 1 +
 1 file changed, 1 insertion(+)
(quoted diff) +0 -0

b/arch/arm/boot/dts/imx6qdl.dtsi index ed613eb..30e21ee 100644

arch/arm/boot/dts/imx6qdl.dtsi line 188

Quoted 4 lines
arch/arm/boot/dts/imx6qdl.dtsi +1 -0
@@ -185,6 +185,7 @@
 			cache-level = <2>;
 			arm,tag-latency = <4 2 3>;
 			arm,data-latency = <4 2 3>;
+			arm,shared-override;

Lucas had an objection to the change [1], considering the case that kernel is booted as non-secure.

Quoted 9 lines
Quoted 5 lines

My objection to this change still stands. Configuring the L2C to be compliant to the ARMv7 ARM is at the same level as the CPU workarounds that can not be applied in secure mode. Those must be done in the firmware, if your firmware doesn't do it it's plain broken.

Sorry, I not understand what's your mean. We only changes L2 configuration when the cache is disabled, what problem will be?

If the kernel is booted in non-secure mode, the L2 cache configuration register is RO and the kernel will crash on the attempt to write into this register.

Regards,
Lucas

author
#6 lore raw

PATCH 1/1 ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

Quoted 42 lines

Am Montag, den 13.06.2016, 09:42 +0000 schrieb Peter Chen:

Quoted 39 lines
Quoted 38 lines

Am Samstag, den 11.06.2016, 19:57 +0800 schrieb Shawn Guo:

Quoted 17 lines

+ Lucas

Quoted 14 lines · Peter Chen

The imx6 SMP system has the same DMA memory coherency issue [1] with
pl310 L2 controller. With this shared override bit set, the customer reports the DMA coherency issue is gone. Besides, I have tested the performance using USB ethernet with/without this bit, it shows no difference.

[1] http://patchwork.ozlabs.org/patch/469362/

---
 arch/arm/boot/dts/imx6qdl.dtsi | 1 +
 1 file changed, 1 insertion(+)
Quoted 12 lines

arch/arm/boot/dts/imx6qdl.dtsi line 188

Quoted 9 lines
(quoted diff) +0 -0

b/arch/arm/boot/dts/imx6qdl.dtsi index ed613eb..30e21ee 100644

arch/arm/boot/dts/imx6qdl.dtsi +1 -0
@@ -185,6 +185,7 @@
 			cache-level = <2>;
 			arm,tag-latency = <4 2 3>;
 			arm,data-latency = <4 2 3>;
+			arm,shared-override;

Lucas had an objection to the change [1], considering the case that kernel is booted as non-secure.

My objection to this change still stands. Configuring the L2C to be compliant to the
ARMv7 ARM is at the same level as the CPU workarounds that can not be applied in secure mode. Those must be done in the firmware, if your firmware doesn't do it it's plain broken.

Quoted 6 lines
Quoted 3 lines

Sorry, I not understand what's your mean. We only changes L2 configuration when the cache is disabled, what problem will be?

If the kernel is booted in non-secure mode, the L2 cache configuration register is RO and the kernel will crash on the attempt to write into this register.

Does that mean current mainline kernel for imx6qdl can't boot with non-secure mode, I see other L2 configurations have been changed like "arm, tag-latency" and "arm,data-latency"? Did you try it?

Peter

#7 lore raw

PATCH 1/1 ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

Hi Peter,

Am Dienstag, den 14.06.2016, 01:30 +0000 schrieb Peter Chen:

Quoted 45 lines
Quoted 45 lines

Am Montag, den 13.06.2016, 09:42 +0000 schrieb Peter Chen:

Quoted 42 lines
Quoted 32 lines

Am Samstag, den 11.06.2016, 19:57 +0800 schrieb Shawn Guo:

Quoted 17 lines

+ Lucas

Quoted 14 lines · Peter Chen

The imx6 SMP system has the same DMA memory coherency issue [1] with
pl310 L2 controller. With this shared override bit set, the customer reports the DMA coherency issue is gone. Besides, I have tested the performance using USB ethernet with/without this bit, it shows no difference.

[1] http://patchwork.ozlabs.org/patch/469362/

---
 arch/arm/boot/dts/imx6qdl.dtsi | 1 +
 1 file changed, 1 insertion(+)
Quoted 12 lines

arch/arm/boot/dts/imx6qdl.dtsi line 188

Quoted 9 lines
(quoted diff) +0 -0

b/arch/arm/boot/dts/imx6qdl.dtsi index ed613eb..30e21ee 100644

arch/arm/boot/dts/imx6qdl.dtsi +1 -0
@@ -185,6 +185,7 @@
 			cache-level = <2>;
 			arm,tag-latency = <4 2 3>;
 			arm,data-latency = <4 2 3>;
+			arm,shared-override;

Lucas had an objection to the change [1], considering the case that kernel is booted as non-secure.

Quoted 6 lines

My objection to this change still stands. Configuring the L2C to be compliant to the
ARMv7 ARM is at the same level as the CPU workarounds that can not be applied in secure mode. Those must be done in the firmware, if your firmware doesn't do it it's plain broken.

Sorry, I not understand what's your mean. We only changes L2 configuration when the cache is disabled, what problem will be?

Quoted 6 lines
Quoted 3 lines

If the kernel is booted in non-secure mode, the L2 cache configuration register is RO and the kernel will crash on the attempt to write into this register.

Does that mean current mainline kernel for imx6qdl can't boot with non-secure mode, I see other L2 configurations have been changed like "arm, tag-latency" and "arm,data-latency"? Did you try it?

I've just re-read the code and hereby withdraw my objection.

The cache configuration registers will only be touched if the DT specified values differ from what the firmware has set up already. As not setting the "shared-override" bit render the platform non compliant to the ARMv7 ARM, one should hope that any firmware booting the kernel in secure mode has already set this bit. The crash that will happen otherwise might be just as good as the silent data corruption resulting from not setting this bit.

Regards,
Lucas

Applied
#8 lore raw

PATCH 1/1 ARM: dts: imx6qdl.dtsi: add "arm, shared-override" for pl310

In reply to Peter Chen
Quoted 9 lines · Peter Chen

The imx6 SMP system has the same DMA memory coherency issue [1] with pl310 L2 controller. With this shared override bit set, the customer reports the DMA coherency issue is gone. Besides, I have tested the performance using USB ethernet with/without this bit, it shows no difference.

[1] http://patchwork.ozlabs.org/patch/469362/

Applied, thanks.