Re: [PATCH] MIPS: mm: tlbex: Replace cpu_has_mips_r2_exec_hazard with cpu_has_mips_r2_r6

[Ralf Baechle <ralf@linux-mips.org>]

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@cavium.com>
> >>Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
> >
> >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