From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 1/2] arch/x86/kvm/x86.c: remove superflous check condition Date: Sun, 27 Apr 2014 16:41:33 +0200 Message-ID: <535D171D.7020809@redhat.com> References: <1398594613-4466-1-git-send-email-toralf.foerster@gmx.de> <535CDFBE.5050806@redhat.com> <535CECB6.5070802@gmx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org To: =?UTF-8?B?VG9yYWxmIEbDtnJzdGVy?= Return-path: Received: from mail-ee0-f51.google.com ([74.125.83.51]:61308 "EHLO mail-ee0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751482AbaD0Olj (ORCPT ); Sun, 27 Apr 2014 10:41:39 -0400 Received: by mail-ee0-f51.google.com with SMTP id c13so4119960eek.10 for ; Sun, 27 Apr 2014 07:41:38 -0700 (PDT) In-Reply-To: <535CECB6.5070802@gmx.de> Sender: kvm-owner@vger.kernel.org List-ID: Il 27/04/2014 13:40, Toralf F=C3=B6rster ha scritto: > On 04/27/2014 12:45 PM, Paolo Bonzini wrote: >> Il 27/04/2014 12:30, Toralf F=C3=B6rster ha scritto: >>> Signed-off-by: Toralf F=C3=B6rster >>> --- >>> 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 =3D is_long_mode(vcpu) && cs_l =3D=3D 1; >>> >>> - if (!longmode) { >>> - param =3D ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << = 32) | >>> - (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); >>> - ingpa =3D ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << = 32) | >>> - (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); >>> - outgpa =3D ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) <<= 32) | >>> - (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); >>> - } >>> #ifdef CONFIG_X86_64 >>> - else { >>> - param =3D kvm_register_read(vcpu, VCPU_REGS_RCX); >>> - ingpa =3D kvm_register_read(vcpu, VCPU_REGS_RDX); >>> - outgpa =3D kvm_register_read(vcpu, VCPU_REGS_R8); >>> - } >>> + param =3D kvm_register_read(vcpu, VCPU_REGS_RCX); >>> + ingpa =3D kvm_register_read(vcpu, VCPU_REGS_RDX); >>> + outgpa =3D kvm_register_read(vcpu, VCPU_REGS_R8); >>> +#else >>> + param =3D ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) = | >>> + (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); >>> + ingpa =3D ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) = | >>> + (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); >>> + outgpa =3D ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32)= | >>> + (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); >>> #endif >>> >>> code =3D 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 =3D ((u64)kvm_register_read(vcpu, VCPU_REGS_RDX) << 32) | > (kvm_register_read(vcpu, VCPU_REGS_RAX) & 0xffffffff); > ingpa =3D ((u64)kvm_register_read(vcpu, VCPU_REGS_RBX) << 32) | > (kvm_register_read(vcpu, VCPU_REGS_RCX) & 0xffffffff); > outgpa =3D ((u64)kvm_register_read(vcpu, VCPU_REGS_RDI) << 32) | > (kvm_register_read(vcpu, VCPU_REGS_RSI) & 0xffffffff); > #ifdef CONFIG_X86_64 > } > else { > param =3D kvm_register_read(vcpu, VCPU_REGS_RCX); > ingpa =3D kvm_register_read(vcpu, VCPU_REGS_RDX); > outgpa =3D kvm_register_read(vcpu, VCPU_REGS_R8); > } > #endif > Yep. :) Note that GCC correctly reports no warning, because it looks=20 through is_long_mode. In the end, #ifdef in the code can just be=20 removed. Please compile-test it on 32-bit though (well, I will too=20 before committing but...). Paolo