mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] MIPS: ingenic: Let Config.VI tell whether the icache is virtual
@ 2026-11-11  4:09 Hunter Laux via B4 Relay
  2026-11-11 16:23 ` Paul Cercueil
  2026-11-11 17:00 ` H. Nikolaus Schaller
  0 siblings, 2 replies; 5+ messages in thread
From: Hunter Laux via B4 Relay @ 2026-11-11  4:09 UTC (permalink / raw)
  To: Paul Cercueil, Thomas Bogendoerfer
  Cc: linux-mips, linux-kernel, Lars-Peter Clausen, Hunter Laux

From: Hunter Laux <hunterlaux@gmail.com>

The Ingenic CPU support treats every XBurst instruction cache as
virtually tagged: the feature overrides fix cpu_has_vtag_icache to 1,
and the probe sets MIPS_CACHE_VTAG. The setting came in with the JZ4740
feature overrides in commit 83ccf69d8f11 ("MIPS: JZ4740: Add base
support for Ingenic JZ4740 System-on-a-Chip") without a stated reason,
and Ingenic's own kernels treat the icache as physically tagged.

Neither XBurst1 nor XBurst II has a virtually tagged icache. Both
programming manuals describe a physically tagged one and hardwire
Config.VI to 0, which the cache probe already reads. On the X2600, code
at a virtual address remapped to another page through the TLB alone
runs the new page, while the old line is still in the icache. XBurst1
has not been tested. Treating the icache as virtually tagged is
incorrect and costs needless icache flushes.

Drop both and leave it to Config.VI.

Cc: Lars-Peter Clausen <lars@metafoo.de>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Hunter Laux <hunterlaux@gmail.com>
---
Lars, the override came with your JZ4740 support in 2010. On the off
chance you remember a reason for setting cpu_has_vtag_icache to 1,
I'd like to hear it.
---
 arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h | 1 -
 arch/mips/kernel/cpu-probe.c                               | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h b/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
index 7ace50127f..71c03f1dd4 100644
--- a/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
+++ b/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
@@ -27,7 +27,6 @@
 #define cpu_has_mips3d 0
 #define cpu_has_smartmips 0
 #define kernel_uses_llsc	1
-#define cpu_has_vtag_icache	1
 #define cpu_has_dc_aliases	0
 #define cpu_has_ic_fills_f_dc	0
 #define cpu_has_pindexed_dcache 0
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index 489612ed9d..bef92e6cd6 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -1763,9 +1763,6 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu)
 	c->options &= ~MIPS_CPU_COUNTER;
 	BUG_ON(__builtin_constant_p(cpu_has_counter) && cpu_has_counter);
 
