On Fri, 2009-01-30 at 03:25 -0800, Amit Shah wrote:
> Please add a Signed-Off-By and a git-style description for patches sent
> for inclusion.
> 
Will do.

> 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.
It is not hitting the limit yet. Bumped it up for future use.

> 
> > --- 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.
This is based on the discussions with Avi last time. Sizer is to get the
size of the list of items. Avi wanted to keep the backward compatibility
of the ioctl API.

> 
> > @@ -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()?
> 
Currently KVM is limiting the basic cpuid limit to 0xb in the kernel
code. That will need to be changed for leaf 0xd to go through. Doing
that would be the next step.

> Amit
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to