* [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