| Message ID | 20230304221524.47160-5-jiaxun.yang@flygoat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS Virt board support | expand |
From: Jiaxun Yang <jiaxun.yang@flygoat.com> Date: Sat, 4 Mar 2023 22:15:16 +0000 > Octeon has a different cache interface with traditional R4K one, > just opt-out this flag for octeon to avoid run R4K cache initialization > code accidentally. > > Also remove ISA level assumption for 4k cache. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > --- > arch/mips/include/asm/cpu-features.h | 2 +- > arch/mips/kernel/cpu-probe.c | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h > index c0983130a44c..c613426b0bfc 100644 > --- a/arch/mips/include/asm/cpu-features.h > +++ b/arch/mips/include/asm/cpu-features.h > @@ -118,7 +118,7 @@ > #define cpu_has_3k_cache __isa_lt_and_opt(1, MIPS_CPU_3K_CACHE) > #endif > #ifndef cpu_has_4k_cache > -#define cpu_has_4k_cache __isa_ge_or_opt(1, MIPS_CPU_4K_CACHE) > +#define cpu_has_4k_cache __opt(MIPS_CPU_4K_CACHE) This breaks compile-time optimization for this feature => hurts performance on non-Octeon machines. Could this be done some other way? E.g. could this be defined depending on CONFIG_SYS_HAS_CPU_OCTEON_WHATEVER? Via its own cpu-features.h or here, that's the second question. So that the platforms which 100% can't have this family of CPUs wouldn't get affected. > #endif > #ifndef cpu_has_octeon_cache > #define cpu_has_octeon_cache 0 > diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c > index 7ddf07f255f3..6d15a398d389 100644 > --- a/arch/mips/kernel/cpu-probe.c > +++ b/arch/mips/kernel/cpu-probe.c > @@ -1602,6 +1602,8 @@ static inline void cpu_probe_broadcom(struct cpuinfo_mips *c, unsigned int cpu) > static inline void cpu_probe_cavium(struct cpuinfo_mips *c, unsigned int cpu) > { > decode_configs(c); > + /* Octeon has different cache interface */ > + c->options &= ~MIPS_CPU_4K_CACHE; > switch (c->processor_id & PRID_IMP_MASK) { > case PRID_IMP_CAVIUM_CN38XX: > case PRID_IMP_CAVIUM_CN31XX: Thanks, Olek
> 2023年3月6日 14:28，Alexander Lobakin <aleksander.lobakin@intel.com> 写道： > > From: Jiaxun Yang <jiaxun.yang@flygoat.com> > Date: Sat, 4 Mar 2023 22:15:16 +0000 > >> Octeon has a different cache interface with traditional R4K one, >> just opt-out this flag for octeon to avoid run R4K cache initialization >> code accidentally. >> >> Also remove ISA level assumption for 4k cache. >> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >> --- >> arch/mips/include/asm/cpu-features.h | 2 +- >> arch/mips/kernel/cpu-probe.c | 2 ++ >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h >> index c0983130a44c..c613426b0bfc 100644 >> --- a/arch/mips/include/asm/cpu-features.h >> +++ b/arch/mips/include/asm/cpu-features.h >> @@ -118,7 +118,7 @@ >> #define cpu_has_3k_cache __isa_lt_and_opt(1, MIPS_CPU_3K_CACHE) >> #endif >> #ifndef cpu_has_4k_cache >> -#define cpu_has_4k_cache __isa_ge_or_opt(1, MIPS_CPU_4K_CACHE) >> +#define cpu_has_4k_cache __opt(MIPS_CPU_4K_CACHE) > > This breaks compile-time optimization for this feature => hurts > performance on non-Octeon machines. Could this be done some other way? > E.g. could this be defined depending on > CONFIG_SYS_HAS_CPU_OCTEON_WHATEVER? Via its own cpu-features.h or here, > that's the second question. So that the platforms which 100% can't have > this family of CPUs wouldn't get affected. Thanks for the comment. This feature is only checked once during boot and never checked at runtime so I think impact should be negligible. Just don’t want to have another ifdef here :-) Thanks - Jiaxun
From: Jiaxun Yang <jiaxun.yang@flygoat.com> Date: Mon, 6 Mar 2023 19:55:23 +0000 > > >> 2023年3月6日 14:28，Alexander Lobakin <aleksander.lobakin@intel.com> 写道： >> >> From: Jiaxun Yang <jiaxun.yang@flygoat.com> >> Date: Sat, 4 Mar 2023 22:15:16 +0000 >> >>> Octeon has a different cache interface with traditional R4K one, >>> just opt-out this flag for octeon to avoid run R4K cache initialization >>> code accidentally. >>> >>> Also remove ISA level assumption for 4k cache. >>> >>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >>> --- >>> arch/mips/include/asm/cpu-features.h | 2 +- >>> arch/mips/kernel/cpu-probe.c | 2 ++ >>> 2 files changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h >>> index c0983130a44c..c613426b0bfc 100644 >>> --- a/arch/mips/include/asm/cpu-features.h >>> +++ b/arch/mips/include/asm/cpu-features.h >>> @@ -118,7 +118,7 @@ >>> #define cpu_has_3k_cache __isa_lt_and_opt(1, MIPS_CPU_3K_CACHE) >>> #endif >>> #ifndef cpu_has_4k_cache >>> -#define cpu_has_4k_cache __isa_ge_or_opt(1, MIPS_CPU_4K_CACHE) >>> +#define cpu_has_4k_cache __opt(MIPS_CPU_4K_CACHE) >> >> This breaks compile-time optimization for this feature => hurts >> performance on non-Octeon machines. Could this be done some other way? >> E.g. could this be defined depending on >> CONFIG_SYS_HAS_CPU_OCTEON_WHATEVER? Via its own cpu-features.h or here, >> that's the second question. So that the platforms which 100% can't have >> this family of CPUs wouldn't get affected. > > Thanks for the comment. > > This feature is only checked once during boot and never checked at runtime so I think impact > should be negligible. Ah, didn't notice it's not used anywhere on hotpath. Just forgot that MIPS installs cache sync callbacks once at boot and then use them. > > Just don’t want to have another ifdef here :-) I'm not sure one ifdef would hurt, we probably need a third opinion here :D Maybe Thomas'. But I'm fine with the current implementation. > > > Thanks > - Jiaxun Thanks, Olek
diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h index c0983130a44c..c613426b0bfc 100644 --- a/arch/mips/include/asm/cpu-features.h +++ b/arch/mips/include/asm/cpu-features.h @@ -118,7 +118,7 @@ #define cpu_has_3k_cache __isa_lt_and_opt(1, MIPS_CPU_3K_CACHE) #endif #ifndef cpu_has_4k_cache -#define cpu_has_4k_cache __isa_ge_or_opt(1, MIPS_CPU_4K_CACHE) +#define cpu_has_4k_cache __opt(MIPS_CPU_4K_CACHE) #endif #ifndef cpu_has_octeon_cache #define cpu_has_octeon_cache 0 diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c index 7ddf07f255f3..6d15a398d389 100644 --- a/arch/mips/kernel/cpu-probe.c +++ b/arch/mips/kernel/cpu-probe.c @@ -1602,6 +1602,8 @@ static inline void cpu_probe_broadcom(struct cpuinfo_mips *c, unsigned int cpu) static inline void cpu_probe_cavium(struct cpuinfo_mips *c, unsigned int cpu) { decode_configs(c); + /* Octeon has different cache interface */ + c->options &= ~MIPS_CPU_4K_CACHE; switch (c->processor_id & PRID_IMP_MASK) { case PRID_IMP_CAVIUM_CN38XX: case PRID_IMP_CAVIUM_CN31XX:
Octeon has a different cache interface with traditional R4K one, just opt-out this flag for octeon to avoid run R4K cache initialization code accidentally. Also remove ISA level assumption for 4k cache. Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> --- arch/mips/include/asm/cpu-features.h | 2 +- arch/mips/kernel/cpu-probe.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)