Details

Reviewers

markj
jhb

Commits

rG20c09e6fece2: amd64: use WRMSRNS immediate form to update splitlock control, when available
rG74d325fee2db: amd64: cache MSR_MEMORY_CTL in pcpu
rG51cea5416a24: amd64: add userspace control for disabling splitlocks
rGe318f24c0f53: amd64: support for tracking per-thread 'disable splitlocks' state
rGd8c1abb0e340: amd64: calculate if hardware supports disabling splitlocks
rGc6d7235ac15d: amd64: handle #AC in kernel mode
rGdc9756122133: amd64: add md thread flags word
rG4a8acccc2e59: amd64 cpufunc.h: add WRMSRNS helpers
rG71f08bfa02a9: x86: add cpu_stdext_feature5
rG9aa031eb9277: x86: add definitions for the CORE_CAP and MEMORY_CTL MSRs

Summary

If hw supports it, on atomic operation that requires exclusive ownership of more than one cache line, #AC is generated.
Thread can control it with sysarch(I386_SET_SPLITLOCK). The global default is set with hw.splitlock_force.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib created this revision.Fri, Sep 18, 4:53 PM

Herald added a subscriber: imp. · View Herald TranscriptFri, Sep 18, 4:53 PM

Test program I used with simics:

/* $Id: splitlock.c,v 1.5 2026/09/18 16:39:53 kostik Exp kostik $ */

#include <sys/param.h>
#include <sys/mman.h>
#include <machine/atomic.h>
#include <machine/sysarch.h>
#include <err.h>
#include <signal.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

#ifndef I386_SET_SPLITLOCK
#define	I386_SET_SPLITLOCK	14
#define	I386_GET_SPLITLOCK	15
#endif

static void
sigbus_handler(int signo, siginfo_t *si, void *mc)
{
	printf("sig%d si_code %d addr %p\n", signo, si->si_code, si->si_addr);
	exit(0);
}

int
main(void)
{
	char *m;
	struct sigaction sa;
	int error, val;

	val = 1;
	error = sysarch(I386_SET_SPLITLOCK, &val);
	if (error == -1)
		err(1, "I386_SET_SPLITLOCK");
	error = sysarch(I386_GET_SPLITLOCK, &val);
	if (error == -1)
		err(1, "I386_GET_SPLITLOCK");
	printf("setlock: %d\n", val);

	memset(&sa, 0, sizeof(sa));
	sa.sa_sigaction = sigbus_handler;
	sa.sa_flags = SA_SIGINFO;
	if (sigaction(SIGBUS, &sa, NULL) == -1)
		err(1, "sigaction");

	m = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
	    MAP_PRIVATE | MAP_ANON, -1, 0);
	if (m == MAP_FAILED)
		err(1, "mmap");

	m += 256 - 1;
	atomic_fetchadd_long((unsigned long *)m, 1);
}

Fix reversed handling of the msr bit on context switch.

Fix splitlock control on exec for non-native ABIs.
Cache MSR_MEMORY_CTL, avoiding msr read on the context switch.
Use non-seializing immediate form for WRMSR when available.

markj added inline comments.Sun, Sep 20, 3:14 PM

sys/amd64/amd64/pmap.c
10268	This should load flags with `atomic_load_*`.
sys/amd64/amd64/sys_machdep.c
457	Extra break.
sys/amd64/amd64/vm_machdep.c
185	Shouldn't it be loading `td1->td_md.md_td_flags`?

If I understand correctly, #AC isn't raised if a splitlock operation happens while in kernel mode. But, what about places that use casuword() and friends? They will behave the same as before even if splitlock detection is enabled.

In D59815#1373664, @markj wrote:

If I understand correctly, #AC isn't raised if a splitlock operation happens while in kernel mode. But, what about places that use casuword() and friends? They will behave the same as before even if splitlock detection is enabled.

Isn't casuword() executed in kernel mode?

Hm, I also had an impression that #AC only happen when CPL is 3. But re-checking SDM 11.1.2.3 Features to Disable Bus Locks and description of the #AC exception, I started doubt it.
I think it costs nothing to add the recovery for #AC handler in kernel mode.

Add kernel-mode #AC handler.
Copy md_td_flags from the right source.
Use atomic_load() in context switch code.

In D59815#1373713, @kib wrote:

In D59815#1373664, @markj wrote:

