SSB status - V18 pushed out

SSB status - V18 pushed out

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

Folks,

we finally reached a stable state with the SSB patches. I've updated all 3
branches master/linux-4.16.y/linux-4.14.y in the repo and attached the
resulting git bundles. They merge cleanly on top of the current HEADs of
the relevant trees.

The lot survived light testing on my side and it would be great if everyone
involved could expose it to their test scenarios.

Thanks to everyone who participated in that effort (patches, review,
testing ...)!

Thanks,

	tglx

[-- Attachment #2: Type: application/octet-stream, Size: 79102 bytes --]

[-- Attachment #3: Type: application/octet-stream, Size: 75724 bytes --]

[-- Attachment #4: Type: application/octet-stream, Size: 75835 bytes --]

[MODERATED] Is: Sleep states ?Was:Re: SSB status - V18 pushed out

[Konrad Rzeszutek Wilk]

On Thu, May 17, 2018 at 10:53:28PM +0200, speck for Thomas Gleixner wrote:
> Folks,
> 
> we finally reached a stable state with the SSB patches. I've updated all 3
> branches master/linux-4.16.y/linux-4.14.y in the repo and attached the
> resulting git bundles. They merge cleanly on top of the current HEADs of
> the relevant trees.
> 
> The lot survived light testing on my side and it would be great if everyone
> involved could expose it to their test scenarios.
> 
> Thanks to everyone who participated in that effort (patches, review,
> testing ...)!

Yeey! Thank you.

I was reading the updated Intel doc today (instead of skim reading it) and it mentioned:

"Intel recommends that the SSBD MSR bit be cleared when in a sleep state on such processors."

We don't seem to be doing that? 

To do that we would need to:

1) Revert 4b59bdb56945 x86/bugs: Remove x86_spec_ctrl_set()

2) Peppering 

	if (static_cpu_has(X86_FEATURE_SPEC_STORE_BYPASS_DISABLE))
		x86_spec_ctrl_set(~SPEC_CTRL_SSBD);

	[when enterring sleep state]

	and:
	if (static_cpu_has(X86_FEATURE_SPEC_STORE_BYPASS_DISABLE))
		x86_spec_ctrl_set(SPEC_CTRL_SSBD);

	[when coming out]

	in mwait_idle_with_hints, mwait_idle, and native_play_dead 

	Or alternatively fiddle with the MSR directly.

3) And then uhuh, I am not sure how you would want to deal when the applications
   are running. That is when the !static_cpu_has(X86_FEATURE_SPEC_STORE_BYPASS_DISABLE)
   but still want the MSR toggled.


> 
> Thanks,
> 
> 	tglx

Re: Is: Sleep states ?Was:Re: SSB status - V18 pushed out

[Thomas Gleixner]

On Fri, 18 May 2018, speck for Konrad Rzeszutek Wilk wrote:
> On Thu, May 17, 2018 at 10:53:28PM +0200, speck for Thomas Gleixner wrote:
> > Folks,
> > 
> > we finally reached a stable state with the SSB patches. I've updated all 3
> > branches master/linux-4.16.y/linux-4.14.y in the repo and attached the
> > resulting git bundles. They merge cleanly on top of the current HEADs of
> > the relevant trees.
> > 
> > The lot survived light testing on my side and it would be great if everyone
> > involved could expose it to their test scenarios.
> > 
> > Thanks to everyone who participated in that effort (patches, review,
> > testing ...)!
> 
> Yeey! Thank you.
> 
> I was reading the updated Intel doc today (instead of skim reading it) and it mentioned:
> 
> "Intel recommends that the SSBD MSR bit be cleared when in a sleep state on such processors."

Well, the same recommendation was for IBRS and the reason is that with HT
enabled the other hyperthread will not be able to go full speed because the
sleeping one vanished with IBRS set. SSBD works the same way.

" SW should clear [SSBD] when enter sleep state, just as is suggested for
  IBRS and STIBP on existing implementations"

and that document says:

