Re: [RFC PATCH v2 1/6] kvm: add device control API

Scott Wood
2013-04-03T17:37:41Z

On 04/03/2013 08:22:37 AM, Alexander Graf wrote:
>
> On 03.04.2013, at 04:17, Paul Mackerras wrote:
>
> > On Tue, Apr 02, 2013 at 08:19:56PM -0500, Scott Wood wrote:
> >> On 04/02/2013 08:02:39 PM, Paul Mackerras wrote:
> >>> On Mon, Apr 01, 2013 at 05:47:48PM -0500, Scott Wood wrote:
> >>>> +4.79 KVM_CREATE_DEVICE
> >>>> +
> >>>> +Capability: KVM_CAP_DEVICE_CTRL
> >>>
> >>> I notice this patch doesn't add this capability;
> >>
> >> Yes, it does (see below).
> >>
> >>> you add it in a later patch.
> >>
> >> Maybe you're thinking of KVM_CAP_IRQ_MPIC?
> >
> > No, I was referring to the addition to
> kvm_dev_ioctl_check_extension()
> > of a KVM_CAP_DEVICE_CTRL case. Since this patch adds the code to
> handle
> > KVM_CREATE_DEVICE ioctl it should also add the code to return 1 if
> > userspace queries the KVM_CAP_DEVICE_CTRL capability.
> >
> >>>> +/* ioctl for vm fd */
> >>>> +#define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct
> >>> kvm_create_device)
> >>>
> >>> This define should go with the other VM ioctls, otherwise the next
> >>> person to add a VM ioctl will probably miss it and reuse the 0xe0
> >>> code.
> >>
> >> That's actually why I moved it to a new section, with device
> control
> >> ioctls getting their own range, as the legacy "device model" and
> >> some other things did. 0xe0 is not the next ioctl that would be
> >> used for either vm or vcpu. The ioctl numbering is actually
> already
> >> a mess, with sometimes care being taken to keep vcpu and vm ioctls
> >> from overlapping, but on other places overlapping does happen. I'm
> >> not sure what exactly I should do here.
> >
> > Well, even if you are using a new range, I still think that
> > KVM_CREATE_DEVICE, being a VM ioctl, should go next to the other VM
> > ioctls. I guess it's ultimately up to the maintainers.
>
> I agree. Things get confusing for VM ioctls otherwise.

Things are already confusing. :-)

I can move KVM_CREATE_DEVICE back with the other VM ioctls, but what
number should it get? The last VM ioctl is 0xab (which is also a VCPU
ioctl). Should I use 0xac (which is also a VCPU ioctl)? Or should I
try to avoid a conflict, as was sometimes done in the past -- in which
case, which number should I use?

-Scott