Re: [PATCH v6 13/19] i386: prefer system KVM_GET_SUPPORTED_HV_CPUID ioctl over vCPU's one

[Eduardo Habkost <ehabkost@redhat.com>]

On Thu, Apr 22, 2021 at 06:11:24PM +0200, Vitaly Kuznetsov wrote:
> KVM_GET_SUPPORTED_HV_CPUID was made a system wide ioctl which can be called
> prior to creating vCPUs and we are going to use that to expand Hyper-V cpu
> features early. Use it when it is supported by KVM.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  target/i386/kvm/kvm.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index feec9f25ba12..5d08f3a39ef7 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -928,7 +928,8 @@ static struct {
>      },
>  };
>  
> -static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max)
> +static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max,
> +                                           bool do_sys_ioctl)
>  {
>      struct kvm_cpuid2 *cpuid;
>      int r, size;
> @@ -937,7 +938,11 @@ static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max)
>      cpuid = g_malloc0(size);
>      cpuid->nent = max;
>  
> -    r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid);
> +    if (do_sys_ioctl) {
> +        r = kvm_ioctl(kvm_state, KVM_GET_SUPPORTED_HV_CPUID, cpuid);
> +    } else {
> +        r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid);
> +    }
>      if (r == 0 && cpuid->nent >= max) {
>          r = -E2BIG;
>      }
> @@ -964,13 +969,17 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
>      /* 0x40000000..0x40000005, 0x4000000A, 0x40000080..0x40000080 leaves */
>      int max = 10;
>      int i;
> +    bool do_sys_ioctl;
> +
> +    do_sys_ioctl =
> +        kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID) > 0;
>  
>      /*
>       * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with
>       * -E2BIG, however, it doesn't report back the right size. Keep increasing
>       * it and re-trying until we succeed.
>       */
> -    while ((cpuid = try_get_hv_cpuid(cs, max)) == NULL) {
> +    while ((cpuid = try_get_hv_cpuid(cs, max, do_sys_ioctl)) == NULL) {
>          max++;
>      }
>  
> @@ -980,7 +989,7 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
>       * information early, just check for the capability and set the bit
>       * manually.
>       */
> -    if (kvm_check_extension(cs->kvm_state,
> +    if (!do_sys_ioctl && kvm_check_extension(cs->kvm_state,
>                              KVM_CAP_HYPERV_ENLIGHTENED_VMCS) > 0) {

Oh, this conditional replaces the comment I suggested in patch
10/19.  It makes it obvious that the hack can be deleted if we
remove support for the VCPU ioctl.

So, when exactly will we be able to delete the VCPU ioctl code
and support only the system ioctl?

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

>          for (i = 0; i < cpuid->nent; i++) {
>              if (cpuid->entries[i].function == HV_CPUID_ENLIGHTMENT_INFO) {
> -- 
> 2.30.2
> 

-- 
Eduardo