Received-SPF: pass (google.com: domain of linux-kernel+bounces-103135-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 14 Mar 2024 18:39:28 +0800
From: Chao Gao <chao.gao@intel.com>
To: <isaku.yamahata@intel.com>
CC: <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<isaku.yamahata@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>,
	<erdemaktas@google.com>, Sean Christopherson <seanjc@google.com>, Sagi Shahar
	<sagis@google.com>, Kai Huang <kai.huang@intel.com>, <chen.bo@intel.com>,
	<hang.yuan@intel.com>, <tina.zhang@intel.com>
Subject: Re: [PATCH v18 023/121] KVM: TDX: Make KVM_CAP_MAX_VCPUS backend
 specific
Message-ID: <ZfLT4AwkhGUQGPxD@chao-email>
References: <cover.1705965634.git.isaku.yamahata@intel.com>
 <ed33ebe29b231e8e657cd610a983fa603b10f530.1705965634.git.isaku.yamahata@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ed33ebe29b231e8e657cd610a983fa603b10f530.1705965634.git.isaku.yamahata@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?PF9auE3XJBdsOv3R/ttv3O0hSjC4EDxjJ9tdLuSSM4cPuy1WaSdYcr+Bp8wp?=
 =?us-ascii?Q?+nCVVex6i56qESrBrdMXGy9ODBsJL5IHnjiwQMrkDYxnKTVbuv53PPIAwTG+?=
 =?us-ascii?Q?AMORsfqohflR0oG1/6rg2OV4cPLJlhACMgurZ2N3bU8f8UaaLZ1T8D2dx49R?=
 =?us-ascii?Q?PHOoxCxn7xSeyvlddg7tkxmlFNV9LA52hIN0VcBqNiWHcj8278QK76hIUZIh?=
 =?us-ascii?Q?H68sRpGSuD/VwwEaVPcmZxkLMJLq4qUWokh2WL4oQ6UOyRRuziRsdNMjnfr6?=
 =?us-ascii?Q?mmXi/10UV5rHT64GcTeDMbghHsxwLw+4K3FrC8Luk0smOe7Ru5CI/75gXkIU?=
 =?us-ascii?Q?Ql5Sqom8YtY3ik7iIdJcS0xChO7e56MvwZMLYu22LX3uxLLQtUY7KdrwPMVj?=
 =?us-ascii?Q?wKGBmWB7fm6ZRj3RmBzCy9g3xwbPCJUdPp2/Z0mSl3CtzjkDXG0Mmq3mJbxP?=
 =?us-ascii?Q?IhZI5qbq8NP443R4s4G4nHUmkAlyc3jOpbSZRU7dXmvTVi6cVxIFoJHjSVj/?=
 =?us-ascii?Q?zMfUlmgP2CL4dGaiAF+jaaFwJ+dJUe+59fXaHVfnWBy5jdW5EeSvdkedGMno?=
 =?us-ascii?Q?AC1AJ/GMz184HgZHxNe9TmwPAzKGpvDDQLqN2G5lq84G3/cXAW8I+v3DSZJU?=
 =?us-ascii?Q?WRXQMUd51RSI/ZGfSDzjVOw7wiUTyIYaErZ2d1reNu9PB0RQqbHLLgtgOKbY?=
 =?us-ascii?Q?Y2nUQog8t/K1UQOLprgftletTq+EHQD0MmH/M/SwOWbT5QN4cY95wTyz6WLt?=
 =?us-ascii?Q?hoUarYbDxgaRt41atW29jl0cG5PSBRAEn5s4k72oUFw2hnUWwVDtHuztapT/?=
 =?us-ascii?Q?P2eWHM3dfXNHMSaHeigvAM79OCHwzuAmTG1WQPWa3KF6/1cY9r5DVSDGH2dn?=
 =?us-ascii?Q?ehjTSzBJEs78Hkz0pZI6eMtl9eTwrfbvSkof9V0ndRi002UP0bS4gtoHiOWM?=
 =?us-ascii?Q?XNFtRiC8K/rMgvA6aqC/kooOrov5ZXGW108pgXXTydxcoZiyFZaLu+12yGGO?=
 =?us-ascii?Q?asmSCz2QRM6p+JqAxJRmJEVsfudR+gvXZks9N8tH7qGXbvontmRXyLM4hNCW?=
 =?us-ascii?Q?buJAhYIDhr0eEp3PjhHkC6rHwpUzNVcIS+d4T/eADOse4NbQ39jcau5EHfIQ?=
 =?us-ascii?Q?WOs2qIFF4Eh13pEbCT00hBqFXYKJbgJsUU3UjLtR5PBC4OOEO0DBMDIdeHdP?=
 =?us-ascii?Q?f2RliUi0sh4JH6oYET9TRYL7GPyHz9nmTQ7Pspe+YJAJHPv9fc8wPj1BfDZH?=
 =?us-ascii?Q?6vOxV7aw1zWrmQN0AlsomROoBPCoGC7ehog+PSlgvsOTO/WfruBkAQ15Nfxd?=
 =?us-ascii?Q?/jevnAWpN8bD6mkX7hUU6peogofQZuKFRAG0GF/fSVDCgkjy7/Fzxfpc1V4+?=
 =?us-ascii?Q?RvI2B4mhDfEflViDbULyVY5Mqhaiejjep8QDC+9bG4njGQJKSqcmp+KX+SGe?=
 =?us-ascii?Q?5xA4jj1Ss9h/AjXiTIhX3FmxAZHYYIEpZjQz6Rc8eskCHrb45W0naoRdjKd8?=
 =?us-ascii?Q?SqGzJtuNlEGWKIJ3l6tNKCw1UUs6PIx8Z4dhO64NBru7E6AijbvRI5v74prt?=
 =?us-ascii?Q?+r27Wy+GEOQGjgALC67IcoXp7BJ9yW7JLA1V5xWW?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a6ab4872-5270-43ee-f783-08dc44130c65
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Mar 2024 10:39:38.6984
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /VY27ytSNxIO+KWgf62LpK+FilVqxSha+5xVAPila1OyYmmC1R1lyumJksF8c7PpKEkdn3LBl8bfjV2mHFAgpQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB6982
X-OriginatorOrg: intel.com

>+static int vt_max_vcpus(struct kvm *kvm)
>+{
>+	if (!kvm)
>+		return KVM_MAX_VCPUS;
>+
>+	if (is_td(kvm))
>+		return min(kvm->max_vcpus, TDX_MAX_VCPUS);

I suppose kvm->max_vcpus should be always smaller than TDX_MAX_VCPUS, right?
if that's the case, the min() is pointless.

I don't get why kvm->max_vcpus is concerned. do you want to enable userspace to
read back the max_vcpus configured last time? this looks useless because userspace
can keep track of that value. how about:

	/*
	 * TDX module imposes additional restrictions on the maximum number of
	 * vCPUs of a TD guest.
	 */
	if (kvm && is_td(kvm))
		return min(TDX_MAX_VCPUS, KVM_MAX_VCPUS);
	else
		return KVM_MAX_VCPUS;

>+
>+	return kvm->max_vcpus;
>+}
>+
> static int vt_hardware_enable(void)
> {
> 	int ret;
>@@ -54,6 +66,14 @@ static void vt_hardware_unsetup(void)
> 	vmx_hardware_unsetup();
> }
> 
>+static int vt_vm_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
>+{
>+	if (is_td(kvm))
>+		return tdx_vm_enable_cap(kvm, cap);
>+
>+	return -EINVAL;
>+}
>+
> static int vt_vm_init(struct kvm *kvm)
> {
> 	if (is_td(kvm))
>@@ -91,7 +111,9 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
> 	.has_emulated_msr = vmx_has_emulated_msr,
> 
> 	.is_vm_type_supported = vt_is_vm_type_supported,
>+	.max_vcpus = vt_max_vcpus,
> 	.vm_size = sizeof(struct kvm_vmx),
>+	.vm_enable_cap = vt_vm_enable_cap,
> 	.vm_init = vt_vm_init,
> 	.vm_destroy = vmx_vm_destroy,
> 
>diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>index 8c463407f8a8..876ad7895b88 100644
>--- a/arch/x86/kvm/vmx/tdx.c
>+++ b/arch/x86/kvm/vmx/tdx.c
>@@ -100,6 +100,35 @@ struct tdx_info {
> /* Info about the TDX module. */
> static struct tdx_info *tdx_info;
> 
>+int tdx_vm_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
>+{
>+	int r;
>+
>+	switch (cap->cap) {
>+	case KVM_CAP_MAX_VCPUS: {
>+		if (cap->flags || cap->args[0] == 0)
>+			return -EINVAL;
>+		if (cap->args[0] > KVM_MAX_VCPUS ||
>+		    cap->args[0] > TDX_MAX_VCPUS)
>+			return -E2BIG;
>+
>+		mutex_lock(&kvm->lock);
>+		if (kvm->created_vcpus)
>+			r = -EBUSY;

Curly brackets are missing.

And -EBUSY looks improper because it isn't a temporary error.

>+		else {
>+			kvm->max_vcpus = cap->args[0];
>+			r = 0;
>+		}
>+		mutex_unlock(&kvm->lock);
>+		break;