PR/60426 CVS commit: [netbsd-11] src
"Martin Husemann" <[email protected]> Sun, 19 Jul 2026 16:00:03 +0000 (UTC)
| Newsgroups | gmane.os.netbsd.bugs |
|---|---|
| Message-ID | <[email protected]> |
The following reply was made to PR kern/60426; it has been noted by GNATS. From: "Martin Husemann" <[email protected]> To: [email protected] Cc: Subject: PR/60426 CVS commit: [netbsd-11] src Date: Sun, 19 Jul 2026 15:57:29 +0000 Module Name: src Committed By: martin Date: Sun Jul 19 15:57:28 UTC 2026 Modified Files: src/distrib/sets/lists/debug [netbsd-11]: mi src/distrib/sets/lists/tests [netbsd-11]: mi src/sys/arch/amd64/amd64 [netbsd-11]: machdep.c netbsd32_machdep.c src/sys/arch/amd64/include [netbsd-11]: mcontext.h src/sys/arch/i386/i386 [netbsd-11]: machdep.c src/sys/arch/i386/include [netbsd-11]: mcontext.h src/sys/arch/x86/include [netbsd-11]: cpu_extended_state.h fpu.h specialreg.h src/sys/arch/x86/x86 [netbsd-11]: fpu.c identcpu.c src/tests/kernel [netbsd-11]: Makefile Added Files: src/tests/kernel [netbsd-11]: t_signal_and_fpu.c src/tests/kernel/arch/x86 [netbsd-11]: sig_fpu.c sig_fpu.h Log Message: Pull up following revision(s) (requested by riastradh in ticket #381): sys/arch/amd64/amd64/machdep.c: revision 1.380 sys/arch/amd64/include/mcontext.h: revision 1.25 sys/arch/amd64/amd64/netbsd32_machdep.c: revision 1.143 distrib/sets/lists/tests/mi: revision 1.1424 sys/arch/x86/include/specialreg.h: revision 1.222 sys/arch/x86/x86/identcpu.c: revision 1.140 sys/arch/i386/include/mcontext.h: revision 1.20 sys/arch/x86/x86/identcpu.c: revision 1.141 tests/kernel/t_signal_and_fpu.c: revision 1.1 tests/kernel/t_signal_and_fpu.c: revision 1.2 tests/kernel/t_signal_and_fpu.c: revision 1.3 sys/arch/x86/include/fpu.h: revision 1.24 sys/arch/x86/include/cpu_extended_state.h: revision 1.20 sys/arch/i386/i386/machdep.c: revision 1.852 distrib/sets/lists/debug/mi: revision 1.516 tests/kernel/Makefile: revision 1.98 tests/kernel/arch/x86/sig_fpu.c: revision 1.1 sys/arch/x86/x86/fpu.c: revision 1.95 tests/kernel/arch/x86/sig_fpu.h: revision 1.1 Verify signal delivery preserves various FPU state. Should also test AVX512 but I'm not sure I have hardware handy to do this right now and qemu doesn't emulate it, as far as I know! TBD. (Plus: there's umpteen different cpuid feature bits to consider, annoyingly, and that means a lot of manual reading to sort through them.) PR kern/60426: Signal handler corrupts AVX (YMM) registers x86: Save and restore all supported extended CPU state on signals. While here, disable Intel AMX, whose state size (>>8 KiB) exceeds MINSIGSTKSZ (8 KiB), until we are ready to safely update the ABI for sigaltstack(2). This isn't a regression: we've never had a release with Intel AMX support. Previously, on signal delivery, we would only save and restore at most what FXSAVE does, which is the x87 and SSE registers that always exist on amd64. To save and restore the upper halves of the YMM or ZMM registers (AVX/AVX2/AVX512), or the AVX512 registers ZMM16..ZMM31, or the enormous AMX state, we need to do more. And we need to do that even if the signal handler doesn't use AVX instructions, because an SSE instruction modifying xmmN (e.g., in a vectorized memcpy) will, as a side effect, zero the high half of ymmN. Fortunately, the x86 architecture has an extensible mechanism for saving and restoring extended CPU state, the XSAVE/XRSTOR instruction family, so we don't need to update this code for every extension -- just handling the generic XSAVE/XRSTOR mechanism will work for a while. When delivering a signal to a thread that has actually made use of these extended registers, we: 1. allocate a separate space on the user's stack for an XSAVE area of whatever size is necessary (as long as it fits in the sigaltstack, if one was provided), and 2. store a pointer to and the length of the separate XSAVE area in an architecturally unused section of the FXSAVE area of the mcontext_t, with the _UC_XSAVE bit set. This way existing applications that have SA_SIGINFO signal handlers and expect to find an FXSAVE area in the mcontext_t with x87 and SSE register content will continue to work, but if the thread has used any state beyond that like AVX registers requiring XSAVE/XRSTOR, on return from signal it can restore everything. Unfortunately, the _size_ that XSAVE needs is variable and might be expanded by architectural extensions, such as Intel AMX. So the minimum stack size to store it all might exceed MINSIGSTKSZ. The current MINSIGSTKSZ is plenty for AVX/AVX2/AVX-512. But before raising MINSIGSTKSZ to support larger XSAVE sizes by adding them to XCR0_FPU, we need to make sure that older binaries built with the old MINSIGSTKSZ continue to work -- of course, they won't be able to use (e.g.) Intel AMX state. To make sure we don't break the ABI accidentally, I've added some __CTASSERTs relating XSAVE_MAX_BYTES, the maximum supported XSAVE area size, and MINSIGSTKSZ, and a panic at boot if the CPU advertises an XSAVE size larger than XSAVE_MAX_BYTES, which would mean either: (a) the CPU's advertised size for various architecturally defined extended CPU state does not match the documentation, or (b) we have inadvertently bitten off more extended state CPU state than we can chew in XCR0_FPU and we have to adjust the ABI and add compatibility for userland programs. Fortunately for getcontext(2), all of the registers in question are caller-saves, so it doesn't need to store any extra state. PR kern/60426: Signal handler corrupts AVX (YMM) registers t_signal_and_fpu: Fix machine-independent double test. Copypasta error! Float and long double tests were fine. PR kern/60426: Signal handler corrupts AVX (YMM) registers x86: Redo boot-time XSAVE area size ABI check. Instead of checking each component's offset+size from size = CPUID[EAX=0x0d,ECX=i].EAX, offset = CPUID[EAX=0x0d,ECX=i].EBX, to make sure it fits in the XSAVE_MAX_BYTES implied by the MINSIGSTKSZ ABI parameter, just check the total _enabled_ XSAVE area size from CPUID[EAX=0x0d,ECX=0].EBX, which is what we use to allocate the XSAVE area in software anyway. The Intel documentation[1] is not very clear on exactly what CPUID[EAX=0x0d,ECX=i] reports for i >= 2, saying `valid bit in the XCR0 register' without distinguishing `supported' from `enabled' bits, and the AMD documentation I skimmed didn't have these leaves in any obvious place. But it seems CPUID[EAX=0xd,ECX=i].EAX/.EBX give the size and offset of the CPU's _supported_ XSAVE features, rather than only those for _enabled_ XSAVE features, so the original panic would have tripped on features we haven't enabled in XCR0_FPU. In any case, better to verify the parameter we use directly for allocating space. [1] Intel 64 and IA-32 Architectures Software Developer's Manual, Volume 1: Basic Architecture, Intel, Order Number: 253665-092US, June 2026, Sec. 21.3 `CPUID Leaves', Subsection `CPUID.0DH -- Processor Extended State', Subsubsection `CPUID.0DH.SUB-LEAVES - Sub-leaves', pp. 21-43 -- 21-44. https://web.archive.org/web/20260709150417/https://cdrdv2-public.intel.com/922477/253665-092-sdm-vol-1.pdf PR kern/60426: Signal handler corrupts AVX (YMM) registers To generate a diff of this commit: cvs rdiff -u -r1.485.2.10 -r1.485.2.11 src/distrib/sets/lists/debug/mi cvs rdiff -u -r1.1387.2.11 -r1.1387.2.12 src/distrib/sets/lists/tests/mi cvs rdiff -u -r1.376.2.1 -r1.376.2.2 src/sys/arch/amd64/amd64/machdep.c cvs rdiff -u -r1.142 -r1.142.2.1 src/sys/arch/amd64/amd64/netbsd32_machdep.c cvs rdiff -u -r1.24 -r1.24.2.1 src/sys/arch/amd64/include/mcontext.h cvs rdiff -u -r1.849 -r1.849.2.1 src/sys/arch/i386/i386/machdep.c cvs rdiff -u -r1.19 -r1.19.2.1 src/sys/arch/i386/include/mcontext.h cvs rdiff -u -r1.19 -r1.19.2.1 src/sys/arch/x86/include/cpu_extended_state.h cvs rdiff -u -r1.23 -r1.23.28.1 src/sys/arch/x86/include/fpu.h cvs rdiff -u -r1.219 -r1.219.2.1 src/sys/arch/x86/include/specialreg.h cvs rdiff -u -r1.93.2.1 -r1.93.2.2 src/sys/arch/x86/x86/fpu.c cvs rdiff -u -r1.138 -r1.138.2.1 src/sys/arch/x86/x86/identcpu.c cvs rdiff -u -r1.96.2.1 -r1.96.2.2 src/tests/kernel/Makefile cvs rdiff -u -r0 -r1.3.2.2 src/tests/kernel/t_signal_and_fpu.c cvs rdiff -u -r0 -r1.3.2.2 src/tests/kernel/arch/x86/sig_fpu.c cvs rdiff -u -r0 -r1.1.2.2 src/tests/kernel/arch/x86/sig_fpu.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.