[PATCH] Revert "MIPS: mm: tlbex: Use cpu_has_mips_r2_exec_hazard for the EHB instruction"

[PATCH] Revert "MIPS: mm: tlbex: Use cpu_has_mips_r2_exec_hazard for the EHB instruction"

[David Daney]

From: David Daney <david.daney@cavium.com>

This reverts commit 77f3ee59ee7cfe19e0ee48d9a990c7967fbfcbed.

There are two problems:

1) It breaks OCTEON, which will now crash in early boot with:

  Kernel panic - not syncing: No TLB refill handler yet (CPU type: 80)

2) The logic is broken.

The meaning of cpu_has_mips_r2_exec_hazard is that the EHB instruction
is required.  The offending patch attempts (and fails) to change the
meaning to be that EHB is part of the ISA.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 arch/mips/mm/tlbex.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index d75ff73..ff8d99c 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -501,7 +501,7 @@ static void build_tlb_write_entry(u32 **p, struct uasm_label **l,
 	case tlb_indexed: tlbw = uasm_i_tlbwi; break;
 	}
 
-	if (cpu_has_mips_r2_exec_hazard) {
+	if (cpu_has_mips_r2) {
 		/*
 		 * The architecture spec says an ehb is required here,
 		 * but a number of cores do not have the hazard and
@@ -1953,7 +1953,7 @@ static void build_r4000_tlb_load_handler(void)
 
 		switch (current_cpu_type()) {
 		default:
-			if (cpu_has_mips_r2_exec_hazard) {
+			if (cpu_has_mips_r2) {
 				uasm_i_ehb(&p);
 
 		case CPU_CAVIUM_OCTEON:
@@ -2020,7 +2020,7 @@ static void build_r4000_tlb_load_handler(void)
 
 		switch (current_cpu_type()) {
 		default:
-			if (cpu_has_mips_r2_exec_hazard) {
+			if (cpu_has_mips_r2) {
 				uasm_i_ehb(&p);
 
 		case CPU_CAVIUM_OCTEON:
-- 
1.7.11.7

Re: [PATCH] Revert "MIPS: mm: tlbex: Use cpu_has_mips_r2_exec_hazard for the EHB instruction"

[Maciej W. Rozycki]

On Mon, 23 Feb 2015, David Daney wrote:

> From: David Daney <david.daney@cavium.com>
> 
> This reverts commit 77f3ee59ee7cfe19e0ee48d9a990c7967fbfcbed.
> 
> There are two problems:
> 
> 1) It breaks OCTEON, which will now crash in early boot with:
> 
>   Kernel panic - not syncing: No TLB refill handler yet (CPU type: 80)
> 
> 2) The logic is broken.
> 
> The meaning of cpu_has_mips_r2_exec_hazard is that the EHB instruction
> is required.  The offending patch attempts (and fails) to change the
> meaning to be that EHB is part of the ISA.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---

 Code affected will have to be reconsidered including possibly older 
changes as well.  Meanwhile, to revert the immediate regression, you have 
my:

Reviewed-by: Maciej W. Rozycki <macro@linux-mips.org>

Next time please try to use the imperative mood for the commit message 
though, as per Documentation/SubmittingPatches.

 Overall I think it makes sense to have a look back there every once in a 
while to avoid getting trapped in routine.  Some breakage we fall into 
from time to time results from missing the guidelines set there, sigh.

  Maciej

Re: [PATCH] Revert "MIPS: mm: tlbex: Use cpu_has_mips_r2_exec_hazard for the EHB instruction"

[Markos Chandras]

On 02/23/2015 10:52 PM, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> This reverts commit 77f3ee59ee7cfe19e0ee48d9a990c7967fbfcbed.
> 
> There are two problems:
> 
> 1) It breaks OCTEON, which will now crash in early boot with:
> 
>   Kernel panic - not syncing: No TLB refill handler yet (CPU type: 80)
> 
> 2) The logic is broken.
> 
> The meaning of cpu_has_mips_r2_exec_hazard is that the EHB instruction
> is required.  The offending patch attempts (and fails) to change the
> meaning to be that EHB is part of the ISA.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
Hi,

First of all sorry about the octeon breakage.

However, whilst this patch will fix Octeon it will break R6

Can we please consider another patch that will simply use
cpu_has_mips_r2_r6 instead of cpu_has_mips_r2 so both will work in 4.0?

-- 
markos

Re: [PATCH] Revert "MIPS: mm: tlbex: Use cpu_has_mips_r2_exec_hazard for the EHB instruction"

[Markos Chandras]

On 02/23/2015 10:52 PM, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> This reverts commit 77f3ee59ee7cfe19e0ee48d9a990c7967fbfcbed.
> 
> There are two problems:
> 
> 1) It breaks OCTEON, which will now crash in early boot with:
> 
>   Kernel panic - not syncing: No TLB refill handler yet (CPU type: 80)
> 
> 2) The logic is broken.
> 
> The meaning of cpu_has_mips_r2_exec_hazard is that the EHB instruction
> is required.  The offending patch attempts (and fails) to change the
> meaning to be that EHB is part of the ISA.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
Hi,

First of all sorry about the octeon breakage.

However, whilst this patch will fix Octeon it will break R6

Can we please consider another patch that will simply use
cpu_has_mips_r2_r6 instead of cpu_has_mips_r2 so both will work in 4.0?

-- 
markos

Re: [PATCH] Revert "MIPS: mm: tlbex: Use cpu_has_mips_r2_exec_hazard for the EHB instruction"

[David Daney]

On 03/11/2015 01:28 AM, Markos Chandras wrote:
> On 02/23/2015 10:52 PM, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> This reverts commit 77f3ee59ee7cfe19e0ee48d9a990c7967fbfcbed.
>>
>> There are two problems:
>>
>> 1) It breaks OCTEON, which will now crash in early boot with:
>>
>>    Kernel panic - not syncing: No TLB refill handler yet (CPU type: 80)
>>
>> 2) The logic is broken.
>>
>> The meaning of cpu_has_mips_r2_exec_hazard is that the EHB instruction
>> is required.  The offending patch attempts (and fails) to change the
>> meaning to be that EHB is part of the ISA.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
> Hi,
>
> First of all sorry about the octeon breakage.
>
> However, whilst this patch will fix Octeon it will break R6
>

But breaking R6 is not a regression, breaking OCTEON is.  For new code, 
there is this bit of asymmetry.

> Can we please consider another patch that will simply use
> cpu_has_mips_r2_r6 instead of cpu_has_mips_r2 so both will work in 4.0?
>

If you have a patch that fixes the problem properly, please post it for 
consideration.

Thanks,
David Daney

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

[Markos Chandras]

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>
---
 arch/mips/mm/tlbex.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index d75ff73a2012..e38d21b62d0f 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -501,7 +501,7 @@ static void build_tlb_write_entry(u32 **p, struct uasm_label **l,
 	case tlb_indexed: tlbw = uasm_i_tlbwi; break;
 	}
 
-	if (cpu_has_mips_r2_exec_hazard) {
+	if (cpu_has_mips_r2_r6) {
 		/*
 		 * The architecture spec says an ehb is required here,
 		 * but a number of cores do not have the hazard and
@@ -1953,7 +1953,7 @@ static void build_r4000_tlb_load_handler(void)
 
 		switch (current_cpu_type()) {
 		default:
-			if (cpu_has_mips_r2_exec_hazard) {
+			if (cpu_has_mips_r2_r6) {
 				uasm_i_ehb(&p);
 
 		case CPU_CAVIUM_OCTEON:
@@ -2020,7 +2020,7 @@ static void build_r4000_tlb_load_handler(void)
 
 		switch (current_cpu_type()) {
 		default:
-			if (cpu_has_mips_r2_exec_hazard) {
+			if (cpu_has_mips_r2_r6) {
 				uasm_i_ehb(&p);
 
 		case CPU_CAVIUM_OCTEON:
-- 
2.3.1

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

[Markos Chandras]

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>
---
 arch/mips/mm/tlbex.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index d75ff73a2012..e38d21b62d0f 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -501,7 +501,7 @@ static void build_tlb_write_entry(u32 **p, struct uasm_label **l,
 	case tlb_indexed: tlbw = uasm_i_tlbwi; break;
 	}
 
-	if (cpu_has_mips_r2_exec_hazard) {
+	if (cpu_has_mips_r2_r6) {
 		/*
 		 * The architecture spec says an ehb is required here,
 		 * but a number of cores do not have the hazard and
@@ -1953,7 +1953,7 @@ static void build_r4000_tlb_load_handler(void)
 
 		switch (current_cpu_type()) {
 		default:
-			if (cpu_has_mips_r2_exec_hazard) {
+			if (cpu_has_mips_r2_r6) {
 				uasm_i_ehb(&p);
 
 		case CPU_CAVIUM_OCTEON:
@@ -2020,7 +2020,7 @@ static void build_r4000_tlb_load_handler(void)
 
 		switch (current_cpu_type()) {
 		default:
-			if (cpu_has_mips_r2_exec_hazard) {
+			if (cpu_has_mips_r2_r6) {
 				uasm_i_ehb(&p);
 
 		case CPU_CAVIUM_OCTEON:
-- 
2.3.1

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

[Ralf Baechle]

On Fri, Mar 13, 2015 at 09:18:08AM +0000, Markos Chandras 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:					\
		__res = 0;						\
		break;							\
									\
	default:							\
		__res = 1;						\
	}								\
									\
	__res;								\
})

?

  Ralf

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

[Markos Chandras]

Hi,

On 03/13/2015 02:44 PM, Ralf Baechle wrote:
> On Fri, Mar 13, 2015 at 09:18:08AM +0000, Markos Chandras 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

Indeed that looks an old regression. Thanks for spotting that.

> 
> #define cpu_has_mips_r2_exec_hazard					\
> ({									\
> 	int __res;							\
> 									\
> 	switch (current_cpu_type()) {					\
> 	case CPU_M14KC:							\
> 	case CPU74K:							\

More MIPS/IMG cores need to be added here but ok

> 	case CPU_CAVIUM_OCTEON:						\
> 	case CPU_CAVIUM_OCTEON_PLUS:					\
>         case CPU_CAVIUM_OCTEON2:					\
> 	case CPU_CAVIUM_OCTEON3:					\
> 		__res = 0;						\
> 		break;							\
> 									\
> 	default:							\
> 		__res = 1;						\
> 	}								\
> 									\
> 	__res;								\
> })
> 
> ?

Overall, that looks like a good solution to me

-- 
markos

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

[Markos Chandras]

Hi,

On 03/13/2015 02:44 PM, Ralf Baechle wrote:
> On Fri, Mar 13, 2015 at 09:18:08AM +0000, Markos Chandras 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

Indeed that looks an old regression. Thanks for spotting that.

> 
> #define cpu_has_mips_r2_exec_hazard					\
> ({									\
> 	int __res;							\
> 									\
> 	switch (current_cpu_type()) {					\
> 	case CPU_M14KC:							\
> 	case CPU74K:							\

More MIPS/IMG cores need to be added here but ok

> 	case CPU_CAVIUM_OCTEON:						\
> 	case CPU_CAVIUM_OCTEON_PLUS:					\
>         case CPU_CAVIUM_OCTEON2:					\
> 	case CPU_CAVIUM_OCTEON3:					\
> 		__res = 0;						\
> 		break;							\
> 									\
> 	default:							\
> 		__res = 1;						\
> 	}								\
> 									\
> 	__res;								\
> })
> 
> ?

Overall, that looks like a good solution to me

-- 
markos

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

[Maciej W. Rozycki]

On Fri, 13 Mar 2015, Markos Chandras wrote:

> > 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
> 
> Indeed that looks an old regression. Thanks for spotting that.

 Noted here:

http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=alpine.LFD.2.11.1502240011420.17311%40eddie.linux-mips.org

several weeks ago already.

  Maciej

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

[David Daney]

On 03/13/2015 07:44 AM, Ralf Baechle wrote:
> On Fri, Mar 13, 2015 at 09:18:08AM +0000, Markos Chandras 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.

David Daney

>    Ralf
>

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

[David Daney]

On 03/13/2015 07:44 AM, Ralf Baechle wrote:
> On Fri, Mar 13, 2015 at 09:18:08AM +0000, Markos Chandras 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.

David Daney

>    Ralf
>

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

[Ralf Baechle]

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