Re: [PATCH] KVM: x86: fixup kvm_set_cr4()

[Avi Kivity <avi@redhat.com>]

  On 09/04/2010 03:43 PM, Hillf Danton wrote:


Subject lines such as "fixup $x" are too general.  Try to make them more 
specific.

> X86_CR4_VMXE is checked earlier, since
> [1] virtualization is not allowed in guest,

Why does that matter?  Note it may change one day.

> [2] load_pdptrs() could be saved.

The common case is that the mov does not fault and we have to call 
load_pdptrs() anyway.

It's a little cleaner to check before doing anything, though.

>
> Signed-off-by: Hillf Danton <dhillf@gmail.com <mailto:dhillf@gmail.com>>
> ---
>
> --- o/linux-2.6.36-rc1/arch/x86/kvm/x86.c 2010-08-16 
> 08:41:38.000000000 +0800
> +++ m/linux-2.6.36-rc1/arch/x86/kvm/x86.c 2010-09-04 
> 20:25:04.000000000 +0800
> @@ -539,6 +539,9 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, u
> if (cr4 & CR4_RESERVED_BITS)
> return 1;
> + if (cr4 & X86_CR4_VMXE)
> + return 1;
> +
> if (!guest_cpuid_has_xsave(vcpu) && (cr4 & X86_CR4_OSXSAVE))
> return 1;
> @@ -550,9 +553,6 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, u
> && !load_pdptrs(vcpu, vcpu->arch.cr3))
> return 1;
> - if (cr4 & X86_CR4_VMXE)
> - return 1;
> -
> kvm_x86_ops->set_cr4(vcpu, cr4);
> if ((cr4 ^ old_cr4) & pdptr_bits)
>


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.