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  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]
Date:   Sat, 23 Mar 2019 10:56:10 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Andi Kleen <ak@...ux.intel.com>
Cc:     kan.liang@...ux.intel.com, acme@...nel.org, mingo@...hat.com,
        linux-kernel@...r.kernel.org, tglx@...utronix.de, jolsa@...nel.org,
        eranian@...gle.com, alexander.shishkin@...ux.intel.com
Subject: Re: [PATCH V3 01/23] perf/x86: Support outputting XMM registers

On Fri, Mar 22, 2019 at 10:22:50AM -0700, Andi Kleen wrote:
> > > diff --git a/arch/x86/include/uapi/asm/perf_regs.h b/arch/x86/include/uapi/asm/perf_regs.h
> > > index f3329cabce5c..b33995313d17 100644
> > > --- a/arch/x86/include/uapi/asm/perf_regs.h
> > > +++ b/arch/x86/include/uapi/asm/perf_regs.h
> > > @@ -28,7 +28,29 @@ enum perf_event_x86_regs {
> > >  	PERF_REG_X86_R14,
> > >  	PERF_REG_X86_R15,
> > >  
> > > -	PERF_REG_X86_32_MAX = PERF_REG_X86_GS + 1,
> > > -	PERF_REG_X86_64_MAX = PERF_REG_X86_R15 + 1,
> > 
> > So this changes UAPI visible symbols... did we think about that?
> 
> Should be fine. Old programs won't use the new bits,
> and it just uses not yet used bits.

Old programs (that used the above symbols) will now fail to compile.
Even if they won't use the new bits, that seems like a bad thing.

> > > +	/* These all need two bits set because they are 128bit */
> > > +	PERF_REG_X86_XMM0  = 32,
> > > +	PERF_REG_X86_XMM1  = 34,
> > > +	PERF_REG_X86_XMM2  = 36,
> > > +	PERF_REG_X86_XMM3  = 38,
> > > +	PERF_REG_X86_XMM4  = 40,
> > > +	PERF_REG_X86_XMM5  = 42,
> > > +	PERF_REG_X86_XMM6  = 44,
> > > +	PERF_REG_X86_XMM7  = 46,
> > > +	PERF_REG_X86_XMM8  = 48,
> > > +	PERF_REG_X86_XMM9  = 50,
> > > +	PERF_REG_X86_XMM10 = 52,
> > > +	PERF_REG_X86_XMM11 = 54,
> > > +	PERF_REG_X86_XMM12 = 56,
> > > +	PERF_REG_X86_XMM13 = 58,
> > > +	PERF_REG_X86_XMM14 = 60,
> > > +	PERF_REG_X86_XMM15 = 62,
> > > +
> > > +	/* This does not include the XMMX registers */
> > > +	PERF_REG_GPR_X86_32_MAX = PERF_REG_X86_GS + 1,
> > > +	PERF_REG_GPR_X86_64_MAX = PERF_REG_X86_R15 + 1,
> > > +
> > > +	/* All registers include the XMMX registers */
> > > +	PERF_REG_X86_MAX = PERF_REG_X86_XMM15 + 2,
> > >  };
> > >  #endif /* _ASM_X86_PERF_REGS_H */
> > 
> > Also, what happens if we run a 32bit kernel or 32bit compat task?
> > 
> > Then the register dump will report PERF_SAMPLE_REGS_ABI_32, should we
> > then still interpret the XMM registers as 2x64bit?
> 
> Yes XMM registers are 128bit in 32bit mode too.
> 
> > 
> > Are they still at the same offset?
> 
> Yes.

I think that is broken.. perf_prepare_sample() does:

 size += hweight(mask) * sizeof(u64);

And since 32bits will not have r8-r15 set, the XMM registers will shift
forward no?

> > Do we need additional PERF_SAMPLE_REGS_ABI_* definitions for this?
> 
> I don't think so.

because....?

Powered by blists - more mailing lists