-	/* XBurst has virtually tagged icache */
-	c->icache.flags |= MIPS_CACHE_VTAG;
-
 	switch (c->processor_id & PRID_IMP_MASK) {
 
 	/* XBurst®1 with MXU1.0/MXU1.1 SIMD ISA */

---
base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
change-id: 20261110-mips-xburst-icache-vi-db10a8d03dd6

Best regards,
-- 
Hunter Laux <hunterlaux@gmail.com>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: ingenic: Let Config.VI tell whether the icache is virtual
  2026-11-11  4:09 [PATCH] MIPS: ingenic: Let Config.VI tell whether the icache is virtual Hunter Laux via B4 Relay
@ 2026-11-11 16:23 ` Paul Cercueil
  2026-11-11 19:56   ` Hunter Laux
  2026-11-11 17:00 ` H. Nikolaus Schaller
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Cercueil @ 2026-11-11 16:23 UTC (permalink / raw)
  To: hunterlaux, Thomas Bogendoerfer
  Cc: linux-mips, linux-kernel, Lars-Peter Clausen

Hi Hunter,

Le mardi 10 novembre 2026 à 21:09 -0700, Hunter Laux via B4 Relay a écrit :
> From: Hunter Laux <hunterlaux@gmail.com>
> 
> The Ingenic CPU support treats every XBurst instruction cache as
> virtually tagged: the feature overrides fix cpu_has_vtag_icache to 1,
> and the probe sets MIPS_CACHE_VTAG. The setting came in with the JZ4740
> feature overrides in commit 83ccf69d8f11 ("MIPS: JZ4740: Add base
> support for Ingenic JZ4740 System-on-a-Chip") without a stated reason,
> and Ingenic's own kernels treat the icache as physically tagged.
> 
> Neither XBurst1 nor XBurst II has a virtually tagged icache. Both
> programming manuals describe a physically tagged one and hardwire
> Config.VI to 0, which the cache probe already reads. On the X2600, code
> at a virtual address remapped to another page through the TLB alone
> runs the new page, while the old line is still in the icache. XBurst1
> has not been tested. Treating the icache as virtually tagged is
> incorrect and costs needless icache flushes.
> 
> Drop both and leave it to Config.VI.
> 
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Hunter Laux <hunterlaux@gmail.com>

The docs I have do say that both caches are "virtually indexed, physically
tagged". So the patch looks good.

If anything it's missing a Fixes: tag (with stable@vger.kernel.org in Cc).

> ---
> Lars, the override came with your JZ4740 support in 2010. On the off
> chance you remember a reason for setting cpu_has_vtag_icache to 1,
> I'd like to hear it.
> ---
>  arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h | 1 -
>  arch/mips/kernel/cpu-probe.c                               | 3 ---
>  2 files changed, 4 deletions(-)
> 
> diff --git a/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
> b/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
> index 7ace50127f..71c03f1dd4 100644
> --- a/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
> +++ b/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
> @@ -27,7 +27,6 @@
>  #define cpu_has_mips3d 0
>  #define cpu_has_smartmips 0
>  #define kernel_uses_llsc	1
> -#define cpu_has_vtag_icache	1

Don't remove it, set it to 0.

Cheers,
-Paul

>  #define cpu_has_dc_aliases	0
>  #define cpu_has_ic_fills_f_dc	0
>  #define cpu_has_pindexed_dcache 0
> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> index 489612ed9d..bef92e6cd6 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -1763,9 +1763,6 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips
> *c, unsigned int cpu)
>  	c->options &= ~MIPS_CPU_COUNTER;
>  	BUG_ON(__builtin_constant_p(cpu_has_counter) && cpu_has_counter);
>  
> -	/* XBurst has virtually tagged icache */
> -	c->icache.flags |= MIPS_CACHE_VTAG;
> -
>  	switch (c->processor_id & PRID_IMP_MASK) {
>  
>  	/* XBurst®1 with MXU1.0/MXU1.1 SIMD ISA */
> 
> ---
> base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
> change-id: 20261110-mips-xburst-icache-vi-db10a8d03dd6
> 
> Best regards,

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: ingenic: Let Config.VI tell whether the icache is virtual
  2026-11-11  4:09 [PATCH] MIPS: ingenic: Let Config.VI tell whether the icache is virtual Hunter Laux via B4 Relay
  2026-11-11 16:23 ` Paul Cercueil
@ 2026-11-11 17:00 ` H. Nikolaus Schaller
  2026-11-11 19:56   ` Hunter Laux
  1 sibling, 1 reply; 5+ messages in thread
From: H. Nikolaus Schaller @ 2026-11-11 17:00 UTC (permalink / raw)
  To: hunterlaux, Paul Cercueil
  Cc: Thomas Bogendoerfer, linux-mips, linux-kernel,
	Lars-Peter Clausen, Paul Boddie,
	Discussions about the Letux Kernel

Hi,

> Am 11.11.2026 um 05:09 schrieb Hunter Laux via B4 Relay <devnull+hunterlaux.gmail.com@kernel.org>:
> 
> From: Hunter Laux <hunterlaux@gmail.com>
> 
> The Ingenic CPU support treats every XBurst instruction cache as
> virtually tagged: the feature overrides fix cpu_has_vtag_icache to 1,
> and the probe sets MIPS_CACHE_VTAG. The setting came in with the JZ4740
> feature overrides in commit 83ccf69d8f11 ("MIPS: JZ4740: Add base
> support for Ingenic JZ4740 System-on-a-Chip") without a stated reason,
> and Ingenic's own kernels treat the icache as physically tagged.
> 
> Neither XBurst1 nor XBurst II has a virtually tagged icache. Both
> programming manuals describe a physically tagged one and hardwire
> Config.VI to 0, which the cache probe already reads. On the X2600, code
> at a virtual address remapped to another page through the TLB alone
> runs the new page, while the old line is still in the icache. XBurst1
> has not been tested. Treating the icache as virtually tagged is
> incorrect and costs needless icache flushes.
> 
> Drop both and leave it to Config.VI.
> 
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Hunter Laux <hunterlaux@gmail.com>
> ---
> Lars, the override came with your JZ4740 support in 2010. On the off
> chance you remember a reason for setting cpu_has_vtag_icache to 1,
> I'd like to hear it.
> ---
> arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h | 1 -
> arch/mips/kernel/cpu-probe.c                               | 3 ---
> 2 files changed, 4 deletions(-)
> 
> diff --git a/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h b/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
> index 7ace50127f..71c03f1dd4 100644
> --- a/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
> +++ b/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
> @@ -27,7 +27,6 @@
> #define cpu_has_mips3d 0
> #define cpu_has_smartmips 0
> #define kernel_uses_llsc 1
> -#define cpu_has_vtag_icache 1
> #define cpu_has_dc_aliases 0
> #define cpu_has_ic_fills_f_dc 0
> #define cpu_has_pindexed_dcache 0
> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> index 489612ed9d..bef92e6cd6 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -1763,9 +1763,6 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu)
> c->options &= ~MIPS_CPU_COUNTER;
> BUG_ON(__builtin_constant_p(cpu_has_counter) && cpu_has_counter);
> 
> - /* XBurst has virtually tagged icache */
> - c->icache.flags |= MIPS_CACHE_VTAG;
> -
> switch (c->processor_id & PRID_IMP_MASK) {
> 
> /* XBurst®1 with MXU1.0/MXU1.1 SIMD ISA */
> 
> ---
> base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
> change-id: 20261110-mips-xburst-icache-vi-db10a8d03dd6
> 
> Best regards,
> -- 
> Hunter Laux <hunterlaux@gmail.com>

it appears that this looks quite similar to what had been sent to us by 周琰杰 (Zhou Yanjie)
5 years ago for testing and we integrated it into our out-of-tree kernel but it never
arrived upstream (attached as plain text just for review).

So it is likely the right way to go to solve such an old bug if independently suggested
and did work for us for such a long time...

Please compare and verify, as I am not an expert for MIPS cache.

BR and thanks,
Nikolaus


See also: https://github.com/goldelico/letux-kernel/commit/ff0761012124d

commit ff0761012124d0d06a9fc01c644c52399b7dc1f4
Author: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
Date:   Tue Jun 22 23:20:22 2021 +0800

   MIPS: Ingenic: Fix the wrong cpu features.
        1.Ingenic SoCs have never had a instruction cache with virtual tag,
     so the "cpu_has_vtag_icache" in cpu-feature-overrides.h should be
     0, and the flag in cpu-probe.c should also be removed.
   2.Ingenic XBurst®1 CPU does not implement the UserLocal register but
     XBurst®2 CPU does, so remove the "#define cpu_has_userlocal 0" and
     detect whether the processor supports UserLocal register through
     "cpu_probe_ingenic()".
        Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>

diff --git a/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h b/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
index 7ace50127f5aa..24d6157ed6da6 100644
--- a/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
+++ b/arch/mips/include/asm/mach-ingenic/cpu-feature-overrides.h
@@ -27,7 +27,7 @@
#define cpu_has_mips3d 0
#define cpu_has_smartmips 0
#define kernel_uses_llsc       1
-#define cpu_has_vtag_icache    1
+#define cpu_has_vtag_icache    0
#define cpu_has_dc_aliases     0
#define cpu_has_ic_fills_f_dc  0
#define cpu_has_pindexed_dcache 0
@@ -37,7 +37,6 @@
#define cpu_has_dsp            0
#define cpu_has_dsp2           0
#define cpu_has_mipsmt         0
-#define cpu_has_userlocal      0
#define cpu_has_nofpuex 0
#define cpu_has_64bits         0
#define cpu_has_64bit_zero_reg 0
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index 489612ed9d498..bef92e6cd6950 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -1763,9 +1763,6 @@ static inline void cpu_probe_ingenic(struct cpuinfo_mips *c, unsigned int cpu)
       c->options &= ~MIPS_CPU_COUNTER;
       BUG_ON(__builtin_constant_p(cpu_has_counter) && cpu_has_counter);
-       /* XBurst has virtually tagged icache */
-       c->icache.flags |= MIPS_CACHE_VTAG;
-
       switch (c->processor_id & PRID_IMP_MASK) {
         /* XBurst®1 with MXU1.0/MXU1.1 SIMD ISA */


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: ingenic: Let Config.VI tell whether the icache is virtual
  2026-11-11 16:23 ` Paul Cercueil
@ 2026-11-11 19:56   ` Hunter Laux
  0 siblings, 0 replies; 5+ messages in thread
From: Hunter Laux @ 2026-11-11 19:56 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Thomas Bogendoerfer, linux-mips, linux-kernel,
	Lars-Peter Clausen, H. Nikolaus Schaller

Hi Paul,

On Wed, 11 Nov 2026 17:23:31 +0100, Paul Cercueil wrote:
> The docs I have do say that both caches are "virtually indexed, physically
> tagged". So the patch looks good.

Thanks for checking.

> If anything it's missing a Fixes: tag (with stable@vger.kernel.org in Cc).

I don't think it needs to go to stable. The vtag setting doesn't cause
wrong behaviour, only extra icache flushes. I can add a Fixes: tag
without Cc: stable if you'd like the origin recorded.

>> -#define cpu_has_vtag_icache	1
>
> Don't remove it, set it to 0.

The Config.VI probe already reports it correctly, so I left it to the
probe. Would you prefer the constant anyway?

Hunter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: ingenic: Let Config.VI tell whether the icache is virtual
  2026-11-11 17:00 ` H. Nikolaus Schaller
@ 2026-11-11 19:56   ` Hunter Laux
  0 siblings, 0 replies; 5+ messages in thread
From: Hunter Laux @ 2026-11-11 19:56 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Paul Cercueil, Thomas Bogendoerfer, linux-mips, linux-kernel,
	Lars-Peter Clausen, Paul Boddie,
	Discussions about the Letux Kernel

Hi Nikolaus,

On Wed, 11 Nov 2026 18:00:47 +0100, H. Nikolaus Schaller wrote:
> it appears that this looks quite similar to what had been sent to us by
> Zhou Yanjie 5 years ago for testing and we integrated it into our
> out-of-tree kernel but it never arrived upstream

Thanks, that's the same change, and good to know it has run in Letux
for that long. v2 will credit Zhou's patch and link the Letux commit.

The other half of that commit, the UserLocal part, is needed too:
XBurst II implements UserLocal, and the override hides it. I have a
patch for it on my list, but I'm still looking into it.

Hunter

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-11 19:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-11  4:09 [PATCH] MIPS: ingenic: Let Config.VI tell whether the icache is virtual Hunter Laux via B4 Relay
2026-11-11 16:23 ` Paul Cercueil
2026-11-11 19:56   ` Hunter Laux
2026-11-11 17:00 ` H. Nikolaus Schaller
2026-11-11 19:56   ` Hunter Laux

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®