Re: [PATCH v2 24/25] KVM: x86: Introduce KVM_TDX_GET_CPUID

[Francesco Lavra <francescolavra.fl@gmail.com>]

On 2024-10-30 at 19:00, Rick Edgecombe wrote:
> @@ -1055,6 +1144,81 @@ static int tdx_td_vcpu_init(struct kvm_vcpu
> *vcpu, u64 vcpu_rcx)
>  	return ret;
>  }
>  
> +/* Sometimes reads multipple subleafs. Return how many enties were
> written. */
> +static int tdx_vcpu_get_cpuid_leaf(struct kvm_vcpu *vcpu, u32 leaf,
> int max_cnt,
> +				   struct kvm_cpuid_entry2
> *output_e)
> +{
> +	int i;
> +
> +	if (!max_cnt)
> +		return 0;
> +
> +	/* First try without a subleaf */
> +	if (!tdx_read_cpuid(vcpu, leaf, 0, false, output_e))
> +		return 1;
> +
> +	/*
> +	 * If the try without a subleaf failed, try reading subleafs
> until
> +	 * failure. The TDX module only supports 6 bits of subleaf
> index.

It actually supports 7 bits, i.e. bits 6:0, so the limit below should
be 0b1111111.

> +	 */
> +	for (i = 0; i < 0b111111; i++) {
> +		if (i > max_cnt)
> +			goto out;

This will make this function return (max_cnt + 1) instead of max_cnt.
I think the code would be simpler if max_cnt was initialized to
min(max_cnt, 0x80) (because 0x7f is a supported subleaf index, as far
as I can tell), and the for() condition was changed to `i < max_cnt`.

> +		/* Keep reading subleafs until there is a failure.
> */
> +		if (tdx_read_cpuid(vcpu, leaf, i, true, output_e))
> +			return i;
> +
> +		output_e++;
> +	}
> +
> +out:
> +	return i;
> +}
> +
> +static int tdx_vcpu_get_cpuid(struct kvm_vcpu *vcpu, struct
> kvm_tdx_cmd *cmd)
> +{
> +	struct kvm_cpuid2 __user *output, *td_cpuid;
> +	struct kvm_cpuid_entry2 *output_e;
> +	int r = 0, i = 0, leaf;
> +
> +	output = u64_to_user_ptr(cmd->data);
> +	td_cpuid = kzalloc(sizeof(*td_cpuid) +
> +			sizeof(output->entries[0]) *
> KVM_MAX_CPUID_ENTRIES,
> +			GFP_KERNEL);
> +	if (!td_cpuid)
> +		return -ENOMEM;
> +
> +	for (leaf = 0; leaf <= 0x1f; leaf++) {
> +		output_e = &td_cpuid->entries[i];
> +		i += tdx_vcpu_get_cpuid_leaf(vcpu, leaf,
> +					     KVM_MAX_CPUID_ENTRIES -
> i - 1,

This should be KVM_MAX_CPUID_ENTRIES - i.

> +					     output_e);
> +	}
> +
> +	for (leaf = 0x80000000; leaf <= 0x80000008; leaf++) {
> +		output_e = &td_cpuid->entries[i];
> +		i += tdx_vcpu_get_cpuid_leaf(vcpu, leaf,
> +					     KVM_MAX_CPUID_ENTRIES -
> i - 1,

Same here.