Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Date:   Wed, 29 Nov 2023 17:24:38 -0800
In-Reply-To: <20231025152406.1879274-12-vkuznets@redhat.com>
Mime-Version: 1.0
References: <20231025152406.1879274-1-vkuznets@redhat.com> <20231025152406.1879274-12-vkuznets@redhat.com>
Message-ID: <ZWfkVnsYytDeeaDL@google.com>
Subject: Re: [PATCH 11/14] KVM: nVMX: hyper-v: Introduce nested_vmx_evmptr12()
 and nested_vmx_is_evmptr12_valid() helpers
From:   Sean Christopherson <seanjc@google.com>
To:     Vitaly Kuznetsov <vkuznets@redhat.com>
Cc:     kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        Maxim Levitsky <mlevitsk@redhat.com>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Wed, Oct 25, 2023, Vitaly Kuznetsov wrote:
> 'vmx->nested.hv_evmcs_vmptr' accesses are all over the place so hiding
> 'hv_evmcs_vmptr' under 'ifdef CONFIG_KVM_HYPERV' would take a lot of
> ifdefs. Introduce 'nested_vmx_evmptr12()' accessor and
> 'nested_vmx_is_evmptr12_valid()' checker instead. Note, several explicit
> 
>   nested_vmx_evmptr12(vmx) != EVMPTR_INVALID
> 
> comparisons exist for a reson: 'nested_vmx_is_evmptr12_valid()' also checks
> against 'EVMPTR_MAP_PENDING' and in these places this is undesireable. It
> is possible to e.g. introduce 'nested_vmx_is_evmptr12_invalid()' and turn
> these sites into
> 
>   !nested_vmx_is_evmptr12_invalid(vmx)
> 
> eliminating the need for 'nested_vmx_evmptr12()' but this seems to create
> even more confusion.
> 
> No functional change intended.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  arch/x86/kvm/vmx/hyperv.h | 10 +++++++++
>  arch/x86/kvm/vmx/nested.c | 44 +++++++++++++++++++--------------------
>  arch/x86/kvm/vmx/nested.h |  2 +-
>  3 files changed, 33 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/hyperv.h b/arch/x86/kvm/vmx/hyperv.h
> index 933ef6cad5e6..ba1a95ea72b7 100644
> --- a/arch/x86/kvm/vmx/hyperv.h
> +++ b/arch/x86/kvm/vmx/hyperv.h
> @@ -4,6 +4,7 @@
>  
>  #include <linux/kvm_host.h>
>  #include "vmcs12.h"
> +#include "vmx.h"
>  
>  #define EVMPTR_INVALID (-1ULL)
>  #define EVMPTR_MAP_PENDING (-2ULL)
> @@ -20,7 +21,14 @@ enum nested_evmptrld_status {
>  	EVMPTRLD_ERROR,
>  };
>  
> +struct vcpu_vmx;

This forward declaration should be unnecessary as it's defined by vmx.h, which
is included above.