Re: [patch] VMX Unrestricted mode support

[Avi Kivity <avi@redhat.com>]

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