On 03/08/2017 08:16 AM, Petar Jovanovic wrote:
Hi David,
Cavium overrides [1] cpu_has_mips32r1, cpu_has_mips32r2,
cpu_has_mips64r1 with zeros in kernel.
#define cpu_has_mips32r1 0
#define cpu_has_mips32r2 0
#define cpu_has_mips64r1 0
#define cpu_has_mips64r2 1
Is there a reason to do this, and can we change that to '1' like it is
done, for instance, for Netlogic XLP [2]?
I have looked at the linux mailing list when those changes [3] were
added and I have not seen explanations for it (I might have missed it if
it was in a different thread though).
That was added with:
commit a96102be700f ("MIPS: Add printing of ISA version in cpuinfo.")
The problem with this whole thing is that it conflates two different things:
1) Which instructions can be used in userspace programs.
2) What CP0 features are available to the kernel, and how to best
optimize kernel exception handling.
When we initially wrote
arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h there
were sites in the kernel where setting "r1" or "mips32" was the wrong
thing to do for OCTEON. There is actually a reason that we did these
overrides, although I cannot recall exactly what the issues were.
The reason why I am bringing this to you is that some userspace software
relies on what gets written after "isa\t\t\t: " in /proc/cpuinfo.
See show_cpuinfo() in kernel code [4].
As support for MIPS32R1/R2 and MIPS64R1 is not exposed on Cavium boards,
this will incorrectly lead software to believe the boards do not
support these ISAs, and may prevent code to be executed or raise
exceptions. One example for that would be Valgrind [5].
A change like this one:
--- a/arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h
+++ b/arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h
@@ -46,9 +46,9 @@
#define cpu_has_64bits 1
#define cpu_has_octeon_cache 1
#define cpu_has_saa octeon_has_saa()
-#define cpu_has_mips32r1 0
-#define cpu_has_mips32r2 0
-#define cpu_has_mips64r1 0
+#define cpu_has_mips32r1 1
+#define cpu_has_mips32r2 1
+#define cpu_has_mips64r1 1
#define cpu_has_mips64r2 1
#define cpu_has_dsp 0
#define cpu_has_dsp2 0
Really, I think you need to cleanly separate the concepts of userspace
instruction availability and CP0 structure.
At a minimum, you should audit all the usage sites reported by:
$ git grep cpu_has_mips
to see if anything would change with your suggested patch.
would be sufficient to solve issues like that one.
Let me know what you think. Thanks.
Regards,
Petar
[1] ./arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h
[2] ./arch/mips/include/asm/mach-netlogic/cpu-feature-overrides.h
[3] https://www.linux-mips.org/archives/linux-mips/2008-12/msg00185.html
[4] ./arch/mips/kernel/proc.c
[5] http://valgrind.org/
