On 07/07/2017 08:04 AM, Maciej W. Rozycki wrote:
On Fri, 7 Jul 2017, Petar Jovanovic wrote:
As I said earlier in the thread, "in the current ToT, I have not seen
where this change would affect apart from show_cpuinfo()"[1]. So, if
someone implements Octeon-specific controls, where this should be used?
Right, `egrep -r 'cpu_has_(mips_r1|mips32r1|mips64r1|mips32r2)' arch/mips'
does not show anything else indeed. Please make it unambiguous in the
patch description then, i.e. that there is no functional change beyond
reporting in `show_cpuinfo'.
I am not aware of the places where Octeon (ab)uses it in the current
kernel code. David says he "cannot recall exactly what the issues
were" [2].
In my recent review of the code while working the the eBPF JIT, I tried
to audit the use of cpu_has_* as it relates to OCTEON. My current
thoughts are that there is no reason not to merge Petar's patch.
Ralf, please add ...
Acked-by: David Daney <david.daney@xxxxxxxxxx>
Sorry for the pain here.
Thanks for the pointer as this message has not been recorded in
patchwork due to its changed `Subject'.
I disagree with David there. The intended use for generic ISA level
controls is the same between userland and the kernel. That is say
`cpu_has_mips_r2' can be used to conditionally turn on a piece of code
that uses a MIPSr2 mandatory architectural feature, such as the ROTR
instruction or the CP0.EBase register.
Anything that is allowed to vary between implementations, be it the
availability of a feature or a choice made between possible solutions
for optimisation reasons, has to use a separate control, either the CPU
identifier or a dedicated `cpu_<foo>' setting, like we do with cache
controls for example.
Maciej
