Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <25257849-8e1a-17ff-5008-bb2d1efecf80@redhat.com>
Date:   Tue, 5 Apr 2022 14:39:51 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [RFC PATCH v5 020/104] KVM: TDX: allocate per-package mutex
Content-Language: en-US
To:     isaku.yamahata@intel.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     isaku.yamahata@gmail.com, Jim Mattson <jmattson@google.com>,
        erdemaktas@google.com, Connor Kuehl <ckuehl@redhat.com>,
        Sean Christopherson <seanjc@google.com>
References: <cover.1646422845.git.isaku.yamahata@intel.com>
 <f7b44d1d5a61f788294c399b63b505b3ff4d301b.1646422845.git.isaku.yamahata@intel.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <f7b44d1d5a61f788294c399b63b505b3ff4d301b.1646422845.git.isaku.yamahata@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/4/22 20:48, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> Several TDX SEAMCALLs are per-package scope (concretely per memory
> controller) and they need to be serialized per-package.  Allocate mutex for
> it.
> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
>   arch/x86/kvm/vmx/main.c    |  8 +++++++-
>   arch/x86/kvm/vmx/tdx.c     | 18 ++++++++++++++++++
>   arch/x86/kvm/vmx/x86_ops.h |  2 ++
>   3 files changed, 27 insertions(+), 1 deletion(-)

Please define here the lock/unlock functions as well:

static inline int tdx_mng_key_lock(void)
{
	int cpu = get_cpu();
	cur_pkg = topology_physical_package_id(cpu);

	mutex_lock(&tdx_mng_key_config_lock[cur_pkg]);
	return cur_pkg;
}

static inline void tdx_mng_key_unlock(int cur_pkg)
{
	mutex_unlock(&tdx_mng_key_config_lock[cur_pkg]);
	put_cpu();
}

Thanks,

Paolo


> 
> diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
> index 8103d1c32cc9..6111c6485d8e 100644
> --- a/arch/x86/kvm/vmx/main.c
> +++ b/arch/x86/kvm/vmx/main.c
> @@ -25,6 +25,12 @@ static __init int vt_hardware_setup(void)
>   	return 0;
>   }
>   
> +static void vt_hardware_unsetup(void)
> +{
> +	tdx_hardware_unsetup();
> +	vmx_hardware_unsetup();
> +}
> +
>   static int vt_vm_init(struct kvm *kvm)
>   {
>   	int ret;
> @@ -42,7 +48,7 @@ static int vt_vm_init(struct kvm *kvm)
>   struct kvm_x86_ops vt_x86_ops __initdata = {
>   	.name = "kvm_intel",
>   
> -	.hardware_unsetup = vmx_hardware_unsetup,
> +	.hardware_unsetup = vt_hardware_unsetup,
>   
>   	.hardware_enable = vmx_hardware_enable,
>   	.hardware_disable = vmx_hardware_disable,
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index e8d293a3c11c..1c8222f54764 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -34,6 +34,8 @@ struct tdx_capabilities {
>   /* Capabilities of KVM + the TDX module. */
>   struct tdx_capabilities tdx_caps;
>   
> +static struct mutex *tdx_mng_key_config_lock;
> +
>   static u64 hkid_mask __ro_after_init;
>   static u8 hkid_start_pos __ro_after_init;
>   
> @@ -112,7 +114,9 @@ bool tdx_is_vm_type_supported(unsigned long type)
>   
>   static int __init __tdx_hardware_setup(struct kvm_x86_ops *x86_ops)
>   {
> +	int max_pkgs;
>   	u32 max_pa;
> +	int i;
>   
>   	if (!enable_ept) {
>   		pr_warn("Cannot enable TDX with EPT disabled\n");
> @@ -127,6 +131,14 @@ static int __init __tdx_hardware_setup(struct kvm_x86_ops *x86_ops)
>   	if (WARN_ON_ONCE(x86_ops->tlb_remote_flush))
>   		return -EIO;
>   
> +	max_pkgs = topology_max_packages();
> +	tdx_mng_key_config_lock = kcalloc(max_pkgs, sizeof(*tdx_mng_key_config_lock),
> +				   GFP_KERNEL);
> +	if (!tdx_mng_key_config_lock)
> +		return -ENOMEM;
> +	for (i = 0; i < max_pkgs; i++)
> +		mutex_init(&tdx_mng_key_config_lock[i]);
> +
>   	max_pa = cpuid_eax(0x80000008) & 0xff;
>   	hkid_start_pos = boot_cpu_data.x86_phys_bits;
>   	hkid_mask = GENMASK_ULL(max_pa - 1, hkid_start_pos);
> @@ -147,6 +159,12 @@ void __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops)
>   		enable_tdx = false;
>   }
>   
> +void tdx_hardware_unsetup(void)
> +{
> +	/* kfree accepts NULL. */
> +	kfree(tdx_mng_key_config_lock);
> +}
> +
>   void __init tdx_pre_kvm_init(unsigned int *vcpu_size,
>   			unsigned int *vcpu_align, unsigned int *vm_size)
>   {
> diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h
> index 78331dbc29f7..da32b4b86b19 100644
> --- a/arch/x86/kvm/vmx/x86_ops.h
> +++ b/arch/x86/kvm/vmx/x86_ops.h
> @@ -131,11 +131,13 @@ void __init tdx_pre_kvm_init(unsigned int *vcpu_size,
>   			unsigned int *vcpu_align, unsigned int *vm_size);
>   bool tdx_is_vm_type_supported(unsigned long type);
>   void __init tdx_hardware_setup(struct kvm_x86_ops *x86_ops);
> +void tdx_hardware_unsetup(void);
>   #else
>   static inline void tdx_pre_kvm_init(
>   	unsigned int *vcpu_size, unsigned int *vcpu_align, unsigned int *vm_size) {}
>   static inline bool tdx_is_vm_type_supported(unsigned long type) { return false; }
>   static inline void tdx_hardware_setup(struct kvm_x86_ops *x86_ops) {}
> +static inline void tdx_hardware_unsetup(void) {}
>   #endif
>   
>   #endif /* __KVM_X86_VMX_X86_OPS_H */