| Message ID | 20230619170115.81398-2-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Add some missing ISBs after updating the PTEs | expand |
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
> 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, > 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 >
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)