linux-kernel - Re: [RFC PATCH V2 03/13] x86/fpu/xstate: Add xsaves

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3e95a8f7-88aa-4732-b38b-ccef74634819@intel.com>
Date: Tue, 1 Jul 2025 17:18:01 -0700
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: <kan.liang@...ux.intel.com>
CC: <peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
	<namhyung@...nel.org>, <tglx@...utronix.de>, <dave.hansen@...ux.intel.com>,
	<irogers@...gle.com>, <adrian.hunter@...el.com>, <jolsa@...nel.org>,
	<alexander.shishkin@...ux.intel.com>, <linux-kernel@...r.kernel.org>,
	<dapeng1.mi@...ux.intel.com>, <ak@...ux.intel.com>, <zide.chen@...el.com>,
	<mark.rutland@....com>, <broonie@...nel.org>, <ravi.bangoria@....com>
Subject: Re: [RFC PATCH V2 03/13] x86/fpu/xstate: Add xsaves_nmi

On 6/26/2025 12:56 PM, kan.liang@...ux.intel.com wrote:
<snip>> Add an interface to retrieve the actual register contents when 
the NMI
> hit. The interface is different from the other interfaces of FPU. The
> other mechanisms that deal with xstate try to get something coherent.
> But this interface is *in*coherent. There's no telling what was in the
> registers when a NMI hits. It writes whatever was in the registers when
> the NMI hit. It's the invoker's responsibility to make sure the contents
> are properly filtered before exposing them to the end user.

<snip>

>   
> +/**
> + * xsaves_nmi - Save selected components to a kernel xstate buffer in NMI
> + * @xstate:	Pointer to the buffer
> + * @mask:	Feature mask to select the components to save
> + *
> + * The @xstate buffer must be 64 byte aligned.
> + *
> + * Caution: The interface is different from the other interfaces of FPU.
> + * The other mechanisms that deal with xstate try to get something coherent.
> + * But this interface is *in*coherent. There's no telling what was in the
> + * registers when a NMI hits. It writes whatever was in the registers when
> + * the NMI hit.
> + * The only user for the interface is perf_event. There is already a
> + * hardware feature (See Intel PEBS XMMs group), which can handle XSAVE
> + * "snapshots" from random code running. This just provides another XSAVE
> + * data source at a random time.
> + * This function can only be invoked in an NMI. It returns the *ACTUAL*
> + * register contents when the NMI hit.
> + */
> +void xsaves_nmi(struct xregs_state *xstate, u64 mask)
> +{
> +	int err;
> +
> +	if (!in_nmi())
> +		return;
> +
> +	XSTATE_OP(XSAVES, xstate, (u32)mask, (u32)(mask >> 32), err);
> +	WARN_ON_ONCE(err);
> +}
> +
There are xsaves()/xrstors() functions, already narrowed to the 
"independent" feature set only. So, adding a new xsaves_yyy() variant 
for a different use case -- without renaming the existing helpers to 
something like xsaves_xxx() -- would make the naming scheme appear 
inconsistent at a glance.

But looking back at history:

1. These helpers were established with "independent" in the name (though
    they were initially described as for “dynamic” features):
    copy_kernel_to_independent_supervisor()/
    copy_independent_supervisor_to_kernel()

2. Later, Thomas reworked them, renaming and simplifying them to
    xsaves()/xrstors(), and adding a refactored validator:
    validate_xsaves_xrstors() [1]. At that point, their usage was
    *relaxed* and not strictly limited to independent features.

3. Subsequently, in preparation for dynamic feature support, the helpers
    were restricted again to independent features only [2]. This involved
    renaming and enforcing stricter validation via
    validate_independent_components().

Given that, rather than introducing a new wrapper for every additional 
use case, another option could be to retain xsaves() naming but modestly 
expand its scope. That would mean to add another allowance: features in 
tightly constrained contexts (e.g., NMI). Perhaps, this approach can 
keep the API simple while still expanding usage.

[1] a75c52896b6d ("x86/fpu/xstate: Sanitize handling of independent 
features")
[2] f5daf836f292 ("x86/fpu: Restrict xsaves()/xrstors() to independent 
states")