Re: [PATCH v18 023/121] KVM: TDX: Make KVM_CAP_MAX_VCPUS backend specific

[Chao Gao]

>+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;