Re: [PATCH v19 027/130] KVM: TDX: Define TDX architectural definitions

Isaku Yamahata
2024-03-05T08:21:39Z

On Fri, Mar 01, 2024 at 03:25:31PM +0800,
Yan Zhao <yan.y.zhao@intel.com> wrote:

> > + * TD_PARAMS is provided as an input to TDH_MNG_INIT, the size of which is 1024B.
> > + */
> > +#define TDX_MAX_VCPUS (~(u16)0)
> This value will be treated as -1 in tdx_vm_init(),
> "kvm->max_vcpus = min(kvm->max_vcpus, TDX_MAX_VCPUS);"
>
> This will lead to kvm->max_vcpus being -1 by default.
> Is this by design or just an error?
> If it's by design, why not set kvm->max_vcpus = -1 in tdx_vm_init() directly.
> If an unexpected error, may below is better?
>
> #define TDX_MAX_VCPUS (int)((u16)(~0UL))
> or
> #define TDX_MAX_VCPUS 65536

You're right. I'll use ((int)U16_MAX).
As TDX 1.5 introduced metadata MAX_VCPUS_PER_TD, I'll update to get the value
and trim it further. Something following.

diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index f964d99f8701..31205f84d594 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -23,7 +23,6 @@ KVM_X86_OP(has_emulated_msr)
/* TODO: Once all backend implemented this op, remove _OPTIONAL_RET0. */
KVM_X86_OP_OPTIONAL_RET0(vcpu_check_cpuid)
KVM_X86_OP(vcpu_after_set_cpuid)
-KVM_X86_OP_OPTIONAL(max_vcpus);
KVM_X86_OP_OPTIONAL(vm_enable_cap)
KVM_X86_OP(vm_init)
KVM_X86_OP_OPTIONAL(flush_shadow_all_private)
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index 6dd78230c9d4..deb59e94990f 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -13,17 +13,6 @@
static bool enable_tdx __ro_after_init;
module_param_named(tdx, enable_tdx, bool, 0444);

-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);
-
- return kvm->max_vcpus;
-}
-
#if IS_ENABLED(CONFIG_HYPERV) || IS_ENABLED(CONFIG_INTEL_TDX_HOST)
static int vt_flush_remote_tlbs(struct kvm *kvm);
static int vt_flush_remote_tlbs_range(struct kvm *kvm, gfn_t gfn, gfn_t nr_pages);
@@ -1130,7 +1119,6 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
.hardware_disable = vt_hardware_disable,
.has_emulated_msr = vt_has_emulated_msr,

- .max_vcpus = vt_max_vcpus,
.vm_size = sizeof(struct kvm_vmx),
.vm_enable_cap = vt_vm_enable_cap,
.vm_init = vt_vm_init,
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 7cca6a33ad97..a8cfb4f214a6 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -63,6 +63,8 @@ struct tdx_info {
u8 nr_tdcs_pages;
u8 nr_tdvpx_pages;

+ u16 max_vcpus_per_td;
+
/*
* The number of WBINVD domains. 0 means that wbinvd domain is cpu
* package.
@@ -100,7 +102,8 @@ int tdx_vm_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
if (cap->flags || cap->args[0] == 0)
return -EINVAL;
if (cap->args[0] > KVM_MAX_VCPUS ||
- cap->args[0] > TDX_MAX_VCPUS)
+ cap->args[0] > TDX_MAX_VCPUS ||
+ cap->args[0] > tdx_info->max_vcpus_per_td)
return -E2BIG;

mutex_lock(&kvm->lock);
@@ -729,7 +732,8 @@ int tdx_vm_init(struct kvm *kvm)
* TDX has its own limit of the number of vcpus in addition to
* KVM_MAX_VCPUS.
*/
- kvm->max_vcpus = min(kvm->max_vcpus, TDX_MAX_VCPUS);
+ kvm->max_vcpus = min3(kvm->max_vcpus, tdx_info->max_vcpus_per_td,
+ TDX_MAX_VCPUS);

mutex_init(&to_kvm_tdx(kvm)->source_lock);
return 0;
@@ -4667,6 +4671,7 @@ static int __init tdx_module_setup(void)
TDX_INFO_MAP(ATTRS_FIXED1, attributes_fixed1),
TDX_INFO_MAP(XFAM_FIXED0, xfam_fixed0),
TDX_INFO_MAP(XFAM_FIXED1, xfam_fixed1),
+ TDX_INFO_MAP(MAX_VCPUS_PER_TD, max_vcpus_per_td),
};

ret = tdx_enable();
diff --git a/arch/x86/kvm/vmx/tdx_arch.h b/arch/x86/kvm/vmx/tdx_arch.h
index b10dad8f46bb..711855be6c83 100644
--- a/arch/x86/kvm/vmx/tdx_arch.h
+++ b/arch/x86/kvm/vmx/tdx_arch.h
@@ -144,7 +144,7 @@ struct tdx_cpuid_value {
/*
* TD_PARAMS is provided as an input to TDH_MNG_INIT, the size of which is 1024B.
*/
-#define TDX_MAX_VCPUS (~(u16)0)
+#define TDX_MAX_VCPUS ((int)U16_MAX)

struct td_params {
u64 attributes;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d097024fd974..6822a50e1d5d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4733,8 +4733,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
break;
case KVM_CAP_MAX_VCPUS:
r = KVM_MAX_VCPUS;
- if (kvm_x86_ops.max_vcpus)
- r = static_call(kvm_x86_max_vcpus)(kvm);
+ if (kvm)
+ r = kvm->max_vcpus;
break;
case KVM_CAP_MAX_VCPU_ID:
r = KVM_MAX_VCPU_IDS;

--
Isaku Yamahata <isaku.yamahata@linux.intel.com>