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

[Sasha Levin <levinsasha928@gmail.com>]

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?

-- 

Sasha.