[MODERATED] Re: [patch V7 15/15] SBB 15

[Tim Chen <tim.c.chen@linux.intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3134 bytes --]

On 04/30/2018 01:20 PM, speck for Konrad Rzeszutek Wilk wrote:
> On Mon, Apr 30, 2018 at 01:12:28PM -0700, speck for Tim Chen wrote:
>> On 04/30/2018 12:36 PM, speck for Thomas Gleixner wrote:
>>> On Mon, 30 Apr 2018, speck for Tim Chen wrote:
>>>
>>>> On 04/29/2018 12:31 PM, speck for Thomas Gleixner wrote:
>>>>
>>>>> +static int sbb_prctl_set(unsigned long ctrl)
>>>>> +{
>>>>> +	bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
>>>>> +
>>>>> +	if (ssb_mode != SPEC_STORE_BYPASS_PRCTL)
>>>>> +		return -ENXIO;
>>>>
>>>> Perhaps we should not use the prctl if we don't have
>>>> the SSB bug.  Something like:
>>>>
>>>> 	if (ssb_mode != SPEC_STORE_BYPASS_PRCTL ||
>>>> 	    !boot_cpu_has_bug(X86_BUG_SPEC_STORE_BYPASS))
>>>> 		return -ENXIO; 
>>>
>>> What? If the CPU does not have the bug then the kernel does not enable
>>> PRCTL mode at all unless
>>>
>>>       1) RDS is available
>>>
>>> AND
>>>
>>>       2) The user enforced PRTCL mode on the kernel command line.
>>>
>>> So what exactly are you trying to solve?
>>
>> Right, we will only do the PRCTL only for the case when
>> admin explicitly ask for it at kernel command line.
> 
> No. It will be automatically enabled. That is the '=auto'

If the CPU doesn't have the bug, the auto case will default
to SPEC_STORE_BYPASS_CMD_NONE.  So I'm not concerned about
that case.

        if (!boot_cpu_has_bug(X86_BUG_SPEC_STORE_BYPASS) &&
            (cmd == SPEC_STORE_BYPASS_CMD_NONE ||
             cmd == SPEC_STORE_BYPASS_CMD_AUTO))
                return mode;

The case that's of interest is the prctl case on command line and cpu doesn't
have the bug.  So if I understand Thomas' reply, we allow the
setting of MSR for that case (though unnecessary), as the admin explicitly asked for it.

Thanks.

Tim

> sets prctl by default on Intel... You know why don't I just
> paste to you the code:
> 
> 477         switch (cmd) {                                                          
> 478         case SPEC_STORE_BYPASS_CMD_AUTO:                                        
> 479                 /*                                                              
> 480                  * AMD platforms by default don't need SSB mitigation.          
> 481                  */                                                             
> 482                 if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)                 
> 483                         break;                                                  
> 484                 /* Choose prctl as the default mode */                          
> 485                 mode = SPEC_STORE_BYPASS_PRCTL;                                 
> 486                 break;                                              
> 
> 
>>
>> We'll let the admin have the choice to do prctl, if he
>> explicitly ask for it. Even though it
>> is unnecessary for cpus that don't have the SSB bug and
>> there will be some added cost in changing the MSR on context
>> switches.  Just want to clear in my mind that this is the choice
>> we are making.
>>
>> Tim
> 
>