To migrate vDPA-net device state though CVQ we need to start the dataplane
after the control virtqueue.
As vdpa frontend doesn't know if the parent vdpa driver supports it, let's
allow them to expose custom backend features by themselves.
Comments are welcome.
Thanks!
v2: address doc issues from checkpath and fix lack of signed-off-by.
v1: from RFC: Tweak doc as Shannon suggestion.
Eugenio Pérez (4):
vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
vdpa: add get_backend_features vdpa operation
vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
drivers/vdpa/vdpa_sim/vdpa_sim.c | 8 ++++++++
drivers/vhost/vdpa.c | 15 ++++++++++++++-
include/linux/vdpa.h | 4 ++++
include/uapi/linux/vhost_types.h | 4 ++++
4 files changed, 30 insertions(+), 1 deletion(-)
--
2.31.1
This feature flag allows the driver enabling virtqueues both before and
after DRIVER_OK.
This is needed for software assisted live migration, so userland can
restore the device status in devices with control virtqueue before the
dataplane is enabled.
Signed-off-by: Eugenio Pérez <[email protected]>
Acked-by: Shannon Nelson <[email protected]>
---
include/uapi/linux/vhost_types.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
index c5690a8992d8..4889e6d70b15 100644
--- a/include/uapi/linux/vhost_types.h
+++ b/include/uapi/linux/vhost_types.h
@@ -165,5 +165,9 @@ struct vhost_vdpa_iova_range {
#define VHOST_BACKEND_F_SUSPEND 0x4
/* Device can be resumed */
#define VHOST_BACKEND_F_RESUME 0x5
+/* Device supports the driver enabling virtqueues both before and after
+ * DRIVER_OK
+ */
+#define VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK 0x6
#endif
--
2.31.1
Accepting VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature if
userland sets it.
Signed-off-by: Eugenio Pérez <[email protected]>
Acked-by: Shannon Nelson <[email protected]>
---
drivers/vhost/vdpa.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index bf77924d5b60..a3204406b73d 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -680,7 +680,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
return -EFAULT;
if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
- BIT_ULL(VHOST_BACKEND_F_RESUME)))
+ BIT_ULL(VHOST_BACKEND_F_RESUME) |
+ BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK)))
return -EOPNOTSUPP;
if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
!vhost_vdpa_can_suspend(v))
--
2.31.1
Start offering the feature in the simulator. Other parent drivers can
follow this code to offer it too.
Signed-off-by: Eugenio Pérez <[email protected]>
Acked-by: Shannon Nelson <[email protected]>
---
drivers/vdpa/vdpa_sim/vdpa_sim.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index d343af4fa60e..76d41058add9 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -18,6 +18,7 @@
#include <linux/vdpa.h>
#include <linux/vhost_iotlb.h>
#include <uapi/linux/vdpa.h>
+#include <uapi/linux/vhost_types.h>
#include "vdpa_sim.h"
@@ -410,6 +411,11 @@ static u64 vdpasim_get_device_features(struct vdpa_device *vdpa)
return vdpasim->dev_attr.supported_features;
}
+static u64 vdpasim_get_backend_features(const struct vdpa_device *vdpa)
+{
+ return BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK);
+}
+
static int vdpasim_set_driver_features(struct vdpa_device *vdpa, u64 features)
{
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
@@ -733,6 +739,7 @@ static const struct vdpa_config_ops vdpasim_config_ops = {
.get_vq_align = vdpasim_get_vq_align,
.get_vq_group = vdpasim_get_vq_group,
.get_device_features = vdpasim_get_device_features,
+ .get_backend_features = vdpasim_get_backend_features,
.set_driver_features = vdpasim_set_driver_features,
.get_driver_features = vdpasim_get_driver_features,
.set_config_cb = vdpasim_set_config_cb,
@@ -770,6 +777,7 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
.get_vq_align = vdpasim_get_vq_align,
.get_vq_group = vdpasim_get_vq_group,
.get_device_features = vdpasim_get_device_features,
+ .get_backend_features = vdpasim_get_backend_features,
.set_driver_features = vdpasim_set_driver_features,
.get_driver_features = vdpasim_get_driver_features,
.set_config_cb = vdpasim_set_config_cb,
--
2.31.1
This operation allow vdpa parent to expose its own backend feature bits.
Next patches introduce a feature not compatible with all parent drivers:
the ability to enable vq after driver_ok. Each parent must declare if
it allows it or not.
Signed-off-by: Eugenio Pérez <[email protected]>
Acked-by: Shannon Nelson <[email protected]>
---
drivers/vhost/vdpa.c | 12 ++++++++++++
include/linux/vdpa.h | 4 ++++
2 files changed, 16 insertions(+)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index a3204406b73d..e1abf29fed5b 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -403,6 +403,17 @@ static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user *featurep)
return 0;
}
+static u64 vhost_vdpa_get_backend_features(const struct vhost_vdpa *v)
+{
+ struct vdpa_device *vdpa = v->vdpa;
+ const struct vdpa_config_ops *ops = vdpa->config;
+
+ if (!ops->get_backend_features)
+ return 0;
+ else
+ return ops->get_backend_features(vdpa);
+}
+
static long vhost_vdpa_set_features(struct vhost_vdpa *v, u64 __user *featurep)
{
struct vdpa_device *vdpa = v->vdpa;
@@ -742,6 +753,7 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
features |= BIT_ULL(VHOST_BACKEND_F_SUSPEND);
if (vhost_vdpa_can_resume(v))
features |= BIT_ULL(VHOST_BACKEND_F_RESUME);
+ features |= vhost_vdpa_get_backend_features(v);
if (copy_to_user(featurep, &features, sizeof(features)))
r = -EFAULT;
break;
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index db1b0eaef4eb..0e652026b776 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -208,6 +208,9 @@ struct vdpa_map_file {
* @vdev: vdpa device
* Returns the virtio features support by the
* device
+ * @get_backend_features: Get parent-specific backend features (optional)
+ * Returns the vdpa features supported by the
+ * device.
* @set_driver_features: Set virtio features supported by the driver
* @vdev: vdpa device
* @features: feature support by the driver
@@ -358,6 +361,7 @@ struct vdpa_config_ops {
u32 (*get_vq_align)(struct vdpa_device *vdev);
u32 (*get_vq_group)(struct vdpa_device *vdev, u16 idx);
u64 (*get_device_features)(struct vdpa_device *vdev);
+ u64 (*get_backend_features)(const struct vdpa_device *vdev);
int (*set_driver_features)(struct vdpa_device *vdev, u64 features);
u64 (*get_driver_features)(struct vdpa_device *vdev);
void (*set_config_cb)(struct vdpa_device *vdev,
--
2.31.1
On Fri, Jun 09, 2023 at 11:21:25AM +0200, Eugenio P?rez wrote:
> Accepting VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature if
> userland sets it.
>
> Signed-off-by: Eugenio P?rez <[email protected]>
> Acked-by: Shannon Nelson <[email protected]>
I don't get it, so all vdpa devices accept this automatically?
Should this not be up to the parent?
> ---
> drivers/vhost/vdpa.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index bf77924d5b60..a3204406b73d 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -680,7 +680,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> return -EFAULT;
> if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
> BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
> - BIT_ULL(VHOST_BACKEND_F_RESUME)))
> + BIT_ULL(VHOST_BACKEND_F_RESUME) |
> + BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK)))
> return -EOPNOTSUPP;
> if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
> !vhost_vdpa_can_suspend(v))
> --
> 2.31.1
On Fri, Jun 9, 2023 at 6:13 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Fri, Jun 09, 2023 at 11:21:25AM +0200, Eugenio Pérez wrote:
> > Accepting VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature if
> > userland sets it.
> >
> > Signed-off-by: Eugenio Pérez <[email protected]>
> > Acked-by: Shannon Nelson <[email protected]>
>
> I don't get it, so all vdpa devices accept this automatically?
> Should this not be up to the parent?
>
At the moment I don't see a reason why if a parent offers this
feature, it could reject it afterwards. However I think we can add a
fail if userland acks the backend feature but the parent does not
offer it however.
Would it work to add such fail in vdpa frontend and move it to the
backend if and when any parent driver needs it in the future?
Thanks!
> > ---
> > drivers/vhost/vdpa.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > index bf77924d5b60..a3204406b73d 100644
> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
> > @@ -680,7 +680,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> > return -EFAULT;
> > if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
> > BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
> > - BIT_ULL(VHOST_BACKEND_F_RESUME)))
> > + BIT_ULL(VHOST_BACKEND_F_RESUME) |
> > + BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK)))
> > return -EOPNOTSUPP;
> > if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
> > !vhost_vdpa_can_suspend(v))
> > --
> > 2.31.1
>