Re: [Qemu-devel] [PATCH] i386: derive '-cpu host' from KVM_GET_SUPPORTED_CPUID

[Avi Kivity <avi@redhat.com>]

On 11/09/2011 08:21 PM, Sasha Levin wrote:
> On Wed, 2011-11-09 at 20:00 +0200, Avi Kivity wrote:
> > On 11/09/2011 07:56 PM, Anthony Liguori wrote:
> > > On 11/09/2011 07:44 AM, Avi Kivity wrote:
> > >> The fact that a host cpu supports a feature doesn't mean that QEMU
> > >> and KVM
> > >> will also support it, yet -cpuid host brings host features wholesale.
> > >>
> > >> We need to whitelist each feature separately to make sure we support it.
> > >> This patch adds KVM whitelisting (by simply using
> > >> KVM_GET_SUPPORTED_CPUID
> > >> instead of the CPUID instruction).
> > >>
> > >> Signed-off-by: Avi Kivity<avi@redhat.com>
> > >
> > > This seems like a 1.0 candidate, yes?
> > 
> > There is a distinct possibility this will uncover bugs in kvm's
> > KVM_GET_SUPPORTED_CPUID.  Those won't be qemu bugs, so I think it's good
> > for 1.0.
> > 
>
> Avi, we have a problem in the KVM tool of KVM_GET_SUPPORTED_CPUID
> sometimes returning -E2BIG. I've sent a mail about it some time ago, but
> we couldn't really find the reason.
>
> It's somewhat non-deterministic, and theres no sure way to reproduce it,
> but it doesn't happen that rarely.
>
> The block of code that uses it from usermode it pretty simple:
>
> struct kvm_cpuid2 *kvm_cpuid;
>
> kvm_cpuid = calloc(1, sizeof(*kvm_cpuid) + 
> 	MAX_KVM_CPUID_ENTRIES * sizeof(*kvm_cpuid->entries));
>
> kvm_cpuid->nent = MAX_KVM_CPUID_ENTRIES;
> if (ioctl(vcpu->kvm->sys_fd, KVM_GET_SUPPORTED_CPUID, kvm_cpuid) < 0)
> 	die_perror("KVM_GET_SUPPORTED_CPUID failed");
>
> MAX_KVM_CPUID_ENTRIES is set to 100, which is more than the 80 defined
> in the kernel, so it shouldn't be an issue. It wouldn't explain the non
> deterministic behavior either.
>
> QEMU's code around it allows it to hide the bug if it does happen:
>
> uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
>                                        uint32_t index, int reg)
> {
>      struct kvm_cpuid2 *cpuid;
>      int i, max;
>      uint32_t ret = 0;
>      uint32_t cpuid_1_edx;
>      int has_kvm_features = 0;
>
>      max = 1;
>      while ((cpuid = try_get_cpuid(s, max)) == NULL) {
>          max *= 2;
>      }
> [snip]
>
> Which means that if it fails it will silently retry until it makes it.
>
> Any guess on why it might happen?
>

No idea.  If you run your code block in a loop, how soon will it reproduce?

-- 
error compiling committee.c: too many arguments to function