Re: [PATCH] allow enabling/disabling NPT by reloading only the architecture module

["Yang, Sheng" <sheng.yang@intel.com>]

On Tuesday 15 July 2008 18:55:37 Avi Kivity wrote:
> Yang, Sheng wrote:
> > On Tuesday 15 July 2008 02:36:36 Joerg Roedel wrote:
> >> If NPT is enabled after loading both KVM modules on AMD and it
> >> should be disabled, both KVM modules must be reloaded. If only
> >> the architecture module is reloaded the behavior is undefined.
> >> With this patch it is possible to disable NPT only by reloading
> >> the kvm_amd module.
> >>
> >> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> >> ---
> >
> > From 3dd7fa4abb1cfc702b3fbd7038d585b541f981a4 Mon Sep 17 00:00:00
> > 2001 From: Sheng Yang <sheng.yang@intel.com>
> > Date: Tue, 15 Jul 2008 14:18:29 +0800
> > Subject: [PATCH] KVM: VMX: Fix undefined beaviour of EPT after
> > reload kvm-intel.ko
> >
> > Based on Joerg Roedel's fix for NPT.
> >
> > Thanks Joerg!
> >
> > Signed-off-by: Sheng Yang <sheng.yang@intel.com>
> > ---
> >  arch/x86/kvm/vmx.c |   15 +++++++++------
> >  1 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 5f807e3..374e1ca 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -3108,14 +3108,17 @@ static struct kvm_vcpu
> > *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
> >  		return ERR_PTR(-ENOMEM);
> >
> >  	allocate_vpid(vmx);
> > -	if (id == 0 && vm_need_ept()) {
> > -		kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK |
> > -			VMX_EPT_WRITABLE_MASK |
> > -			VMX_EPT_DEFAULT_MT << VMX_EPT_MT_EPTE_SHIFT);
> > -		kvm_mmu_set_mask_ptes(0ull, VMX_EPT_FAKE_ACCESSED_MASK,
> > +	if (id == 0) {
> > +		if (vm_need_ept()) {
> > +			kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK |
> > +				VMX_EPT_WRITABLE_MASK |
> > +				VMX_EPT_DEFAULT_MT << VMX_EPT_MT_EPTE_SHIFT);
> > +			kvm_mmu_set_mask_ptes(0ull, VMX_EPT_FAKE_ACCESSED_MASK,
> >  				VMX_EPT_FAKE_DIRTY_MASK, 0ull,
> >  				VMX_EPT_EXECUTABLE_MASK);
> > -		kvm_enable_tdp();
> > +			kvm_enable_tdp();
> > +		} else
> > +			kvm_disable_tdp();
> >  	}
>
> hmm, what is this code doing in vmx_create_vcpu()?  surely
> vmx_init() is a better place?

Oh, may be a historic reason :)

Move it to vmx_init() now.

-- 
regards
Yang, Sheng