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

[Paul Mackerras <paulus@samba.org>]

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.

Paul.