On (Thu) Jan 29 2009 [17:12:56], Nitin A Kamble wrote:
> Avi,
> I reworked the earlier patch for exposing the host cpuid bits to
> guests. Attached is the patch for your kvm.git tree. With this new code
> in the kernel both the old and new qemu binaries are working.
Please add a Signed-Off-By and a git-style description for patches sent
for inclusion.
Also, it'll be better to reply to the patch if you include in the
message instead of attaching them (git send-email).
> -#define KVM_MAX_CPUID_ENTRIES 40
> +#define KVM_MAX_CPUID_ENTRIES 100
Are we already hitting this limit? I don't think so.
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1060,19 +1060,25 @@ long kvm_arch_dev_ioctl(struct file *filp,
> case KVM_GET_SUPPORTED_CPUID: {
> struct kvm_cpuid2 __user *cpuid_arg = argp;
> struct kvm_cpuid2 cpuid;
> + int retry = 0;
>
> r = -EFAULT;
> if (copy_from_user(&cpuid, cpuid_arg, sizeof cpuid))
> goto out;
> r = kvm_dev_ioctl_get_supported_cpuid(&cpuid,
> cpuid_arg->entries);
> - if (r)
> + if (r == -EAGAIN)
> + retry = 1;
> + else if (r)
> goto out;
>
> r = -EFAULT;
> if (copy_to_user(cpuid_arg, &cpuid, sizeof cpuid))
> goto out;
> - r = 0;
> + if (retry)
> + r = -EAGAIN;
> + else
> + r = 0;
> break;
> }
> default:
I can't really get the point of doing this: do you want to return
-EAGAIN when there are multiple values to be read off a given cpuid
function? If that's the case, I'd suggest adding the necessary
intelligence to the userspace to poll for as much data as can be fetched
rather than (ab)using EAGAIN.
> @@ -1325,10 +1331,14 @@ static int
> kvm_dev_ioctl_get_supported_cpuid(struct kvm_cpuid2 *cpuid,
> {
> struct kvm_cpuid_entry2 *cpuid_entries;
> int limit, nent = 0, r = -E2BIG;
> + int sizer = 0;
What does 'sizer' mean?
Also, I've added support for handling function 0xd in userspace; can you
add the handler for that in get_supported_cpuid()?
Amit
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
