Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras <Markos.Chandras@imgtec.com>]

On 01/26/2015 11:36 AM, James Hogan wrote:

> 
>> +		raw_current_cpu_data.htw_seq++;				\
> 
> not "if (!raw_current_cpu_data.htw_seq++)) {"?
Why?

on _stop() calls you just increment it. The _start() will do the right
thing then.

I think what you suggest it to move the if() condition from the _start()
to _stop().

> 
>>  		write_c0_pwctl(read_c0_pwctl() &			\
>>  			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
>>  		back_to_back_c0_hazard();				\
>> +		local_irq_restore(flags);				\
>>  	}								\
>>  } while(0)
>>  
>>  #define htw_start()							\
>>  do {									\
>> +	unsigned long flags;						\
>> +									\
>>  	if (cpu_has_htw) {						\
>> -		write_c0_pwctl(read_c0_pwctl() |			\
>> -			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
>> -		back_to_back_c0_hazard();				\
>> +		local_irq_save(flags);					\
>> +		if (!--raw_current_cpu_data.htw_seq) {			\
>> +			write_c0_pwctl(read_c0_pwctl() |		\
>> +				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
>> +			back_to_back_c0_hazard();			\
>> +		}							\
>> +		local_irq_restore(flags);				\
>>  	}								\
>>  } while(0)
>>  
>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>> index dc49cf30c2db..5d6e59f20750 100644
>> --- a/arch/mips/kernel/cpu-probe.c
>> +++ b/arch/mips/kernel/cpu-probe.c
>> @@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
>>  	if (config3 & MIPS_CONF3_MSA)
>>  		c->ases |= MIPS_ASE_MSA;
>>  	/* Only tested on 32-bit cores */
>> -	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
>> +	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
>> +		c->htw_seq = 0;
> 
> is that necessary, since cpu_data[] is global?

I checked and it is not placed in the .data instead of .bss section so i
was not sure if it is initialized properly.

-- 
markos