arch/x86/kernel/fpu/xstate.h | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)
The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: 2883b4c2169a435488f7845e1b6fdc6f3438c7c6
Gitweb: https://git.kernel.org/tip/2883b4c2169a435488f7845e1b6fdc6f3438c7c6
Author: Uros Bizjak <ubizjak@gmail.com>
AuthorDate: Thu, 13 Mar 2025 14:02:27 +01:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Thu, 13 Mar 2025 18:36:52 +01:00
x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
Current minimum required version of binutils is 2.25, which
supports XSAVE{,OPT,C,S} and XRSTOR{,S} instruction mnemonics.
Replace the byte-wise specification of XSAVE{,OPT,C,S}
and XRSTOR{,S} with these proper mnemonics.
No functional change intended.
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/r/20250313130251.383204-1-ubizjak@gmail.com
---
arch/x86/kernel/fpu/xstate.h | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index aa16f1a..1418423 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -94,18 +94,17 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
/* XSAVE/XRSTOR wrapper functions */
#ifdef CONFIG_X86_64
-#define REX_PREFIX "0x48, "
+#define REX_SUFFIX "64"
#else
-#define REX_PREFIX
+#define REX_SUFFIX
#endif
-/* These macros all use (%edi)/(%rdi) as the single memory argument. */
-#define XSAVE ".byte " REX_PREFIX "0x0f,0xae,0x27"
-#define XSAVEOPT ".byte " REX_PREFIX "0x0f,0xae,0x37"
-#define XSAVEC ".byte " REX_PREFIX "0x0f,0xc7,0x27"
-#define XSAVES ".byte " REX_PREFIX "0x0f,0xc7,0x2f"
-#define XRSTOR ".byte " REX_PREFIX "0x0f,0xae,0x2f"
-#define XRSTORS ".byte " REX_PREFIX "0x0f,0xc7,0x1f"
+#define XSAVE "xsave" REX_SUFFIX " %[xa]"
+#define XSAVEOPT "xsaveopt" REX_SUFFIX " %[xa]"
+#define XSAVEC "xsavec" REX_SUFFIX " %[xa]"
+#define XSAVES "xsaves" REX_SUFFIX " %[xa]"
+#define XRSTOR "xrstor" REX_SUFFIX " %[xa]"
+#define XRSTORS "xrstors" REX_SUFFIX " %[xa]"
/*
* After this @err contains 0 on success or the trap number when the
@@ -114,10 +113,10 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
#define XSTATE_OP(op, st, lmask, hmask, err) \
asm volatile("1:" op "\n\t" \
"xor %[err], %[err]\n" \
- "2:\n\t" \
+ "2:\n" \
_ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT_MCE_SAFE) \
: [err] "=a" (err) \
- : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
+ : [xa] "m" (*(st)), "a" (lmask), "d" (hmask) \
: "memory")
/*
@@ -137,12 +136,12 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
XSAVEOPT, X86_FEATURE_XSAVEOPT, \
XSAVEC, X86_FEATURE_XSAVEC, \
XSAVES, X86_FEATURE_XSAVES) \
- "\n" \
+ "\n\t" \
"xor %[err], %[err]\n" \
"3:\n" \
_ASM_EXTABLE_TYPE_REG(1b, 3b, EX_TYPE_EFAULT_REG, %[err]) \
: [err] "=r" (err) \
- : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
+ : [xa] "m" (*(st)), "a" (lmask), "d" (hmask) \
: "memory")
/*
@@ -156,7 +155,7 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
"3:\n" \
_ASM_EXTABLE_TYPE(1b, 3b, EX_TYPE_FPU_RESTORE) \
: \
- : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
+ : [xa] "m" (*(st)), "a" (lmask), "d" (hmask) \
: "memory")
#if defined(CONFIG_X86_64) && defined(CONFIG_X86_DEBUG_FPU)On Thu, Mar 13, 2025 at 05:50:34PM -0000, tip-bot2 for Uros Bizjak wrote:
> The following commit has been merged into the x86/fpu branch of tip:
>
> Commit-ID: 2883b4c2169a435488f7845e1b6fdc6f3438c7c6
> Gitweb: https://git.kernel.org/tip/2883b4c2169a435488f7845e1b6fdc6f3438c7c6
> Author: Uros Bizjak <ubizjak@gmail.com>
> AuthorDate: Thu, 13 Mar 2025 14:02:27 +01:00
> Committer: Ingo Molnar <mingo@kernel.org>
> CommitterDate: Thu, 13 Mar 2025 18:36:52 +01:00
>
> x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
>
> Current minimum required version of binutils is 2.25, which
> supports XSAVE{,OPT,C,S} and XRSTOR{,S} instruction mnemonics.
>
> Replace the byte-wise specification of XSAVE{,OPT,C,S}
> and XRSTOR{,S} with these proper mnemonics.
>
> No functional change intended.
>
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Brian Gerst <brgerst@gmail.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Link: https://lore.kernel.org/r/20250313130251.383204-1-ubizjak@gmail.com
> ---
> arch/x86/kernel/fpu/xstate.h | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
> index aa16f1a..1418423 100644
> --- a/arch/x86/kernel/fpu/xstate.h
> +++ b/arch/x86/kernel/fpu/xstate.h
> @@ -94,18 +94,17 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
> /* XSAVE/XRSTOR wrapper functions */
>
> #ifdef CONFIG_X86_64
> -#define REX_PREFIX "0x48, "
> +#define REX_SUFFIX "64"
> #else
> -#define REX_PREFIX
> +#define REX_SUFFIX
> #endif
>
> -/* These macros all use (%edi)/(%rdi) as the single memory argument. */
> -#define XSAVE ".byte " REX_PREFIX "0x0f,0xae,0x27"
> -#define XSAVEOPT ".byte " REX_PREFIX "0x0f,0xae,0x37"
> -#define XSAVEC ".byte " REX_PREFIX "0x0f,0xc7,0x27"
> -#define XSAVES ".byte " REX_PREFIX "0x0f,0xc7,0x2f"
> -#define XRSTOR ".byte " REX_PREFIX "0x0f,0xae,0x2f"
> -#define XRSTORS ".byte " REX_PREFIX "0x0f,0xc7,0x1f"
> +#define XSAVE "xsave" REX_SUFFIX " %[xa]"
> +#define XSAVEOPT "xsaveopt" REX_SUFFIX " %[xa]"
> +#define XSAVEC "xsavec" REX_SUFFIX " %[xa]"
> +#define XSAVES "xsaves" REX_SUFFIX " %[xa]"
> +#define XRSTOR "xrstor" REX_SUFFIX " %[xa]"
> +#define XRSTORS "xrstors" REX_SUFFIX " %[xa]"
>
> /*
> * After this @err contains 0 on success or the trap number when the
> @@ -114,10 +113,10 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
> #define XSTATE_OP(op, st, lmask, hmask, err) \
> asm volatile("1:" op "\n\t" \
> "xor %[err], %[err]\n" \
> - "2:\n\t" \
> + "2:\n" \
> _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT_MCE_SAFE) \
> : [err] "=a" (err) \
> - : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
> + : [xa] "m" (*(st)), "a" (lmask), "d" (hmask) \
This [xa] needs documenting in the comment above this.
What does "xa" even mean?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, Mar 17, 2025 at 11:14 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Mar 13, 2025 at 05:50:34PM -0000, tip-bot2 for Uros Bizjak wrote:
> > The following commit has been merged into the x86/fpu branch of tip:
> >
> > Commit-ID: 2883b4c2169a435488f7845e1b6fdc6f3438c7c6
> > Gitweb: https://git.kernel.org/tip/2883b4c2169a435488f7845e1b6fdc6f3438c7c6
> > Author: Uros Bizjak <ubizjak@gmail.com>
> > AuthorDate: Thu, 13 Mar 2025 14:02:27 +01:00
> > Committer: Ingo Molnar <mingo@kernel.org>
> > CommitterDate: Thu, 13 Mar 2025 18:36:52 +01:00
> >
> > x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
> >
> > Current minimum required version of binutils is 2.25, which
> > supports XSAVE{,OPT,C,S} and XRSTOR{,S} instruction mnemonics.
> >
> > Replace the byte-wise specification of XSAVE{,OPT,C,S}
> > and XRSTOR{,S} with these proper mnemonics.
> >
> > No functional change intended.
> >
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Brian Gerst <brgerst@gmail.com>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: Dave Hansen <dave.hansen@linux.intel.com>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Link: https://lore.kernel.org/r/20250313130251.383204-1-ubizjak@gmail.com
> > ---
> > arch/x86/kernel/fpu/xstate.h | 27 +++++++++++++--------------
> > 1 file changed, 13 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
> > index aa16f1a..1418423 100644
> > --- a/arch/x86/kernel/fpu/xstate.h
> > +++ b/arch/x86/kernel/fpu/xstate.h
> > @@ -94,18 +94,17 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
> > /* XSAVE/XRSTOR wrapper functions */
> >
> > #ifdef CONFIG_X86_64
> > -#define REX_PREFIX "0x48, "
> > +#define REX_SUFFIX "64"
> > #else
> > -#define REX_PREFIX
> > +#define REX_SUFFIX
> > #endif
> >
> > -/* These macros all use (%edi)/(%rdi) as the single memory argument. */
> > -#define XSAVE ".byte " REX_PREFIX "0x0f,0xae,0x27"
> > -#define XSAVEOPT ".byte " REX_PREFIX "0x0f,0xae,0x37"
> > -#define XSAVEC ".byte " REX_PREFIX "0x0f,0xc7,0x27"
> > -#define XSAVES ".byte " REX_PREFIX "0x0f,0xc7,0x2f"
> > -#define XRSTOR ".byte " REX_PREFIX "0x0f,0xae,0x2f"
> > -#define XRSTORS ".byte " REX_PREFIX "0x0f,0xc7,0x1f"
> > +#define XSAVE "xsave" REX_SUFFIX " %[xa]"
> > +#define XSAVEOPT "xsaveopt" REX_SUFFIX " %[xa]"
> > +#define XSAVEC "xsavec" REX_SUFFIX " %[xa]"
> > +#define XSAVES "xsaves" REX_SUFFIX " %[xa]"
> > +#define XRSTOR "xrstor" REX_SUFFIX " %[xa]"
> > +#define XRSTORS "xrstors" REX_SUFFIX " %[xa]"
> >
> > /*
> > * After this @err contains 0 on success or the trap number when the
> > @@ -114,10 +113,10 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
> > #define XSTATE_OP(op, st, lmask, hmask, err) \
> > asm volatile("1:" op "\n\t" \
> > "xor %[err], %[err]\n" \
> > - "2:\n\t" \
> > + "2:\n" \
> > _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT_MCE_SAFE) \
> > : [err] "=a" (err) \
> > - : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
> > + : [xa] "m" (*(st)), "a" (lmask), "d" (hmask) \
>
> This [xa] needs documenting in the comment above this.
>
> What does "xa" even mean?
xsave area.
Uros.
On Mon, Mar 17, 2025 at 11:28:58AM +0100, Uros Bizjak wrote:
> > > @@ -114,10 +113,10 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
> > > #define XSTATE_OP(op, st, lmask, hmask, err) \
> > > asm volatile("1:" op "\n\t" \
> > > "xor %[err], %[err]\n" \
> > > - "2:\n\t" \
> > > + "2:\n" \
> > > _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT_MCE_SAFE) \
> > > : [err] "=a" (err) \
> > > - : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
> > > + : [xa] "m" (*(st)), "a" (lmask), "d" (hmask) \
> >
> > This [xa] needs documenting in the comment above this.
> >
> > What does "xa" even mean?
>
> xsave area.
That's struct xregs_state in kernel nomenclature.
And the macro's argument is called "st".
And when it says [xa] there, one wonders where that "xa" comes from. So please
add a comment above the macro explaining that.
And you can redo the whole patch - it is the topmost one in tip:x86/fpu and
can simply be replaced.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, Mar 17, 2025 at 11:46 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Mar 17, 2025 at 11:28:58AM +0100, Uros Bizjak wrote:
> > > > @@ -114,10 +113,10 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
> > > > #define XSTATE_OP(op, st, lmask, hmask, err) \
> > > > asm volatile("1:" op "\n\t" \
> > > > "xor %[err], %[err]\n" \
> > > > - "2:\n\t" \
> > > > + "2:\n" \
> > > > _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT_MCE_SAFE) \
> > > > : [err] "=a" (err) \
> > > > - : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
> > > > + : [xa] "m" (*(st)), "a" (lmask), "d" (hmask) \
> > >
> > > This [xa] needs documenting in the comment above this.
> > >
> > > What does "xa" even mean?
> >
> > xsave area.
>
> That's struct xregs_state in kernel nomenclature.
>
> And the macro's argument is called "st".
>
> And when it says [xa] there, one wonders where that "xa" comes from. So please
> add a comment above the macro explaining that.
This is an internal label for a named argument. The name shouldn't
bother anybody, it could be anything, [xa], [ptr], [arg] or whatnot,
so I see no reason why a comment should explain the choice. It's like
arguing about the name of a variable.
Uros.
On March 17, 2025 4:06:11 AM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>On Mon, Mar 17, 2025 at 11:46 AM Borislav Petkov <bp@alien8.de> wrote:
>>
>> On Mon, Mar 17, 2025 at 11:28:58AM +0100, Uros Bizjak wrote:
>> > > > @@ -114,10 +113,10 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
>> > > > #define XSTATE_OP(op, st, lmask, hmask, err) \
>> > > > asm volatile("1:" op "\n\t" \
>> > > > "xor %[err], %[err]\n" \
>> > > > - "2:\n\t" \
>> > > > + "2:\n" \
>> > > > _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT_MCE_SAFE) \
>> > > > : [err] "=a" (err) \
>> > > > - : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
>> > > > + : [xa] "m" (*(st)), "a" (lmask), "d" (hmask) \
>> > >
>> > > This [xa] needs documenting in the comment above this.
>> > >
>> > > What does "xa" even mean?
>> >
>> > xsave area.
>>
>> That's struct xregs_state in kernel nomenclature.
>>
>> And the macro's argument is called "st".
>>
>> And when it says [xa] there, one wonders where that "xa" comes from. So please
>> add a comment above the macro explaining that.
>
>This is an internal label for a named argument. The name shouldn't
>bother anybody, it could be anything, [xa], [ptr], [arg] or whatnot,
>so I see no reason why a comment should explain the choice. It's like
>arguing about the name of a variable.
>
>Uros.
>
Ok, I'm going to argue, but only because the argument is called "st" and the assembly parameter "xa". That's needlessly different and means having to look extra hard
We can obviously not use the same token, but IMO it would make a lot more sense to call one of them _st or perhaps st_p (being a pointer).
On Mon, Mar 17, 2025 at 11:38:12AM -0700, H. Peter Anvin wrote:
> Ok, I'm going to argue, but only because the argument is called "st" and the
> assembly parameter "xa". That's needlessly different and means having to
> look extra hard
>
> We can obviously not use the same token, but IMO it would make a lot more
> sense to call one of them _st or perhaps st_p (being a pointer).
Already documented:
https://git.kernel.org/tip/4348e9177813656d5d8bd18f34b3e611df004032
Got tired of discussing the obvious.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On March 17, 2025 11:48:39 AM PDT, Borislav Petkov <bp@alien8.de> wrote: >On Mon, Mar 17, 2025 at 11:38:12AM -0700, H. Peter Anvin wrote: >> Ok, I'm going to argue, but only because the argument is called "st" and the >> assembly parameter "xa". That's needlessly different and means having to >> look extra hard >> >> We can obviously not use the same token, but IMO it would make a lot more >> sense to call one of them _st or perhaps st_p (being a pointer). > >Already documented: > >https://git.kernel.org/tip/4348e9177813656d5d8bd18f34b3e611df004032 > >Got tired of discussing the obvious. > Ok.
© 2016 - 2025 Red Hat, Inc.