All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Feng Tang <feng.tang@intel.com>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	x86@kernel.org, kernel test robot <oliver.sang@intel.com>,
	linux-kernel@vger.kernel.org
Cc: Feng Tang <feng.tang@intel.com>
Subject: Re: [PATCH] x86/fpu: set X86_FEATURE_OSXSAVE feature after enabling OSXSAVE in CR4
Date: Thu, 24 Aug 2023 11:01:18 +0200	[thread overview]
Message-ID: <87r0nsddb5.ffs@tglx> (raw)
In-Reply-To: <20230823065747.92257-1-feng.tang@intel.com>

On Wed, Aug 23 2023 at 14:57, Feng Tang wrote:
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 0bab497c9436..8ebea0d522d2 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -173,6 +173,9 @@ void fpu__init_cpu_xstate(void)
>  
>  	cr4_set_bits(X86_CR4_OSXSAVE);
>  
> +	if (!boot_cpu_has(X86_FEATURE_OSXSAVE))
> +		setup_force_cpu_cap(X86_FEATURE_OSXSAVE);

This is wrong in several aspects:

     1) You force the feature bit _before_ XSAVE is completely
        initialized. fpu__init_system_xstate() has error paths which
        disable XSAVE.

     2) This conditional should have been a red flag for you simply
        because fpu__init_cpu_xstate() is invoked on all CPUs not only
        on the BSP.

I fixed it up and added a proper comment explaining it.

  parent reply	other threads:[~2023-08-24  9:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-23  6:57 [PATCH] x86/fpu: set X86_FEATURE_OSXSAVE feature after enabling OSXSAVE in CR4 Feng Tang
2023-08-23 22:16 ` Edgecombe, Rick P
2023-08-24  1:46   ` Feng Tang
2023-08-24  9:01 ` Thomas Gleixner [this message]
2023-08-24 11:58   ` Feng Tang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r0nsddb5.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=feng.tang@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oliver.sang@intel.com \
    --cc=peterz@infradead.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.