linux-kernel - Re: [RFC PATCH 05/12] 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] [day] [month] [year] [list]

Message-ID: <067e0923-d904-4382-bf49-34083f9927e7@linux.intel.com>
Date: Fri, 13 Jun 2025 14:14:05 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Dave Hansen <dave.hansen@...el.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
Subject: Re: [RFC PATCH 05/12] perf/x86: Support XMM register for non-PEBS and
 REGS_USER



On 2025-06-13 11:34 a.m., Dave Hansen wrote:
> On 6/13/25 06:49, kan.liang@...ux.intel.com wrote:
>> +static void x86_pmu_get_ext_regs(struct x86_perf_regs *perf_regs, u64 mask)
>> +{
>> +	void *xsave = (void *)ALIGN((unsigned long)per_cpu(ext_regs_buf, smp_processor_id()), 64);
>> +	struct xregs_state *xregs_xsave = xsave;
>> +	u64 xcomp_bv;
>> +
>> +	if (WARN_ON_ONCE(!xsave))
>> +		return;
>> +
>> +	xsaves_nmi(xsave, mask);
>> +
>> +	xcomp_bv = xregs_xsave->header.xcomp_bv;
>> +	if (mask & XFEATURE_MASK_SSE && xcomp_bv & XFEATURE_SSE)
>> +		perf_regs->xmm_regs = (u64 *)xregs_xsave->i387.xmm_space;
>> +}
> 
> Now that I'm thinking about the init optimization... This is buggy.
> 
> Isn't XSAVE fun?
> 
> Here's a little primer:
> 
> 	xcomp_bv - tells you what the format of the buffer is.
> 		   Which states are where.
> 	xstate_bv - (aka. xfeatures) tells you which things XSAVES
> 		    wrote to the buffer.

I got the definitions of the two reversed. :(

> 
> It's totally valid to have a feature set in xcomp_bv but not xstate_bv.
> 
> xcomp_bv is actually pretty boring:
> 
> 	The XSAVES instructions sets bit 63 of the XCOMP_BV field of the
> 	XSAVE header while writing RFBM[62:0] to XCOMP_BV[62:0]
> 
> Since you know the RFBM, you also know xstate_bv. You don't need to read
> it out of the buffer even.
> 
> Oh, and what's with the:
> 
> 	xcomp_bv & XFEATURE_SSE
> 
> ? xcomp_bv is a bitmap, just like 'mask'
> 
> So, what do you do when
> 
> 	if (!(xregs_xsave->header.xfeatures & XFEATURE_MASK_SSE))
> 		... here?
> 
> The "XSAVE-Enabled Registers Group" docs say:
> 
> 	The first eight bytes include the XSAVES instruction’s XSTATE_BV
> 	bit vector (reflecting INIT optimization). This field
> 	is in XCR0 format.
> 
> So the PEBS parsing code has to know how to deal with this situation too
> and not copy the xmm_regs out to users.

Now, perf will copy all 0 to users if !perf_regs->xmm_regs. But 0 should
be a valid value for the xmm_regs. Perf probably need to dump a bitmap
which indicates what regs are really collected. It may be slightly
different from the requested bitmap because of the above case.

Thanks,
Kan