Re: [Patch v8 07/23] x86/fpu/xstate: Add xsaves_nmi() helper

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 5/29/2026 7:32 PM, Peter Zijlstra wrote:
> On Fri, May 29, 2026 at 03:56:29PM +0800, Dapeng Mi wrote:
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> Add xsaves_nmi() to save supported xsave states in NMI handler.
>>
>> This function is similar to xsaves(), but should only be called within
>> a NMI handler. This function returns the actual register contents at
>> the moment the NMI occurs.
>>
>> Currently the perf subsystem is the sole user of this helper. It uses
>> this function to snapshot SIMD (XMM/YMM/ZMM) and APX eGPRs registers
>> which would be added in subsequent patches.
>>
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
>> ---
>>  arch/x86/include/asm/fpu/xstate.h |  1 +
>>  arch/x86/kernel/fpu/xstate.c      | 23 +++++++++++++++++++++++
>>  2 files changed, 24 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
>> index 7a7dc9d56027..38fa8ff26559 100644
>> --- a/arch/x86/include/asm/fpu/xstate.h
>> +++ b/arch/x86/include/asm/fpu/xstate.h
>> @@ -110,6 +110,7 @@ int xfeature_size(int xfeature_nr);
>>  
>>  void xsaves(struct xregs_state *xsave, u64 mask);
>>  void xrstors(struct xregs_state *xsave, u64 mask);
>> +void xsaves_nmi(struct xregs_state *xsave, u64 mask);
>>  
>>  int xfd_enable_feature(u64 xfd_err);
>>  
>> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
>> index a7b6524a9dea..4394091c4791 100644
>> --- a/arch/x86/kernel/fpu/xstate.c
>> +++ b/arch/x86/kernel/fpu/xstate.c
>> @@ -1474,6 +1474,29 @@ void xrstors(struct xregs_state *xstate, u64 mask)
>>  	WARN_ON_ONCE(err);
>>  }
>>  
>> +/**
>> + * 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
>> + *
>> + * This function is similar to xsaves(), but should only be called within
>> + * a NMI handler. This function returns the actual register contents at
>> + * the moment the NMI occurs.
>> + *
>> + * Currently, the perf subsystem is the sole user of this helper. It uses
>> + * the function to snapshot SIMD (XMM/YMM/ZMM) and APX eGPRs registers.
>> + */
>> +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);
>> +}
> Sashiko raises a fun point vs skid; if an exclude_kernel=1 event trips
> inside the kernel this can potentially leak a whole pile of kernel regs.
>
> But of course the same thing is true for the existing setup. So perhaps
> that doesn't need to concern us now.
>
> There used to be discussions about this case, and I think we had generic
> code to sanitize such boundary events, but I can't seem to find that in
> the current tree.
>
> Mark, ISTR you were involved at some point, any idea what happened?

Just consult this question if current code has such kind of boundary check
with Gemini, his answer is no.

Currently perf_sample_regs_intr() unconditionally save the intr regs and
set the ABI, a simple way to mitigate this security risk may be like this,

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1654c493be56..e6116eab44c5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7920,10 +7920,21 @@ static void perf_sample_regs_user(struct perf_regs
*regs_user,
 }

 static void perf_sample_regs_intr(struct perf_regs *regs_intr,
-                                 struct pt_regs *regs)
+                                 struct pt_regs *regs,
+                                 bool exclude_kernel)
 {
-       regs_intr->regs = regs;
-       regs_intr->abi  = perf_reg_abi(current);
+       /*
+        * Hardware skid can lead to PMI is delivered after
+        * the CPU has already entered kernel mode. In that case,
+        * user-space sampling must not expose kernel register state.
+        */
+       if (exclude_kernel && !user_mode(regs)) {
+               regs_intr->abi = PERF_SAMPLE_REGS_ABI_NONE;
+               regs_intr->regs = NULL;
+       } else {
+               regs_intr->regs = regs;
+               regs_intr->abi  = perf_reg_abi(current);
+       }
 }

The PERF_SAMPLE_REGS_ABI_NONE would prevent the later code to sample and
expose the SIMD registers further. 

Thanks.


>