* [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition @ 2014-04-27 10:30 Toralf Förster 2014-04-27 10:30 ` [PATCH 2/2] arch/x86/kvm/x86.c: variable longmode ist just used in one place, remove it therefore Toralf Förster 2014-04-27 10:45 ` [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition Paolo Bonzini 0 siblings, 2 replies; 6+ messages in thread From: Toralf Förster @ 2014-04-27 10:30 UTC (permalink / raw) To: pbonzini; +Cc: kvm, Toralf Förster Signed-off-by: Toralf Förster <toralf.foerster@gmx.de> --- arch/x86/kvm/x86.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 8b8fc0b..93cf454 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5680,20 +5680,17 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu) kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l); longmode = is_long_mode(vcpu) && cs_l == 1; - if (!longmode) { - param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | - (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); - ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | - (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); - outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | - (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); - } #ifdef CONFIG_X86_64 - else { - param = kvm_register_read(vcpu, VCPU_REGS_RCX); - ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); - outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); - } + param = kvm_register_read(vcpu, VCPU_REGS_RCX); + ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); + outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); +#else + param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | + (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); + ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | + (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); + outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | + (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); #endif code = param & 0xffff; -- 1.9.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] arch/x86/kvm/x86.c: variable longmode ist just used in one place, remove it therefore 2014-04-27 10:30 [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition Toralf Förster @ 2014-04-27 10:30 ` Toralf Förster 2014-04-27 10:45 ` [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition Paolo Bonzini 1 sibling, 0 replies; 6+ messages in thread From: Toralf Förster @ 2014-04-27 10:30 UTC (permalink / raw) To: pbonzini; +Cc: kvm, Toralf Förster Signed-off-by: Toralf Förster <toralf.foerster@gmx.de> --- arch/x86/kvm/x86.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 93cf454..05166a0 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5665,7 +5665,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu) { u64 param, ingpa, outgpa, ret; uint16_t code, rep_idx, rep_cnt, res = HV_STATUS_SUCCESS, rep_done = 0; - bool fast, longmode; + bool fast; int cs_db, cs_l; /* @@ -5678,7 +5678,6 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu) } kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l); - longmode = is_long_mode(vcpu) && cs_l == 1; #ifdef CONFIG_X86_64 param = kvm_register_read(vcpu, VCPU_REGS_RCX); @@ -5710,7 +5709,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu) } ret = res | (((u64)rep_done & 0xfff) << 32); - if (longmode) { + if (is_long_mode(vcpu) && cs_l == 1) { kvm_register_write(vcpu, VCPU_REGS_RAX, ret); } else { kvm_register_write(vcpu, VCPU_REGS_RDX, ret >> 32); -- 1.9.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition 2014-04-27 10:30 [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition Toralf Förster 2014-04-27 10:30 ` [PATCH 2/2] arch/x86/kvm/x86.c: variable longmode ist just used in one place, remove it therefore Toralf Förster @ 2014-04-27 10:45 ` Paolo Bonzini 2014-04-27 11:40 ` Toralf Förster 1 sibling, 1 reply; 6+ messages in thread From: Paolo Bonzini @ 2014-04-27 10:45 UTC (permalink / raw) To: Toralf Förster; +Cc: kvm Il 27/04/2014 12:30, Toralf Förster ha scritto: > Signed-off-by: Toralf Förster <toralf.foerster@gmx.de> > --- > arch/x86/kvm/x86.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 8b8fc0b..93cf454 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5680,20 +5680,17 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu) > kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l); > longmode = is_long_mode(vcpu) && cs_l == 1; > > - if (!longmode) { > - param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | > - (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); > - ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | > - (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); > - outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | > - (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); > - } > #ifdef CONFIG_X86_64 > - else { > - param = kvm_register_read(vcpu, VCPU_REGS_RCX); > - ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); > - outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); > - } > + param = kvm_register_read(vcpu, VCPU_REGS_RCX); > + ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); > + outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); > +#else > + param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | > + (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); > + ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | > + (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); > + outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | > + (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); > #endif > > code = param & 0xffff; > No, wait, it's only superfluous in the sense that you don't need longmode for !CONFIG_X86_64. It's definitely needed for CONFIG_X86_64, because the guest can run in 32-bit mode. Paolo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition 2014-04-27 10:45 ` [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition Paolo Bonzini @ 2014-04-27 11:40 ` Toralf Förster 2014-04-27 14:41 ` Paolo Bonzini 0 siblings, 1 reply; 6+ messages in thread From: Toralf Förster @ 2014-04-27 11:40 UTC (permalink / raw) To: Paolo Bonzini; +Cc: kvm On 04/27/2014 12:45 PM, Paolo Bonzini wrote: > Il 27/04/2014 12:30, Toralf Förster ha scritto: >> Signed-off-by: Toralf Förster <toralf.foerster@gmx.de> >> --- >> arch/x86/kvm/x86.c | 23 ++++++++++------------- >> 1 file changed, 10 insertions(+), 13 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 8b8fc0b..93cf454 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5680,20 +5680,17 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu) >> kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l); >> longmode = is_long_mode(vcpu) && cs_l == 1; >> >> - if (!longmode) { >> - param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | >> - (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); >> - ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | >> - (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); >> - outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | >> - (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); >> - } >> #ifdef CONFIG_X86_64 >> - else { >> - param = kvm_register_read(vcpu, VCPU_REGS_RCX); >> - ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); >> - outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); >> - } >> + param = kvm_register_read(vcpu, VCPU_REGS_RCX); >> + ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); >> + outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); >> +#else >> + param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | >> + (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); >> + ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | >> + (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); >> + outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | >> + (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); >> #endif >> >> code = param & 0xffff; >> > > No, wait, it's only superfluous in the sense that you don't need > longmode for !CONFIG_X86_64. It's definitely needed for CONFIG_X86_64, > because the guest can run in 32-bit mode. > > Paolo > Ah, so the following would work, but looks too ugly, right ? : #ifdef CONFIG_X86_64 if (!longmode) { #endif param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); #ifdef CONFIG_X86_64 } else { param = kvm_register_read(vcpu, VCPU_REGS_RCX); ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); } #endif -- Toralf ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition 2014-04-27 11:40 ` Toralf Förster @ 2014-04-27 14:41 ` Paolo Bonzini 2014-04-27 15:29 ` Toralf Förster 0 siblings, 1 reply; 6+ messages in thread From: Paolo Bonzini @ 2014-04-27 14:41 UTC (permalink / raw) To: Toralf Förster; +Cc: kvm Il 27/04/2014 13:40, Toralf Förster ha scritto: > On 04/27/2014 12:45 PM, Paolo Bonzini wrote: >> Il 27/04/2014 12:30, Toralf Förster ha scritto: >>> Signed-off-by: Toralf Förster <toralf.foerster@gmx.de> >>> --- >>> arch/x86/kvm/x86.c | 23 ++++++++++------------- >>> 1 file changed, 10 insertions(+), 13 deletions(-) >>> >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index 8b8fc0b..93cf454 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -5680,20 +5680,17 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu) >>> kvm_x86_ops->get_cs_db_l_bits(vcpu, &cs_db, &cs_l); >>> longmode = is_long_mode(vcpu) && cs_l == 1; >>> >>> - if (!longmode) { >>> - param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | >>> - (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); >>> - ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | >>> - (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); >>> - outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | >>> - (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); >>> - } >>> #ifdef CONFIG_X86_64 >>> - else { >>> - param = kvm_register_read(vcpu, VCPU_REGS_RCX); >>> - ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); >>> - outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); >>> - } >>> + param = kvm_register_read(vcpu, VCPU_REGS_RCX); >>> + ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); >>> + outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); >>> +#else >>> + param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | >>> + (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); >>> + ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | >>> + (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); >>> + outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | >>> + (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); >>> #endif >>> >>> code = param & 0xffff; >>> >> >> No, wait, it's only superfluous in the sense that you don't need >> longmode for !CONFIG_X86_64. It's definitely needed for CONFIG_X86_64, >> because the guest can run in 32-bit mode. >> >> Paolo >> > > Ah, so the following would work, but looks too ugly, right ? : > > > #ifdef CONFIG_X86_64 > if (!longmode) { > #endif > param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | > (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); > ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | > (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); > outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | > (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); > #ifdef CONFIG_X86_64 > } > else { > param = kvm_register_read(vcpu, VCPU_REGS_RCX); > ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); > outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); > } > #endif > Yep. :) Note that GCC correctly reports no warning, because it looks through is_long_mode. In the end, #ifdef in the code can just be removed. Please compile-test it on 32-bit though (well, I will too before committing but...). Paolo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition 2014-04-27 14:41 ` Paolo Bonzini @ 2014-04-27 15:29 ` Toralf Förster 0 siblings, 0 replies; 6+ messages in thread From: Toralf Förster @ 2014-04-27 15:29 UTC (permalink / raw) To: Paolo Bonzini; +Cc: kvm On 04/27/2014 04:41 PM, Paolo Bonzini wrote: > Il 27/04/2014 13:40, Toralf Förster ha scritto: >> Ah, so the following would work, but looks too ugly, right ? : >> >> >> #ifdef CONFIG_X86_64 >> if (!longmode) { >> #endif >> param = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | >> (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); >> ingpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | >> (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); >> outgpa = ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | >> (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); >> #ifdef CONFIG_X86_64 >> } >> else { >> param = kvm_register_read(vcpu, VCPU_REGS_RCX); >> ingpa = kvm_register_read(vcpu, VCPU_REGS_RDX); >> outgpa = kvm_register_read(vcpu, VCPU_REGS_R8); >> } >> #endif >> > > Yep. :) Note that GCC correctly reports no warning, because it looks > through is_long_mode. In the end, #ifdef in the code can just be > removed. Please compile-test it on 32-bit though (well, I will too > before committing but...). > > Paolo > Will send out a patch, which was compile tested and runtime tested using a tails KVM image udner a stable 32 bit Gentoo Linux -- Toralf ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-27 15:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-27 10:30 [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition Toralf Förster 2014-04-27 10:30 ` [PATCH 2/2] arch/x86/kvm/x86.c: variable longmode ist just used in one place, remove it therefore Toralf Förster 2014-04-27 10:45 ` [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition Paolo Bonzini 2014-04-27 11:40 ` Toralf Förster 2014-04-27 14:41 ` Paolo Bonzini 2014-04-27 15:29 ` Toralf Förster
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox