From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [patch] VMX Unrestricted mode support
Date: Sun, 31 May 2009 11:39:05 +0300
Message-ID: <4A224229.6090108@redhat.com>
References: <1243552292.25456.23.camel@mukti.sc.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org
To: Nitin A Kamble <nitin.a.kamble@intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:38450 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1757419AbZEaIjF (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 31 May 2009 04:39:05 -0400
In-Reply-To: <1243552292.25456.23.camel@mukti.sc.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Nitin A Kamble wrote:
> Avi,
>
> A new VMX feature "Unrestricted Guest" feature is added in the VMX
> specification. You can look at the latest Intel processor manual for
> details of the feature here:
>
>  http://www.intel.com/products/processor/manuals
>
>     It allows kvm guests to run real mode and unpaged mode
> code natively in the VMX mode when EPT is turned on. With the
> unrestricted guest there is no need to emulate the guest real mode code
> in the vm86 container or in the emulator. Also the guest big real mode
> code works like native. 
>
>   The attached patch enhances KVM to use the unrestricted guest feature
> if available on the processor. It also adds a new kernel/module
> parameter to disable the unrestricted guest feature at the boot time. 
>   

Thanks, this is much needed.  Review comments below.

>  #define KVM_GUEST_CR0_MASK				   \
>  	(X86_CR0_PG | X86_CR0_PE | X86_CR0_WP | X86_CR0_NE \
>  	 | X86_CR0_NW | X86_CR0_CD)
> +#define KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST				\
> +	(X86_CR0_WP | X86_CR0_NE | X86_CR0_TS | X86_CR0_MP)
> +#define KVM_VM_CR0_ALWAYS_ON_RESTRICTED_GUEST				\
> +	(KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST | X86_CR0_PG | X86_CR0_PE)
>  #define KVM_VM_CR0_ALWAYS_ON						\
> -	(X86_CR0_PG | X86_CR0_PE | X86_CR0_WP | X86_CR0_NE | X86_CR0_TS \
> -	 | X86_CR0_MP)
> +	(enable_unrestricted_guest ? KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST \
> +	: KVM_VM_CR0_ALWAYS_ON_RESTRICTED_GUEST)
>   

Please avoid hiding computations in macros.  Just change the call sites.

>  static inline int vm_need_virtualize_apic_accesses(struct kvm *kvm)
>  {
>  	return flexpriority_enabled &&
> @@ -731,7 +741,7 @@ static unsigned long vmx_get_rflags(struct kvm_vcpu
> *vcpu)
>  
>  static void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags)
>  {
> -	if (vcpu->arch.rmode.active)
> +	if (vcpu->arch.rmode.active && !enable_unrestricted_guest)
>  		rflags |= X86_EFLAGS_IOPL | X86_EFLAGS_VM;
>  	vmcs_writel(GUEST_RFLAGS, rflags);
>  }
>   

Instead of changing all the checks like this, you can make rmode.active 
be false when unrestricted guest is enabled.  We can interpret 
rmode.active as "emulating real mode via vm86", not as "guest is in real 
mode".

You can just have enter_rmode() exit immediately when called.

>  
> +static inline u32 get_segment_ar(int seg)
> +{
> +	if (!enable_unrestricted_guest)
> +		return 0xf3;
> +
> +	switch (seg) {
> +	case VCPU_SREG_CS:
> +		return 0x9b;
> +	case VCPU_SREG_TR:
> +		return 0x8b;
> +	case VCPU_SREG_LDTR:
> +		return 0x82;
> +	default:
> +		return 0x93;
> +	}
> +}
> +
>  static void vmx_set_segment(struct kvm_vcpu *vcpu,
>  			    struct kvm_segment *var, int seg)
>  {
> @@ -1755,7 +1799,7 @@ static void vmx_set_segment(struct kvm_vcpu *vcpu,
>  		 */
>  		if (var->base == 0xffff0000 && var->selector == 0xf000)
>  			vmcs_writel(sf->base, 0xf0000);
> -		ar = 0xf3;
> +		ar = get_segment_ar(seg);
>   

I think this can go away if rmode.active == 0.

-- 
error compiling committee.c: too many arguments to function