Re: [PATCH] nVMX: Get rid of prepare_vmcs02_early_full and inline its content in the caller

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Fri, Jun 07, 2019 at 02:05:44PM -0400, Krish Sadhukhan wrote:
>  ..as there is no need for a separate function
> 
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index f1a69117ac0f..4643eb3a97f7 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -1963,28 +1963,24 @@ static void prepare_vmcs02_constant_state(struct vcpu_vmx *vmx)
>  	vmx_set_constant_host_state(vmx);
>  }
>  
> -static void prepare_vmcs02_early_full(struct vcpu_vmx *vmx,
> -				      struct vmcs12 *vmcs12)
> -{
> -	prepare_vmcs02_constant_state(vmx);
> -
> -	vmcs_write64(VMCS_LINK_POINTER, -1ull);
> -
> -	if (enable_vpid) {
> -		if (nested_cpu_has_vpid(vmcs12) && vmx->nested.vpid02)
> -			vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->nested.vpid02);
> -		else
> -			vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
> -	}
> -}
> -
>  static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
>  {
>  	u32 exec_control, vmcs12_exec_ctrl;
>  	u64 guest_efer = nested_vmx_calc_efer(vmx, vmcs12);
>  
> -	if (vmx->nested.dirty_vmcs12 || vmx->nested.hv_evmcs)
> -		prepare_vmcs02_early_full(vmx, vmcs12);
> +	if (vmx->nested.dirty_vmcs12 || vmx->nested.hv_evmcs) {
> +		prepare_vmcs02_constant_state(vmx);
> +
> +		vmcs_write64(VMCS_LINK_POINTER, -1ull);
> +
> +		if (enable_vpid) {
> +			if (nested_cpu_has_vpid(vmcs12) && vmx->nested.vpid02)
> +				vmcs_write16(VIRTUAL_PROCESSOR_ID,
> +					     vmx->nested.vpid02);
> +			else
> +				vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
> +		}
> +	}

My vote is to keep the separate helper.  I agree that it's a bit
superfluous, but I really like having symmetry across the "early" and
"late" prepartion flows, i.e. there's value in having both
prepare_vmcs02_full() and prepare_vmcs02_early_full().

>  
>  	/*
>  	 * PIN CONTROLS
> -- 
> 2.20.1
>