From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Subject: Re: [PATCH v2 15/16] KVM: introduce a KVM_DESTROY_DEVICE ioctl Date: Mon, 25 Feb 2019 15:15:07 +1100 Message-ID: <20190225041507.GS7668@umbus.fritz.box> References: <20190222112840.25000-1-clg@kaod.org> <20190222112840.25000-16-clg@kaod.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="szdyR02yM8NCQUEm" Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, Paul Mackerras , linuxppc-dev@lists.ozlabs.org To: =?iso-8859-1?Q?C=E9dric?= Le Goater Return-path: Content-Disposition: inline In-Reply-To: <20190222112840.25000-16-clg@kaod.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" List-Id: kvm.vger.kernel.org --szdyR02yM8NCQUEm Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Feb 22, 2019 at 12:28:39PM +0100, C=E9dric Le Goater wrote: > The 'destroy' method is currently used to destroy all devices when the > VM is destroyed after the vCPUs have been freed. >=20 > 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. >=20 > The 'destroy' method could be improved by returning an error code. >=20 > Signed-off-by: C=E9dric Le Goater 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(+) >=20 > 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 */ > }; > =20 > +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) > =20 > +#define KVM_DESTROY_DEVICE _IOWR(KVMIO, 0xf0, struct kvm_destroy_devi= ce) > + > /* > * 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; > } > =20 > +static int kvm_ioctl_destroy_device(struct kvm *kvm, > + struct kvm_destroy_device *dd) > +{ > + struct fd f; > + struct kvm_device *dev; > + > + f =3D fdget(dd->fd); > + if (!f.file) > + return -EBADF; > + > + dev =3D 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 a= rg) > { > switch (arg) { > @@ -3270,6 +3294,20 @@ static long kvm_vm_ioctl(struct file *filp, > r =3D 0; > break; > } > + case KVM_DESTROY_DEVICE: { > + struct kvm_destroy_device dd; > + > + r =3D -EFAULT; > + if (copy_from_user(&dd, argp, sizeof(dd))) > + goto out; > + > + r =3D kvm_ioctl_destroy_device(kvm, &dd); > + if (r) > + goto out; > + > + r =3D 0; > + break; > + } > case KVM_CHECK_EXTENSION: > r =3D kvm_vm_ioctl_check_extension_generic(kvm, arg); > break; > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kv= m/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, wh= ich is then filled. > 'index' and 'flags' fields in 'struct kvm_cpuid_entry2' are currently re= served, > userspace should not expect to get any particular value there. > =20 > +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 > ------------------------ > =20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --szdyR02yM8NCQUEm Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEdfRlhq5hpmzETofcbDjKyiDZs5IFAlxza8oACgkQbDjKyiDZ s5L9wQ/7BYKQKJJ1z6IMH2MuO8VHMKZyEaoLVcfyA5mCANl8eq1Kfqj/qWVd9zLl 1af5sGXIzp1esAQuKVX5zBk4epWDFc29xh8oesPVrd72gIV6CcLsciAsbSeEmVzP P0r39crlCG2bFBEFJdRGJx/TLySbxYU4ZgFqoSDG6mxyD9btRgB3Meb0sd+k0w8t 9Jfl++VnrT8lB5pRUqLyUPioc89yJ1NgzRryHV9YwhNSOZdkaJYXJ7bMKdGFq4m9 QtiPZaacFPTTbxZJ9YzPnV3sEyh+BbW/rYyPn1/wNdtJMWUfASu6hkMbzEl6gzc/ lSp7MliTTNpE9x4MxR6v/iyEXza8vWIubZuw3DEZMrPscQVxSviGcgV/9pI/ihlz 9nTustpENQz4GG4x1ReBdCr4NCKejvyrRLv1Nvd/HiH0uDNjjVJ/GPxmUnRAXJZf knlypP7FlF7pbR4Nqzmn5nQBNYoOXmGHmJjIOcs8cPnAKlLGEFP3ROOscrZfRXDP aHmQxRibc61xTeuT+mOWUDzUPlJMpBaiy/7b8b2Jt7YJyNnl/uGOLQ2xqnCFmwQc DbBCdmhA7ZQCHPZ3oZa+UNF8u5cFStSBquLd7mIrI39kfeCUNKZdM/ata07EZcVk oymyZNGta4Q1pXTZ70ZfGluMQ10odbgRLYQr/SAJlsZW+vgY3bk= =wK9Q -----END PGP SIGNATURE----- --szdyR02yM8NCQUEm-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Date: Mon, 25 Feb 2019 04:15:07 +0000 Subject: Re: [PATCH v2 15/16] KVM: introduce a KVM_DESTROY_DEVICE ioctl Message-Id: <20190225041507.GS7668@umbus.fritz.box> MIME-Version: 1 Content-Type: multipart/mixed; boundary="szdyR02yM8NCQUEm" List-Id: References: <20190222112840.25000-1-clg@kaod.org> <20190222112840.25000-16-clg@kaod.org> In-Reply-To: <20190222112840.25000-16-clg@kaod.org> To: =?iso-8859-1?Q?C=E9dric?= Le Goater Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, Paul Mackerras , linuxppc-dev@lists.ozlabs.org --szdyR02yM8NCQUEm Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Feb 22, 2019 at 12:28:39PM +0100, C=E9dric Le Goater wrote: > The 'destroy' method is currently used to destroy all devices when the > VM is destroyed after the vCPUs have been freed. >=20 > 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. >=20 > The 'destroy' method could be improved by returning an error code. >=20 > Signed-off-by: C=E9dric Le Goater 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(+) >=20 > 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 */ > }; > =20 > +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) > =20 > +#define KVM_DESTROY_DEVICE _IOWR(KVMIO, 0xf0, struct kvm_destroy_devi= ce) > + > /* > * 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; > } > =20 > +static int kvm_ioctl_destroy_device(struct kvm *kvm, > + struct kvm_destroy_device *dd) > +{ > + struct fd f; > + struct kvm_device *dev; > + > + f =3D fdget(dd->fd); > + if (!f.file) > + return -EBADF; > + > + dev =3D 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 a= rg) > { > switch (arg) { > @@ -3270,6 +3294,20 @@ static long kvm_vm_ioctl(struct file *filp, > r =3D 0; > break; > } > + case KVM_DESTROY_DEVICE: { > + struct kvm_destroy_device dd; > + > + r =3D -EFAULT; > + if (copy_from_user(&dd, argp, sizeof(dd))) > + goto out; > + > + r =3D kvm_ioctl_destroy_device(kvm, &dd); > + if (r) > + goto out; > + > + r =3D 0; > + break; > + } > case KVM_CHECK_EXTENSION: > r =3D kvm_vm_ioctl_check_extension_generic(kvm, arg); > break; > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kv= m/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, wh= ich is then filled. > 'index' and 'flags' fields in 'struct kvm_cpuid_entry2' are currently re= served, > userspace should not expect to get any particular value there. > =20 > +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 > ------------------------ > =20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --szdyR02yM8NCQUEm Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEdfRlhq5hpmzETofcbDjKyiDZs5IFAlxza8oACgkQbDjKyiDZ s5L9wQ/7BYKQKJJ1z6IMH2MuO8VHMKZyEaoLVcfyA5mCANl8eq1Kfqj/qWVd9zLl 1af5sGXIzp1esAQuKVX5zBk4epWDFc29xh8oesPVrd72gIV6CcLsciAsbSeEmVzP P0r39crlCG2bFBEFJdRGJx/TLySbxYU4ZgFqoSDG6mxyD9btRgB3Meb0sd+k0w8t 9Jfl++VnrT8lB5pRUqLyUPioc89yJ1NgzRryHV9YwhNSOZdkaJYXJ7bMKdGFq4m9 QtiPZaacFPTTbxZJ9YzPnV3sEyh+BbW/rYyPn1/wNdtJMWUfASu6hkMbzEl6gzc/ lSp7MliTTNpE9x4MxR6v/iyEXza8vWIubZuw3DEZMrPscQVxSviGcgV/9pI/ihlz 9nTustpENQz4GG4x1ReBdCr4NCKejvyrRLv1Nvd/HiH0uDNjjVJ/GPxmUnRAXJZf knlypP7FlF7pbR4Nqzmn5nQBNYoOXmGHmJjIOcs8cPnAKlLGEFP3ROOscrZfRXDP aHmQxRibc61xTeuT+mOWUDzUPlJMpBaiy/7b8b2Jt7YJyNnl/uGOLQ2xqnCFmwQc DbBCdmhA7ZQCHPZ3oZa+UNF8u5cFStSBquLd7mIrI39kfeCUNKZdM/ata07EZcVk oymyZNGta4Q1pXTZ70ZfGluMQ10odbgRLYQr/SAJlsZW+vgY3bk= =wK9Q -----END PGP SIGNATURE----- --szdyR02yM8NCQUEm--