lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 8 Jul 2022 13:38:45 +0200 From: Eugenio Perez Martin <eperezma@...hat.com> To: Stefano Garzarella <sgarzare@...hat.com> Cc: netdev <netdev@...r.kernel.org>, Jason Wang <jasowang@...hat.com>, linux-kernel <linux-kernel@...r.kernel.org>, virtualization <virtualization@...ts.linux-foundation.org>, kvm list <kvm@...r.kernel.org>, "Michael S. Tsirkin" <mst@...hat.com>, Parav Pandit <parav@...dia.com>, Christophe JAILLET <christophe.jaillet@...adoo.fr>, Cindy Lu <lulu@...hat.com>, "Kamde, Tanuj" <tanuj.kamde@....com>, Si-Wei Liu <si-wei.liu@...cle.com>, "Uminski, Piotr" <Piotr.Uminski@...el.com>, habetsm.xilinx@...il.com, "Dawar, Gautam" <gautam.dawar@....com>, Pablo Cascon Katchadourian <pabloc@...inx.com>, Zhu Lingshan <lingshan.zhu@...el.com>, Laurent Vivier <lvivier@...hat.com>, Longpeng <longpeng2@...wei.com>, Dinan Gunawardena <dinang@...inx.com>, Martin Petrus Hubertus Habets <martinh@...inx.com>, Martin Porter <martinpo@...inx.com>, Eli Cohen <elic@...dia.com>, ecree.xilinx@...il.com, Wu Zongyong <wuzongyong@...ux.alibaba.com>, Dan Carpenter <dan.carpenter@...cle.com>, Harpreet Singh Anand <hanand@...inx.com>, Xie Yongji <xieyongji@...edance.com>, Zhang Min <zhang.min9@....com.cn> Subject: Re: [PATCH v6 2/4] vhost-vdpa: introduce SUSPEND backend feature bit On Tue, Jun 28, 2022 at 3:43 PM Stefano Garzarella <sgarzare@...hat.com> wrote: > > On Thu, Jun 23, 2022 at 06:07:36PM +0200, Eugenio Pérez wrote: > >Userland knows if it can suspend the device or not by checking this feature > >bit. > > > >It's only offered if the vdpa driver backend implements the suspend() > >operation callback, and to offer it or userland to ack it if the backend > >does not offer that callback is an error. > > Should we document in the previous patch that the callback must be > implemented only if the drive/device support it? > It's marked as optional in the doc, following other optional callbacks like set_group_asid for example. But I'm ok with documenting this behavior further. > The rest LGTM although I have a doubt whether it is better to move this > patch after patch 3, or merge it with patch 3, for bisectability since > we enable the feature here but if the userspace calls ioctl() with > VHOST_VDPA_SUSPEND we reply back that it is not supported. > I'm fine with moving it, but we will have that behavior with all the devices anyway. Regarding userspace, we just replace ENOIOCTL with EOPNOTSUPP. Or I'm missing something? Thanks! > Thanks, > Stefano > > > > >Signed-off-by: Eugenio Pérez <eperezma@...hat.com> > >--- > > drivers/vhost/vdpa.c | 16 +++++++++++++++- > > include/uapi/linux/vhost_types.h | 2 ++ > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > >index 23dcbfdfa13b..3d636e192061 100644 > >--- a/drivers/vhost/vdpa.c > >+++ b/drivers/vhost/vdpa.c > >@@ -347,6 +347,14 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v, > > return 0; > > } > > > >+static bool vhost_vdpa_can_suspend(const struct vhost_vdpa *v) > >+{ > >+ struct vdpa_device *vdpa = v->vdpa; > >+ const struct vdpa_config_ops *ops = vdpa->config; > >+ > >+ return ops->suspend; > >+} > >+ > > static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user *featurep) > > { > > struct vdpa_device *vdpa = v->vdpa; > >@@ -577,7 +585,11 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, > > if (cmd == VHOST_SET_BACKEND_FEATURES) { > > if (copy_from_user(&features, featurep, sizeof(features))) > > return -EFAULT; > >- if (features & ~VHOST_VDPA_BACKEND_FEATURES) > >+ if (features & ~(VHOST_VDPA_BACKEND_FEATURES | > >+ BIT_ULL(VHOST_BACKEND_F_SUSPEND))) > >+ return -EOPNOTSUPP; > >+ if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) && > >+ !vhost_vdpa_can_suspend(v)) > > return -EOPNOTSUPP; > > vhost_set_backend_features(&v->vdev, features); > > return 0; > >@@ -628,6 +640,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, > > break; > > case VHOST_GET_BACKEND_FEATURES: > > features = VHOST_VDPA_BACKEND_FEATURES; > >+ if (vhost_vdpa_can_suspend(v)) > >+ features |= BIT_ULL(VHOST_BACKEND_F_SUSPEND); > > if (copy_to_user(featurep, &features, sizeof(features))) > > r = -EFAULT; > > break; > >diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h > >index 634cee485abb..1bdd6e363f4c 100644 > >--- a/include/uapi/linux/vhost_types.h > >+++ b/include/uapi/linux/vhost_types.h > >@@ -161,5 +161,7 @@ struct vhost_vdpa_iova_range { > > * message > > */ > > #define VHOST_BACKEND_F_IOTLB_ASID 0x3 > >+/* Device can be suspended */ > >+#define VHOST_BACKEND_F_SUSPEND 0x4 > > > > #endif > >-- > >2.31.1 > > >
Powered by blists - more mailing lists