Re: [PATCH] x86/split_lock: Zap the unwieldy switch-case in sld_state_show()

[Borislav Petkov]

On Wed, Jan 07, 2026 at 04:41:22PM +0000, Luck, Tony wrote:
> It matters because the mode is shared by both hyper-threads, and you may
> only offline one of them.
> 
> The offline handler isn't needed in the fatal case because split lock is
> always enabled. So the "Unconditionally re-enable detection here."
> won't change anything.

/me goes and reads

b041b525dab9 ("x86/split_lock: Make life miserable for split lockers")

Oh wow, that's a fancy dance there :)
 
> That said, it wouldn't break anything to run that. So if it makes the setup
> code easier to read, it's OK to do this. But should have a comment to
> describe what it going on.

Right, exactly.

My goal is to have it be more readable by dialing down on the repetitive
stuff.

IOW, this (pasting the whole function):

static void sld_state_show(void)
{
        const char *action = "warning";
 
        if (!boot_cpu_has(X86_FEATURE_BUS_LOCK_DETECT) &&
            !boot_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT))
                return;
 
        if (sld_state == sld_off) {
                pr_info("disabled\n");
                return;
        } else if (sld_state == sld_ratelimit) {
                if (boot_cpu_has(X86_FEATURE_BUS_LOCK_DETECT))
                        pr_info("#DB: setting system wide bus lock rate limit to %u/sec\n", bld_ratelimit.burst);
                return;
        }
 
        if (sld_state == sld_fatal)
                action = "sending SIGBUS";
 
        if (boot_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT)) {
                pr_info("#AC: crashing the kernel on kernel split_locks and %s on user-space split_locks\n", action);
 
                /*
                 * This is handling the case where a CPU goes offline at the
                 * moment where split lock detection is disabled in the warn
                 * setting, see split_lock_warn(). It doesn't have any effect
                 * in the fatal case.
                 */
                if (cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
                                      "x86/splitlock", NULL, splitlock_cpu_offline) < 0)
                        pr_warn("No splitlock CPU offline handler\n");
        } else if (boot_cpu_has(X86_FEATURE_BUS_LOCK_DETECT)) {
                pr_info("#DB: %s on user-space bus_locks\n", action);
        }
}

Btw, looking some more, that MSR writing function could save a write if not
necessary:

diff --git a/arch/x86/kernel/cpu/bus_lock.c b/arch/x86/kernel/cpu/bus_lock.c
index 811f87906c1e..bdf518d28310 100644
--- a/arch/x86/kernel/cpu/bus_lock.c
+++ b/arch/x86/kernel/cpu/bus_lock.c
@@ -164,7 +164,10 @@ static void sld_update_msr(bool on)
        if (on)
                test_ctrl_val |= MSR_TEST_CTRL_SPLIT_LOCK_DETECT;
 
-       wrmsrq(MSR_TEST_CTRL, test_ctrl_val);
+       if (test_ctrl_val != msr_test_ctrl_cache) {
+               wrmsrq(MSR_TEST_CTRL, test_ctrl_val);
+               msr_test_ctrl_cache = test_ctrl_val;
+       }
 }
 
 void split_lock_init(void)

---

but maybe that's not important as slow path...

Also, from the looks of it, that cached value could be dropped in favor of
using msr_{set,clear}_bit(). But maybe something for another day...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette