| Message ID | 20240616173105.7430-3-heinrich.schuchardt@canonical.com |
|---|---|
| State | Accepted |
| Commit | 2da287ad13e1b488c1c39afc4721d733d6b83cd7 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | cmd: avoid duplicate weak functions | expand |
Hi Heinrich, On Sun, 16 Jun 2024 at 20:31, Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: > > In EFI sub-system we rely on invalidate_icache_all() to invalidate the > instruction cache after loading binaries. Add the missing implementation on > ARM1136, ARM1176. > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > --- > arch/arm/cpu/arm11/cpu.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c > index 01d2e1a125d..4bf0446b543 100644 > --- a/arch/arm/cpu/arm11/cpu.c > +++ b/arch/arm/cpu/arm11/cpu.c > @@ -116,3 +116,15 @@ void enable_caches(void) > #endif > } > #endif > + > +#if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF) > +/* Invalidate entire I-cache */ > +void invalidate_icache_all(void) > +{ > + unsigned long i = 0; > + > + asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i)); This looks correct, but can't we define it as __asm__("mcr p15, 0, %0, c7, c5, 0" : : "r" (0)); ? Thanks /Ilias > +} > +#else > +void invalidate_icache_all(void) {} > +#endif > -- > 2.43.0 >
On 19.06.24 14:22, Ilias Apalodimas wrote: > Hi Heinrich, > > On Sun, 16 Jun 2024 at 20:31, Heinrich Schuchardt > <heinrich.schuchardt@canonical.com> wrote: >> >> In EFI sub-system we rely on invalidate_icache_all() to invalidate the >> instruction cache after loading binaries. Add the missing implementation on >> ARM1136, ARM1176. >> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >> --- >> arch/arm/cpu/arm11/cpu.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c >> index 01d2e1a125d..4bf0446b543 100644 >> --- a/arch/arm/cpu/arm11/cpu.c >> +++ b/arch/arm/cpu/arm11/cpu.c >> @@ -116,3 +116,15 @@ void enable_caches(void) >> #endif >> } >> #endif >> + >> +#if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF) >> +/* Invalidate entire I-cache */ >> +void invalidate_icache_all(void) >> +{ >> + unsigned long i = 0; >> + >> + asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i)); > > This looks correct, but can't we define it as > __asm__("mcr p15, 0, %0, c7, c5, 0" : : "r" (0)); ? Both compile to the same code. So we should simplify the expression. arch/arm/cpu/armv7/cache_v7.c and other modules always uses asm and not __asm__. @Tom: Can we specify what is preferred in doc/develop/codingstyle.rst? https://gcc.gnu.org/onlinedocs/gcc/Alternate-Keywords.html gives some background. Best regards Heinrich > > Thanks > /Ilias >> +} >> +#else >> +void invalidate_icache_all(void) {} >> +#endif >> -- >> 2.43.0 >>
On Wed, Jun 19, 2024 at 02:56:24PM +0200, Heinrich Schuchardt wrote: > On 19.06.24 14:22, Ilias Apalodimas wrote: > > Hi Heinrich, > > > > On Sun, 16 Jun 2024 at 20:31, Heinrich Schuchardt > > <heinrich.schuchardt@canonical.com> wrote: > > > > > > In EFI sub-system we rely on invalidate_icache_all() to invalidate the > > > instruction cache after loading binaries. Add the missing implementation on > > > ARM1136, ARM1176. > > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > --- > > > arch/arm/cpu/arm11/cpu.c | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c > > > index 01d2e1a125d..4bf0446b543 100644 > > > --- a/arch/arm/cpu/arm11/cpu.c > > > +++ b/arch/arm/cpu/arm11/cpu.c > > > @@ -116,3 +116,15 @@ void enable_caches(void) > > > #endif > > > } > > > #endif > > > + > > > +#if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF) > > > +/* Invalidate entire I-cache */ > > > +void invalidate_icache_all(void) > > > +{ > > > + unsigned long i = 0; > > > + > > > + asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i)); > > > > This looks correct, but can't we define it as > > __asm__("mcr p15, 0, %0, c7, c5, 0" : : "r" (0)); ? > > Both compile to the same code. So we should simplify the expression. > > arch/arm/cpu/armv7/cache_v7.c and other modules always uses asm and not > __asm__. > > @Tom: > Can we specify what is preferred in doc/develop/codingstyle.rst? > > https://gcc.gnu.org/onlinedocs/gcc/Alternate-Keywords.html gives some > background. Sure.
diff --git a/arch/arm/cpu/arm11/cpu.c b/arch/arm/cpu/arm11/cpu.c index 01d2e1a125d..4bf0446b543 100644 --- a/arch/arm/cpu/arm11/cpu.c +++ b/arch/arm/cpu/arm11/cpu.c @@ -116,3 +116,15 @@ void enable_caches(void) #endif } #endif + +#if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF) +/* Invalidate entire I-cache */ +void invalidate_icache_all(void) +{ + unsigned long i = 0; + + asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i)); +} +#else +void invalidate_icache_all(void) {} +#endif
In EFI sub-system we rely on invalidate_icache_all() to invalidate the instruction cache after loading binaries. Add the missing implementation on ARM1136, ARM1176. Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> --- arch/arm/cpu/arm11/cpu.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)