Re: [PATCH 0/5] x86/speculation: Disable IBRS when idle

Re: [PATCH 0/5] x86/speculation: Disable IBRS when idle

[Robin Jarry]

Waiman Long, Jun 16, 2023 at 21:59:
> For Intel processors that need to turn on IBRS to protect against
> Spectre v2 and Retbleed, the IBRS bit in the SPEC_CTRL MSR affects
> the performance of the whole core even if only one thread is turning
> it on when running in the kernel. For user space heavy applications,
> the performance impact of occasionally turning IBRS on during syscalls
> shouldn't be significant. Unfortunately, that is not the case when the
> sibling thread is idling in the kernel. In that case, the performance
> impact can be significant.
>
> When DPDK is running on an isolated CPU thread processing network packets
> in user space while its sibling thread is idle. The performance of the
> busy DPDK thread with IBRS on and off in the sibling idle thread are:
>
>                                 IBRS on               IBRS off
>                                 -------               --------
>   packets/second:                  7.8M                  10.4M
>   avg tsc cycles/packet:         282.26                 209.86
>
> This is a 25% performance degradation. The test system is a Intel Xeon
> 4114 CPU @ 2.20GHz.
>
> This patch series turns off IBRS when in various idle mode to eliminate
> the performance impact of the idling thread on its busy sibling thread.

Hi Longman,

thanks a lot for the quick turnaround on this issue.

Tested-by: Robin Jarry <rjarry@redhat.com>

Re: [PATCH 0/5] x86/speculation: Disable IBRS when idle

[Peter Zijlstra]

On Fri, Jun 16, 2023 at 10:14:52PM +0200, Robin Jarry wrote:
> Waiman Long, Jun 16, 2023 at 21:59:
> > For Intel processors that need to turn on IBRS to protect against
> > Spectre v2 and Retbleed, the IBRS bit in the SPEC_CTRL MSR affects
> > the performance of the whole core even if only one thread is turning
> > it on when running in the kernel. For user space heavy applications,
> > the performance impact of occasionally turning IBRS on during syscalls
> > shouldn't be significant. Unfortunately, that is not the case when the
> > sibling thread is idling in the kernel. In that case, the performance
> > impact can be significant.
> >
> > When DPDK is running on an isolated CPU thread processing network packets
> > in user space while its sibling thread is idle. The performance of the
> > busy DPDK thread with IBRS on and off in the sibling idle thread are:
> >
> >                                 IBRS on               IBRS off
> >                                 -------               --------
> >   packets/second:                  7.8M                  10.4M
> >   avg tsc cycles/packet:         282.26                 209.86
> >
> > This is a 25% performance degradation. The test system is a Intel Xeon
> > 4114 CPU @ 2.20GHz.
> >
> > This patch series turns off IBRS when in various idle mode to eliminate
> > the performance impact of the idling thread on its busy sibling thread.
> 
> Hi Longman,
> 
> thanks a lot for the quick turnaround on this issue.
> 
> Tested-by: Robin Jarry <rjarry@redhat.com>

I can't see the patches -- they didn't arrive in my mailbox nor can I
find them in the archive, in fact this here mail is the only evidence
they exist at all.

However, did you all see intel_idle_ibrs() and how that is selected for
C6 and up?

What exactly isn't working there?

Also, instead of investing more in this IBRS trainwreck, did you all try
call-depth-stuffing ?

Re: [PATCH 0/5] x86/speculation: Disable IBRS when idle

[Robin Jarry]

Peter Zijlstra, Jun 17, 2023 at 14:21:
> I can't see the patches -- they didn't arrive in my mailbox nor can I
> find them in the archive, in fact this here mail is the only evidence
> they exist at all.

I was also looking at them on the public archives but only found my own
reply. Maybe something got wrong with Red Hat internal SMTP.

Re: [PATCH 0/5] x86/speculation: Disable IBRS when idle

[Waiman Long]

On 6/17/23 08:21, Peter Zijlstra wrote:
> On Fri, Jun 16, 2023 at 10:14:52PM +0200, Robin Jarry wrote:
>> Waiman Long, Jun 16, 2023 at 21:59:
>>> For Intel processors that need to turn on IBRS to protect against
>>> Spectre v2 and Retbleed, the IBRS bit in the SPEC_CTRL MSR affects
>>> the performance of the whole core even if only one thread is turning
>>> it on when running in the kernel. For user space heavy applications,
>>> the performance impact of occasionally turning IBRS on during syscalls
>>> shouldn't be significant. Unfortunately, that is not the case when the
>>> sibling thread is idling in the kernel. In that case, the performance
>>> impact can be significant.
>>>
>>> When DPDK is running on an isolated CPU thread processing network packets
>>> in user space while its sibling thread is idle. The performance of the
>>> busy DPDK thread with IBRS on and off in the sibling idle thread are:
>>>
>>>                                  IBRS on               IBRS off
>>>                                  -------               --------
>>>    packets/second:                  7.8M                  10.4M
>>>    avg tsc cycles/packet:         282.26                 209.86
>>>
>>> This is a 25% performance degradation. The test system is a Intel Xeon
>>> 4114 CPU @ 2.20GHz.
>>>
>>> This patch series turns off IBRS when in various idle mode to eliminate
>>> the performance impact of the idling thread on its busy sibling thread.
>> Hi Longman,
>>
>> thanks a lot for the quick turnaround on this issue.
>>
>> Tested-by: Robin Jarry <rjarry@redhat.com>
> I can't see the patches -- they didn't arrive in my mailbox nor can I
> find them in the archive, in fact this here mail is the only evidence
> they exist at all.

I got a rebound message from your mail server about incorrect message 
format. It is probably caused by some problem in my end.


> However, did you all see intel_idle_ibrs() and how that is selected for
> C6 and up?
>
> What exactly isn't working there?

