On 2/25/19 5:15 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 12:28:39PM +0100, Cédric Le Goater wrote:
>> The 'destroy' method is currently used to destroy all devices when the
>> VM is destroyed after the vCPUs have been freed.
>>
>> This new KVM ioctl exposes the same KVM device method. It acts as a
>> software reset of the VM to 'destroy' selected devices when necessary
>> and perform the required cleanups on the vCPUs. Called with the
>> kvm->lock.
>>
>> The 'destroy' method could be improved by returning an error code.
>>
>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>
> Again, has this been discussed with Paolo and/or other KVM core
> people?
Not yet. Adding Paolo for feedback.
>> ---
>> include/uapi/linux/kvm.h | 7 ++++++
>> virt/kvm/kvm_main.c | 38 +++++++++++++++++++++++++++++++
>> Documentation/virtual/kvm/api.txt | 19 ++++++++++++++++
>> 3 files changed, 64 insertions(+)
>>
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 52bf74a1616e..d78fafa54274 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -1183,6 +1183,11 @@ struct kvm_create_device {
>> __u32 flags; /* in: KVM_CREATE_DEVICE_xxx */
>> };
>>
>> +struct kvm_destroy_device {
>> + __u32 fd; /* in: device handle */
>> + __u32 flags; /* in: unused */
>> +};
>> +
>> struct kvm_device_attr {
>> __u32 flags; /* no flags currently defined */
>> __u32 group; /* device-defined */
>> @@ -1331,6 +1336,8 @@ struct kvm_s390_ucas_mapping {
>> #define KVM_GET_DEVICE_ATTR _IOW(KVMIO, 0xe2, struct kvm_device_attr)
>> #define KVM_HAS_DEVICE_ATTR _IOW(KVMIO, 0xe3, struct kvm_device_attr)
>>
>> +#define KVM_DESTROY_DEVICE _IOWR(KVMIO, 0xf0, struct kvm_destroy_device)
>> +
>> /*
>> * ioctls for vcpu fds
>> */
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 84717d8cb5e4..983d5c37f710 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -3026,6 +3026,30 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
>> return 0;
>> }
>>
>> +static int kvm_ioctl_destroy_device(struct kvm *kvm,
>> + struct kvm_destroy_device *dd)
>> +{
>> + struct fd f;
>> + struct kvm_device *dev;
>> +
>> + f = fdget(dd->fd);
>> + if (!f.file)
>> + return -EBADF;
>> +
>> + dev = kvm_device_from_filp(f.file);
>> + fdput(f);
>> +
>> + if (!dev)
>> + return -ENODEV;
>
> Don't you need to verify that the device belongs to this KVM instance?
ah yes. I should at least check : dev->kvm == kvm
>> + mutex_lock(&kvm->lock);
>> + list_del(&dev->vm_node);
>> + dev->ops->destroy(dev);
>> + mutex_unlock(&kvm->lock);
And there, I should problably drop a ref count on the VM. I am not sure
of that.
Thanks,
C.
>> + return 0;
>> +}
>> +
>> static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
>> {
>> switch (arg) {
>> @@ -3270,6 +3294,20 @@ static long kvm_vm_ioctl(struct file *filp,
>> r = 0;
>> break;
>> }
>> + case KVM_DESTROY_DEVICE: {
>> + struct kvm_destroy_device dd;
>> +
>> + r = -EFAULT;
>> + if (copy_from_user(&dd, argp, sizeof(dd)))
>> + goto out;
>> +
>> + r = kvm_ioctl_destroy_device(kvm, &dd);
>> + if (r)
>> + goto out;
>> +
>> + r = 0;
>> + break;
>> + }
>> case KVM_CHECK_EXTENSION:
>> r = kvm_vm_ioctl_check_extension_generic(kvm, arg);
>> break;
>> diff --git a/Documentation/virtual/kvm/api.txt
>> b/Documentation/virtual/kvm/api.txt
>> index 1db1435769b4..c2ba18279298 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -3857,6 +3857,25 @@ number of valid entries in the 'entries' array, which
>> is then filled.
>> 'index' and 'flags' fields in 'struct kvm_cpuid_entry2' are currently
>> reserved,
>> userspace should not expect to get any particular value there.
>>
>> +4.119 KVM_DESTROY_DEVICE
>> +
>> +Capability: KVM_CAP_DEVICE_CTRL
>> +Type: vm ioctl
>> +Parameters: struct kvm_destroy_device (in)
>> +Returns: 0 on success, -1 on error
>> +Errors:
>> + ENODEV: The device type is unknown or unsupported
>> +
>> + Other error conditions may be defined by individual device types or
>> + have their standard meanings.
>> +
>> +Destroys an emulated device in the kernel.
>> +
>> +struct kvm_destroy_device {
>> + __u32 fd; /* in: device handle */
>> + __u32 flags; /* unused */
>> +};
>> +
>> 5. The kvm_run structure
>> ------------------------
>>
>
