Re: [PATCH v10 06/28] x86/fred: Export this_cpu_fred_rsp() for KVM usage

[Sohil Mehta]

> 
> Two nits below.
> 
> [...]
> 
>>  
>> +enum fred_stack_level {
>> +	FRED_STACK_LEVEL_0,
>> +	FRED_STACK_LEVEL_1,
>> +	FRED_STACK_LEVEL_2,
>> +	FRED_STACK_LEVEL_3
>> +};
>> +
> 
> [...]
> 
>>  
>> +unsigned long this_cpu_fred_rsp(enum fred_stack_level lvl)
>> +{
>> +	switch (lvl) {
>> +	case FRED_STACK_LEVEL_0:
>> +		return __this_cpu_read(fred_rsp0);
>> +	case FRED_STACK_LEVEL_1:
>> +		return __this_cpu_ist_top_va(ESTACK_DB);
>> +	case FRED_STACK_LEVEL_2:
>> +		return __this_cpu_ist_top_va(ESTACK_NMI);
> 
> Nit:
> How about adding a comment to state NMI and MC share this one? 
> 

Sure, I'll add one.

> 
> Also, currently, FRED_{DB,NMI,MC,DF}_STACK_LEVEL are defined as
> number, may be it can be defined using the new added 
> FRED_STACK_LEVEL_{1,2,3}
> 

Good point. Something like this?

#define FRED_DB_STACK_LEVEL	FRED_STACK_LEVEL_1
#define FRED_NMI_STACK_LEVEL	FRED_STACK_LEVEL_2
#define FRED_MC_STACK_LEVEL	FRED_STACK_LEVEL_2
#define FRED_DF_STACK_LEVEL	FRED_STACK_LEVEL_3

By itself that leads to a compile issue with the macro FRED_STKLVL()
which needs the following change.

-#define FRED_STKLVL(vector, lvl)	((lvl) << (2 * (vector)))
+#define FRED_STKLVL(vector, lvl)	(_AT(unsigned long, lvl) << (2 *
(vector)))

It seems reasonable to me.

>> +	case FRED_STACK_LEVEL_3:
>> +		return __this_cpu_ist_top_va(ESTACK_DF);
>> +	default:
>> +		BUG();
>> +	}
>> +}
>> +EXPORT_SYMBOL_FOR_KVM(this_cpu_fred_rsp);
>> +
>>  /* Must be called after setup_cpu_entry_areas() */
>>  void cpu_init_fred_rsps(void)
>>  {
>> @@ -84,7 +102,7 @@ void cpu_init_fred_rsps(void)
>>  	       FRED_STKLVL(X86_TRAP_DF,  FRED_DF_STACK_LEVEL));
>>  
>>  	/* The FRED equivalents to IST stacks... */
>> -	wrmsrq(MSR_IA32_FRED_RSP1, __this_cpu_ist_top_va(ESTACK_DB));
>> -	wrmsrq(MSR_IA32_FRED_RSP2, __this_cpu_ist_top_va(ESTACK_NMI));
>> -	wrmsrq(MSR_IA32_FRED_RSP3, __this_cpu_ist_top_va(ESTACK_DF));
>> +	wrmsrq(MSR_IA32_FRED_RSP1, this_cpu_fred_rsp(FRED_STACK_LEVEL_1));
>> +	wrmsrq(MSR_IA32_FRED_RSP2, this_cpu_fred_rsp(FRED_STACK_LEVEL_2));
>> +	wrmsrq(MSR_IA32_FRED_RSP3, this_cpu_fred_rsp(FRED_STACK_LEVEL_3));
>>  }
>