| Message ID | 20230617013138.1823-3-namit@vmware.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: fix paging issues | expand |
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 > >
> On Jul 14, 2023, at 3:55 AM, Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > !! External Email > > 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. You are completely correct. I forgot about the implicit “volatile”. This one can be dropped.
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"); + 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(); }