2023-06-28 02:46:09

by Waiman Long

[permalink] [raw]
Subject: [PATCH v4 0/4] x86/speculation: Disable IBRS when idle

v4:
- Add a new __update_spec_ctrl() helper in patch 1.
- Rebased to the latest linux kernel.

v3:
- Drop patches 1 ("x86/speculation: Provide a debugfs file to dump
SPEC_CTRL MSRs") and 5 ("x86/idle: Disable IBRS entering mwait idle
and enable it on wakeup") for now.
- Drop the MSR restoration code in ("x86/idle: Disable IBRS when cpu
is offline") as native_play_dead() does not return.
- For patch ("intel_idle: Add ibrs_off module parameter to force
disable IBRS"), change the name from "no_ibrs" to "ibrs_off" and
document the new parameter in intel_idle.rst.

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.

Commit bf5835bcdb96 ("intel_idle: Disable IBRS during long idle")
disables IBRS when the CPU enters long idle (C6 or below). However, there
are existing users out there who have set "intel_idle.max_cstate=1"
to decrease latency. Those users won't be able to benefit from this
commit. This patch series extends this commit by providing a new
"intel_idle.ibrs_off" module parameter to force disable IBRS even when
"intel_idle.max_cstate=1" at the expense of increased IRQ response
latency. It also includes a commit to allow the disabling of IBRS when
a CPU becomes offline.


Waiman Long (4):
x86/speculation: Add __update_spec_ctrl() helper
x86/idle: Disable IBRS when cpu is offline
intel_idle: Use __update_spec_ctrl() in intel_idle_ibrs()
intel_idle: Add ibrs_off module parameter to force disable IBRS

Documentation/admin-guide/pm/intel_idle.rst | 17 ++++++++++++++++-
arch/x86/include/asm/nospec-branch.h | 11 ++++++++++-
arch/x86/kernel/smpboot.c | 8 ++++++++
drivers/idle/intel_idle.c | 18 ++++++++++++++----
4 files changed, 48 insertions(+), 6 deletions(-)

--
2.31.1



2023-06-28 03:06:23

by Waiman Long

[permalink] [raw]
Subject: [PATCH v4 3/4] intel_idle: Use __update_spec_ctrl() in intel_idle_ibrs()

When intel_idle_ibrs() is called, it modifies the SPEC_CTRL MSR to 0
in order disable IBRS. However, the new MSR value isn't reflected in
x86_spec_ctrl_current which is at odd with the other code that keep track
of its state in that percpu variable. Use the new __update_spec_ctrl()
to have the x86_spec_ctrl_current percpu value properly updated.

Signed-off-by: Waiman Long <[email protected]>
---
drivers/idle/intel_idle.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 34201d7ef33e..e32ff09051af 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -182,12 +182,12 @@ static __cpuidle int intel_idle_ibrs(struct cpuidle_device *dev,
int ret;

if (smt_active)
- native_wrmsrl(MSR_IA32_SPEC_CTRL, 0);
+ __update_spec_ctrl(0);

ret = __intel_idle(dev, drv, index);

if (smt_active)
- native_wrmsrl(MSR_IA32_SPEC_CTRL, spec_ctrl);
+ __update_spec_ctrl(spec_ctrl);

return ret;
}
--
2.31.1


2023-06-28 17:26:02

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] x86/speculation: Disable IBRS when idle

On Wed, Jun 28, 2023 at 4:27 AM Waiman Long <[email protected]> wrote:
>
> v4:
> - Add a new __update_spec_ctrl() helper in patch 1.
> - Rebased to the latest linux kernel.
>
> v3:
> - Drop patches 1 ("x86/speculation: Provide a debugfs file to dump
> SPEC_CTRL MSRs") and 5 ("x86/idle: Disable IBRS entering mwait idle
> and enable it on wakeup") for now.
> - Drop the MSR restoration code in ("x86/idle: Disable IBRS when cpu
> is offline") as native_play_dead() does not return.
> - For patch ("intel_idle: Add ibrs_off module parameter to force
> disable IBRS"), change the name from "no_ibrs" to "ibrs_off" and
> document the new parameter in intel_idle.rst.
>
> 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.
>
> Commit bf5835bcdb96 ("intel_idle: Disable IBRS during long idle")
> disables IBRS when the CPU enters long idle (C6 or below). However, there
> are existing users out there who have set "intel_idle.max_cstate=1"
> to decrease latency. Those users won't be able to benefit from this
> commit. This patch series extends this commit by providing a new
> "intel_idle.ibrs_off" module parameter to force disable IBRS even when
> "intel_idle.max_cstate=1" at the expense of increased IRQ response
> latency. It also includes a commit to allow the disabling of IBRS when
> a CPU becomes offline.
>
>
> Waiman Long (4):
> x86/speculation: Add __update_spec_ctrl() helper
> x86/idle: Disable IBRS when cpu is offline
> intel_idle: Use __update_spec_ctrl() in intel_idle_ibrs()
> intel_idle: Add ibrs_off module parameter to force disable IBRS

x86 maintainers, if you want to take care of this series, please feel
free to add

Acked-by: Rafael J. Wysocki <[email protected]>

to all of the patches in it.

Thanks!