"Enabling IBRS on one logical processor of a core with Intel
 Hyper-Threading Technology may affect branch prediction on other logical
 processors of the same core. For this reason, software should disable IBRS
 (by clearing IA32_SPEC_CTRL.IBRS) prior to entering a sleep state (e.g.,
 by executing HLT or MWAIT) and re-enable IBRS upon wakeup and prior to
 executing any indirect branch."

So it's only a performance issue and not a fundamental problem to have it
on when executing HLT/MWAIT

So we have two situations here:

1) ssbd = on, i.e X86_FEATURE_SPEC_STORE_BYPASS_DISABLE

   There it is irrelevant because both threads have SSBD set permanentely,
   so unsetting it on HLT/MWAIT is not going to lift the restriction for
   the running sibling thread. And HLT/MWAIT is not going to be faster by
   unsetting it and then setting it on wakeup again....

2) SSBD via prctl/seccomp

   Nothing to do there, because idle task does not have TIF_SSBD set so it
   never goes with SSBD set into HLT/MWAIT.

So I think we're good, but it would be nice if Intel folks would confirm
that.

Thanks,

	tglx

  

[MODERATED] Encrypted Message

[Tim Chen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/rfc822-headers; protected-headers="v1", Size: 163 bytes --]

From: Tim Chen <tim.c.chen@linux.intel.com>
To: speck for Thomas Gleixner <speck@linutronix.de>
Subject: Re: Is: Sleep states ?Was:Re: SSB status - V18 pushed out

[-- Attachment #2: Type: text/plain, Size: 2667 bytes --]

On 05/18/2018 07:29 AM, speck for Thomas Gleixner wrote:
> On Fri, 18 May 2018, speck for Konrad Rzeszutek Wilk wrote:
>> On Thu, May 17, 2018 at 10:53:28PM +0200, speck for Thomas Gleixner wrote:
>>> Folks,
>>>
>>> we finally reached a stable state with the SSB patches. I've updated all 3
>>> branches master/linux-4.16.y/linux-4.14.y in the repo and attached the
>>> resulting git bundles. They merge cleanly on top of the current HEADs of
>>> the relevant trees.
>>>
>>> The lot survived light testing on my side and it would be great if everyone
>>> involved could expose it to their test scenarios.
>>>
>>> Thanks to everyone who participated in that effort (patches, review,
>>> testing ...)!
>>
>> Yeey! Thank you.
>>
>> I was reading the updated Intel doc today (instead of skim reading it) and it mentioned:
>>
>> "Intel recommends that the SSBD MSR bit be cleared when in a sleep state on such processors."
> 
> Well, the same recommendation was for IBRS and the reason is that with HT
> enabled the other hyperthread will not be able to go full speed because the
> sleeping one vanished with IBRS set. SSBD works the same way.
> 
> " SW should clear [SSBD] when enter sleep state, just as is suggested for
>   IBRS and STIBP on existing implementations"
> 
> and that document says:
> 
> "Enabling IBRS on one logical processor of a core with Intel
>  Hyper-Threading Technology may affect branch prediction on other logical
>  processors of the same core. For this reason, software should disable IBRS
>  (by clearing IA32_SPEC_CTRL.IBRS) prior to entering a sleep state (e.g.,
>  by executing HLT or MWAIT) and re-enable IBRS upon wakeup and prior to
>  executing any indirect branch."
> 
> So it's only a performance issue and not a fundamental problem to have it
> on when executing HLT/MWAIT
> 
> So we have two situations here:
> 
> 1) ssbd = on, i.e X86_FEATURE_SPEC_STORE_BYPASS_DISABLE
> 
>    There it is irrelevant because both threads have SSBD set permanentely,
>    so unsetting it on HLT/MWAIT is not going to lift the restriction for
>    the running sibling thread. And HLT/MWAIT is not going to be faster by
>    unsetting it and then setting it on wakeup again....
> 
> 2) SSBD via prctl/seccomp
> 
>    Nothing to do there, because idle task does not have TIF_SSBD set so it
>    never goes with SSBD set into HLT/MWAIT.
> 
> So I think we're good, but it would be nice if Intel folks would confirm
> that.

Yes, we have thought about turning off SSBD in the mwait path earlier. But
decided that it was unnecessary for the exact reasons Thomas mentioned.

Thanks.

Tim