linux-kernel - RE: [PATCH 3/3] X86, mpx: Intel MPX xstate feature definition

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: <9E0BE1322F2F2246BD820DA9FC397ADE014A4374@SHSMSX102.ccr.corp.intel.com>
Date:	Fri, 6 Dec 2013 16:08:45 +0000
From:	"Ren, Qiaowei" <qiaowei.ren@...el.com>
To:	Borislav Petkov <bp@...en8.de>
CC:	Paolo Bonzini <pbonzini@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"x86@...nel.org" <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"qemu-devel@...gnu.org" <qemu-devel@...gnu.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	Xudong Hao <xudong.hao@...el.com>,
	"Liu, Jinsong" <jinsong.liu@...el.com>
Subject: RE: [PATCH 3/3] X86, mpx: Intel MPX xstate feature definition


> -----Original Message-----
> From: Borislav Petkov [mailto:bp@...en8.de]
> Sent: Friday, December 06, 2013 9:47 PM
> To: Ren, Qiaowei
> Cc: Paolo Bonzini; H. Peter Anvin; Ingo Molnar; Thomas Gleixner;
> x86@...nel.org; linux-kernel@...r.kernel.org; qemu-devel@...gnu.org;
> kvm@...r.kernel.org; Xudong Hao; Liu, Jinsong
> Subject: Re: [PATCH 3/3] X86, mpx: Intel MPX xstate feature definition
> 
> On Sat, Dec 07, 2013 at 02:52:56AM +0800, Qiaowei Ren wrote:
> 
> Commit message please.
> 
> > Signed-off-by: Qiaowei Ren <qiaowei.ren@...el.com>
> > Signed-off-by: Xudong Hao <xudong.hao@...el.com>
> > Signed-off-by: Liu Jinsong <jinsong.liu@...el.com>
> > ---
> >  arch/x86/include/asm/processor.h |   23 +++++++++++++++++++++++
> >  arch/x86/include/asm/xsave.h     |    6 +++++-
> >  2 files changed, 28 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/processor.h
> > b/arch/x86/include/asm/processor.h
> > index 987c75e..43be6f6 100644
> > --- a/arch/x86/include/asm/processor.h
> > +++ b/arch/x86/include/asm/processor.h
> > @@ -370,6 +370,26 @@ struct ymmh_struct {
> >  	u32 ymmh_space[64];
> >  };
> >
> > +struct lwp_struct {
> > +	u64 lwpcb_addr;
> > +	u32 flags;
> > +	u32 buf_head_offset;
> > +	u64 buf_base;
> > +	u32 buf_size;
> > +	u32 filters;
> > +	u64 saved_event_record[4];
> > +	u32 event_counter[16];
> > +};
> > +
> > +struct bndregs_struct {
> > +	u64 bndregs[8];
> > +} __packed;
> > +
> > +struct bndcsr_struct {
> > +	u64 cfg_reg_u;
> > +	u64 status_reg;
> > +} __packed;
> > +
> >  struct xsave_hdr_struct {
> >  	u64 xstate_bv;
> >  	u64 reserved1[2];
> > @@ -380,6 +400,9 @@ struct xsave_struct {
> >  	struct i387_fxsave_struct i387;
> >  	struct xsave_hdr_struct xsave_hdr;
> >  	struct ymmh_struct ymmh;
> > +	struct lwp_struct lwp;
> 
> I'm guessing this and the struct lwp_struct above is being added so that you
> can have the LWP XSAVE area size? If so, you don't need it: LWP XSAVE area is
> 128 bytes at offset 832 according to my manuals so I'd guess having a u8
> lwp_area[128] should be fine.
> 
Yes, currently it is only for the LWP XSAVE area size. I will directly use a u8 lwp_area[128].

> 
> > +	struct bndregs_struct bndregs;
> > +	struct bndcsr_struct bndcsr;
> >  	/* new processor state extensions will go here */  } __attribute__
> > ((packed, aligned (64)));
> >
> > diff --git a/arch/x86/include/asm/xsave.h
> > b/arch/x86/include/asm/xsave.h index 0415cda..5cd9de3 100644
> > --- a/arch/x86/include/asm/xsave.h
> > +++ b/arch/x86/include/asm/xsave.h
> > @@ -9,6 +9,8 @@
> >  #define XSTATE_FP	0x1
> >  #define XSTATE_SSE	0x2
> >  #define XSTATE_YMM	0x4
> > +#define XSTATE_BNDREGS	0x8
> > +#define XSTATE_BNDCSR	0x10
> >
> >  #define XSTATE_FPSSE	(XSTATE_FP | XSTATE_SSE)
> >
> > @@ -20,10 +22,12 @@
> >  #define XSAVE_YMM_SIZE	    256
> >  #define XSAVE_YMM_OFFSET    (XSAVE_HDR_SIZE +
> XSAVE_HDR_OFFSET)
> >
> > +#define XSTATE_FLEXIBLE (XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> 
> What's the use of that macro if it is used only once?
> 
Yes. Maybe it is best to use previous pattern.

> > +#define XSTATE_EAGER	(XSTATE_BNDREGS | XSTATE_BNDCSR)
> >  /*
> >   * These are the features that the OS can handle currently.
> >   */
> > -#define XCNTXT_MASK	(XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> > +#define XCNTXT_MASK	(XSTATE_FLEXIBLE | XSTATE_EAGER)
> >
> >  #ifdef CONFIG_X86_64
> >  #define REX_PREFIX	"0x48, "
> 
Thanks,
Qiaowei