Not all paths that lead to fpu__init_disable_system_xstate() currently
emit a message indicating that XSAVE has been disabled. Move the print
statement into the function to ensure the message in all cases.
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Link: https://lore.kernel.org/lkml/d6d19e39-2749-4d45-aeab-a209a0ecba17@intel.com
---
New patch for following up patch 3.
---
arch/x86/kernel/fpu/xstate.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 2ac1fc182273..8b14c9d3a1df 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -751,6 +751,8 @@ static int __init init_xstate_size(void)
*/
static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
{
+ pr_info("x86/fpu: XSAVE disabled\n");
+
fpu_kernel_cfg.max_features = 0;
cr4_clear_bits(X86_CR4_OSXSAVE);
setup_clear_cpu_cap(X86_FEATURE_XSAVE);
@@ -821,7 +823,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
* This is a problematic CPU configuration where two
* conflicting state components are both enumerated.
*/
- pr_err("x86/fpu: Both APX/MPX present in the CPU's xstate features: 0x%llx, disabling XSAVE.\n",
+ pr_err("x86/fpu: Both APX/MPX present in the CPU's xstate features: 0x%llx.\n",
fpu_kernel_cfg.max_features);
goto out_disable;
}
@@ -900,7 +902,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
init_fpstate.xfeatures = fpu_kernel_cfg.default_features;
if (init_fpstate.size > sizeof(init_fpstate.regs)) {
- pr_warn("x86/fpu: init_fpstate buffer too small (%zu < %d), disabling XSAVE\n",
+ pr_warn("x86/fpu: init_fpstate buffer too small (%zu < %d)\n",
sizeof(init_fpstate.regs), init_fpstate.size);
goto out_disable;
}
@@ -912,7 +914,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
* xfeatures mask.
*/
if (xfeatures != fpu_kernel_cfg.max_features) {
- pr_err("x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during init, disabling XSAVE\n",
+ pr_err("x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during init\n",
xfeatures, fpu_kernel_cfg.max_features);
goto out_disable;
}
--
2.45.2On 4/15/2025 7:16 PM, Chang S. Bae wrote:
> Not all paths that lead to fpu__init_disable_system_xstate() currently
> emit a message indicating that XSAVE has been disabled. Move the print
> statement into the function to ensure the message in all cases.
>
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
> Link: https://lore.kernel.org/lkml/d6d19e39-2749-4d45-aeab-a209a0ecba17@intel.com
> ---
> New patch for following up patch 3.
> ---
> arch/x86/kernel/fpu/xstate.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 2ac1fc182273..8b14c9d3a1df 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -751,6 +751,8 @@ static int __init init_xstate_size(void)
> */
> static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
> {
> + pr_info("x86/fpu: XSAVE disabled\n");
> +
There is a mix of pr_info(), pr_warn() and pr_err() to log these related
messages. Would it be useful to make the log level consistent in this
patch or a follow-up?
I think new the "XSAVE disabled" print should be a pr_warn() at least.
> fpu_kernel_cfg.max_features = 0;
> cr4_clear_bits(X86_CR4_OSXSAVE);
> setup_clear_cpu_cap(X86_FEATURE_XSAVE);
> @@ -821,7 +823,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
> * This is a problematic CPU configuration where two
> * conflicting state components are both enumerated.
> */
> - pr_err("x86/fpu: Both APX/MPX present in the CPU's xstate features: 0x%llx, disabling XSAVE.\n",
> + pr_err("x86/fpu: Both APX/MPX present in the CPU's xstate features: 0x%llx.\n",
> fpu_kernel_cfg.max_features);
> goto out_disable;
> }
> @@ -900,7 +902,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
> init_fpstate.xfeatures = fpu_kernel_cfg.default_features;
>
> if (init_fpstate.size > sizeof(init_fpstate.regs)) {
> - pr_warn("x86/fpu: init_fpstate buffer too small (%zu < %d), disabling XSAVE\n",
> + pr_warn("x86/fpu: init_fpstate buffer too small (%zu < %d)\n",
> sizeof(init_fpstate.regs), init_fpstate.size);
> goto out_disable;
> }
> @@ -912,7 +914,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
> * xfeatures mask.
> */
> if (xfeatures != fpu_kernel_cfg.max_features) {
> - pr_err("x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during init, disabling XSAVE\n",
> + pr_err("x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during init\n",
> xfeatures, fpu_kernel_cfg.max_features);
> goto out_disable;
> }
On 4/16/2025 9:56 AM, Sohil Mehta wrote:
> On 4/15/2025 7:16 PM, Chang S. Bae wrote:
>>
>> static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
>> {
>> + pr_info("x86/fpu: XSAVE disabled\n");
>> +
>
> There is a mix of pr_info(), pr_warn() and pr_err() to log these related
> messages. Would it be useful to make the log level consistent in this
> patch or a follow-up?
>
> I think new the "XSAVE disabled" print should be a pr_warn() at least.
I think pr_info() makes sense here, as it aligns with this hunk:
static __init void disable_smp(void)
{
pr_info("SMP disabled\n");
disable_ioapic_support();
topology_reset_possible_cpus_up();
cpumask_set_cpu(0, topology_sibling_cpumask(0));
cpumask_set_cpu(0, topology_core_cpumask(0));
cpumask_set_cpu(0, topology_die_cpumask(0));
}
If you strongly feel that pr_warn() is more appropriate, then it would
make sense to update all related messages consistently. But honestly, I
don’t think it’s a big deal either way.
On 4/16/2025 10:03 AM, Chang S. Bae wrote:
> If you strongly feel that pr_warn() is more appropriate, then it would
> make sense to update all related messages consistently. But honestly, I
> don’t think it’s a big deal either way.
I don't have a strong preference.
But, for someone who is using log levels to reduce the kernel log
details, the below fpu-specific messages probably don't seem to be in
priority order.
pr_info("x86/fpu: XSAVE disabled\n");
pr_warn("x86/fpu: init_fpstate buffer too small (%zu < %d)\n",
pr_err("x86/fpu: Both APX/MPX present in the CPU's xstate features:
0x%llx.\n",
pr_err("x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during
init\n",
pr_err("x86/fpu: FP/SSE not present amongst the CPU's xstate features:
0x%llx.\n",
"XSAVE disabled" seems most important, with the rest just being internal
details. Maybe it's just me... Feel free to ignore unless someone else
chimes in.
The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: 39cd7fad39ce2ffbeb21939c805ee55f3ec808d4
Gitweb: https://git.kernel.org/tip/39cd7fad39ce2ffbeb21939c805ee55f3ec808d4
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:16:56 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 09:44:15 +02:00
x86/fpu: Log XSAVE disablement consistently
Not all paths that lead to fpu__init_disable_system_xstate() currently
emit a message indicating that XSAVE has been disabled. Move the print
statement into the function to ensure the message in all cases.
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/20250416021720.12305-7-chang.seok.bae@intel.com
---
arch/x86/kernel/fpu/xstate.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 2ac1fc1..8b14c9d 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -751,6 +751,8 @@ static int __init init_xstate_size(void)
*/
static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
{
+ pr_info("x86/fpu: XSAVE disabled\n");
+
fpu_kernel_cfg.max_features = 0;
cr4_clear_bits(X86_CR4_OSXSAVE);
setup_clear_cpu_cap(X86_FEATURE_XSAVE);
@@ -821,7 +823,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
* This is a problematic CPU configuration where two
* conflicting state components are both enumerated.
*/
- pr_err("x86/fpu: Both APX/MPX present in the CPU's xstate features: 0x%llx, disabling XSAVE.\n",
+ pr_err("x86/fpu: Both APX/MPX present in the CPU's xstate features: 0x%llx.\n",
fpu_kernel_cfg.max_features);
goto out_disable;
}
@@ -900,7 +902,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
init_fpstate.xfeatures = fpu_kernel_cfg.default_features;
if (init_fpstate.size > sizeof(init_fpstate.regs)) {
- pr_warn("x86/fpu: init_fpstate buffer too small (%zu < %d), disabling XSAVE\n",
+ pr_warn("x86/fpu: init_fpstate buffer too small (%zu < %d)\n",
sizeof(init_fpstate.regs), init_fpstate.size);
goto out_disable;
}
@@ -912,7 +914,7 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
* xfeatures mask.
*/
if (xfeatures != fpu_kernel_cfg.max_features) {
- pr_err("x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during init, disabling XSAVE\n",
+ pr_err("x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during init\n",
xfeatures, fpu_kernel_cfg.max_features);
goto out_disable;
}© 2016 - 2025 Red Hat, Inc.