Re: [PATCH] KVM: nVMX: Don't return error on nested bitmap memory allocation failure

[Nadav Amit <nadav.amit@gmail.com>]

Bandan Das <bsd@redhat.com> 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 <bsd@redhat.com>
> ---
> 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