Roman Kagan <[email protected]> writes: > On Mon, May 27, 2019 at 06:39:53PM +0200, Vitaly Kuznetsov wrote: >> Roman Kagan <[email protected]> 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.
No problem at all! > (What was the reason for not following this pattern in > kvm_vcpu_ioctl_get_hv_cpuid BTW?) The opportunity to set nent in E2BIG case was probabbly overlooked. I was looking at QEMU's get_supported_cpuid() implementation which iterates trying to find the right number and used this as a pattern. While setting nent in E2BIG case seems to be very convenient, this is an unobvious side-effect: usually, where the return value indicates an error we don't inspect the payload. I'm, however, not at all against changing kvm_vcpu_ioctl_get_hv_cpuid(). Unfortunately, this won't help QEMU and we'll still have to iterate to support legacy kernels. -- Vitaly
