diff mbox series

[v4,02/14] xen/arm64: flushtlb: Implement the TLBI repeat workaround for TLB flush by VA

Message ID 20230113101136.479-3-julien@xen.org (mailing list archive)
State Superseded
Headers show
Series xen/arm: Don't switch TTBR while the MMU is on | expand

Commit Message

Julien Grall Jan. 13, 2023, 10:11 a.m. UTC 
From: Julien Grall <jgrall@amazon.com>

Looking at the Neoverse N1 errata document, it is not clear to me
why the TLBI repeat workaround is not applied for TLB flush by VA.

The TBL flush by VA helpers are used in flush_xen_tlb_range_va_local()
and flush_xen_tlb_range_va(). So if the range size if a fixed size smaller
than a PAGE_SIZE, it would be possible that the compiler remove the loop
and therefore replicate the sequence described in the erratum 1286807.

So the TLBI repeat workaround should also be applied for the TLB flush
by VA helpers.

Fixes: 22e323d115d8 ("xen/arm: Add workaround for Cortex-A76/Neoverse-N1 erratum #1286807")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

----
    This was spotted while looking at reducing the scope of the memory
    barriers. I don't have any HW affected.

    Changes in v4:
        - Add Michal's reviewed-by tag

    Changes in v3:
        - Patch added
---
 xen/arch/arm/include/asm/arm64/flushtlb.h | 31 +++++++++++++++++------
 1 file changed, 23 insertions(+), 8 deletions(-)

Comments

Henry Wang Jan. 13, 2023, 1:22 p.m. UTC | #1 
Hi Julien,

> -----Original Message-----
> Subject: [PATCH v4 02/14] xen/arm64: flushtlb: Implement the TLBI repeat
> workaround for TLB flush by VA
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Looking at the Neoverse N1 errata document, it is not clear to me
> why the TLBI repeat workaround is not applied for TLB flush by VA.
> 
> The TBL flush by VA helpers are used in flush_xen_tlb_range_va_local()
> and flush_xen_tlb_range_va(). So if the range size if a fixed size smaller
> than a PAGE_SIZE, it would be possible that the compiler remove the loop
> and therefore replicate the sequence described in the erratum 1286807.
> 
> So the TLBI repeat workaround should also be applied for the TLB flush
> by VA helpers.
> 
> Fixes: 22e323d115d8 ("xen/arm: Add workaround for Cortex-A76/Neoverse-
> N1 erratum #1286807")
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> ----
>     This was spotted while looking at reducing the scope of the memory
>     barriers. I don't have any HW affected.

Seeing this scissors line comment, I tried to test this patch using basically
the same approach that I did for patch#1 on every board that I can find,
including some Neoverse N1 boards, and this patch looks good, so:

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

Kind regards,
Henry
Luca Fancellu Jan. 13, 2023, 5:56 p.m. UTC | #2 
> On 13 Jan 2023, at 10:11, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Looking at the Neoverse N1 errata document, it is not clear to me
> why the TLBI repeat workaround is not applied for TLB flush by VA.
> 
> The TBL flush by VA helpers are used in flush_xen_tlb_range_va_local()
> and flush_xen_tlb_range_va(). So if the range size if a fixed size smaller

NIT: is a fixed size
Julien Grall Jan. 16, 2023, 8:36 a.m. UTC | #3 
Hi Luca,

On 13/01/2023 17:56, Luca Fancellu wrote:
> 
> 
>> On 13 Jan 2023, at 10:11, Julien Grall <julien@xen.org> wrote:
>>
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Looking at the Neoverse N1 errata document, it is not clear to me
>> why the TLBI repeat workaround is not applied for TLB flush by VA.
>>
>> The TBL flush by VA helpers are used in flush_xen_tlb_range_va_local()

s/TBL/TLB/

>> and flush_xen_tlb_range_va(). So if the range size if a fixed size smaller
> 
> NIT: is a fixed size

Thanks. I will fix it on commit unless there is another round needed.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/arm64/flushtlb.h b/xen/arch/arm/include/asm/arm64/flushtlb.h
index a40693b08dd3..7f81cbbb93f9 100644
--- a/xen/arch/arm/include/asm/arm64/flushtlb.h
+++ b/xen/arch/arm/include/asm/arm64/flushtlb.h
@@ -44,6 +44,27 @@  static inline void name(void)                    \
         : : : "memory");                         \
 }
 
+/*
+ * FLush TLB by VA. This will likely be used in a loop, so the caller
+ * is responsible to use the appropriate memory barriers before/after
+ * the sequence.
+ *
+ * See above about the ARM64_WORKAROUND_REPEAT_TLBI sequence.
+ */
+#define TLB_HELPER_VA(name, tlbop)               \
+static inline void name(vaddr_t va)              \
+{                                                \
+    asm volatile(                                \
+        "tlbi "  # tlbop  ", %0;"                \
+        ALTERNATIVE(                             \
+            "nop; nop;",                         \
+            "dsb  ish;"                          \
+            "tlbi "  # tlbop  ", %0;",           \
+            ARM64_WORKAROUND_REPEAT_TLBI,        \
+            CONFIG_ARM64_WORKAROUND_REPEAT_TLBI) \
+        : : "r" (va >> PAGE_SHIFT) : "memory");  \
+}
+
 /* Flush local TLBs, current VMID only. */
 TLB_HELPER(flush_guest_tlb_local, vmalls12e1, nsh);
 
@@ -60,16 +81,10 @@  TLB_HELPER(flush_all_guests_tlb, alle1is, ish);
 TLB_HELPER(flush_xen_tlb_local, alle2, nsh);
 
 /* Flush TLB of local processor for address va. */
-static inline void  __flush_xen_tlb_one_local(vaddr_t va)
-{
-    asm volatile("tlbi vae2, %0;" : : "r" (va>>PAGE_SHIFT) : "memory");
-}
+TLB_HELPER_VA(__flush_xen_tlb_one_local, vae2);
 
 /* Flush TLB of all processors in the inner-shareable domain for address va. */
-static inline void __flush_xen_tlb_one(vaddr_t va)
-{
-    asm volatile("tlbi vae2is, %0;" : : "r" (va>>PAGE_SHIFT) : "memory");
-}
+TLB_HELPER_VA(__flush_xen_tlb_one, vae2is);
 
 #endif /* __ASM_ARM_ARM64_FLUSHTLB_H__ */
 /*