Intel Advanced Performance Extensions (APX) introduce a new set of
general-purpose registers, managed as an extended state component via the
xstate management facility.
Before enabling this new xstate, define a feature flag to clarify the
dependency in xsave_cpuid_features[]. APX is enumerated under CPUID level
7 with EDX=1. Since this CPUID leaf is not yet allocated, place the flag
in a scattered feature word.
While this feature is intended only for userspace, exposing it via
/proc/cpuinfo is unnecessary. Instead, the existing arch_prctl(2)
mechanism with the ARCH_GET_XCOMP_SUPP option can be used to query the
feature availability.
Finally, clarify that APX depends on XSAVE.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
Allocating a new feature word for this bit seems excessive at this stage,
given that no other imminent features are quite known.
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/kernel/cpu/cpuid-deps.c | 1 +
arch/x86/kernel/cpu/scattered.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 8b7cf13e0acb..51178d4a6308 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -479,6 +479,7 @@
#define X86_FEATURE_AMD_FAST_CPPC (21*32 + 5) /* Fast CPPC */
#define X86_FEATURE_AMD_HETEROGENEOUS_CORES (21*32 + 6) /* Heterogeneous Core Topology */
#define X86_FEATURE_AMD_WORKLOAD_CLASS (21*32 + 7) /* Workload Classification */
+#define X86_FEATURE_APX (21*32 + 8) /* Advanced Performance Extensions */
/*
* BUG word(s)
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index a2fbea0be535..d5e5013e0e9f 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -84,6 +84,7 @@ static const struct cpuid_dep cpuid_deps[] = {
{ X86_FEATURE_AMX_TILE, X86_FEATURE_XFD },
{ X86_FEATURE_SHSTK, X86_FEATURE_XSAVES },
{ X86_FEATURE_FRED, X86_FEATURE_LKGS },
+ { X86_FEATURE_APX, X86_FEATURE_XSAVE },
{}
};
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 16f3ca30626a..6c40d5af8479 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -54,6 +54,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_AMD_LBR_V2, CPUID_EAX, 1, 0x80000022, 0 },
{ X86_FEATURE_AMD_LBR_PMC_FREEZE, CPUID_EAX, 2, 0x80000022, 0 },
{ X86_FEATURE_AMD_HETEROGENEOUS_CORES, CPUID_EAX, 30, 0x80000026, 0 },
+ { X86_FEATURE_APX, CPUID_EDX, 21, 0x00000007, 1 },
{ 0, 0, 0, 0, 0 }
};
--
2.45.2Intel Advanced Performance Extensions (APX) introduce a new set of
general-purpose registers, managed as an extended state component via the
xstate management facility.
Before enabling this new xstate, define a feature flag to clarify the
dependency in xsave_cpuid_features[]. APX is enumerated under CPUID level
7 with EDX=1. Since this CPUID leaf is not yet allocated, place the flag
in a scattered feature word.
While this feature is intended only for userspace, exposing it via
/proc/cpuinfo is unnecessary. Instead, the existing arch_prctl(2)
mechanism with the ARCH_GET_XCOMP_SUPP option can be used to query the
feature availability.
Finally, clarify that APX depends on XSAVE.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
Rebased onto v6.15-rc1 with commit:
968e9bc4cef8 ("x86: move ZMM exclusion list into CPU feature flag")
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/kernel/cpu/cpuid-deps.c | 1 +
arch/x86/kernel/cpu/scattered.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 6c2c152d8a67..5445937eff3d 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -481,6 +481,7 @@
#define X86_FEATURE_AMD_HETEROGENEOUS_CORES (21*32 + 6) /* Heterogeneous Core Topology */
#define X86_FEATURE_AMD_WORKLOAD_CLASS (21*32 + 7) /* Workload Classification */
#define X86_FEATURE_PREFER_YMM (21*32 + 8) /* Avoid ZMM registers due to downclocking */
+#define X86_FEATURE_APX (21*32 + 9) /* Advanced #Performance Extensions */
/*
* BUG word(s)
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 94c062cddfa4..7c1268138a7a 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -88,6 +88,7 @@ static const struct cpuid_dep cpuid_deps[] = {
{ X86_FEATURE_SHSTK, X86_FEATURE_XSAVES },
{ X86_FEATURE_FRED, X86_FEATURE_LKGS },
{ X86_FEATURE_SPEC_CTRL_SSBD, X86_FEATURE_SPEC_CTRL },
+ { X86_FEATURE_APX, X86_FEATURE_XSAVE },
{}
};
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 16f3ca30626a..6c40d5af8479 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -54,6 +54,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_AMD_LBR_V2, CPUID_EAX, 1, 0x80000022, 0 },
{ X86_FEATURE_AMD_LBR_PMC_FREEZE, CPUID_EAX, 2, 0x80000022, 0 },
{ X86_FEATURE_AMD_HETEROGENEOUS_CORES, CPUID_EAX, 30, 0x80000026, 0 },
+ { X86_FEATURE_APX, CPUID_EDX, 21, 0x00000007, 1 },
{ 0, 0, 0, 0, 0 }
};
--
2.45.2On 4/11/2025 9:12 AM, Chang S. Bae wrote:
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 6c2c152d8a67..5445937eff3d 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -481,6 +481,7 @@
> #define X86_FEATURE_AMD_HETEROGENEOUS_CORES (21*32 + 6) /* Heterogeneous Core Topology */
> #define X86_FEATURE_AMD_WORKLOAD_CLASS (21*32 + 7) /* Workload Classification */
> #define X86_FEATURE_PREFER_YMM (21*32 + 8) /* Avoid ZMM registers due to downclocking */
> +#define X86_FEATURE_APX (21*32 + 9) /* Advanced #Performance Extensions */
>
Is the '#' before 'Performance' intentional? The previous version didn't
seem to have it.
> /*
> * BUG word(s)
> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
> index 94c062cddfa4..7c1268138a7a 100644
> --- a/arch/x86/kernel/cpu/cpuid-deps.c
> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> @@ -88,6 +88,7 @@ static const struct cpuid_dep cpuid_deps[] = {
> { X86_FEATURE_SHSTK, X86_FEATURE_XSAVES },
> { X86_FEATURE_FRED, X86_FEATURE_LKGS },
> { X86_FEATURE_SPEC_CTRL_SSBD, X86_FEATURE_SPEC_CTRL },
> + { X86_FEATURE_APX, X86_FEATURE_XSAVE },
I don't think this table follows any specific logic, but recent patches
have tried grouping by similar features or features with similar
dependencies.
I don't have a preference, but should this be inserted closer to other
XSAVE dependent features?
{ X86_FEATURE_PKU, X86_FEATURE_XSAVE },
{ X86_FEATURE_MPX, X86_FEATURE_XSAVE },
{ X86_FEATURE_XGETBV1, X86_FEATURE_XSAVE },
+ { X86_FEATURE_APX, X86_FEATURE_XSAVE },
{ X86_FEATURE_CMOV, X86_FEATURE_FXSR },
> {}
> };
>
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 16f3ca30626a..6c40d5af8479 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -54,6 +54,7 @@ static const struct cpuid_bit cpuid_bits[] = {
> { X86_FEATURE_AMD_LBR_V2, CPUID_EAX, 1, 0x80000022, 0 },
> { X86_FEATURE_AMD_LBR_PMC_FREEZE, CPUID_EAX, 2, 0x80000022, 0 },
> { X86_FEATURE_AMD_HETEROGENEOUS_CORES, CPUID_EAX, 30, 0x80000026, 0 },
> + { X86_FEATURE_APX, CPUID_EDX, 21, 0x00000007, 1 },
There is a comment on top of that table that says:
"Please keep the leaf sorted by cpuid_bit.level for faster search."
Based on that, APX should be inserted here:
{ X86_FEATURE_INTEL_PPIN, CPUID_EBX, 0, 0x00000007, 1 },
+{ X86_FEATURE_APX, CPUID_EDX, 21, 0x00000007, 1 },
{ X86_FEATURE_RRSBA_CTRL, CPUID_EDX, 2, 0x00000007, 2 },
> { 0, 0, 0, 0, 0 }
> };
>
On 4/11/2025 9:54 AM, Sohil Mehta wrote:
> On 4/11/2025 9:12 AM, Chang S. Bae wrote:
>
>> +#define X86_FEATURE_APX (21*32 + 9) /* Advanced #Performance Extensions */
>
> Is the '#' before 'Performance' intentional? The previous version didn't
> seem to have it.
Oh no — I only meant to update the bit position. My bad!
> I don't think this table follows any specific logic, but recent patches
> have tried grouping by similar features or features with similar
> dependencies.
>
> I don't have a preference, but should this be inserted closer to other
> XSAVE dependent features?
>
> { X86_FEATURE_PKU, X86_FEATURE_XSAVE },
> { X86_FEATURE_MPX, X86_FEATURE_XSAVE },
> { X86_FEATURE_XGETBV1, X86_FEATURE_XSAVE },
> + { X86_FEATURE_APX, X86_FEATURE_XSAVE },
> { X86_FEATURE_CMOV, X86_FEATURE_FXSR },
> There is a comment on top of that table that says:
> "Please keep the leaf sorted by cpuid_bit.level for faster search."
>
> Based on that, APX should be inserted here:
>
> { X86_FEATURE_INTEL_PPIN, CPUID_EBX, 0, 0x00000007, 1 },
> +{ X86_FEATURE_APX, CPUID_EDX, 21, 0x00000007, 1 },
> { X86_FEATURE_RRSBA_CTRL, CPUID_EDX, 2, 0x00000007, 2 },Yeah, both suggestions make sense. Thanks for pointing them out!
I've attached the patch revision.
Thanks,
Chang
From a3a68edf35c43162a57dad80ea2eecc9c1101246 Mon Sep 17 00:00:00 2001
From: "Chang S. Bae" <chang.seok.bae@intel.com>
Date: Thu, 13 Feb 2025 23:00:28 -0800
Subject: [PATCH RFC v2b 5/9] x86/cpufeatures: Add X86_FEATURE_APX
Intel Advanced Performance Extensions (APX) introduce a new set of
general-purpose registers, managed as an extended state component via the
xstate management facility.
Before enabling this new xstate, define a feature flag to clarify the
dependency in xsave_cpuid_features[]. APX is enumerated under CPUID level
7 with EDX=1. Since this CPUID leaf is not yet allocated, place the flag
in a scattered feature word.
While this feature is intended only for userspace, exposing it via
/proc/cpuinfo is unnecessary. Instead, the existing arch_prctl(2)
mechanism with the ARCH_GET_XCOMP_SUPP option can be used to query the
feature availability.
Finally, clarify that APX depends on XSAVE.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
RFC-V2a -> RFC-V2b: Fix typo and organize APX entries orderly (Sohil)
RFC-V2 -> RFC-V2a: Rebased onto v6.15-rc1; resolve conflict with commit
968e9bc4cef8 ("x86: move ZMM exclusion list into CPU feature flag")
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/kernel/cpu/cpuid-deps.c | 1 +
arch/x86/kernel/cpu/scattered.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 6c2c152d8a67..eb73f3f0ec70 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -481,6 +481,7 @@
#define X86_FEATURE_AMD_HETEROGENEOUS_CORES (21*32 + 6) /* Heterogeneous Core Topology */
#define X86_FEATURE_AMD_WORKLOAD_CLASS (21*32 + 7) /* Workload Classification */
#define X86_FEATURE_PREFER_YMM (21*32 + 8) /* Avoid ZMM registers due to downclocking */
+#define X86_FEATURE_APX (21*32 + 9) /* Advanced Performance Extensions */
/*
* BUG word(s)
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 94c062cddfa4..46efcbd6afa4 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -28,6 +28,7 @@ static const struct cpuid_dep cpuid_deps[] = {
{ X86_FEATURE_PKU, X86_FEATURE_XSAVE },
{ X86_FEATURE_MPX, X86_FEATURE_XSAVE },
{ X86_FEATURE_XGETBV1, X86_FEATURE_XSAVE },
+ { X86_FEATURE_APX, X86_FEATURE_XSAVE },
{ X86_FEATURE_CMOV, X86_FEATURE_FXSR },
{ X86_FEATURE_MMX, X86_FEATURE_FXSR },
{ X86_FEATURE_MMXEXT, X86_FEATURE_MMX },
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 16f3ca30626a..ffb80b5ad97f 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -27,6 +27,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 },
{ X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 },
{ X86_FEATURE_INTEL_PPIN, CPUID_EBX, 0, 0x00000007, 1 },
+ { X86_FEATURE_APX, CPUID_EDX, 21, 0x00000007, 1 },
{ X86_FEATURE_RRSBA_CTRL, CPUID_EDX, 2, 0x00000007, 2 },
{ X86_FEATURE_BHI_CTRL, CPUID_EDX, 4, 0x00000007, 2 },
{ X86_FEATURE_CQM_LLC, CPUID_EDX, 1, 0x0000000f, 0 },
--
2.45.2
On 4/11/2025 11:23 AM, Chang S. Bae wrote: > I've attached the patch revision. > LGTM, Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
* Sohil Mehta <sohil.mehta@intel.com> wrote: > On 4/11/2025 11:23 AM, Chang S. Bae wrote: > > > I've attached the patch revision. > > > > LGTM, > > Reviewed-by: Sohil Mehta <sohil.mehta@intel.com> Chang, mind sending a series of the latest version of all the pending APX patches you have at the moment (and any other pending FPU patches you may have), with Reviewed-by tags rolled in, etc., on top of: git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/fpu Thanks, Ingo
On 4/12/2025 1:43 AM, Ingo Molnar wrote:
>
> Chang, mind sending a series of the latest version of all the pending
> APX patches you have at the moment (and any other pending FPU patches
> you may have), with Reviewed-by tags rolled in, etc.
Hi Ingo,
Here’s the updated patch set following up on the previous APX series [1],
along with a collection of additional FPU-related cleanups and
improvements that were previously posted or discussed.
The series is organized into two parts:
1. APX Enabling (PATCH 1–5)
These patches complete the APX bring-up. After laying the groundwork,
this portion finalizes the enablement:
* Patches 1, 2, and 4 are typical xfeature plumbing.
* Patch 3 handles MPX conflict -- unexpected hardware issue
* Patch 5 adds a test case.
2. Miscellaneous FPU Code Improvements (PATCH 6–10)
This batch includes various standalone improvements:
* Patch 6: Centralizes the XSAVE disablement message
* Patches 7-8: Simplifies PKRU update in XSTATE_BV on sigframe
* Patch 9: Removes the unused mxcsr_feature_mask export
* Patch 10: Renames fpu_reset_fpregs() for clarity
Each patch includes context and links to earlier discussions or
revisions.
As you noted in [2], the series is based on the x86/fpu branch and is
available at:
git://github.com/intel/apx.git apx-and-misc
[1]: https://lore.kernel.org/lkml/20250320234301.8342-1-chang.seok.bae@intel.com
[2]: https://lore.kernel.org/lkml/Z_zGCCNE_Qt3IlMZ@gmail.com
Thanks,
Chang
Chang S. Bae (10):
x86/cpufeatures: Add X86_FEATURE_APX
x86/fpu/apx: Define APX state component
x86/fpu/apx: Disallow conflicting MPX presence
x86/fpu/apx: Enable APX state support
selftests/x86/apx: Add APX test
x86/fpu: Log XSAVE disablement consistently
x86/fpu: Refactor xfeature bitmask update code for sigframe XSAVE
x86/pkeys: Simplify PKRU update in signal frame
x86/fpu: Remove export of mxcsr_feature_mask
x86/fpu: Rename fpu_reset_fpregs() to fpu_reset_fpstate_regs()
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/fpu/types.h | 9 +++++++++
arch/x86/include/asm/fpu/xstate.h | 3 ++-
arch/x86/kernel/cpu/cpuid-deps.c | 1 +
arch/x86/kernel/cpu/scattered.c | 1 +
arch/x86/kernel/fpu/core.c | 6 +++---
arch/x86/kernel/fpu/init.c | 1 -
arch/x86/kernel/fpu/signal.c | 11 +----------
arch/x86/kernel/fpu/xstate.c | 23 ++++++++++++++++++++---
arch/x86/kernel/fpu/xstate.h | 21 +++++++++++++++------
tools/testing/selftests/x86/Makefile | 3 ++-
tools/testing/selftests/x86/apx.c | 10 ++++++++++
tools/testing/selftests/x86/xstate.c | 3 ++-
tools/testing/selftests/x86/xstate.h | 2 ++
14 files changed, 69 insertions(+), 26 deletions(-)
create mode 100644 tools/testing/selftests/x86/apx.c
base-commit: e3a52b67f54aa36ab21265eeea016460b5fe1c46
--
2.45.2
* Chang S. Bae <chang.seok.bae@intel.com> wrote: > On 4/12/2025 1:43 AM, Ingo Molnar wrote: > > > > Chang, mind sending a series of the latest version of all the pending > > APX patches you have at the moment (and any other pending FPU patches > > you may have), with Reviewed-by tags rolled in, etc. > > Hi Ingo, > > Here’s the updated patch set following up on the previous APX series [1], > along with a collection of additional FPU-related cleanups and > improvements that were previously posted or discussed. > > The series is organized into two parts: > > 1. APX Enabling (PATCH 1–5) > > These patches complete the APX bring-up. After laying the groundwork, > this portion finalizes the enablement: > > * Patches 1, 2, and 4 are typical xfeature plumbing. > > * Patch 3 handles MPX conflict -- unexpected hardware issue > > * Patch 5 adds a test case. > > 2. Miscellaneous FPU Code Improvements (PATCH 6–10) > > This batch includes various standalone improvements: > > * Patch 6: Centralizes the XSAVE disablement message > > * Patches 7-8: Simplifies PKRU update in XSTATE_BV on sigframe > > * Patch 9: Removes the unused mxcsr_feature_mask export > > * Patch 10: Renames fpu_reset_fpregs() for clarity > > Each patch includes context and links to earlier discussions or > revisions. > 14 files changed, 69 insertions(+), 26 deletions(-) Applied to tip:x86/fpu, thanks! Note that I've merged the currently pending tip:x86/cpu bits into tip:x86/fpu before applying these patches, to resolve a conflict with <asm/cpufeatures.h>, and re-formatted the new X86_FEATURE_APX line to have the canonical format. Ingo
On 4/16/2025 1:07 AM, Ingo Molnar wrote: > >> 14 files changed, 69 insertions(+), 26 deletions(-) > > Applied to tip:x86/fpu, thanks! > > Note that I've merged the currently pending tip:x86/cpu bits into > tip:x86/fpu before applying these patches, to resolve a conflict with > <asm/cpufeatures.h>, and re-formatted the new X86_FEATURE_APX line to > have the canonical format. Appreciate! Chang
Intel Advanced Performance Extensions (APX) introduce a new set of
general-purpose registers, managed as an extended state component via the
xstate management facility.
Before enabling this new xstate, define a feature flag to clarify the
dependency in xsave_cpuid_features[]. APX is enumerated under CPUID level
7 with EDX=1. Since this CPUID leaf is not yet allocated, place the flag
in a scattered feature word.
While this feature is intended only for userspace, exposing it via
/proc/cpuinfo is unnecessary. Instead, the existing arch_prctl(2)
mechanism with the ARCH_GET_XCOMP_SUPP option can be used to query the
feature availability.
Finally, clarify that APX depends on XSAVE.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
---
Changes from the last posting:
https://lore.kernel.org/lkml/20250320234301.8342-6-chang.seok.bae@intel.com/
* Rebase onto v6.15-rc1; resolve conflict with commit:
968e9bc4cef8 ("x86: move ZMM exclusion list into CPU feature flag")
* Organize APX entries orderly (Sohil). Then, include his review tag
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/kernel/cpu/cpuid-deps.c | 1 +
arch/x86/kernel/cpu/scattered.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 6c2c152d8a67..eb73f3f0ec70 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -481,6 +481,7 @@
#define X86_FEATURE_AMD_HETEROGENEOUS_CORES (21*32 + 6) /* Heterogeneous Core Topology */
#define X86_FEATURE_AMD_WORKLOAD_CLASS (21*32 + 7) /* Workload Classification */
#define X86_FEATURE_PREFER_YMM (21*32 + 8) /* Avoid ZMM registers due to downclocking */
+#define X86_FEATURE_APX (21*32 + 9) /* Advanced Performance Extensions */
/*
* BUG word(s)
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index a2fbea0be535..72f4fb66ac20 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -28,6 +28,7 @@ static const struct cpuid_dep cpuid_deps[] = {
{ X86_FEATURE_PKU, X86_FEATURE_XSAVE },
{ X86_FEATURE_MPX, X86_FEATURE_XSAVE },
{ X86_FEATURE_XGETBV1, X86_FEATURE_XSAVE },
+ { X86_FEATURE_APX, X86_FEATURE_XSAVE },
{ X86_FEATURE_CMOV, X86_FEATURE_FXSR },
{ X86_FEATURE_MMX, X86_FEATURE_FXSR },
{ X86_FEATURE_MMXEXT, X86_FEATURE_MMX },
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 16f3ca30626a..ffb80b5ad97f 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -27,6 +27,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 },
{ X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 },
{ X86_FEATURE_INTEL_PPIN, CPUID_EBX, 0, 0x00000007, 1 },
+ { X86_FEATURE_APX, CPUID_EDX, 21, 0x00000007, 1 },
{ X86_FEATURE_RRSBA_CTRL, CPUID_EDX, 2, 0x00000007, 2 },
{ X86_FEATURE_BHI_CTRL, CPUID_EDX, 4, 0x00000007, 2 },
{ X86_FEATURE_CQM_LLC, CPUID_EDX, 1, 0x0000000f, 0 },
--
2.45.2The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: b02dc185ee86836cf1d8a37b81349374e4018ee0
Gitweb: https://git.kernel.org/tip/b02dc185ee86836cf1d8a37b81349374e4018ee0
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:16:51 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 09:44:13 +02:00
x86/cpufeatures: Add X86_FEATURE_APX
Intel Advanced Performance Extensions (APX) introduce a new set of
general-purpose registers, managed as an extended state component via the
xstate management facility.
Before enabling this new xstate, define a feature flag to clarify the
dependency in xsave_cpuid_features[]. APX is enumerated under CPUID level
7 with EDX=1. Since this CPUID leaf is not yet allocated, place the flag
in a scattered feature word.
While this feature is intended only for userspace, exposing it via
/proc/cpuinfo is unnecessary. Instead, the existing arch_prctl(2)
mechanism with the ARCH_GET_XCOMP_SUPP option can be used to query the
feature availability.
Finally, clarify that APX depends on XSAVE.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20250416021720.12305-2-chang.seok.bae@intel.com
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/kernel/cpu/cpuid-deps.c | 1 +
arch/x86/kernel/cpu/scattered.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index bc81b9d..478ab36 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -481,6 +481,7 @@
#define X86_FEATURE_AMD_HTR_CORES (21*32+ 6) /* Heterogeneous Core Topology */
#define X86_FEATURE_AMD_WORKLOAD_CLASS (21*32+ 7) /* Workload Classification */
#define X86_FEATURE_PREFER_YMM (21*32+ 8) /* Avoid ZMM registers due to downclocking */
+#define X86_FEATURE_APX (21*32+ 9) /* Advanced Performance Extensions */
/*
* BUG word(s)
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 94c062c..46efcbd 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -28,6 +28,7 @@ static const struct cpuid_dep cpuid_deps[] = {
{ X86_FEATURE_PKU, X86_FEATURE_XSAVE },
{ X86_FEATURE_MPX, X86_FEATURE_XSAVE },
{ X86_FEATURE_XGETBV1, X86_FEATURE_XSAVE },
+ { X86_FEATURE_APX, X86_FEATURE_XSAVE },
{ X86_FEATURE_CMOV, X86_FEATURE_FXSR },
{ X86_FEATURE_MMX, X86_FEATURE_FXSR },
{ X86_FEATURE_MMXEXT, X86_FEATURE_MMX },
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index c75c57b..dbf6d71 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -27,6 +27,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 },
{ X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 },
{ X86_FEATURE_INTEL_PPIN, CPUID_EBX, 0, 0x00000007, 1 },
+ { X86_FEATURE_APX, CPUID_EDX, 21, 0x00000007, 1 },
{ X86_FEATURE_RRSBA_CTRL, CPUID_EDX, 2, 0x00000007, 2 },
{ X86_FEATURE_BHI_CTRL, CPUID_EDX, 4, 0x00000007, 2 },
{ X86_FEATURE_CQM_LLC, CPUID_EDX, 1, 0x0000000f, 0 },Advanced Performance Extensions (APX) is associated with a new state
component number 19. To support saving and restoring of the corresponding
registers via the XSAVE mechanism, introduce the component definition
along with the necessary sanity checks.
Define the new component number, state name, and those register data
type. Then, extend the size checker to validate the register data type
and explicitly list the APX feature flag as a dependency for the new
component in xsave_cpuid_features[].
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
---
Changes from the last posting:
https://lore.kernel.org/lkml/20250320234301.8342-7-chang.seok.bae@intel.com/
* Move the check to be grouped with other XCHECK_SZ() entries (Sohil)
* Massage the changelog (Sohil)
* Add Sohil's tag
---
arch/x86/include/asm/fpu/types.h | 9 +++++++++
arch/x86/kernel/fpu/xstate.c | 3 +++
2 files changed, 12 insertions(+)
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index de16862bf230..97310df3ea13 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -125,6 +125,7 @@ enum xfeature {
XFEATURE_RSRVD_COMP_16,
XFEATURE_XTILE_CFG,
XFEATURE_XTILE_DATA,
+ XFEATURE_APX,
XFEATURE_MAX,
};
@@ -145,6 +146,7 @@ enum xfeature {
#define XFEATURE_MASK_LBR (1 << XFEATURE_LBR)
#define XFEATURE_MASK_XTILE_CFG (1 << XFEATURE_XTILE_CFG)
#define XFEATURE_MASK_XTILE_DATA (1 << XFEATURE_XTILE_DATA)
+#define XFEATURE_MASK_APX (1 << XFEATURE_APX)
#define XFEATURE_MASK_FPSSE (XFEATURE_MASK_FP | XFEATURE_MASK_SSE)
#define XFEATURE_MASK_AVX512 (XFEATURE_MASK_OPMASK \
@@ -303,6 +305,13 @@ struct xtile_data {
struct reg_1024_byte tmm;
} __packed;
+/*
+ * State component 19: 8B extended general purpose register.
+ */
+struct apx_state {
+ u64 egpr[16];
+} __packed;
+
/*
* State component 10 is supervisor state used for context-switching the
* PASID state.
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index a288597065fd..dfd07af10037 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -63,6 +63,7 @@ static const char *xfeature_names[] =
"unknown xstate feature",
"AMX Tile config",
"AMX Tile data",
+ "APX registers",
"unknown xstate feature",
};
@@ -81,6 +82,7 @@ static unsigned short xsave_cpuid_features[] __initdata = {
[XFEATURE_CET_USER] = X86_FEATURE_SHSTK,
[XFEATURE_XTILE_CFG] = X86_FEATURE_AMX_TILE,
[XFEATURE_XTILE_DATA] = X86_FEATURE_AMX_TILE,
+ [XFEATURE_APX] = X86_FEATURE_APX,
};
static unsigned int xstate_offsets[XFEATURE_MAX] __ro_after_init =
@@ -569,6 +571,7 @@ static bool __init check_xstate_against_struct(int nr)
case XFEATURE_PASID: return XCHECK_SZ(sz, nr, struct ia32_pasid_state);
case XFEATURE_XTILE_CFG: return XCHECK_SZ(sz, nr, struct xtile_cfg);
case XFEATURE_CET_USER: return XCHECK_SZ(sz, nr, struct cet_user_state);
+ case XFEATURE_APX: return XCHECK_SZ(sz, nr, struct apx_state);
case XFEATURE_XTILE_DATA: check_xtile_data_against_struct(sz); return true;
default:
XSTATE_WARN_ON(1, "No structure for xstate: %d\n", nr);
--
2.45.2The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: bd0b10b795c5c4c587e83c0498251356874c655c
Gitweb: https://git.kernel.org/tip/bd0b10b795c5c4c587e83c0498251356874c655c
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:16:52 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 09:44:14 +02:00
x86/fpu/apx: Define APX state component
Advanced Performance Extensions (APX) is associated with a new state
component number 19. To support saving and restoring of the corresponding
registers via the XSAVE mechanism, introduce the component definition
along with the necessary sanity checks.
Define the new component number, state name, and those register data
type. Then, extend the size checker to validate the register data type
and explicitly list the APX feature flag as a dependency for the new
component in xsave_cpuid_features[].
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20250416021720.12305-3-chang.seok.bae@intel.com
---
arch/x86/include/asm/fpu/types.h | 9 +++++++++
arch/x86/kernel/fpu/xstate.c | 3 +++
2 files changed, 12 insertions(+)
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index de16862..97310df 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -125,6 +125,7 @@ enum xfeature {
XFEATURE_RSRVD_COMP_16,
XFEATURE_XTILE_CFG,
XFEATURE_XTILE_DATA,
+ XFEATURE_APX,
XFEATURE_MAX,
};
@@ -145,6 +146,7 @@ enum xfeature {
#define XFEATURE_MASK_LBR (1 << XFEATURE_LBR)
#define XFEATURE_MASK_XTILE_CFG (1 << XFEATURE_XTILE_CFG)
#define XFEATURE_MASK_XTILE_DATA (1 << XFEATURE_XTILE_DATA)
+#define XFEATURE_MASK_APX (1 << XFEATURE_APX)
#define XFEATURE_MASK_FPSSE (XFEATURE_MASK_FP | XFEATURE_MASK_SSE)
#define XFEATURE_MASK_AVX512 (XFEATURE_MASK_OPMASK \
@@ -304,6 +306,13 @@ struct xtile_data {
} __packed;
/*
+ * State component 19: 8B extended general purpose register.
+ */
+struct apx_state {
+ u64 egpr[16];
+} __packed;
+
+/*
* State component 10 is supervisor state used for context-switching the
* PASID state.
*/
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index a288597..dfd07af 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -63,6 +63,7 @@ static const char *xfeature_names[] =
"unknown xstate feature",
"AMX Tile config",
"AMX Tile data",
+ "APX registers",
"unknown xstate feature",
};
@@ -81,6 +82,7 @@ static unsigned short xsave_cpuid_features[] __initdata = {
[XFEATURE_CET_USER] = X86_FEATURE_SHSTK,
[XFEATURE_XTILE_CFG] = X86_FEATURE_AMX_TILE,
[XFEATURE_XTILE_DATA] = X86_FEATURE_AMX_TILE,
+ [XFEATURE_APX] = X86_FEATURE_APX,
};
static unsigned int xstate_offsets[XFEATURE_MAX] __ro_after_init =
@@ -569,6 +571,7 @@ static bool __init check_xstate_against_struct(int nr)
case XFEATURE_PASID: return XCHECK_SZ(sz, nr, struct ia32_pasid_state);
case XFEATURE_XTILE_CFG: return XCHECK_SZ(sz, nr, struct xtile_cfg);
case XFEATURE_CET_USER: return XCHECK_SZ(sz, nr, struct cet_user_state);
+ case XFEATURE_APX: return XCHECK_SZ(sz, nr, struct apx_state);
case XFEATURE_XTILE_DATA: check_xtile_data_against_struct(sz); return true;
default:
XSTATE_WARN_ON(1, "No structure for xstate: %d\n", nr);XSTATE components are architecturally independent. There is no rule
requiring their offsets in the non-compacted format to be strictly
ascending or mutually non-overlapping. However, in practice, such
overlaps have not occurred -- until now.
APX is introduced as xstate component 19, following AMX. In the
non-compacted XSAVE format, its offset overlaps with the space previously
occupied by the now-deprecated MPX feature:
45fc24e89b7c ("x86/mpx: remove MPX from arch/x86")
To prevent conflicts, the kernel must ensure the CPU never expose both
features at the same time. If so, it indicates unreliable hardware. In
such cases, XSAVE should be disabled entirely as a precautionary measure.
Add a sanity check to detect this condition and disable XSAVE if an
invalid hardware configuration is identified.
Note: MPX state components remain enabled on legacy systems solely for
KVM guest support.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
---
Changes from the last posting:
https://lore.kernel.org/lkml/20250320234301.8342-8-chang.seok.bae@intel.com/
* Add background in the changelog (Sohil/Dave)
* Clarify XSAVE disablement (Sohil).
* Collect review tag
The related warning message will be moved to the XSAVE-disabling function
in patch 6, per Dave’s suggestion.
---
arch/x86/kernel/fpu/xstate.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index dfd07af10037..14f5c1bb2080 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -814,6 +814,17 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
goto out_disable;
}
+ if (fpu_kernel_cfg.max_features & XFEATURE_MASK_APX &&
+ fpu_kernel_cfg.max_features & (XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR)) {
+ /*
+ * 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",
+ fpu_kernel_cfg.max_features);
+ goto out_disable;
+ }
+
fpu_kernel_cfg.independent_features = fpu_kernel_cfg.max_features &
XFEATURE_MASK_INDEPENDENT;
--
2.45.2
The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: ea68e39190cff86f457bd286c70b535e2a99a94d
Gitweb: https://git.kernel.org/tip/ea68e39190cff86f457bd286c70b535e2a99a94d
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:16:53 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 09:44:14 +02:00
x86/fpu/apx: Disallow conflicting MPX presence
XSTATE components are architecturally independent. There is no rule
requiring their offsets in the non-compacted format to be strictly
ascending or mutually non-overlapping. However, in practice, such
overlaps have not occurred -- until now.
APX is introduced as xstate component 19, following AMX. In the
non-compacted XSAVE format, its offset overlaps with the space previously
occupied by the now-deprecated MPX feature:
45fc24e89b7c ("x86/mpx: remove MPX from arch/x86")
To prevent conflicts, the kernel must ensure the CPU never expose both
features at the same time. If so, it indicates unreliable hardware. In
such cases, XSAVE should be disabled entirely as a precautionary measure.
Add a sanity check to detect this condition and disable XSAVE if an
invalid hardware configuration is identified.
Note: MPX state components remain enabled on legacy systems solely for
KVM guest support.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20250416021720.12305-4-chang.seok.bae@intel.com
---
arch/x86/kernel/fpu/xstate.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index dfd07af..14f5c1b 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -814,6 +814,17 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
goto out_disable;
}
+ if (fpu_kernel_cfg.max_features & XFEATURE_MASK_APX &&
+ fpu_kernel_cfg.max_features & (XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR)) {
+ /*
+ * 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",
+ fpu_kernel_cfg.max_features);
+ goto out_disable;
+ }
+
fpu_kernel_cfg.independent_features = fpu_kernel_cfg.max_features &
XFEATURE_MASK_INDEPENDENT;With securing APX against conflicting MPX, it is now ready to be enabled.
Include APX in the enabled xfeature set.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
---
Changes from the last posting:
https://lore.kernel.org/lkml/20250320234301.8342-9-chang.seok.bae@intel.com/
* Include review tag
---
arch/x86/include/asm/fpu/xstate.h | 3 ++-
arch/x86/kernel/fpu/xstate.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 7f39fe7980c5..b308a76afbb7 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -32,7 +32,8 @@
XFEATURE_MASK_PKRU | \
XFEATURE_MASK_BNDREGS | \
XFEATURE_MASK_BNDCSR | \
- XFEATURE_MASK_XTILE)
+ XFEATURE_MASK_XTILE | \
+ XFEATURE_MASK_APX)
/*
* Features which are restored when returning to user space.
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 14f5c1bb2080..2ac1fc182273 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -371,7 +371,8 @@ static __init void os_xrstor_booting(struct xregs_state *xstate)
XFEATURE_MASK_BNDCSR | \
XFEATURE_MASK_PASID | \
XFEATURE_MASK_CET_USER | \
- XFEATURE_MASK_XTILE)
+ XFEATURE_MASK_XTILE | \
+ XFEATURE_MASK_APX)
/*
* setup the xstate image representing the init state
--
2.45.2The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: 50c5b071e2833d2b61e3774cd792620311df157c
Gitweb: https://git.kernel.org/tip/50c5b071e2833d2b61e3774cd792620311df157c
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:16:54 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 09:44:14 +02:00
x86/fpu/apx: Enable APX state support
With securing APX against conflicting MPX, it is now ready to be enabled.
Include APX in the enabled xfeature set.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20250416021720.12305-5-chang.seok.bae@intel.com
---
arch/x86/include/asm/fpu/xstate.h | 3 ++-
arch/x86/kernel/fpu/xstate.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 7f39fe7..b308a76 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -32,7 +32,8 @@
XFEATURE_MASK_PKRU | \
XFEATURE_MASK_BNDREGS | \
XFEATURE_MASK_BNDCSR | \
- XFEATURE_MASK_XTILE)
+ XFEATURE_MASK_XTILE | \
+ XFEATURE_MASK_APX)
/*
* Features which are restored when returning to user space.
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 14f5c1b..2ac1fc1 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -371,7 +371,8 @@ static __init void os_xrstor_booting(struct xregs_state *xstate)
XFEATURE_MASK_BNDCSR | \
XFEATURE_MASK_PASID | \
XFEATURE_MASK_CET_USER | \
- XFEATURE_MASK_XTILE)
+ XFEATURE_MASK_XTILE | \
+ XFEATURE_MASK_APX)
/*
* setup the xstate image representing the init stateThe extended general-purpose registers for APX may contain random data,
which is currently assumed by the xstate testing framework. This allows
the testing of the new userspace feature using the common test code.
Invoke the test entry function from apx.c after enumerating the
state component and adding it to the support list
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
---
Changes from the last posting:
https://lore.kernel.org/lkml/20250320234301.8342-10-chang.seok.bae@intel.com/
* Add review tag
Some might view this standalone test as potentially converging with other
tests. I encountered similar situation when posting the selftest rework:
https://lore.kernel.org/lkml/20250226010731.2456-10-chang.seok.bae@intel.com/
Maybe it's worthwhile to clarify the consideration again here:
Alternatively, this invocation could be placed directly in
xstate.c::main(). However, the current test file naming convention,
which clearly specifies the tested area, seems reasonable. Adding apx.c
considerably aligns with that convention.
---
tools/testing/selftests/x86/Makefile | 3 ++-
tools/testing/selftests/x86/apx.c | 10 ++++++++++
tools/testing/selftests/x86/xstate.c | 3 ++-
tools/testing/selftests/x86/xstate.h | 2 ++
4 files changed, 16 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/x86/apx.c
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index 28422c32cc8f..f703fcfe9f7c 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -19,7 +19,7 @@ TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \
test_FCMOV test_FCOMI test_FISTTP \
vdso_restorer
TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip syscall_numbering \
- corrupt_xstate_header amx lam test_shadow_stack avx
+ corrupt_xstate_header amx lam test_shadow_stack avx apx
# Some selftests require 32bit support enabled also on 64bit systems
TARGETS_C_32BIT_NEEDED := ldt_gdt ptrace_syscall
@@ -136,3 +136,4 @@ $(OUTPUT)/nx_stack_64: CFLAGS += -Wl,-z,noexecstack
$(OUTPUT)/avx_64: CFLAGS += -mno-avx -mno-avx512f
$(OUTPUT)/amx_64: EXTRA_FILES += xstate.c
$(OUTPUT)/avx_64: EXTRA_FILES += xstate.c
+$(OUTPUT)/apx_64: EXTRA_FILES += xstate.c
diff --git a/tools/testing/selftests/x86/apx.c b/tools/testing/selftests/x86/apx.c
new file mode 100644
index 000000000000..d9c8d41b8c5a
--- /dev/null
+++ b/tools/testing/selftests/x86/apx.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+
+#include "xstate.h"
+
+int main(void)
+{
+ test_xstate(XFEATURE_APX);
+}
diff --git a/tools/testing/selftests/x86/xstate.c b/tools/testing/selftests/x86/xstate.c
index 23c1d6c964ea..97fe4bd8bc77 100644
--- a/tools/testing/selftests/x86/xstate.c
+++ b/tools/testing/selftests/x86/xstate.c
@@ -31,7 +31,8 @@
(1 << XFEATURE_OPMASK) | \
(1 << XFEATURE_ZMM_Hi256) | \
(1 << XFEATURE_Hi16_ZMM) | \
- (1 << XFEATURE_XTILEDATA))
+ (1 << XFEATURE_XTILEDATA) | \
+ (1 << XFEATURE_APX))
static inline uint64_t xgetbv(uint32_t index)
{
diff --git a/tools/testing/selftests/x86/xstate.h b/tools/testing/selftests/x86/xstate.h
index 42af36ec852f..e91e3092b5d2 100644
--- a/tools/testing/selftests/x86/xstate.h
+++ b/tools/testing/selftests/x86/xstate.h
@@ -33,6 +33,7 @@ enum xfeature {
XFEATURE_RSRVD_COMP_16,
XFEATURE_XTILECFG,
XFEATURE_XTILEDATA,
+ XFEATURE_APX,
XFEATURE_MAX,
};
@@ -59,6 +60,7 @@ static const char *xfeature_names[] =
"unknown xstate feature",
"AMX Tile config",
"AMX Tile data",
+ "APX registers",
"unknown xstate feature",
};
--
2.45.2The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: ab6f87ddd0c6d3fb114cdf897eb9839cbd429439
Gitweb: https://git.kernel.org/tip/ab6f87ddd0c6d3fb114cdf897eb9839cbd429439
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:16:55 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 09:44:14 +02:00
selftests/x86/apx: Add APX test
The extended general-purpose registers for APX may contain random data,
which is currently assumed by the xstate testing framework. This allows
the testing of the new userspace feature using the common test code.
Invoke the test entry function from apx.c after enumerating the
state component and adding it to the support list
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20250416021720.12305-6-chang.seok.bae@intel.com
---
tools/testing/selftests/x86/Makefile | 3 ++-
tools/testing/selftests/x86/apx.c | 10 ++++++++++
tools/testing/selftests/x86/xstate.c | 3 ++-
tools/testing/selftests/x86/xstate.h | 2 ++
4 files changed, 16 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/x86/apx.c
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index 28422c3..f703fcf 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -19,7 +19,7 @@ TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \
test_FCMOV test_FCOMI test_FISTTP \
vdso_restorer
TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip syscall_numbering \
- corrupt_xstate_header amx lam test_shadow_stack avx
+ corrupt_xstate_header amx lam test_shadow_stack avx apx
# Some selftests require 32bit support enabled also on 64bit systems
TARGETS_C_32BIT_NEEDED := ldt_gdt ptrace_syscall
@@ -136,3 +136,4 @@ $(OUTPUT)/nx_stack_64: CFLAGS += -Wl,-z,noexecstack
$(OUTPUT)/avx_64: CFLAGS += -mno-avx -mno-avx512f
$(OUTPUT)/amx_64: EXTRA_FILES += xstate.c
$(OUTPUT)/avx_64: EXTRA_FILES += xstate.c
+$(OUTPUT)/apx_64: EXTRA_FILES += xstate.c
diff --git a/tools/testing/selftests/x86/apx.c b/tools/testing/selftests/x86/apx.c
new file mode 100644
index 0000000..d9c8d41
--- /dev/null
+++ b/tools/testing/selftests/x86/apx.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+
+#include "xstate.h"
+
+int main(void)
+{
+ test_xstate(XFEATURE_APX);
+}
diff --git a/tools/testing/selftests/x86/xstate.c b/tools/testing/selftests/x86/xstate.c
index 23c1d6c..97fe4bd 100644
--- a/tools/testing/selftests/x86/xstate.c
+++ b/tools/testing/selftests/x86/xstate.c
@@ -31,7 +31,8 @@
(1 << XFEATURE_OPMASK) | \
(1 << XFEATURE_ZMM_Hi256) | \
(1 << XFEATURE_Hi16_ZMM) | \
- (1 << XFEATURE_XTILEDATA))
+ (1 << XFEATURE_XTILEDATA) | \
+ (1 << XFEATURE_APX))
static inline uint64_t xgetbv(uint32_t index)
{
diff --git a/tools/testing/selftests/x86/xstate.h b/tools/testing/selftests/x86/xstate.h
index 42af36e..e91e309 100644
--- a/tools/testing/selftests/x86/xstate.h
+++ b/tools/testing/selftests/x86/xstate.h
@@ -33,6 +33,7 @@ enum xfeature {
XFEATURE_RSRVD_COMP_16,
XFEATURE_XTILECFG,
XFEATURE_XTILEDATA,
+ XFEATURE_APX,
XFEATURE_MAX,
};
@@ -59,6 +60,7 @@ static const char *xfeature_names[] =
"unknown xstate feature",
"AMX Tile config",
"AMX Tile data",
+ "APX registers",
"unknown xstate feature",
};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;
}Currently, saving register states in the signal frame, the legacy feature
bits are always set in xregs_state->header->xfeatures. This code sequence
can be generalized for reuse in similar cases.
Refactor the logic to ensure a consistent approach across similar usages.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
Changes from the last posting:
https://lore.kernel.org/lkml/20250214010607.7067-2-chang.seok.bae@intel.com/
* No change
This patch and the next were previously posted together. I thought this
refactoring is a meaningful step toward decoupling PKRU from an
unnecessary dependency on XGETBV(1).
---
arch/x86/kernel/fpu/signal.c | 11 +----------
arch/x86/kernel/fpu/xstate.h | 12 ++++++++++++
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index b8b4fa9c2d04..c3ec2512f2bb 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -114,7 +114,6 @@ static inline bool save_xstate_epilog(void __user *buf, int ia32_frame,
{
struct xregs_state __user *x = buf;
struct _fpx_sw_bytes sw_bytes = {};
- u32 xfeatures;
int err;
/* Setup the bytes not touched by the [f]xsave and reserved for SW. */
@@ -127,12 +126,6 @@ static inline bool save_xstate_epilog(void __user *buf, int ia32_frame,
err |= __put_user(FP_XSTATE_MAGIC2,
(__u32 __user *)(buf + fpstate->user_size));
- /*
- * Read the xfeatures which we copied (directly from the cpu or
- * from the state in task struct) to the user buffers.
- */
- err |= __get_user(xfeatures, (__u32 __user *)&x->header.xfeatures);
-
/*
* For legacy compatible, we always set FP/SSE bits in the bit
* vector while saving the state to the user context. This will
@@ -144,9 +137,7 @@ static inline bool save_xstate_epilog(void __user *buf, int ia32_frame,
* header as well as change any contents in the memory layout.
* xrestore as part of sigreturn will capture all the changes.
*/
- xfeatures |= XFEATURE_MASK_FPSSE;
-
- err |= __put_user(xfeatures, (__u32 __user *)&x->header.xfeatures);
+ err |= set_xfeature_in_sigframe(x, XFEATURE_MASK_FPSSE);
return !err;
}
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index 9a3a8ccf13bf..aadf02aed071 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -69,6 +69,18 @@ static inline u64 xfeatures_mask_independent(void)
return fpu_kernel_cfg.independent_features;
}
+static inline int set_xfeature_in_sigframe(struct xregs_state __user *xbuf, u64 mask)
+{
+ u64 xfeatures;
+ int err;
+
+ /* Read the xfeatures value already saved in the user buffer */
+ err = __get_user(xfeatures, &xbuf->header.xfeatures);
+ xfeatures |= mask;
+ err |= __put_user(xfeatures, &xbuf->header.xfeatures);
+ return err;
+}
+
/*
* Update the value of PKRU register that was already pushed onto the signal frame.
*/
--
2.45.2
* Chang S. Bae <chang.seok.bae@intel.com> wrote:
> +static inline int set_xfeature_in_sigframe(struct xregs_state __user *xbuf, u64 mask)
> +{
> + u64 xfeatures;
> + int err;
> +
> + /* Read the xfeatures value already saved in the user buffer */
> + err = __get_user(xfeatures, &xbuf->header.xfeatures);
> + xfeatures |= mask;
> + err |= __put_user(xfeatures, &xbuf->header.xfeatures);
> + return err;
> +}
For future reference, please put an extra newline before 'return'
statements, so that the code looks more 'balanced':
> +{
> + u64 xfeatures;
> + int err;
> +
> + /* Read the xfeatures value already saved in the user buffer */
> + err = __get_user(xfeatures, &xbuf->header.xfeatures);
> + xfeatures |= mask;
> + err |= __put_user(xfeatures, &xbuf->header.xfeatures);
> +
> + return err;
> +}
I've done that for this patch, so no need to resend it.
Thanks,
Ingo
On 4/16/2025 1:05 AM, Ingo Molnar wrote:
>
> * Chang S. Bae <chang.seok.bae@intel.com> wrote:
>
>> +static inline int set_xfeature_in_sigframe(struct xregs_state __user *xbuf, u64 mask)
>> +{
>> + u64 xfeatures;
>> + int err;
>> +
>> + /* Read the xfeatures value already saved in the user buffer */
>> + err = __get_user(xfeatures, &xbuf->header.xfeatures);
>> + xfeatures |= mask;
>> + err |= __put_user(xfeatures, &xbuf->header.xfeatures);
>> + return err;
>> +}
>
> For future reference, please put an extra newline before 'return'
> statements, so that the code looks more 'balanced':
>
>> +{
>> + u64 xfeatures;
>> + int err;
>> +
>> + /* Read the xfeatures value already saved in the user buffer */
>> + err = __get_user(xfeatures, &xbuf->header.xfeatures);
>> + xfeatures |= mask;
>> + err |= __put_user(xfeatures, &xbuf->header.xfeatures);
>> +
>> + return err;
>> +}
I see. Thanks for the note.
> I've done that for this patch, so no need to resend it.
Thanks!
Chang
The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: 64e54461ab6e8524a8de4e63b7d1a3e4481b5cf3
Gitweb: https://git.kernel.org/tip/64e54461ab6e8524a8de4e63b7d1a3e4481b5cf3
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:16:57 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 10:01:00 +02:00
x86/fpu: Refactor xfeature bitmask update code for sigframe XSAVE
Currently, saving register states in the signal frame, the legacy feature
bits are always set in xregs_state->header->xfeatures. This code sequence
can be generalized for reuse in similar cases.
Refactor the logic to ensure a consistent approach across similar usages.
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>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20250416021720.12305-8-chang.seok.bae@intel.com
---
arch/x86/kernel/fpu/signal.c | 11 +----------
arch/x86/kernel/fpu/xstate.h | 13 +++++++++++++
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index b8b4fa9..c3ec251 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -114,7 +114,6 @@ static inline bool save_xstate_epilog(void __user *buf, int ia32_frame,
{
struct xregs_state __user *x = buf;
struct _fpx_sw_bytes sw_bytes = {};
- u32 xfeatures;
int err;
/* Setup the bytes not touched by the [f]xsave and reserved for SW. */
@@ -128,12 +127,6 @@ static inline bool save_xstate_epilog(void __user *buf, int ia32_frame,
(__u32 __user *)(buf + fpstate->user_size));
/*
- * Read the xfeatures which we copied (directly from the cpu or
- * from the state in task struct) to the user buffers.
- */
- err |= __get_user(xfeatures, (__u32 __user *)&x->header.xfeatures);
-
- /*
* For legacy compatible, we always set FP/SSE bits in the bit
* vector while saving the state to the user context. This will
* enable us capturing any changes(during sigreturn) to
@@ -144,9 +137,7 @@ static inline bool save_xstate_epilog(void __user *buf, int ia32_frame,
* header as well as change any contents in the memory layout.
* xrestore as part of sigreturn will capture all the changes.
*/
- xfeatures |= XFEATURE_MASK_FPSSE;
-
- err |= __put_user(xfeatures, (__u32 __user *)&x->header.xfeatures);
+ err |= set_xfeature_in_sigframe(x, XFEATURE_MASK_FPSSE);
return !err;
}
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index 9a3a8cc..4231e44 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -69,6 +69,19 @@ static inline u64 xfeatures_mask_independent(void)
return fpu_kernel_cfg.independent_features;
}
+static inline int set_xfeature_in_sigframe(struct xregs_state __user *xbuf, u64 mask)
+{
+ u64 xfeatures;
+ int err;
+
+ /* Read the xfeatures value already saved in the user buffer */
+ err = __get_user(xfeatures, &xbuf->header.xfeatures);
+ xfeatures |= mask;
+ err |= __put_user(xfeatures, &xbuf->header.xfeatures);
+
+ return err;
+}
+
/*
* Update the value of PKRU register that was already pushed onto the signal frame.
*/The signal delivery logic was modified to always set the PKRU bit in
xregs_state->header->xfeatures by this commit:
ae6012d72fa6 ("x86/pkeys: Ensure updated PKRU value is XRSTOR'd")
However, the change derives the bitmask value using XGETBV(1), rather
than simply updating the buffer that already holds the value. Thus, this
approach induces an unnecessary dependency on XGETBV1 for PKRU handling.
Eliminate the dependency by using the established helper function.
Subsequently, remove the now-unused 'mask' argument.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
Cc: Tony W Wang-oc <TonyWWang-oc@zhaoxin.com>
---
Changes from the last posting:
https://lore.kernel.org/lkml/20250214010607.7067-3-chang.seok.bae@intel.com/
* Massage the changelog
Additional Context:
Previously, the concern was raised about environments where XGETBV1 is
unavailable — such as in some virtual machines:
https://lore.kernel.org/lkml/20250102075419.2559-1-TonyWWang-oc@zhaoxin.com
That patch tried to sidestep the problem by skipping PKRU updates
entirely when XGETBV1 is inaccessible. However, this assumed the
dependency was necessary, which isn’t the case.
---
arch/x86/kernel/fpu/xstate.h | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index aadf02aed071..a6d987c16293 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -84,18 +84,15 @@ static inline int set_xfeature_in_sigframe(struct xregs_state __user *xbuf, u64
/*
* Update the value of PKRU register that was already pushed onto the signal frame.
*/
-static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 mask, u32 pkru)
+static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u32 pkru)
{
- u64 xstate_bv;
int err;
if (unlikely(!cpu_feature_enabled(X86_FEATURE_OSPKE)))
return 0;
/* Mark PKRU as in-use so that it is restored correctly. */
- xstate_bv = (mask & xfeatures_in_use()) | XFEATURE_MASK_PKRU;
-
- err = __put_user(xstate_bv, &buf->header.xfeatures);
+ err = set_xfeature_in_sigframe(buf, XFEATURE_MASK_PKRU);
if (err)
return err;
@@ -319,7 +316,7 @@ static inline int xsave_to_user_sigframe(struct xregs_state __user *buf, u32 pkr
clac();
if (!err)
- err = update_pkru_in_sigframe(buf, mask, pkru);
+ err = update_pkru_in_sigframe(buf, pkru);
return err;
}
--
2.45.2
The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: d1e420772cd1eb0afe5858619c73ce36f3e781a1
Gitweb: https://git.kernel.org/tip/d1e420772cd1eb0afe5858619c73ce36f3e781a1
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:16:58 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 10:01:03 +02:00
x86/pkeys: Simplify PKRU update in signal frame
The signal delivery logic was modified to always set the PKRU bit in
xregs_state->header->xfeatures by this commit:
ae6012d72fa6 ("x86/pkeys: Ensure updated PKRU value is XRSTOR'd")
However, the change derives the bitmask value using XGETBV(1), rather
than simply updating the buffer that already holds the value. Thus, this
approach induces an unnecessary dependency on XGETBV1 for PKRU handling.
Eliminate the dependency by using the established helper function.
Subsequently, remove the now-unused 'mask' argument.
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: Aruna Ramakrishna <aruna.ramakrishna@oracle.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Tony W Wang-oc <TonyWWang-oc@zhaoxin.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Link: https://lore.kernel.org/r/20250416021720.12305-9-chang.seok.bae@intel.com
---
arch/x86/kernel/fpu/xstate.h | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index 4231e44..a0256ef 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -85,18 +85,15 @@ static inline int set_xfeature_in_sigframe(struct xregs_state __user *xbuf, u64
/*
* Update the value of PKRU register that was already pushed onto the signal frame.
*/
-static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 mask, u32 pkru)
+static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u32 pkru)
{
- u64 xstate_bv;
int err;
if (unlikely(!cpu_feature_enabled(X86_FEATURE_OSPKE)))
return 0;
/* Mark PKRU as in-use so that it is restored correctly. */
- xstate_bv = (mask & xfeatures_in_use()) | XFEATURE_MASK_PKRU;
-
- err = __put_user(xstate_bv, &buf->header.xfeatures);
+ err = set_xfeature_in_sigframe(buf, XFEATURE_MASK_PKRU);
if (err)
return err;
@@ -320,7 +317,7 @@ static inline int xsave_to_user_sigframe(struct xregs_state __user *buf, u32 pkr
clac();
if (!err)
- err = update_pkru_in_sigframe(buf, mask, pkru);
+ err = update_pkru_in_sigframe(buf, pkru);
return err;
}The variable was previously referenced in KVM code but the last usage was
removed by:
ea4d6938d4c0 ("x86/fpu: Replace KVMs home brewed FPU copy from user")
Remove its export symbol.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
---
Changes from the last posting:
https://lore.kernel.org/lkml/d143cc4c-8f8e-48e5-87f1-dded3272433a@suse.com
* Note the commit that removed its usage (Nikolay)
* Include review tag
Apologies -- given the review tag, I should have followed up this
earlier.
---
arch/x86/kernel/fpu/init.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 16b6611634c3..2d9b5e677559 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -100,7 +100,6 @@ static void __init fpu__init_system_early_generic(void)
* Boot time FPU feature detection code:
*/
unsigned int mxcsr_feature_mask __ro_after_init = 0xffffffffu;
-EXPORT_SYMBOL_GPL(mxcsr_feature_mask);
static void __init fpu__init_system_mxcsr(void)
{
--
2.45.2The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: 70fe4a0266ef156f3a49071da0d9ea6af0f49c44
Gitweb: https://git.kernel.org/tip/70fe4a0266ef156f3a49071da0d9ea6af0f49c44
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:16:59 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 10:01:03 +02:00
x86/fpu: Remove export of mxcsr_feature_mask
The variable was previously referenced in KVM code but the last usage was
removed by:
ea4d6938d4c0 ("x86/fpu: Replace KVMs home brewed FPU copy from user")
Remove its export symbol.
Reviewed-by: Nikolay Borisov <nik.borisov@suse.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>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20250416021720.12305-10-chang.seok.bae@intel.com
---
arch/x86/kernel/fpu/init.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index 16b6611..2d9b5e6 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -100,7 +100,6 @@ static void __init fpu__init_system_early_generic(void)
* Boot time FPU feature detection code:
*/
unsigned int mxcsr_feature_mask __ro_after_init = 0xffffffffu;
-EXPORT_SYMBOL_GPL(mxcsr_feature_mask);
static void __init fpu__init_system_mxcsr(void)
{The original function name came from an overly compressed form of
'fpstate_regs' by commit:
e61d6310a0f8 ("x86/fpu: Reset permission and fpstate on exec()")
However, the term 'fpregs' typically refers to physical FPU registers. In
contrast, this function copies the init values to fpu->fpstate->regs, not
hardware registers.
Rename the function to better reflect what it actually does.
No functional change.
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
Changes from the last posting:
https://lore.kernel.org/lkml/20240530192739.172566-2-chang.seok.bae@intel.com
* Refine the changelog
* Add a note referencing the original naming decision.
This patch was originally submitted as part of the In-Field Scan driver
series. Although that series is now dropped that I believe, this rename
still serves as a useful cleanup to correct a naming choice that I
previously made :(.
---
arch/x86/kernel/fpu/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 3a19877a314e..8d674435f173 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -733,7 +733,7 @@ static inline void restore_fpregs_from_init_fpstate(u64 features_mask)
/*
* Reset current->fpu memory state to the init values.
*/
-static void fpu_reset_fpregs(void)
+static void fpu_reset_fpstate_regs(void)
{
struct fpu *fpu = x86_task_fpu(current);
@@ -768,7 +768,7 @@ void fpu__clear_user_states(struct fpu *fpu)
fpregs_lock();
if (!cpu_feature_enabled(X86_FEATURE_FPU)) {
- fpu_reset_fpregs();
+ fpu_reset_fpstate_regs();
fpregs_unlock();
return;
}
@@ -798,7 +798,7 @@ void fpu__clear_user_states(struct fpu *fpu)
void fpu_flush_thread(void)
{
fpstate_reset(x86_task_fpu(current));
- fpu_reset_fpregs();
+ fpu_reset_fpstate_regs();
}
/*
* Load FPU context before returning to userspace.
--
2.45.2The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: de8304c319bc020ef79d109909ad40e944d82c82
Gitweb: https://git.kernel.org/tip/de8304c319bc020ef79d109909ad40e944d82c82
Author: Chang S. Bae <chang.seok.bae@intel.com>
AuthorDate: Tue, 15 Apr 2025 19:17:00 -07:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 16 Apr 2025 10:01:03 +02:00
x86/fpu: Rename fpu_reset_fpregs() to fpu_reset_fpstate_regs()
The original function name came from an overly compressed form of
'fpstate_regs' by commit:
e61d6310a0f8 ("x86/fpu: Reset permission and fpstate on exec()")
However, the term 'fpregs' typically refers to physical FPU registers. In
contrast, this function copies the init values to fpu->fpstate->regs, not
hardware registers.
Rename the function to better reflect what it actually does.
No functional change.
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>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20250416021720.12305-11-chang.seok.bae@intel.com
---
arch/x86/kernel/fpu/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 3a19877..8d67443 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -733,7 +733,7 @@ static inline void restore_fpregs_from_init_fpstate(u64 features_mask)
/*
* Reset current->fpu memory state to the init values.
*/
-static void fpu_reset_fpregs(void)
+static void fpu_reset_fpstate_regs(void)
{
struct fpu *fpu = x86_task_fpu(current);
@@ -768,7 +768,7 @@ void fpu__clear_user_states(struct fpu *fpu)
fpregs_lock();
if (!cpu_feature_enabled(X86_FEATURE_FPU)) {
- fpu_reset_fpregs();
+ fpu_reset_fpstate_regs();
fpregs_unlock();
return;
}
@@ -798,7 +798,7 @@ void fpu__clear_user_states(struct fpu *fpu)
void fpu_flush_thread(void)
{
fpstate_reset(x86_task_fpu(current));
- fpu_reset_fpregs();
+ fpu_reset_fpstate_regs();
}
/*
* Load FPU context before returning to userspace.* Ingo Molnar <mingo@kernel.org> wrote: > > * Sohil Mehta <sohil.mehta@intel.com> wrote: > > > On 4/11/2025 11:23 AM, Chang S. Bae wrote: > > > > > I've attached the patch revision. > > > > > > > LGTM, > > > > Reviewed-by: Sohil Mehta <sohil.mehta@intel.com> > > Chang, mind sending a series of the latest version of all the pending > APX patches you have at the moment (and any other pending FPU patches > you may have), with Reviewed-by tags rolled in, etc., on top of: > > git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/fpu Note that this is now all in tip:x86/fpu, alongside with a rebased version of Chang S. Bae's preparatory patches for Intel APX: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/fpu I think we could now start merging the rest of the APX patches, for v6.16 upstreaming. Thanks, Ingo
On 4/14/2025 1:23 AM, Ingo Molnar wrote: > >> Chang, mind sending a series of the latest version of all the pending >> APX patches you have at the moment (and any other pending FPU patches >> you may have), with Reviewed-by tags rolled in, etc., on top of: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/fpu > > Note that this is now all in tip:x86/fpu, alongside with a rebased > version of Chang S. Bae's preparatory patches for Intel APX: > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/fpu > > I think we could now start merging the rest of the APX patches, for > v6.16 upstreaming. Thanks for the update! Finalizing a few minor changes based on the feedback so far, and also refreshing a few other FPU patches that are considerably worth to be bundled up. I’ll post them in the next day or two. Thanks, Chang
On 4/14/2025 1:23 AM, Ingo Molnar wrote: > I think we could now start merging the rest of the APX patches, for > v6.16 upstreaming. > Chang, Ingo, The series looks good to me. Functionally, the only change I have requested is in patch 7/9 to add a "disabling XSAVE" message to the error print. The rest of the comments are mainly about clarifications in the commit logs. Feel free to ignore them if they seem redundant.
On 4/14/25 10:28, Sohil Mehta wrote: > Functionally, the only change I have requested is in patch 7/9 to add a > "disabling XSAVE" message to the error print. That's probably a good idea to stick in: fpu__init_disable_system_xstate() for all of the "out_disable" cases to use, no?
On 4/14/2025 10:32 AM, Dave Hansen wrote: > On 4/14/25 10:28, Sohil Mehta wrote: >> Functionally, the only change I have requested is in patch 7/9 to add a >> "disabling XSAVE" message to the error print. > > That's probably a good idea to stick in: > > fpu__init_disable_system_xstate() > > for all of the "out_disable" cases to use, no? That way my initial inclination as well. My suggestion was mainly to keep it consistent. But looking more closely, there is mismatch already. So either of the options work for me. "x86/fpu: FP/SSE not present amongst the CPU's xstate features: 0x%llx." "x86/fpu: init_fpstate buffer too small (%zu < %d), disabling XSAVE" "x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during init, disabling XSAVE"
On 4/14/2025 10:45 AM, Sohil Mehta wrote:
>
> That way my initial inclination as well. My suggestion was mainly to
> keep it consistent. But looking more closely, there is mismatch already.
> So either of the options work for me.
>
> "x86/fpu: FP/SSE not present amongst the CPU's xstate features: 0x%llx."
>
> "x86/fpu: init_fpstate buffer too small (%zu < %d), disabling XSAVE"
>
> "x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during init,
> disabling XSAVE"
And you also left this comment in patch 7:
> It might be useful to add a "disabling XSAVE" print at the end of this
> statement, like the other error messages in the same function.
So it sounds like you were suggesting something along the lines of:
"x86/fpu: Both APX/MPX present in the CPU's xstate features: 0x%llx,
disabling XSAVE"
If so, I see that as an improvement to the error message rather than a
functional change. But I agree it reads better and more in line with the
other cases.
Thanks,
Chang
On 4/14/2025 11:02 AM, Chang S. Bae wrote: > On 4/14/2025 10:45 AM, Sohil Mehta wrote: >> >> That way my initial inclination as well. My suggestion was mainly to >> keep it consistent. But looking more closely, there is mismatch already. >> So either of the options work for me. >> >> "x86/fpu: FP/SSE not present amongst the CPU's xstate features: 0x%llx." >> >> "x86/fpu: init_fpstate buffer too small (%zu < %d), disabling XSAVE" >> >> "x86/fpu: xfeatures modified from 0x%016llx to 0x%016llx during init, >> disabling XSAVE" > > > And you also left this comment in patch 7: > > > It might be useful to add a "disabling XSAVE" print at the end of this > > statement, like the other error messages in the same function. > > So it sounds like you were suggesting something along the lines of: > "x86/fpu: Both APX/MPX present in the CPU's xstate features: 0x%llx, > disabling XSAVE" > Yes, this is what I meant. But Dave's suggestion to add a common print in fpu__init_disable_system_xstate() is fine as well. > If so, I see that as an improvement to the error message rather than a > functional change. Maybe functional is too strong. I meant a minor code/message change. > But I agree it reads better and more in line with the > other cases. >
© 2016 - 2025 Red Hat, Inc.