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

[Huang, Kai]

On 5/03/2024 9:21 pm, Isaku Yamahata wrote:
> On Fri, Mar 01, 2024 at 03:25:31PM +0800,
> Yan Zhao <yan.y.zhao@xxxxxxxxx> 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.

[...]

>   
> +	u16 max_vcpus_per_td;
> +

[...]

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

[...]

> -#define TDX_MAX_VCPUS	(~(u16)0)
> +#define TDX_MAX_VCPUS	((int)U16_MAX)

Why do you even need TDX_MAX_VCPUS, given it cannot exceed U16_MAX and 
you will have the 'u16 max_vcpus_per_td' anyway?

IIUC, in KVM_ENABLE_CAP(KVM_CAP_MAX_VCPUS), we can overwrite the 
kvm->max_vcpus to the 'max_vcpus' provided by the userspace, and make 
sure it doesn't exceed tdx_info->max_vcpus_per_td.

Anything I am missing?