* [PATCH] x86/kvm/emulate.c: simplify NOP (opcode 0x90) check
@ 2013-07-04 8:58 Denys Vlasenko
2013-07-04 9:13 ` Gleb Natapov
0 siblings, 1 reply; 5+ messages in thread
From: Denys Vlasenko @ 2013-07-04 8:58 UTC (permalink / raw)
To: Gleb Natapov, linux-kernel; +Cc: Denys Vlasenko, Paolo Bonzini, Avi Kivity
The check sits in switch() statement which itself can check
for opcode 0x90 far more efficiently.
On assembler level, this change simply eliminates the following
bit of code:
4c 8b a3 d8 00 00 00 mov 0xd8(%rbx),%r12
31 f6 xor %esi,%esi
48 89 df mov %rbx,%rdi
e8 32 8c ff ff callq 490 <reg_read>
48 8d 83 78 01 00 00 lea 0x178(%rbx),%rax
83 8b 70 01 00 00 01 orl $0x1,0x170(%rbx)
83 8b 74 01 00 00 01 orl $0x1,0x174(%rbx)
49 39 c4 cmp %rax,%r12
0f 84 88 fa ff ff je 7304 <x86_emulate_insn+0x204>
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Avi Kivity <avi.kivity@gmail.com>
---
arch/x86/kvm/emulate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2bc1e81..7e6e74a 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -4544,9 +4544,9 @@ special_insn:
case 0x8d: /* lea r16/r32, m */
ctxt->dst.val = ctxt->src.addr.mem.ea;
break;
- case 0x90 ... 0x97: /* nop / xchg reg, rax */
- if (ctxt->dst.addr.reg == reg_rmw(ctxt, VCPU_REGS_RAX))
- break;
+ case 0x90: /* nop */
+ break;
+ case 0x91 ... 0x97: /* xchg reg, rax */
rc = em_xchg(ctxt);
break;
case 0x98: /* cbw/cwde/cdqe */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/kvm/emulate.c: simplify NOP (opcode 0x90) check
2013-07-04 8:58 [PATCH] x86/kvm/emulate.c: simplify NOP (opcode 0x90) check Denys Vlasenko
@ 2013-07-04 9:13 ` Gleb Natapov
2013-07-04 11:06 ` Denys Vlasenko
0 siblings, 1 reply; 5+ messages in thread
From: Gleb Natapov @ 2013-07-04 9:13 UTC (permalink / raw)
To: Denys Vlasenko; +Cc: linux-kernel, Paolo Bonzini, Avi Kivity
On Thu, Jul 04, 2013 at 10:58:29AM +0200, Denys Vlasenko wrote:
> The check sits in switch() statement which itself can check
> for opcode 0x90 far more efficiently.
>
> On assembler level, this change simply eliminates the following
> bit of code:
>
> 4c 8b a3 d8 00 00 00 mov 0xd8(%rbx),%r12
> 31 f6 xor %esi,%esi
> 48 89 df mov %rbx,%rdi
> e8 32 8c ff ff callq 490 <reg_read>
> 48 8d 83 78 01 00 00 lea 0x178(%rbx),%rax
> 83 8b 70 01 00 00 01 orl $0x1,0x170(%rbx)
> 83 8b 74 01 00 00 01 orl $0x1,0x174(%rbx)
> 49 39 c4 cmp %rax,%r12
> 0f 84 88 fa ff ff je 7304 <x86_emulate_insn+0x204>
>
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Avi Kivity <avi.kivity@gmail.com>
> ---
> arch/x86/kvm/emulate.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 2bc1e81..7e6e74a 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -4544,9 +4544,9 @@ special_insn:
> case 0x8d: /* lea r16/r32, m */
> ctxt->dst.val = ctxt->src.addr.mem.ea;
> break;
> - case 0x90 ... 0x97: /* nop / xchg reg, rax */
> - if (ctxt->dst.addr.reg == reg_rmw(ctxt, VCPU_REGS_RAX))
> - break;
> + case 0x90: /* nop */
> + break;
This does not work on 64bit and REX prefix.
> + case 0x91 ... 0x97: /* xchg reg, rax */
> rc = em_xchg(ctxt);
> break;
> case 0x98: /* cbw/cwde/cdqe */
> --
> 1.8.1.4
--
Gleb.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/kvm/emulate.c: simplify NOP (opcode 0x90) check
2013-07-04 9:13 ` Gleb Natapov
@ 2013-07-04 11:06 ` Denys Vlasenko
2013-07-04 11:07 ` Gleb Natapov
2013-07-04 11:08 ` Paolo Bonzini
0 siblings, 2 replies; 5+ messages in thread
From: Denys Vlasenko @ 2013-07-04 11:06 UTC (permalink / raw)
To: Gleb Natapov; +Cc: linux-kernel, Paolo Bonzini, Avi Kivity
On 07/04/2013 11:13 AM, Gleb Natapov wrote:
> On Thu, Jul 04, 2013 at 10:58:29AM +0200, Denys Vlasenko wrote:
>> The check sits in switch() statement which itself can check
>> for opcode 0x90 far more efficiently.
>>
>> On assembler level, this change simply eliminates the following
>> bit of code:
>>
>> 4c 8b a3 d8 00 00 00 mov 0xd8(%rbx),%r12
>> 31 f6 xor %esi,%esi
>> 48 89 df mov %rbx,%rdi
>> e8 32 8c ff ff callq 490 <reg_read>
>> 48 8d 83 78 01 00 00 lea 0x178(%rbx),%rax
>> 83 8b 70 01 00 00 01 orl $0x1,0x170(%rbx)
>> 83 8b 74 01 00 00 01 orl $0x1,0x174(%rbx)
>> 49 39 c4 cmp %rax,%r12
>> 0f 84 88 fa ff ff je 7304 <x86_emulate_insn+0x204>
>>
>> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Avi Kivity <avi.kivity@gmail.com>
>> ---
>> arch/x86/kvm/emulate.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index 2bc1e81..7e6e74a 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -4544,9 +4544,9 @@ special_insn:
>> case 0x8d: /* lea r16/r32, m */
>> ctxt->dst.val = ctxt->src.addr.mem.ea;
>> break;
>> - case 0x90 ... 0x97: /* nop / xchg reg, rax */
>> - if (ctxt->dst.addr.reg == reg_rmw(ctxt, VCPU_REGS_RAX))
>> - break;
>> + case 0x90: /* nop */
>> + break;
> This does not work on 64bit and REX prefix.
Can you elaborate?
0x90 is special-cased in CPU to be a NOP regardless of bit width.
IOW, xchg %eax,%eax ordinarily would clear upper 32 bits of %rax,
but 0x90 doesn't do that.
Do you mean that with REX.R==1, 0x90 will refer to R8?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/kvm/emulate.c: simplify NOP (opcode 0x90) check
2013-07-04 11:06 ` Denys Vlasenko
@ 2013-07-04 11:07 ` Gleb Natapov
2013-07-04 11:08 ` Paolo Bonzini
1 sibling, 0 replies; 5+ messages in thread
From: Gleb Natapov @ 2013-07-04 11:07 UTC (permalink / raw)
To: Denys Vlasenko; +Cc: linux-kernel, Paolo Bonzini, Avi Kivity
On Thu, Jul 04, 2013 at 01:06:16PM +0200, Denys Vlasenko wrote:
> On 07/04/2013 11:13 AM, Gleb Natapov wrote:
> > On Thu, Jul 04, 2013 at 10:58:29AM +0200, Denys Vlasenko wrote:
> >> The check sits in switch() statement which itself can check
> >> for opcode 0x90 far more efficiently.
> >>
> >> On assembler level, this change simply eliminates the following
> >> bit of code:
> >>
> >> 4c 8b a3 d8 00 00 00 mov 0xd8(%rbx),%r12
> >> 31 f6 xor %esi,%esi
> >> 48 89 df mov %rbx,%rdi
> >> e8 32 8c ff ff callq 490 <reg_read>
> >> 48 8d 83 78 01 00 00 lea 0x178(%rbx),%rax
> >> 83 8b 70 01 00 00 01 orl $0x1,0x170(%rbx)
> >> 83 8b 74 01 00 00 01 orl $0x1,0x174(%rbx)
> >> 49 39 c4 cmp %rax,%r12
> >> 0f 84 88 fa ff ff je 7304 <x86_emulate_insn+0x204>
> >>
> >> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> >> CC: Paolo Bonzini <pbonzini@redhat.com>
> >> CC: Avi Kivity <avi.kivity@gmail.com>
> >> ---
> >> arch/x86/kvm/emulate.c | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> >> index 2bc1e81..7e6e74a 100644
> >> --- a/arch/x86/kvm/emulate.c
> >> +++ b/arch/x86/kvm/emulate.c
> >> @@ -4544,9 +4544,9 @@ special_insn:
> >> case 0x8d: /* lea r16/r32, m */
> >> ctxt->dst.val = ctxt->src.addr.mem.ea;
> >> break;
> >> - case 0x90 ... 0x97: /* nop / xchg reg, rax */
> >> - if (ctxt->dst.addr.reg == reg_rmw(ctxt, VCPU_REGS_RAX))
> >> - break;
> >> + case 0x90: /* nop */
> >> + break;
> > This does not work on 64bit and REX prefix.
>
> Can you elaborate?
>
> 0x90 is special-cased in CPU to be a NOP regardless of bit width.
> IOW, xchg %eax,%eax ordinarily would clear upper 32 bits of %rax,
> but 0x90 doesn't do that.
>
> Do you mean that with REX.R==1, 0x90 will refer to R8?
Yes.
--
Gleb.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/kvm/emulate.c: simplify NOP (opcode 0x90) check
2013-07-04 11:06 ` Denys Vlasenko
2013-07-04 11:07 ` Gleb Natapov
@ 2013-07-04 11:08 ` Paolo Bonzini
1 sibling, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2013-07-04 11:08 UTC (permalink / raw)
To: Denys Vlasenko; +Cc: Gleb Natapov, linux-kernel, Avi Kivity
Il 04/07/2013 13:06, Denys Vlasenko ha scritto:
>>> >> - case 0x90 ... 0x97: /* nop / xchg reg, rax */
>>> >> - if (ctxt->dst.addr.reg == reg_rmw(ctxt, VCPU_REGS_RAX))
>>> >> - break;
>>> >> + case 0x90: /* nop */
>>> >> + break;
>> > This does not work on 64bit and REX prefix.
> Can you elaborate?
>
> 0x90 is special-cased in CPU to be a NOP regardless of bit width.
> IOW, xchg %eax,%eax ordinarily would clear upper 32 bits of %rax,
> but 0x90 doesn't do that.
>
> Do you mean that with REX.R==1, 0x90 will refer to R8?
Yes.
$ echo 'xchg %rax,%r8' | as
$ objdump -d a.out
a.out: file format elf64-x86-64
Disassembly of section .text:
0000000000000000 <.text>:
0: 49 90 xchg %rax,%r8
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-07-04 11:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-04 8:58 [PATCH] x86/kvm/emulate.c: simplify NOP (opcode 0x90) check Denys Vlasenko
2013-07-04 9:13 ` Gleb Natapov
2013-07-04 11:06 ` Denys Vlasenko
2013-07-04 11:07 ` Gleb Natapov
2013-07-04 11:08 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox