Re: [PATCH v4 2/2] kvm-unit-tests : The first version of VMX nested test case

[Paolo Bonzini <pbonzini@redhat.com>]

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