Re: [U-Boot] [PATCH 3/9] ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
On 11/21/2013 09:59 AM, Marc Zyngier wrote: A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order. Makes sense ;-) and works on the VExpress TC2. Albert, Tom, please apply for v2014.01. Acked-by: Andre Przywara andre.przyw...@linaro.org Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) + isb #ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/9] ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
On Fri, Nov 22, 2013 at 10:56:05AM +, Marc Zyngier wrote: On 22/11/13 01:51, Christoffer Dall wrote: On 21 November 2013 00:59, Marc Zyngier marc.zyng...@arm.com wrote: A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) + isb #ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value -- 1.8.2.3 Does this matter? Are we not still in monitor mode and therefore secure and the exception return below will surely be ordered by the cpu after the mcr, right? or no? You need to look at what is between the SCR access and the exception return (and you cannot see that from the patch): There's a write to HVBAR that can only be executed if SCR.NS==1. If the write to SCR is delayed, the whole thing will stop very quickly... Ah, I didn't realize this specific about PL2-mode system control registers and relied on the fact that we were just in monitor mode and that the setting of this bit essentially didn't matter; I obviously got this wrong, and I think to be fair that Andre had this isb in his original code and removed it after my review. /my bad. Thanks for the explanation. -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/9] ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
On 22/11/13 01:51, Christoffer Dall wrote: On 21 November 2013 00:59, Marc Zyngier marc.zyng...@arm.com wrote: A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) + isb #ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value -- 1.8.2.3 Does this matter? Are we not still in monitor mode and therefore secure and the exception return below will surely be ordered by the cpu after the mcr, right? or no? You need to look at what is between the SCR access and the exception return (and you cannot see that from the patch): There's a write to HVBAR that can only be executed if SCR.NS==1. If the write to SCR is delayed, the whole thing will stop very quickly... M. -- Jazz is not dead. It just smells funny... ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 3/9] ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) + isb #ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value -- 1.8.2.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/9] ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
On 21 November 2013 00:59, Marc Zyngier marc.zyng...@arm.com wrote: A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order. Signed-off-by: Marc Zyngier marc.zyng...@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) + isb #ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value -- 1.8.2.3 Does this matter? Are we not still in monitor mode and therefore secure and the exception return below will surely be ordered by the cpu after the mcr, right? or no? -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
