On Fri, Mar 13, 2015 at 08:46:03AM -0700, David Daney wrote:
> >>Commit 77f3ee59ee7cf ("MIPS: mm: tlbex: Use cpu_has_mips_r2_exec_hazard
> >>for the EHB instruction") replaced cpu_has_mips_r2 with
> >>cpu_has_mips_r2_exec_hazard to indicate whether the ISA has the EHB
> >>instruction. However, the meaning of the cpu_has_mips_r2_exec_hazard
> >>is different. It was meant to be used as an indication on whether the
> >>running processor needs to run the EHB instruction instead of checking
> >>whether the EHB is available on the ISA. This broke processors that do
> >>not define cpu_has_mips_r2_exec_hazard. We fix this by replacing the
> >>said macro with cpu_has_mips_r2_r6 which covers R2 and R6 processors.
> >>
> >>Fixes: 77f3ee59ee7cf ("MIPS: mm: tlbex: Use cpu_has_mips_r2_exec_hazard for the EHB instruction")
> >>Cc: David Daney <david.daney@xxxxxxxxxx>
> >>Signed-off-by: Markos Chandras <markos.chandras@xxxxxxxxxx>
> >
> >Either of this David's revert or this patch applied will leave
> >cpu_has_mips_r2_exec_hazard unused which at a glance doesn't seem to
> >be right and defeats David's old patches 9e290a19 / 41f0e4d0 from working.
> >
> >cpu_has_mips_r2_exec_hazard was made unused by 625c0a21 which I think
> >should be reverted and cpu_has_mips_r2_exec_hazard be defined to be something
> >like
> >
> >#define cpu_has_mips_r2_exec_hazard \
> >({ \
> > int __res; \
> > \
> > switch (current_cpu_type()) { \
> > case CPU_M14KC: \
> > case CPU74K: \
> > case CPU_CAVIUM_OCTEON: \
> > case CPU_CAVIUM_OCTEON_PLUS: \
> > case CPU_CAVIUM_OCTEON2: \
> > case CPU_CAVIUM_OCTEON3: \
>
> The four octeon models are already covered in
> arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h
>
> > __res = 0; \
> > break; \
> > \
> > default: \
> > __res = 1; \
> > } \
> > \
> > __res; \
> >})
> >
> >?
> >
>
> Something like that is needed somewhere
>
> I would suggest having the default definition contain some generalizations
> about where it should return true, and
> arch/mips/include/asm/mach-*/cpu-feature-overrides.h isolate the specific
> models for each sub-architecture.
In the past we had this very simple kind of constructs like
#ifndef cpu_has_weird_stuff
#define cpu_has_weird_stuff (current_cpu_data()->feature_flags & WEIRD_FLAG)
#endif
plus the platform-specific overrides because GCC wasn't able to properly
optimize something like my suggested definition for
cpu_has_mips_r2_exec_hazard. Since a while GCC howeer is capable of
optimizing that sort of constructs eleminating dead case statements and
so I think we're probably better off eleminating the per-platform
overrides definitions where we can.
Ralf