If I understand correctly, #AC isn't raised if a splitlock operation happens while in kernel mode. But, what about places that use casuword() and friends? They will behave the same as before even if splitlock detection is enabled.

Isn't casuword() executed in kernel mode?

Hm, I also had an impression that #AC only happen when CPL is 3. But re-checking SDM 11.1.2.3 Features to Disable Bus Locks and description of the #AC exception, I started doubt it.
I think it costs nothing to add the recovery for #AC handler in kernel mode.

I checked, simics does generates #AC in CPL 0, the current patch correctly returns EFAULT from umtx_op UMTX_OP_MUTEX_LOCK on mis-aligned struct umutex.

I wonder if it should be a procctl feature instead of sysarch()? If your intent is to set ia32_splitlock_force = 1 by default, then it would be nice if proccontrol could be used to override application settings without having to modify the program or change the global default. Hmm, but I guess this wouldn't work if the policy is to reset the splitlock setting upon exec.

sys/amd64/amd64/exec_machdep.c
415	What if `!ia32_splitlock` is true? disable_splitlock() asserts `MPASS(ia32_splitlock)`.
sys/amd64/amd64/pmap.c
10267	I think the `oldtd != td` check is redundant: if `oldtd == td` then the `oldpmap == pmap` case above should be taken.

In D59815#1373855, @markj wrote:

I wonder if it should be a procctl feature instead of sysarch()? If your intent is to set ia32_splitlock_force = 1 by default, then it would be nice if proccontrol could be used to override application settings without having to modify the program or change the global default. Hmm, but I guess this wouldn't work if the policy is to reset the splitlock setting upon exec.

My first version of the patch controlled it with MD procctl. But I felt it was too 'tweaky' for such minor and probably not too popular feature. After all, we do not provide a procctl knob to enable %rflags.AC, and nobody asked for that.

If somebody wants to enable either alignment checks or disable splitlocks, this can be done with a trivial LD_PRELOADed dso. The static binaries are off (like Go-compiled binaries) but I do not feel guilty.

In exec_splitlock(), do not call disable_splitlock() if hw does not support it.
Remove oldtd != td check in context switch.

markj added inline comments.Mon, Sep 21, 8:13 PM

sys/amd64/amd64/pmap.c
10267	Hmm, but this is a per-thread setting, not per-process. So in the `oldpmap == pmap` case, where we are switching between two threads in the same proc, the per-thread splitlock setting might not be applied correctly.
sys/amd64/include/proc.h
72	It should be (k) I believe.

Fix handling of context switches.

markj accepted this revision.Tue, Sep 22, 1:15 PM

markj added inline comments.

sys/amd64/amd64/sys_machdep.c
436	I guess we don't need the proc lock here.
sys/x86/include/sysarch.h
56	Why does it have the I386_ prefix? Is it just to avoid having to translate in freebsd32_sysarch()?

This revision is now accepted and ready to land.Tue, Sep 22, 1:15 PM

kib marked 2 inline comments as done.Tue, Sep 22, 1:25 PM

kib added inline comments.

sys/amd64/amd64/sys_machdep.c
436	Indeed. md_td_flags was annotated with (c).
sys/x86/include/sysarch.h
56	To mark it as available to the i386 userspace as well. I see no sense in adding the parallel AMD64_GET/SET_SPLITLOCK constants.

Remove proc lock in sysarch()

This was also tested on real hardware

(uarch synth) = Intel Emerald Rapids {Raptor Cove, optim of Golden Cove}, Intel 7
(synth) = Intel Xeon Scalable (5th Gen) Bronze/Silver/Gold/Platinum (Emerald Rapids A1/R1) {Raptor Cove, optim of Golden Cove}, Intel 7

which has the splitlock control.

This revision now requires review to proceed.Tue, Sep 22, 1:27 PM

Presumably sysarch.2 should also be updated.

In D59815#1374449, @markj wrote:

Presumably sysarch.2 should also be updated.

Rather, it should be written. Or might be sysarch.x86.2. Right now the man page is a stub that proclaims the existence of the syscall.
I might do it later.

In D59815#1374524, @kib wrote:

In D59815#1374449, @markj wrote:

Presumably sysarch.2 should also be updated.

Rather, it should be written. Or might be sysarch.x86.2. Right now the man page is a stub that proclaims the existence of the syscall.
I might do it later.