* [PATCH] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
@ 2025-03-13 13:02 Uros Bizjak
2025-03-13 17:50 ` [tip: x86/fpu] " tip-bot2 for Uros Bizjak
0 siblings, 1 reply; 9+ messages in thread
From: Uros Bizjak @ 2025-03-13 13:02 UTC (permalink / raw)
To: x86, linux-kernel
Cc: Uros Bizjak, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin
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>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.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 aa16f1a1bbcf..1418423bc4c9 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)
--
2.48.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [tip: x86/fpu] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
2025-03-13 13:02 [PATCH] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h Uros Bizjak
@ 2025-03-13 17:50 ` tip-bot2 for Uros Bizjak
2025-03-17 10:14 ` Borislav Petkov
0 siblings, 1 reply; 9+ messages in thread
From: tip-bot2 for Uros Bizjak @ 2025-03-13 17:50 UTC (permalink / raw)
To: linux-tip-commits
Cc: Uros Bizjak, Ingo Molnar, Andy Lutomirski, Brian Gerst,
H. Peter Anvin, Dave Hansen, Linus Torvalds, x86, linux-kernel
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)
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [tip: x86/fpu] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
2025-03-13 17:50 ` [tip: x86/fpu] " tip-bot2 for Uros Bizjak
@ 2025-03-17 10:14 ` Borislav Petkov
2025-03-17 10:28 ` Uros Bizjak
0 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2025-03-17 10:14 UTC (permalink / raw)
To: linux-kernel
Cc: linux-tip-commits, Uros Bizjak, Ingo Molnar, Andy Lutomirski,
Brian Gerst, H. Peter Anvin, Dave Hansen, Linus Torvalds, x86
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [tip: x86/fpu] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
2025-03-17 10:14 ` Borislav Petkov
@ 2025-03-17 10:28 ` Uros Bizjak
2025-03-17 10:46 ` Borislav Petkov
0 siblings, 1 reply; 9+ messages in thread
From: Uros Bizjak @ 2025-03-17 10:28 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-kernel, linux-tip-commits, Ingo Molnar, Andy Lutomirski,
Brian Gerst, H. Peter Anvin, Dave Hansen, Linus Torvalds, x86
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.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [tip: x86/fpu] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
2025-03-17 10:28 ` Uros Bizjak
@ 2025-03-17 10:46 ` Borislav Petkov
2025-03-17 11:06 ` Uros Bizjak
0 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2025-03-17 10:46 UTC (permalink / raw)
To: Uros Bizjak
Cc: linux-kernel, linux-tip-commits, Ingo Molnar, Andy Lutomirski,
Brian Gerst, H. Peter Anvin, Dave Hansen, Linus Torvalds, x86
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [tip: x86/fpu] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
2025-03-17 10:46 ` Borislav Petkov
@ 2025-03-17 11:06 ` Uros Bizjak
2025-03-17 18:38 ` H. Peter Anvin
0 siblings, 1 reply; 9+ messages in thread
From: Uros Bizjak @ 2025-03-17 11:06 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-kernel, linux-tip-commits, Ingo Molnar, Andy Lutomirski,
Brian Gerst, H. Peter Anvin, Dave Hansen, Linus Torvalds, x86
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.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [tip: x86/fpu] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
2025-03-17 11:06 ` Uros Bizjak
@ 2025-03-17 18:38 ` H. Peter Anvin
2025-03-17 18:48 ` Borislav Petkov
0 siblings, 1 reply; 9+ messages in thread
From: H. Peter Anvin @ 2025-03-17 18:38 UTC (permalink / raw)
To: Uros Bizjak, Borislav Petkov
Cc: linux-kernel, linux-tip-commits, Ingo Molnar, Andy Lutomirski,
Brian Gerst, Dave Hansen, Linus Torvalds, x86
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).
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [tip: x86/fpu] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
2025-03-17 18:38 ` H. Peter Anvin
@ 2025-03-17 18:48 ` Borislav Petkov
2025-03-17 18:49 ` H. Peter Anvin
0 siblings, 1 reply; 9+ messages in thread
From: Borislav Petkov @ 2025-03-17 18:48 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Uros Bizjak, linux-kernel, linux-tip-commits, Ingo Molnar,
Andy Lutomirski, Brian Gerst, Dave Hansen, Linus Torvalds, x86
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [tip: x86/fpu] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h
2025-03-17 18:48 ` Borislav Petkov
@ 2025-03-17 18:49 ` H. Peter Anvin
0 siblings, 0 replies; 9+ messages in thread
From: H. Peter Anvin @ 2025-03-17 18:49 UTC (permalink / raw)
To: Borislav Petkov
Cc: Uros Bizjak, linux-kernel, linux-tip-commits, Ingo Molnar,
Andy Lutomirski, Brian Gerst, Dave Hansen, Linus Torvalds, x86
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-03-17 18:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-13 13:02 [PATCH] x86/fpu: Use XSAVE{,OPT,C,S} and XRSTOR{,S} mnemonics in xstate.h Uros Bizjak
2025-03-13 17:50 ` [tip: x86/fpu] " tip-bot2 for Uros Bizjak
2025-03-17 10:14 ` Borislav Petkov
2025-03-17 10:28 ` Uros Bizjak
2025-03-17 10:46 ` Borislav Petkov
2025-03-17 11:06 ` Uros Bizjak
2025-03-17 18:38 ` H. Peter Anvin
2025-03-17 18:48 ` Borislav Petkov
2025-03-17 18:49 ` H. Peter Anvin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®