Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966194AbbD2H1X (ORCPT ); Wed, 29 Apr 2015 03:27:23 -0400 Received: from mail-wg0-f51.google.com ([74.125.82.51]:34920 "EHLO mail-wg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966081AbbD2H1V (ORCPT ); Wed, 29 Apr 2015 03:27:21 -0400 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2098\)) Subject: Re: [PATCH] KVM: nVMX: Don't return error on nested bitmap memory allocation failure From: Nadav Amit In-Reply-To: Date: Wed, 29 Apr 2015 10:27:17 +0300 Cc: kvm list , Paolo Bonzini , Jan Kiszka , Wincy Van , linux-kernel@vger.kernel.org Content-Transfer-Encoding: 7bit Message-Id: <9C58F44B-0406-4273-BB36-50D39B7C33F9@gmail.com> References: To: Bandan Das X-Mailer: Apple Mail (2.2098) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2672 Lines: 90 Bandan Das wrote: > > If get_free_page() fails for nested bitmap area, it's evident that > we are gonna get screwed anyway but returning failure because we failed > allocating memory for a nested structure seems like an unnecessary big > hammer. Also, save the call for later; after we are done with other > non-nested allocations. > > Signed-off-by: Bandan Das > --- > arch/x86/kvm/vmx.c | 29 +++++++++++++++-------------- > 1 file changed, 15 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index f7b6168..200bc5c 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -6039,20 +6039,22 @@ static __init int hardware_setup(void) > if (!vmx_msr_bitmap_longmode_x2apic) > goto out4; > > - if (nested) { > - vmx_msr_bitmap_nested = > - (unsigned long *)__get_free_page(GFP_KERNEL); > - if (!vmx_msr_bitmap_nested) > - goto out5; > - } > - > vmx_vmread_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL); > if (!vmx_vmread_bitmap) > - goto out6; > + goto out5; > > vmx_vmwrite_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL); > if (!vmx_vmwrite_bitmap) > - goto out7; > + goto out6; > + > + if (nested) { > + vmx_msr_bitmap_nested = > + (unsigned long *)__get_free_page(GFP_KERNEL); > + if (!vmx_msr_bitmap_nested) { > + printk(KERN_WARNING > + "vmx: Failed getting memory for nested bitmap\n"); > + nested = 0; > + } > > memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE); > memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE); > @@ -6073,7 +6075,7 @@ static __init int hardware_setup(void) > > if (setup_vmcs_config(&vmcs_config) < 0) { > r = -EIO; > - goto out8; > + goto out7; > } > > if (boot_cpu_has(X86_FEATURE_NX)) > @@ -6190,13 +6192,12 @@ static __init int hardware_setup(void) > > return alloc_kvm_area(); > > -out8: > - free_page((unsigned long)vmx_vmwrite_bitmap); > out7: > - free_page((unsigned long)vmx_vmread_bitmap); > -out6: > if (nested) > free_page((unsigned long)vmx_msr_bitmap_nested); > + free_page((unsigned long)vmx_vmwrite_bitmap); > +out6: > + free_page((unsigned long)vmx_vmread_bitmap); > out5: > free_page((unsigned long)vmx_msr_bitmap_longmode_x2apic); > out4: > free_page appears to check whether the address is zero before it actually frees the page. Perhaps it is better to leverage this behaviour to remove all the outX and simplify the code. Nadav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/