We were testing on the RHEL9.2 kernel which doesn't have your 
intel_idle_ibrs() patch yet. My preliminary testing does indicate your 
patch will likely work. I will ask Jerry to test a newer RHEL9.3 kernel 
with the intel_idle_ibrs() patch to see if it helps.

> Also, instead of investing more in this IBRS trainwreck, did you all try
> call-depth-stuffing ?

Yes, we are planning to backport your call-depth-stuffing code, but I 
believe there is still some issue outstanding that you need to address. 
So we need a solution to work around this issue in the mean time.

Cheers,
Longman

Re: [PATCH 0/5] x86/speculation: Disable IBRS when idle

[Waiman Long]

On 6/18/23 21:18, Waiman Long wrote:
> On 6/17/23 08:21, Peter Zijlstra wrote:
>> On Fri, Jun 16, 2023 at 10:14:52PM +0200, Robin Jarry wrote:
>>> Waiman Long, Jun 16, 2023 at 21:59:
>>>> For Intel processors that need to turn on IBRS to protect against
>>>> Spectre v2 and Retbleed, the IBRS bit in the SPEC_CTRL MSR affects
>>>> the performance of the whole core even if only one thread is turning
>>>> it on when running in the kernel. For user space heavy applications,
>>>> the performance impact of occasionally turning IBRS on during syscalls
>>>> shouldn't be significant. Unfortunately, that is not the case when the
>>>> sibling thread is idling in the kernel. In that case, the performance
>>>> impact can be significant.
>>>>
>>>> When DPDK is running on an isolated CPU thread processing network 
>>>> packets
>>>> in user space while its sibling thread is idle. The performance of the
>>>> busy DPDK thread with IBRS on and off in the sibling idle thread are:
>>>>
>>>>                                  IBRS on               IBRS off
>>>>                                  ------- --------
>>>>    packets/second:                  7.8M 10.4M
>>>>    avg tsc cycles/packet:         282.26 209.86
>>>>
>>>> This is a 25% performance degradation. The test system is a Intel Xeon
>>>> 4114 CPU @ 2.20GHz.
>>>>
>>>> This patch series turns off IBRS when in various idle mode to 
>>>> eliminate
>>>> the performance impact of the idling thread on its busy sibling 
>>>> thread.
>>> Hi Longman,
>>>
>>> thanks a lot for the quick turnaround on this issue.
>>>
>>> Tested-by: Robin Jarry <rjarry@redhat.com>
>> I can't see the patches -- they didn't arrive in my mailbox nor can I
>> find them in the archive, in fact this here mail is the only evidence
>> they exist at all.
>
> I got a rebound message from your mail server about incorrect message 
> format. It is probably caused by some problem in my end.
>
>
>> However, did you all see intel_idle_ibrs() and how that is selected for
>> C6 and up?
>>
>> What exactly isn't working there?
>
> We were testing on the RHEL9.2 kernel which doesn't have your 
> intel_idle_ibrs() patch yet. My preliminary testing does indicate your 
> patch will likely work. I will ask Jerry to test a newer RHEL9.3 
> kernel with the intel_idle_ibrs() patch to see if it helps.

We may need to extend your current solution to cover more cases. Perhaps 
adding a module parameter (e.g. idle_no_ibrs) to force the use of 
intel_idle_ibrs(). BTW, is it really the case that we can't disable IBRS 
when irq is enabled? The idle thread does not really interact with any 
user applications. I don't think there is any risk of information 
leakage even if we disable IBRS with interrupt enabled. Is my assumption 
incorrect?

Thanks,
Longman

Re: [PATCH 0/5] x86/speculation: Disable IBRS when idle

[Peter Zijlstra]

On Sun, Jun 18, 2023 at 11:25:29PM -0400, Waiman Long wrote:

> > We were testing on the RHEL9.2 kernel which doesn't have your

Then keep the tinkering in the RHEL tree, please.

> We may need to extend your current solution to cover more cases.

See below.

> Perhaps adding a module parameter (e.g. idle_no_ibrs) to force the use
> of intel_idle_ibrs(). BTW, is it really the case that we can't disable
> IBRS when irq is enabled?

No, that is an entirely artificial constraint due to not having
intel_idle_ibrs_irq() and having no desire to deal with the
ramifications of such a thing.

That said; it also doesn't make any sense what so ever to add this. The
reason for having this intel_idle_irq() is for C1 state to improve IRQ
response latency. Adding WRMSRs will obviously regress that.

Specifically, we very intentionally did not add CPUIDLE_FLAG_IBRS to the
very shallow idle states to avoid regressions. These WRMSRs are
*EXPENSIVE*.

Additionally, if you were to go do this with IRQs enabled, then you have
to worry about enabling IBRS again on the interrupt path from kernel.

> The idle thread does not really interact with any user
> applications. I don't think there is any risk of information leakage even if
> we disable IBRS with interrupt enabled. Is my assumption incorrect?

Yes:

 - doing the WRMSR on C1 makes no sense, the C1 state is only picked if
   the idle time expectation is *VERY* short, the WRMSR overhead in that
   case is probably more than the expected idle time.

 - doing the WRMSR with IRQs enabled means you now get to touch the
   interrupt/exception from kernel paths, nobody wants more of this
   crap.

 - the whole IBRS thing is a trainwreck, let it be.

 - finally, T0 runs userspace, T1 goes into C1 idle, disables IBRS,
   enables IRQs, takes an IRQ and now T0 can 'see' everything T1 does in
   kernel space, you loose.

Also, did I say that IBRS sucks? Like really? It is horrific -- step
away and let it be.

The possibly better solution is to make sure nothing untrusted what so
ever runs on the DPDK machine, then you can forget about all the
mitigation nonsense.