linux-kernel - Re: [RFC PATCH V2 05/13] perf/x86: Support XMM register for non-PEBS and REGS

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: <b894739e-a0e1-4451-92c5-8bc577d3979b@intel.com>
Date: Fri, 27 Jun 2025 07:35:06 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: kan.liang@...ux.intel.com, 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
Cc: 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 05/13] perf/x86: Support XMM register for non-PEBS
 and REGS_USER

On 6/26/25 12:56, kan.liang@...ux.intel.com wrote:
> +static void x86_pmu_get_ext_regs(struct x86_perf_regs *perf_regs, u64 mask)
> +{
> +	struct xregs_state *xsave = per_cpu(ext_regs_buf, smp_processor_id());
> +
> +	if (WARN_ON_ONCE(!xsave))
> +		return;
> +
> +	xsaves_nmi(xsave, mask);

This makes me a little nervous.

Could we maybe keep a mask around that reminds us what 'ext_regs_buf'
was sized for and then ensure that no bits in the passed-in mask are set
in that?

I almost wonder if you want to add a

	struct fpu_state_config fpu_perf_cfg;

I guess it's mostly overkill for this. But please do have a look at the
data structures in:

	arch/x86/include/asm/fpu/types.h

> +	if (mask & XFEATURE_MASK_SSE &&
> +	    xsave->header.xfeatures & BIT_ULL(XFEATURE_SSE))
> +		perf_regs->xmm_space = xsave->i387.xmm_space;
> +}

There's a lot going on here.

'mask' and 'xfeatures' have the exact same format. Why use
XFEATURE_MASK_SSE for one and BIT_ULL(XFEATURE_SSE) for the other?

Why check both? How could a bit get into 'xfeatures' without being in
'mask'?

How does the caller handle the fact that ->xmm_space might be written or
not?