On 16/01/2018 18:27, David Hildenbrand wrote: > On 16.01.2018 17:52, Paolo Bonzini wrote: >> Group together the calls to alloc_vmcs and loaded_vmcs_init. Soon we'll also >> allocate an MSR bitmap there. >> >> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> >> --- >> arch/x86/kvm/vmx.c | 36 ++++++++++++++++++++++-------------- >> 1 file changed, 22 insertions(+), 14 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 00d76292a5d2..d81fd4a87f2b 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -3799,11 +3799,6 @@ static struct vmcs *alloc_vmcs_cpu(int cpu) >> return vmcs; >> } >> >> -static struct vmcs *alloc_vmcs(void) >> -{ >> - return alloc_vmcs_cpu(raw_smp_processor_id()); >> -} >> - >> static void free_vmcs(struct vmcs *vmcs) >> { >> free_pages((unsigned long)vmcs, vmcs_config.order); >> @@ -3835,6 +3830,22 @@ static void vmx_nested_free_vmcs02(struct vcpu_vmx *vmx) >> free_loaded_vmcs(loaded_vmcs); >> } >> >> +static struct vmcs *alloc_vmcs(void) >> +{ >> + return alloc_vmcs_cpu(raw_smp_processor_id()); >> +} >> + > > don't understand why that was moved :) Because later alloc_loaded_vmcs needs to call free_loaded_vmcs so I wanted to put all free before alloc. :) >> +static int alloc_loaded_vmcs(struct loaded_vmcs *loaded_vmcs) > > maybe s/alloc_/init_/ would be a better name? > >> +{ >> + loaded_vmcs->vmcs = alloc_vmcs(); >> + if (!loaded_vmcs->vmcs) >> + return -ENOMEM; >> + >> + loaded_vmcs->shadow_vmcs = NULL; >> + loaded_vmcs_init(loaded_vmcs); >> + return 0; >> +} >> + >> static void free_kvm_area(void) >> { >> int cpu; >> @@ -7168,12 +7179,11 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu) >> { >> struct vcpu_vmx *vmx = to_vmx(vcpu); >> struct vmcs *shadow_vmcs; >> + int r; >> >> - vmx->nested.vmcs02.vmcs = alloc_vmcs(); >> - vmx->nested.vmcs02.shadow_vmcs = NULL; >> - if (!vmx->nested.vmcs02.vmcs) >> + r = alloc_loaded_vmcs(&vmx->nested.vmcs02); >> + if (r < 0) >> goto out_vmcs02; > > local variable r is not needed > > if (alloc_ ...) Yes, but it's less readable. Paolo