* [PATCH] x86: Use INVPCID mnemonic in invpcid.h @ 2020-05-08 9:22 Uros Bizjak 2020-05-08 11:34 ` Peter Zijlstra ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Uros Bizjak @ 2020-05-08 9:22 UTC (permalink / raw) To: x86, linux-kernel Cc: Uros Bizjak, H. Peter Anvin, Ingo Molnar, Thomas Gleixner Current minimum required version of binutils is 2.23, which supports INVPCID instruction mnemonic. Replace the byte-wise specification of INVPCID with this proper mnemonic. Signed-off-by: Uros Bizjak <ubizjak@gmail.com> CC: "H. Peter Anvin" <hpa@zytor.com> CC: Ingo Molnar <mingo@redhat.com> CC: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/include/asm/invpcid.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/arch/x86/include/asm/invpcid.h b/arch/x86/include/asm/invpcid.h index 989cfa86de85..23749bbca0ad 100644 --- a/arch/x86/include/asm/invpcid.h +++ b/arch/x86/include/asm/invpcid.h @@ -12,12 +12,9 @@ static inline void __invpcid(unsigned long pcid, unsigned long addr, * stale TLB entries and, especially if we're flushing global * mappings, we don't want the compiler to reorder any subsequent * memory accesses before the TLB flush. - * - * The hex opcode is invpcid (%ecx), %eax in 32-bit mode and - * invpcid (%rcx), %rax in long mode. */ - asm volatile (".byte 0x66, 0x0f, 0x38, 0x82, 0x01" - : : "m" (desc), "a" (type), "c" (&desc) : "memory"); + asm volatile ("invpcid %1, %0" + : : "r" (type), "m" (desc) : "memory"); } #define INVPCID_TYPE_INDIV_ADDR 0 -- 2.25.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Use INVPCID mnemonic in invpcid.h 2020-05-08 9:22 [PATCH] x86: Use INVPCID mnemonic in invpcid.h Uros Bizjak @ 2020-05-08 11:34 ` Peter Zijlstra 2020-05-08 17:32 ` H. Peter Anvin ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Peter Zijlstra @ 2020-05-08 11:34 UTC (permalink / raw) To: Uros Bizjak Cc: x86, linux-kernel, H. Peter Anvin, Ingo Molnar, Thomas Gleixner On Fri, May 08, 2020 at 11:22:47AM +0200, Uros Bizjak wrote: > Current minimum required version of binutils is 2.23, > which supports INVPCID instruction mnemonic. > > Replace the byte-wise specification of INVPCID with > this proper mnemonic. Excellent, thanks for doing these cleanups. (also for the rand one): Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Use INVPCID mnemonic in invpcid.h 2020-05-08 9:22 [PATCH] x86: Use INVPCID mnemonic in invpcid.h Uros Bizjak 2020-05-08 11:34 ` Peter Zijlstra @ 2020-05-08 17:32 ` H. Peter Anvin 2020-05-09 11:33 ` Borislav Petkov 2020-05-12 14:10 ` [tip: x86/cpu] x86/cpu: " tip-bot2 for Uros Bizjak 3 siblings, 0 replies; 9+ messages in thread From: H. Peter Anvin @ 2020-05-08 17:32 UTC (permalink / raw) To: Uros Bizjak, x86, linux-kernel; +Cc: Ingo Molnar, Thomas Gleixner Reviewed-by: H. Peter Anvin (Intel) <hpa@zytor.com> On 2020-05-08 02:22, Uros Bizjak wrote: > Current minimum required version of binutils is 2.23, > which supports INVPCID instruction mnemonic. > > Replace the byte-wise specification of INVPCID with > this proper mnemonic. > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > CC: "H. Peter Anvin" <hpa@zytor.com> > CC: Ingo Molnar <mingo@redhat.com> > CC: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/include/asm/invpcid.h | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/include/asm/invpcid.h b/arch/x86/include/asm/invpcid.h > index 989cfa86de85..23749bbca0ad 100644 > --- a/arch/x86/include/asm/invpcid.h > +++ b/arch/x86/include/asm/invpcid.h > @@ -12,12 +12,9 @@ static inline void __invpcid(unsigned long pcid, unsigned long addr, > * stale TLB entries and, especially if we're flushing global > * mappings, we don't want the compiler to reorder any subsequent > * memory accesses before the TLB flush. > - * > - * The hex opcode is invpcid (%ecx), %eax in 32-bit mode and > - * invpcid (%rcx), %rax in long mode. > */ > - asm volatile (".byte 0x66, 0x0f, 0x38, 0x82, 0x01" > - : : "m" (desc), "a" (type), "c" (&desc) : "memory"); > + asm volatile ("invpcid %1, %0" > + : : "r" (type), "m" (desc) : "memory"); > } > > #define INVPCID_TYPE_INDIV_ADDR 0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Use INVPCID mnemonic in invpcid.h 2020-05-08 9:22 [PATCH] x86: Use INVPCID mnemonic in invpcid.h Uros Bizjak 2020-05-08 11:34 ` Peter Zijlstra 2020-05-08 17:32 ` H. Peter Anvin @ 2020-05-09 11:33 ` Borislav Petkov 2020-05-12 14:10 ` [tip: x86/cpu] x86/cpu: " tip-bot2 for Uros Bizjak 3 siblings, 0 replies; 9+ messages in thread From: Borislav Petkov @ 2020-05-09 11:33 UTC (permalink / raw) To: Uros Bizjak Cc: x86, linux-kernel, H. Peter Anvin, Ingo Molnar, Thomas Gleixner On Fri, May 08, 2020 at 11:22:47AM +0200, Uros Bizjak wrote: > Current minimum required version of binutils is 2.23, > which supports INVPCID instruction mnemonic. > > Replace the byte-wise specification of INVPCID with > this proper mnemonic. > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > CC: "H. Peter Anvin" <hpa@zytor.com> > CC: Ingo Molnar <mingo@redhat.com> > CC: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/include/asm/invpcid.h | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/include/asm/invpcid.h b/arch/x86/include/asm/invpcid.h > index 989cfa86de85..23749bbca0ad 100644 > --- a/arch/x86/include/asm/invpcid.h > +++ b/arch/x86/include/asm/invpcid.h > @@ -12,12 +12,9 @@ static inline void __invpcid(unsigned long pcid, unsigned long addr, > * stale TLB entries and, especially if we're flushing global > * mappings, we don't want the compiler to reorder any subsequent > * memory accesses before the TLB flush. > - * > - * The hex opcode is invpcid (%ecx), %eax in 32-bit mode and > - * invpcid (%rcx), %rax in long mode. > */ > - asm volatile (".byte 0x66, 0x0f, 0x38, 0x82, 0x01" > - : : "m" (desc), "a" (type), "c" (&desc) : "memory"); > + asm volatile ("invpcid %1, %0" > + : : "r" (type), "m" (desc) : "memory"); How about I make it even more readable when applying? asm volatile("invpcid %[desc], %[type]" :: [desc] "m" (desc), [type] "r" (type) : "memory"); :-) -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 9+ messages in thread
* [tip: x86/cpu] x86/cpu: Use INVPCID mnemonic in invpcid.h 2020-05-08 9:22 [PATCH] x86: Use INVPCID mnemonic in invpcid.h Uros Bizjak ` (2 preceding siblings ...) 2020-05-09 11:33 ` Borislav Petkov @ 2020-05-12 14:10 ` tip-bot2 for Uros Bizjak 2020-05-12 14:26 ` Uros Bizjak 3 siblings, 1 reply; 9+ messages in thread From: tip-bot2 for Uros Bizjak @ 2020-05-12 14:10 UTC (permalink / raw) To: linux-tip-commits Cc: Uros Bizjak, Borislav Petkov, H. Peter Anvin (Intel), Peter Zijlstra (Intel), x86, LKML The following commit has been merged into the x86/cpu branch of tip: Commit-ID: 7e32a9dac9926241d56851e1517c9391d39fb48e Gitweb: https://git.kernel.org/tip/7e32a9dac9926241d56851e1517c9391d39fb48e Author: Uros Bizjak <ubizjak@gmail.com> AuthorDate: Fri, 08 May 2020 11:22:47 +02:00 Committer: Borislav Petkov <bp@suse.de> CommitterDate: Tue, 12 May 2020 16:05:30 +02:00 x86/cpu: Use INVPCID mnemonic in invpcid.h The current minimum required version of binutils is 2.23, which supports the INVPCID instruction mnemonic. Replace the byte-wise specification of INVPCID with the proper mnemonic. [ bp: Add symbolic operand names for increased readability and flip their order like the insn expects them for the AT&T syntax. ] Signed-off-by: Uros Bizjak <ubizjak@gmail.com> Signed-off-by: Borislav Petkov <bp@suse.de> Reviewed-by: H. Peter Anvin (Intel) <hpa@zytor.com> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20200508092247.132147-1-ubizjak@gmail.com Signed-off-by: Borislav Petkov <bp@suse.de> --- arch/x86/include/asm/invpcid.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/arch/x86/include/asm/invpcid.h b/arch/x86/include/asm/invpcid.h index 989cfa8..734482a 100644 --- a/arch/x86/include/asm/invpcid.h +++ b/arch/x86/include/asm/invpcid.h @@ -12,12 +12,9 @@ static inline void __invpcid(unsigned long pcid, unsigned long addr, * stale TLB entries and, especially if we're flushing global * mappings, we don't want the compiler to reorder any subsequent * memory accesses before the TLB flush. - * - * The hex opcode is invpcid (%ecx), %eax in 32-bit mode and - * invpcid (%rcx), %rax in long mode. */ - asm volatile (".byte 0x66, 0x0f, 0x38, 0x82, 0x01" - : : "m" (desc), "a" (type), "c" (&desc) : "memory"); + asm volatile("invpcid %[desc], %[type]" + :: [desc] "m" (desc), [type] "r" (type) : "memory"); } #define INVPCID_TYPE_INDIV_ADDR 0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [tip: x86/cpu] x86/cpu: Use INVPCID mnemonic in invpcid.h 2020-05-12 14:10 ` [tip: x86/cpu] x86/cpu: " tip-bot2 for Uros Bizjak @ 2020-05-12 14:26 ` Uros Bizjak 2020-05-12 15:15 ` Borislav Petkov 0 siblings, 1 reply; 9+ messages in thread From: Uros Bizjak @ 2020-05-12 14:26 UTC (permalink / raw) To: LKML Cc: linux-tip-commits, Borislav Petkov, H. Peter Anvin (Intel), Peter Zijlstra (Intel), x86 On Tue, May 12, 2020 at 4:10 PM tip-bot2 for Uros Bizjak <tip-bot2@linutronix.de> wrote: > > The following commit has been merged into the x86/cpu branch of tip: > > Commit-ID: 7e32a9dac9926241d56851e1517c9391d39fb48e > Gitweb: https://git.kernel.org/tip/7e32a9dac9926241d56851e1517c9391d39fb48e > Author: Uros Bizjak <ubizjak@gmail.com> > AuthorDate: Fri, 08 May 2020 11:22:47 +02:00 > Committer: Borislav Petkov <bp@suse.de> > CommitterDate: Tue, 12 May 2020 16:05:30 +02:00 > > x86/cpu: Use INVPCID mnemonic in invpcid.h > > The current minimum required version of binutils is 2.23, which supports > the INVPCID instruction mnemonic. Replace the byte-wise specification of > INVPCID with the proper mnemonic. > > [ bp: Add symbolic operand names for increased readability and flip > their order like the insn expects them for the AT&T syntax. ] Actually, the order was correct for AT&T syntax in the original patch. The insn template for AT&T syntax goes: insn arg2, arg1, arg0 where rightmost arguments are output operands. The operands in asm template go asm ("insn template" : output0, output1 : input0, input1 : clobbers) so, in effect: asm ("insn template" : arg0, arg1 : arg2, arg3: clobbers) As you can see, the operand order in insn tempate is reversed for AT&T syntax. I didn't notice the reversal of operands in your improvement. Uros. Uros. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tip: x86/cpu] x86/cpu: Use INVPCID mnemonic in invpcid.h 2020-05-12 14:26 ` Uros Bizjak @ 2020-05-12 15:15 ` Borislav Petkov 2020-05-12 15:54 ` Uros Bizjak 0 siblings, 1 reply; 9+ messages in thread From: Borislav Petkov @ 2020-05-12 15:15 UTC (permalink / raw) To: Uros Bizjak Cc: LKML, linux-tip-commits, H. Peter Anvin (Intel), Peter Zijlstra (Intel), x86 On Tue, May 12, 2020 at 04:26:37PM +0200, Uros Bizjak wrote: > Actually, the order was correct for AT&T syntax in the original patch. > > The insn template for AT&T syntax goes: > > insn arg2, arg1, arg0 > > where rightmost arguments are output operands. > > The operands in asm template go > > asm ("insn template" : output0, output1 : input0, input1 : clobbers) > > so, in effect: > > asm ("insn template" : arg0, arg1 : arg2, arg3: clobbers) > > As you can see, the operand order in insn tempate is reversed for AT&T > syntax. I didn't notice the reversal of operands in your improvement. Your version had: + asm volatile ("invpcid %1, %0" + : : "r" (type), "m" (desc) : "memory"); with "type" being the 0th operand and "desc" being the 1st operand in the input operands list. The order of the operands after the "invpcid" mnemonic are the other way around though: you first have %1 which is "desc" and then %0 which is the type. I simply swapped the arguments order in the input operands list, after the second ':' + asm volatile("invpcid %[desc], %[type]" + :: [desc] "m" (desc), [type] "r" (type) : "memory"); so that "desc" comes first and "type" second when reading from left-to-right in both 1. *after* the "invpcid" mnemonic and 2. in the input operands list, after the second ':'. And since I'm using the symbolic operand names, then the order just works because looking at a before-and-after thing doesn't show any opcode differences: $ diff -suprN /tmp/before /tmp/after Files /tmp/before and /tmp/after are identical $ cat /tmp/before ffffffff81058392: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8105c542: 66 44 0f 38 82 04 24 invpcid (%rsp),%r8 ffffffff8105c5b4: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8105ccd2: 66 44 0f 38 82 14 24 invpcid (%rsp),%r10 ffffffff8105d6cf: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8105d7ef: 66 0f 38 82 1c 24 invpcid (%rsp),%rbx ffffffff8105e4db: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8106067b: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8169547d: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff82406698 <x86_noinvpcid_setup>: ffffffff824066a3: 75 27 jne ffffffff824066cc <x86_noinvpcid_setup+0x34> ffffffff824066b4: 73 16 jae ffffffff824066cc <x86_noinvpcid_setup+0x34> ffffffff824124db: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff82412e34: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff82415158: 66 0f 38 82 44 24 10 invpcid 0x10(%rsp),%rax $ cat /tmp/after ffffffff81058392: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8105c542: 66 44 0f 38 82 04 24 invpcid (%rsp),%r8 ffffffff8105c5b4: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8105ccd2: 66 44 0f 38 82 14 24 invpcid (%rsp),%r10 ffffffff8105d6cf: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8105d7ef: 66 0f 38 82 1c 24 invpcid (%rsp),%rbx ffffffff8105e4db: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8106067b: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff8169547d: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff82406698 <x86_noinvpcid_setup>: ffffffff824066a3: 75 27 jne ffffffff824066cc <x86_noinvpcid_setup+0x34> ffffffff824066b4: 73 16 jae ffffffff824066cc <x86_noinvpcid_setup+0x34> ffffffff824124db: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff82412e34: 66 0f 38 82 04 24 invpcid (%rsp),%rax ffffffff82415158: 66 0f 38 82 44 24 10 invpcid 0x10(%rsp),%rax Makes sense? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tip: x86/cpu] x86/cpu: Use INVPCID mnemonic in invpcid.h 2020-05-12 15:15 ` Borislav Petkov @ 2020-05-12 15:54 ` Uros Bizjak 2020-05-12 16:28 ` Borislav Petkov 0 siblings, 1 reply; 9+ messages in thread From: Uros Bizjak @ 2020-05-12 15:54 UTC (permalink / raw) To: Borislav Petkov Cc: LKML, linux-tip-commits, H. Peter Anvin (Intel), Peter Zijlstra (Intel), x86 On Tue, May 12, 2020 at 5:15 PM Borislav Petkov <bp@alien8.de> wrote: > > On Tue, May 12, 2020 at 04:26:37PM +0200, Uros Bizjak wrote: > > Actually, the order was correct for AT&T syntax in the original patch. > > > > The insn template for AT&T syntax goes: > > > > insn arg2, arg1, arg0 > > > > where rightmost arguments are output operands. > > > > The operands in asm template go > > > > asm ("insn template" : output0, output1 : input0, input1 : clobbers) > > > > so, in effect: > > > > asm ("insn template" : arg0, arg1 : arg2, arg3: clobbers) > > > > As you can see, the operand order in insn tempate is reversed for AT&T > > syntax. I didn't notice the reversal of operands in your improvement. > > Your version had: > > + asm volatile ("invpcid %1, %0" > + : : "r" (type), "m" (desc) : "memory"); > > with "type" being the 0th operand and "desc" being the 1st operand in > the input operands list. Correct. > The order of the operands after the "invpcid" mnemonic are the other way > around though: you first have %1 which is "desc" and then %0 which is > the type. Also correct. Because AT&T has right-to-left operand order, while asm statement has left-to-right operand order. > I simply swapped the arguments order in the input operands list, after > the second ':' > > + asm volatile("invpcid %[desc], %[type]" > + :: [desc] "m" (desc), [type] "r" (type) : "memory"); > > so that "desc" comes first and "type" second when reading from > left-to-right in both But this is not correct, AT&T insn template should have right-to-left order. > 1. *after* the "invpcid" mnemonic and > 2. in the input operands list, after the second ':'. This is not the case throughout the kernel source. Please see for example sync_bitops, where: asm volatile("lock; " __ASM_SIZE(bts) " %1,%0" : "+m" (ADDR) : "Ir" (nr) : "memory"); (to support my claim, I tried to find instruction with two input operands; there are some in KVM, but these were also written by myself). > And since I'm using the symbolic operand names, then the order just > works because looking at a before-and-after thing doesn't show any > opcode differences: Symbolic operands are agnostic to the position in the asm clause, so it really doesn't matter much. It just doesn't feel right, when other cases follow different order. > $ diff -suprN /tmp/before /tmp/after > Files /tmp/before and /tmp/after are identical Sure, otherwise assembler would complain. > Makes sense? Well, I don't want to bikeshed around this anymore, so any way is good. Uros. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tip: x86/cpu] x86/cpu: Use INVPCID mnemonic in invpcid.h 2020-05-12 15:54 ` Uros Bizjak @ 2020-05-12 16:28 ` Borislav Petkov 0 siblings, 0 replies; 9+ messages in thread From: Borislav Petkov @ 2020-05-12 16:28 UTC (permalink / raw) To: Uros Bizjak Cc: LKML, linux-tip-commits, H. Peter Anvin (Intel), Peter Zijlstra (Intel), x86 On Tue, May 12, 2020 at 05:54:49PM +0200, Uros Bizjak wrote: > Symbolic operands are agnostic to the position in the asm clause, so > it really doesn't matter much. It just doesn't feel right, when other > cases follow different order. > > > $ diff -suprN /tmp/before /tmp/after > > Files /tmp/before and /tmp/after are identical > > Sure, otherwise assembler would complain. > > > Makes sense? > > Well, I don't want to bikeshed around this anymore, so any way is good. Look at it this way: the symbolic operand names feature has made inline assembly *orders* of magnitude more readable than what it was before. Kernel folks, including myself, have stumbled upon the question which operand is which, on a regular basis and having the operand names there makes reading the inline asm almost trivial. So while we should not convert wholesale, I think we should aim to gradually convert those other cases to the a-lot-more readable variant with symbolic operand names. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-12 16:28 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-08 9:22 [PATCH] x86: Use INVPCID mnemonic in invpcid.h Uros Bizjak 2020-05-08 11:34 ` Peter Zijlstra 2020-05-08 17:32 ` H. Peter Anvin 2020-05-09 11:33 ` Borislav Petkov 2020-05-12 14:10 ` [tip: x86/cpu] x86/cpu: " tip-bot2 for Uros Bizjak 2020-05-12 14:26 ` Uros Bizjak 2020-05-12 15:15 ` Borislav Petkov 2020-05-12 15:54 ` Uros Bizjak 2020-05-12 16:28 ` Borislav Petkov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.