linux-kernel - Re: [PATCH v2 01/22] x86/fpu/xstate: Modify area init helper prototypes to access all the possible areas

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: <2c4c4340feaf8542fa41e9f4563ecb2b58eef996.camel@intel.com>
Date:   Mon, 7 Dec 2020 23:03:27 +0000
From:   "Bae, Chang Seok" <chang.seok.bae@...el.com>
To:     "bp@...e.de" <bp@...e.de>
CC:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "mingo@...nel.org" <mingo@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "Liu, Jing2" <jing2.liu@...el.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "luto@...nel.org" <luto@...nel.org>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "Brown, Len" <len.brown@...el.com>
Subject: Re: [PATCH v2 01/22] x86/fpu/xstate: Modify area init helper
 prototypes to access all the possible areas

> On Dec 8, 2020, at 02:12, Borislav Petkov <bp@...e.de> wrote:
> 
> On Thu, Nov 19, 2020 at 03:32:36PM -0800, Chang S. Bae wrote:
> > The xstate infrastructure is not flexible to support dynamic areas in
> > task->fpu.
> 
> task->fpu?

It was considered to be concise to represent, but it looks to be 
unreadable.

> Do you mean the fpu member in struct thread_struct ?

Yes. Will make sure to use this for clarification at fist.

> > Change the fpstate_init() prototype to access task->fpu directly. It
> > treats a null pointer as indicating init_fpstate, as this initial data
> > does not belong to any task.
> 
> What for? Commit messages should state *why* you're doing a change - not
> *what* you're doing. *What* I can more or less see, *why* is harder.

An earlier version had wordy explanations, but it looks too much trimmed
down.

(I suspect this point applicable to PATCH2-4 as well.)

> /me goes and looks forward into the patchset...
> 
> Are you going to need it for stuff like
> 
> 	fpu ? fpu->state_mask : get_init_fpstate_mask()
> 
> ?

Yes, I think that’s one of the cases.

> If so, why don't you write *why* you're doing those changes here?

Will do that.

Thanks,
Chang