Re: [Qemu-devel] [PATCH v2 2/9] i386/kvm: add support for KVM_GET_SUPPORTED_HV_CPUID

[Roman Kagan]

On Mon, May 27, 2019 at 06:39:53PM +0200, Vitaly Kuznetsov wrote:
> Roman Kagan <address@hidden> writes:
> > On Fri, May 17, 2019 at 04:19:17PM +0200, Vitaly Kuznetsov wrote:
> >> +static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max)
> >> +{
> >> +    struct kvm_cpuid2 *cpuid;
> >> +    int r, size;
> >> +
> >> +    size = sizeof(*cpuid) + max * sizeof(*cpuid->entries);
> >> +    cpuid = g_malloc0(size);
> >> +    cpuid->nent = max;
> >> +
> >> +    r = kvm_vcpu_ioctl(cs, KVM_GET_SUPPORTED_HV_CPUID, cpuid);
> >> +    if (r == 0 && cpuid->nent >= max) {
> >> +        r = -E2BIG;
> >> +    }
> >> +    if (r < 0) {
> >> +        if (r == -E2BIG) {
> >> +            g_free(cpuid);
> >> +            return NULL;
> >> +        } else {
> >> +            fprintf(stderr, "KVM_GET_SUPPORTED_HV_CPUID failed: %s\n",
> >> +                    strerror(-r));
> >> +            exit(1);
> >> +        }
> >> +    }
> >> +    return cpuid;
> >> +}
> >> +
> >> +/*
> >> + * Run KVM_GET_SUPPORTED_HV_CPUID ioctl(), allocating a buffer large 
> >> enough
> >> + * for all entries.
> >> + */
> >> +static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
> >> +{
> >> +    struct kvm_cpuid2 *cpuid;
> >> +    int max = 7; /* 0x40000000..0x40000005, 0x4000000A */
> >> +
> >> +    /*
> >> +     * 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.
> >> +     */
> >
> > I'm still missing the idea of reiterating more than once: the ioctl
> > returns the actual size of the array.
> 
> Hm, I think I checked that and it doesn't seem to be the case.
> 
> The code in kvm_vcpu_ioctl_get_hv_cpuid():
> 
>       if (cpuid->nent < nent)
>               return -E2BIG;
> 
>       if (cpuid->nent > nent)
>               cpuid->nent = nent;
> 
> (I think I even ran a test after your comment on v1 and it it
> confirmed nent is not set on E2BIG). Am I missing something obvious?

Indeed, I saw kvm_vcpu_ioctl_get_cpuid2() always setting ->nent on
return and assumed so did kvm_vcpu_ioctl_get_hv_cpuid().  I stand
corrected, please disregard this comment.
(What was the reason for not following this pattern in
kvm_vcpu_ioctl_get_hv_cpuid BTW?)

Thanks, and sorry for confusion,
Roman.