Re: [kvm-unit-tests PATCH 2/2] arm64: ensure tlbi is safe

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi,

On Sat, Jun 17, 2023 at 01:31:38AM +0000, Nadav Amit wrote:
> From: Nadav Amit <namit@vmware.com>
> 
> While no real problem was encountered, having an inline assembly without
> volatile keyword and output can allow the compiler to ignore it. And
> without a memory clobber, potentially reorder it.
> 
> Add volatile and memory clobber.
> 
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
>  lib/arm64/asm/mmu.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/arm64/asm/mmu.h b/lib/arm64/asm/mmu.h
> index 5c27edb..cf94403 100644
> --- a/lib/arm64/asm/mmu.h
> +++ b/lib/arm64/asm/mmu.h
> @@ -14,7 +14,7 @@
>  static inline void flush_tlb_all(void)
>  {
>  	dsb(ishst);
> -	asm("tlbi	vmalle1is");

From the gas manual [1]:

"asm statements that have no output operands and asm goto statements, are
implicitly volatile."

Looks to me like both TLBIs fall into this category.

And I think the "memory" clobber is not needed because the dsb macro before and
after the TLBI already have it.

[1] https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile

Thanks,
Alex

> +	asm volatile("tlbi	vmalle1is" ::: "memory");
>  	dsb(ish);
>  	isb();
>  }
> @@ -23,7 +23,7 @@ static inline void flush_tlb_page(unsigned long vaddr)
>  {
>  	unsigned long page = vaddr >> 12;
>  	dsb(ishst);
> -	asm("tlbi	vaae1is, %0" :: "r" (page));
> +	asm volatile("tlbi	vaae1is, %0" :: "r" (page) : "memory");
>  	dsb(ish);
>  	isb();
>  }
> -- 
> 2.34.1
> 
>