danmcd: No, you shouldn't have to introduct a new value per se. There isn't another save mechanism really.

There are definitely challenges if you need amx, but we shouldn't be enabling that.

Oh I don't need amx, but for some reason, cpuid_get_xsave_size() is returning 0x2b00 (11008) and it's landing as _plt_save_size .

Node's large use of threads, memory management, etc. appear to put us into a situation where we don't have enough thread-stack to make that happen.

ld.so.1`_plt_save_size: 2b00

THat's on Sapphire Rapids (AMX available...)

Alder Lake is smaller:

ld.so.1`_plt_save_size: a88

Ooops, not alder lake, that's Crystal Lake (11th Gen core)

Same on alder lake.

This explains my segv.

cpuid_get_xsave_size() returns MAX of our xsave_state structure size OR what the CPU reports

`cpuid_info0.cpi_xsave.xsav_max_size` ==> this gets huge on AMX-equipped machines, and the block comment section about coping with AMX in the future lists the long set of problems.

We don't support AMX yet, so maybe just maybe we should make sure cpuid_get_xsave_size() is a bit more conservative?

shouldn't it return MIN instead?

@Woodstock if we're making it static, possibly? I'd like a 3rd opinion, and this coming week is Thanksgiving in the US so some of those opinion-holders may be travelling.

There's big commentary about coping with AMX in $UTS/intel/os/fpu.c. (99.99% sure it's a genuine rmustacc Epic Block Comment(TM).)

danmcd: i haven't looked at the code in quite a while, it just seemed odd that we'd be using a value _larger_ than our structure

Intuitively it makes sense. It was introduce in -gate with fenix illumos#8534 (and friends).

FEATURE 8534: Want AVX-512 Support (Closed)

↳ illumos.org/issues/8534

I know getting MAX/MIN wrong is not uncommon, but IF it was gotten wrong, we had enough thread-stack-size to cope until AMX-machines showed up.

Also, it's quite possible the Node {,ab}usage we have in Triton might be doing things like creating threads with smaller stack sizes.

I'm gonna try a dirty hotpatch trick right now in my kernel...

Yeah it stopped the bleeding.

mdb -ke "cpuid_info0::print -athd cpi_xsave.xsav_max_size" ==> How much is YOUR CPU reporting it needs at most for XSAVE

My reports above from _plt_save_size come right from the kernel's saved CPU state, which is set per-process at exec() time.

My hotpatch was to shrink Sapphire Rapids xsav_max_size to Alder Lake sizes (2696 vs. 11008). The other possible hotpatch is to literally fix cpuid_get_xsave_size() to put MIN in instead of MAX.

(cmovq.ae becomes cmovq.be)

If @Woodstock is correct about cpuid_get_xsave_size() (s/MAX/MIN/), we should file a bug and fix this. If he's not, we need to know why.

Pardon the paste: A quick what-I-Can-see CPU survey (HDC is Zen2 so not included as there's a Zen2 already)

note, sizeof (struct xsave_state) == 832

VMware emulated kneecap ==> 832

Kebecloud nodes: Xeon E[35] - Gen3 (Haswell/Haswell-E) ==> 832

mango: EPYC 7302P (Zen2) ==> 896

My big NUC: Core Gen11 Mobile (Tiger Lake) ==> 2696

My small NUC: N100 (Alder Lake) ==> 2696

Shiny new Box: Xeon Scalable Gen4 ==> 11008

danmcd: would not be surprised if MAX of what-hardware-reports and our struct size is intentional (on the theory that hardware may write up to N bytes so we need that much space reserved)

(if all bells and whistles are enabled, that is).

See illumos.org/issues/8534

→ FEATURE 8534: Want AVX-512 Support (Closed)

or rather illumos.org/issues/8536

→ FEATURE 8536: xsave area should size dynamically, based on CPU features (Closed)

AVX512 adds almost 2k to the xsave area in each LWP. Intel clearly intends for the xsave area to be dynamically sized based on the features available in the current CPU. We should do that instead of blindly just increasing the size as we did for AVX.

(that was a quote from 8536)

(are their BIOS settings to disable AMX and does that shrink the reported max save size?)

[illumos-gate] 17748 installboot: potential null pointer dereference -- Toomas Soome <tsoome⊙mc>