Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC PATCH v2 60/69] KVM: VMX: Add macro framework to read/write
 VMCS for VMs and TDs
To:     isaku.yamahata@intel.com, Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, erdemaktas@google.com,
        Connor Kuehl <ckuehl@redhat.com>,
        Sean Christopherson <seanjc@google.com>, x86@kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc:     isaku.yamahata@gmail.com,
        Sean Christopherson <sean.j.christopherson@intel.com>
References: <cover.1625186503.git.isaku.yamahata@intel.com>
 <5735bf9268130a70b49bc32ff4b68ffc53ee788c.1625186503.git.isaku.yamahata@intel.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <71ee8575-bd72-f51e-38c5-4e8411b8aedd@redhat.com>
Date:   Tue, 6 Jul 2021 16:46:17 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <5735bf9268130a70b49bc32ff4b68ffc53ee788c.1625186503.git.isaku.yamahata@intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 03/07/21 00:05, isaku.yamahata@intel.com wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> Add a macro framework to hide VMX vs. TDX details of VMREAD and VMWRITE
> so the VMX and TDX can shared common flows, e.g. accessing DTs.
> 
> Note, the TDX paths are dead code at this time.  There is no great way
> to deal with the chicken-and-egg scenario of having things in place for
> TDX without first having TDX.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>   arch/x86/kvm/vmx/common.h | 41 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/common.h b/arch/x86/kvm/vmx/common.h
> index 9e5865b05d47..aa6a569b87d1 100644
> --- a/arch/x86/kvm/vmx/common.h
> +++ b/arch/x86/kvm/vmx/common.h
> @@ -11,6 +11,47 @@
>   #include "vmcs.h"
>   #include "vmx.h"
>   #include "x86.h"
> +#include "tdx.h"
> +
> +#ifdef CONFIG_KVM_INTEL_TDX

Is this #ifdef needed at all if tdx.h properly stubs is_td_vcpu (to 
return false) and possibly declares a dummy version of 
td_vmcs_read/td_vmcs_write?

Paolo

> +#define VT_BUILD_VMCS_HELPERS(type, bits, tdbits)			   \
> +static __always_inline type vmread##bits(struct kvm_vcpu *vcpu,		   \
> +					 unsigned long field)		   \
> +{									   \
> +	if (unlikely(is_td_vcpu(vcpu))) {				   \
> +		if (KVM_BUG_ON(!is_debug_td(vcpu), vcpu->kvm))		   \
> +			return 0;					   \
> +		return td_vmcs_read##tdbits(to_tdx(vcpu), field);	   \
> +	}								   \
> +	return vmcs_read##bits(field);					   \
> +}									   \
> +static __always_inline void vmwrite##bits(struct kvm_vcpu *vcpu,	   \
> +					  unsigned long field, type value) \
> +{									   \
> +	if (unlikely(is_td_vcpu(vcpu))) {				   \
> +		if (KVM_BUG_ON(!is_debug_td(vcpu), vcpu->kvm))		   \
> +			return;						   \
> +		return td_vmcs_write##tdbits(to_tdx(vcpu), field, value);  \
> +	}								   \
> +	vmcs_write##bits(field, value);					   \
> +}
> +#else
> +#define VT_BUILD_VMCS_HELPERS(type, bits, tdbits)			   \
> +static __always_inline type vmread##bits(struct kvm_vcpu *vcpu,		   \
> +					 unsigned long field)		   \
> +{									   \
> +	return vmcs_read##bits(field);					   \
> +}									   \
> +static __always_inline void vmwrite##bits(struct kvm_vcpu *vcpu,	   \
> +					  unsigned long field, type value) \
> +{									   \
> +	vmcs_write##bits(field, value);					   \
> +}
> +#endif /* CONFIG_KVM_INTEL_TDX */
> +VT_BUILD_VMCS_HELPERS(u16, 16, 16);
> +VT_BUILD_VMCS_HELPERS(u32, 32, 32);
> +VT_BUILD_VMCS_HELPERS(u64, 64, 64);
> +VT_BUILD_VMCS_HELPERS(unsigned long, l, 64);
>   
>   extern unsigned long vmx_host_idt_base;
>   void vmx_do_interrupt_nmi_irqoff(unsigned long entry);
>