From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH v4 2/2] kvm-unit-tests : The first version of VMX nested test case Date: Wed, 17 Jul 2013 12:13:54 +0200 Message-ID: <51E66E62.3010205@redhat.com> References: <1374041153-32235-1-git-send-email-yzt356@gmail.com> <1374041153-32235-3-git-send-email-yzt356@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, jan.kiszka@web.de, gleb@redhat.com To: Arthur Chunqi Li Return-path: Received: from mail-qa0-f42.google.com ([209.85.216.42]:50522 "EHLO mail-qa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752805Ab3GQKOH (ORCPT ); Wed, 17 Jul 2013 06:14:07 -0400 Received: by mail-qa0-f42.google.com with SMTP id hu16so2881428qab.1 for ; Wed, 17 Jul 2013 03:14:06 -0700 (PDT) In-Reply-To: <1374041153-32235-3-git-send-email-yzt356@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: Il 17/07/2013 08:05, Arthur Chunqi Li ha scritto: > + guest_rip = vmcs_read(GUEST_RIP); > + reason = vmcs_read(EXI_REASON) & 0xff; > + > + switch (reason) { > + case VMX_VMCALL: > + print_vmexit_info(); > + vmcs_write(GUEST_RIP, guest_rip + 3); > + goto vmx_resume; > + case VMX_HLT: > + goto vmx_halt; > + default: > + break; > + } Could you reorganize this code, so that it reuses the switch statement you have in the "current != NULL && current->exit_handler != NULL" case? In other words, it looks like this code: > + if ((read_cr4() & CR4_PAE) && (read_cr0() & CR0_PG) > + && !(rdmsr(MSR_EFER) & EFER_LMA)) > + printf("ERROR : PDPTEs should be checked\n"); > + > + guest_rip = vmcs_read(GUEST_RIP); > + reason = vmcs_read(EXI_REASON) & 0xff; > + > + switch (reason) { > + case VMX_VMCALL: > + print_vmexit_info(); > + vmcs_write(GUEST_RIP, guest_rip + 3); > + goto vmx_resume; > + case VMX_HLT: > + goto vmx_halt; > + default: > + break; > + } > + printf("ERROR : Unhandled vmx exit.\n"); > + print_vmexit_info(); is only alternative to this: > + current->exits ++; > + current->guest_regs = regs; > + ret = current->exit_handler(); > + regs = current->guest_regs; Everything that comes after "regs = current->guest_regs;" should be outside the "if". Please tell me if I'm not clear. :) > +#define VMX_HALT 1 > +#define VMX_EXIT 2 > +#define VMX_RESUME 3 These are not VMX constants, so perhaps you can rename them to VMXTEST_{HANDLE,EXIT,RESUME}? Paolo