| Message ID | 20190222112840.25000-16-clg@kaod.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KVM: PPC: Book3S HV: add XIVE native exploitation mode | expand |
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 <clg@kaod.org> Again, has this been discussed with Paolo and/or other KVM core people? > --- > 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? > + mutex_lock(&kvm->lock); > + list_del(&dev->vm_node); > + dev->ops->destroy(dev); > + mutex_unlock(&kvm->lock); > + > + 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 > ------------------------ >
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 <clg@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 >> ------------------------ >> >
On 13/03/19 09:02, 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 <clg@kaod.org> Looks good to me. Paolo
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; + + mutex_lock(&kvm->lock); + list_del(&dev->vm_node); + dev->ops->destroy(dev); + mutex_unlock(&kvm->lock); + + 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 ------------------------
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 <clg@kaod.org> --- include/uapi/linux/kvm.h | 7 ++++++ virt/kvm/kvm_main.c | 38 +++++++++++++++++++++++++++++++ Documentation/virtual/kvm/api.txt | 19 ++++++++++++++++ 3 files changed, 64 insertions(+)