linux-kernel - Re: [PATCH v2 1/2] x86/fpu: Add a helper to prepare AMX state for low-power CPU idle

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: <13346402-7580-d60e-bb88-3172dd60406f@intel.com>
Date:   Thu, 10 Mar 2022 13:00:56 -0800
From:   "Chang S. Bae" <chang.seok.bae@...el.com>
To:     Dave Hansen <dave.hansen@...el.com>,
        <linux-kernel@...r.kernel.org>, <x86@...nel.org>,
        <linux-pm@...r.kernel.org>
CC:     <tglx@...utronix.de>, <dave.hansen@...ux.intel.com>,
        <peterz@...radead.org>, <bp@...en8.de>, <rafael@...nel.org>,
        <ravi.v.shankar@...el.com>
Subject: Re: [PATCH v2 1/2] x86/fpu: Add a helper to prepare AMX state for
 low-power CPU idle

On 3/9/2022 4:24 PM, Dave Hansen wrote:
> 
> I assume that splat is because 0day found a CPU which doesn't support
> XGETBV1.  Since fpu_state_size_dynamic() only ever returns true on
> XGETBV1 systems so it works as a proxy for checking XGETBV1 support.
> 
> Right? >
> If so, then fpu_state_size_dynamic() is a *bit* of an oblique way to
> check for XGETBV1 support.
> > Why don't we do a good old:
> 
> 	cpu_feature_enabled(X86_FEATURE_XGETBV1)
> 
> check?

Agreed, checking XGETBV1 support is the reason for this, so this looks 
to be straightforward here.

> 
> Also, did we get the asm constraints wrong on xgetbv()?  Surely we
> shouldn't be allowing the compiler to reorder it.  Do we need a "memory"
> constraint?

I think this is a good point. Perhaps x{get|set}bv() may follow this 
change [1] to prevent any reordering.

BTW, now I'm suspicious of this JMP as patched at runtime with 
fpu_state_size_dynamic():

   22:   eb 01                    jmp    0x25
   24:   c3                       retq
   25:   b9 01 00 00 00           mov    $0x1,%ecx
   2a:*  0f 01 d0                 xgetbv           <-- trapping instruction

Still, the question is, if so, why it was patched on non-XFD systems. 
Let me analyze the case a bit further with 0day folks.

Thanks,
Chang

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aa5cacdc29d76a005cbbee018a47faa6e724dd2d