diff mbox series

[5/7] xen/arm: pmap: Add missing ISB in arch_pmap_map()

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

Commit Message

Julien Grall June 19, 2023, 5:01 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

Per the Arm Arm, (Armv7 DDI406C.d A3.8.3 and Armv8 DDI 0487J.a B2.3.12):

"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."

Note that second sentence is not part of the newer Armv8 spec. But the
interpretation is not much different.

As the entry created by arch_pmap_map() will be used soon after
pmap_map() returns, we want to ensure the DSB in write_pte() has
completed. So add an ISB.

Fixes: 4f17357b52f6 ("xen/arm: add Persistent Map (PMAP) infrastructure")
Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/arm/include/asm/pmap.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Henry Wang June 20, 2023, 3:07 a.m. UTC | #1
Hi Julien,

> -----Original Message-----
> Subject: [PATCH 5/7] xen/arm: pmap: Add missing ISB in arch_pmap_map()
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Per the Arm Arm, (Armv7 DDI406C.d A3.8.3 and Armv8 DDI 0487J.a B2.3.12):
> 
> "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."
> 
> Note that second sentence is not part of the newer Armv8 spec. But the
> interpretation is not much different.
> 
> As the entry created by arch_pmap_map() will be used soon after
> pmap_map() returns, we want to ensure the DSB in write_pte() has
> completed. So add an ISB.
> 
> Fixes: 4f17357b52f6 ("xen/arm: add Persistent Map (PMAP) infrastructure")
> 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, and this
patch looks good, so:

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

Kind regards,
Henry
Bertrand Marquis July 4, 2023, 2:56 p.m. UTC | #2
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, (Armv7 DDI406C.d A3.8.3 and Armv8 DDI 0487J.a B2.3.12):
> 
> "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."
> 
> Note that second sentence is not part of the newer Armv8 spec. But the
> interpretation is not much different.
> 
> As the entry created by arch_pmap_map() will be used soon after
> pmap_map() returns, we want to ensure the DSB in write_pte() has
> completed. So add an ISB.
> 
> Fixes: 4f17357b52f6 ("xen/arm: add Persistent Map (PMAP) infrastructure")
> Signed-off-by: Julien Grall <jgrall@amazon.com>

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

Cheers
Bertrand

> ---
> xen/arch/arm/include/asm/pmap.h | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/xen/arch/arm/include/asm/pmap.h b/xen/arch/arm/include/asm/pmap.h
> index e094d13dd282..bca3381796f3 100644
> --- a/xen/arch/arm/include/asm/pmap.h
> +++ b/xen/arch/arm/include/asm/pmap.h
> @@ -15,6 +15,11 @@ static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
>     pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW);
>     pte.pt.table = 1;
>     write_pte(entry, pte);
> +    /*
> +     * The new entry will be used very soon after arch_pmap_map() returns.
> +     * So ensure the DSB in write_pte() has completed before continuing.
> +     */
> +    isb();
> }
> 
> static inline void arch_pmap_unmap(unsigned int slot)
> -- 
> 2.40.1
>
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/pmap.h b/xen/arch/arm/include/asm/pmap.h
index e094d13dd282..bca3381796f3 100644
--- a/xen/arch/arm/include/asm/pmap.h
+++ b/xen/arch/arm/include/asm/pmap.h
@@ -15,6 +15,11 @@  static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
     pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW);
     pte.pt.table = 1;
     write_pte(entry, pte);
+    /*
+     * The new entry will be used very soon after arch_pmap_map() returns.
+     * So ensure the DSB in write_pte() has completed before continuing.
+     */
+    isb();
 }
 
 static inline void arch_pmap_unmap(unsigned int slot)