From: Julien Grall <jgrall@amazon.com>

Per the Arm Arm (ARM DDI 0406C.d A3.8.3):

"The DMB and DSB memory barriers affect reads and writes to the memory
system generated by load/store instructions and data or unified cache
maintenance operations being executed by the processor. Instruction
fetches or accesses caused by a hardware translation table access are
not explicit accesses."

In setup_fixmap(), we write the fixmap area and may be used soon after,
for instance, to write to the UART. IOW, there could be hardware
translation table access. So we need to ensure the 'dsb' has completed
before continuing. Therefore add an 'isb'.

Fixes: e79999e587d7 ("xen/arm32: head: Remove 1:1 mapping as soon as it is not used")
Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/arm/arm32/head.S | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index f9f7be9588b1..6ca3329138e3 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -751,6 +751,11 @@ setup_fixmap:
         create_table_entry boot_second, xen_fixmap, r0, 2
         /* Ensure any page table updates made above have occurred. */
         dsb   nshst
+        /*
+         * The fixmap area will be used soon after. So ensure no hardware
+         * translation happens before the dsb completes.
+         */
+        isb
 
         mov   pc, lr
 ENDPROC(setup_fixmap)
-- 
2.40.1

Hi Julien,

> On 19 Jun 2023, at 19:01, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Per the Arm Arm (ARM DDI 0406C.d A3.8.3):
> 
> "The DMB and DSB memory barriers affect reads and writes to the memory
> system generated by load/store instructions and data or unified cache
> maintenance operations being executed by the processor. Instruction
> fetches or accesses caused by a hardware translation table access are
> not explicit accesses."
> 
> In setup_fixmap(), we write the fixmap area and may be used soon after,
> for instance, to write to the UART. IOW, there could be hardware
> translation table access. So we need to ensure the 'dsb' has completed
> before continuing. Therefore add an 'isb'.
> 
> Fixes: e79999e587d7 ("xen/arm32: head: Remove 1:1 mapping as soon as it is not used")
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> xen/arch/arm/arm32/head.S | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index f9f7be9588b1..6ca3329138e3 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -751,6 +751,11 @@ setup_fixmap:
>         create_table_entry boot_second, xen_fixmap, r0, 2
>         /* Ensure any page table updates made above have occurred. */
>         dsb   nshst
> +        /*
> +         * The fixmap area will be used soon after. So ensure no hardware
> +         * translation happens before the dsb completes.
> +         */
> +        isb
> 
>         mov   pc, lr
> ENDPROC(setup_fixmap)
> -- 
> 2.40.1
> 

> On 19 Jun 2023, at 18:01, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Per the Arm Arm (ARM DDI 0406C.d A3.8.3):
> 
> "The DMB and DSB memory barriers affect reads and writes to the memory
> system generated by load/store instructions and data or unified cache
> maintenance operations being executed by the processor. Instruction
> fetches or accesses caused by a hardware translation table access are
> not explicit accesses."
> 
> In setup_fixmap(), we write the fixmap area and may be used soon after,
> for instance, to write to the UART. IOW, there could be hardware
> translation table access. So we need to ensure the 'dsb' has completed
> before continuing. Therefore add an 'isb'.
> 
> Fixes: e79999e587d7 ("xen/arm32: head: Remove 1:1 mapping as soon as it is not used")
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> ---

Hi Julien,

Yeah makes sense!

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Hi Julien,

> -----Original Message-----
> Subject: [PATCH 1/7] xen/arm32: head: Add missing isb in setup_fixmap()
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Per the Arm Arm (ARM DDI 0406C.d A3.8.3):
> 
> "The DMB and DSB memory barriers affect reads and writes to the memory
> system generated by load/store instructions and data or unified cache
> maintenance operations being executed by the processor. Instruction
> fetches or accesses caused by a hardware translation table access are
> not explicit accesses."
> 
> In setup_fixmap(), we write the fixmap area and may be used soon after,
> for instance, to write to the UART. IOW, there could be hardware
> translation table access. So we need to ensure the 'dsb' has completed
> before continuing. Therefore add an 'isb'.
> 
> Fixes: e79999e587d7 ("xen/arm32: head: Remove 1:1 mapping as soon as it is
> not used")
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Henry Wang <Henry.Wang@arm.com>

I've also tested this patch on top of today's staging by our internal CI, which
includes the FVP arm32 mode and qemu-arm32. Our CI says this patch looks
good, so:

Tested-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry