Re: [PATCH v19 037/130] KVM: TDX: Make KVM_CAP_MAX_VCPUS backend specific

[Huang, Kai]

On 26/02/2024 9:25 pm, Yamahata, Isaku wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>
> TDX has its own limitation on the maximum number of vcpus that the guest
> can accommodate.  

"limitation" -> "control".

"the guest" -> "a guest".

Allow x86 kvm backend to implement its own KVM_ENABLE_CAP
> handler and implement TDX backend for KVM_CAP_MAX_VCPUS.  

I am not sure we normally say "x86 KVM backend".  Just say "Allow KVM 
x86 ...".

user space VMM,
> e.g. qemu, can specify its value instead of KVM_MAX_VCPUS.

Grammar check.

> When creating TD (TDH.MNG.INIT), the maximum number of vcpu needs to be
> specified as struct td_params_struct.  

'struct td_params_struct'??

Anyway, I don't think you need to mention such details.

and the value is a part of
> measurement.  The user space has to specify the value somehow.  

"and" -> "And" (grammar check please).

And add an empty line to start below as a new paragraph.

There are
> two options for it.
> option 1. API (Set KVM_CAP_MAX_VCPU) to specify the value (this patch)
> option 2. Add max_vcpu as a parameter to initialize the guest.
>            (TDG.MNG.INIT)

First of all, it seems to me that the two are not conflicting.

Based on the uapi/kvm.h:

  #define KVM_CAP_MAX_VCPUS 66       /* returns max vcpus per vm */

Currently KVM x86 doesn't allow to configure MAX_VCPU on VM-basis, but 
always reports KVM_MAX_VCPUS for _ALL_ VMs.  I.e., it doesn't support 
userspace to explicitly enable KVM_CAP_MAX_VCPUS for a given VM.

Now, if we allow the userspace to configure the MAX_VCPU for TDX guest 
(this could be a separate discussion in fact) due to attestation 
whatever, we need to support allowing userspace to configure MAX_VCPUS 
on VM-basis.

Therefore, option 1 isn't really an option to me, but is the thing that 
we _SHOULD_ do to support TDX.

So this pach should really just add "per-VM max vcpus" support for TDX, 
starting from:

	struct kvm_tdx {	/* or 'struct kvm_arch' ?? */
		...
		int max_vcpus;
	}

And in TDH.MNG.INIT, we need to manually check the MAX_VCPU specified in 
TD_PARAMS structure to make sure it matches to the record that we 
specified via KVM_CAP_MAX_VCPUS.

So how about:

"
TDX has its own mechanism to control the maximum number of VCPUs that 
the TDX guest can use.  When creating a TDX guest, the maximum number of 
vcpus needs to be passed to the TDX module as part of the measurement of 
the guest.

Because the value is part of the measurement, thus part of attestation, 
it better to allow the userspace to be able to configure it.  E.g. the 
users may want to precisely control the maximum number of vcpus their 
precious VMs can use.

The actual control itself must be done via the TDH.MNG.INIT SEAMCALL 
itself, where the number of maximum cpus is an input to the TDX module, 
but KVM needs to support the "per-VM number of maximum vcpus" and 
reflect that in the KVM_CAP_MAX_VCPUS.

Currently, the KVM x86 always reports KVM_MAX_VCPUS for all VMs but 
doesn't allow to enable KVM_CAP_MAX_VCPUS to configure the number of 
maximum vcpus on VM-basis.

Add "per-VM maximum vcpus" to KVM x86/TDX to accommodate TDX's needs.

The userspace-configured value then can be verified when KVM is actually 
creating the TDX guest.
"