| Message ID | 20220623160738.632852-5-eperezma@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Implement vdpasim suspend operation | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Jun 24, 2022 at 12:08 AM Eugenio Pérez <eperezma@redhat.com> wrote: > > Implement suspend operation for vdpa_sim devices, so vhost-vdpa will > offer that backend feature and userspace can effectively suspend the > device. > > This is a must before get virtqueue indexes (base) for live migration, > since the device could modify them after userland gets them. There are > individual ways to perform that action for some devices > (VHOST_NET_SET_BACKEND, VHOST_VSOCK_SET_RUNNING, ...) but there was no > way to perform it for any vhost device (and, in particular, vhost-vdpa). > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > drivers/vdpa/vdpa_sim/vdpa_sim.c | 21 +++++++++++++++++++++ > drivers/vdpa/vdpa_sim/vdpa_sim.h | 1 + > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 3 +++ > drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 3 +++ > 4 files changed, 28 insertions(+) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c > index 0f2865899647..213883487f9b 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c > @@ -107,6 +107,7 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim) > for (i = 0; i < vdpasim->dev_attr.nas; i++) > vhost_iotlb_reset(&vdpasim->iommu[i]); > > + vdpasim->running = true; > spin_unlock(&vdpasim->iommu_lock); > > vdpasim->features = 0; > @@ -505,6 +506,24 @@ static int vdpasim_reset(struct vdpa_device *vdpa) > return 0; > } > > +static int vdpasim_suspend(struct vdpa_device *vdpa) > +{ > + struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > + int i; > + > + spin_lock(&vdpasim->lock); > + vdpasim->running = false; > + if (vdpasim->running) { > + /* Check for missed buffers */ > + for (i = 0; i < vdpasim->dev_attr.nvqs; ++i) > + vdpasim_kick_vq(vdpa, i); This seems only valid if we allow resuming? Thanks > + > + } > + spin_unlock(&vdpasim->lock); > + > + return 0; > +} > + > static size_t vdpasim_get_config_size(struct vdpa_device *vdpa) > { > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > @@ -694,6 +713,7 @@ static const struct vdpa_config_ops vdpasim_config_ops = { > .get_status = vdpasim_get_status, > .set_status = vdpasim_set_status, > .reset = vdpasim_reset, > + .suspend = vdpasim_suspend, > .get_config_size = vdpasim_get_config_size, > .get_config = vdpasim_get_config, > .set_config = vdpasim_set_config, > @@ -726,6 +746,7 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = { > .get_status = vdpasim_get_status, > .set_status = vdpasim_set_status, > .reset = vdpasim_reset, > + .suspend = vdpasim_suspend, > .get_config_size = vdpasim_get_config_size, > .get_config = vdpasim_get_config, > .set_config = vdpasim_set_config, > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h > index 622782e92239..061986f30911 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h > @@ -66,6 +66,7 @@ struct vdpasim { > u32 generation; > u64 features; > u32 groups; > + bool running; > /* spinlock to synchronize iommu table */ > spinlock_t iommu_lock; > }; > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > index 42d401d43911..bcdb1982c378 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > @@ -204,6 +204,9 @@ static void vdpasim_blk_work(struct work_struct *work) > if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) > goto out; > > + if (!vdpasim->running) > + goto out; > + > for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { > struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c > index 5125976a4df8..886449e88502 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c > @@ -154,6 +154,9 @@ static void vdpasim_net_work(struct work_struct *work) > > spin_lock(&vdpasim->lock); > > + if (!vdpasim->running) > + goto out; > + > if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) > goto out; > > -- > 2.31.1 >
On Wed, Jun 29, 2022 at 6:18 AM Jason Wang <jasowang@redhat.com> wrote: > > On Fri, Jun 24, 2022 at 12:08 AM Eugenio Pérez <eperezma@redhat.com> wrote: > > > > Implement suspend operation for vdpa_sim devices, so vhost-vdpa will > > offer that backend feature and userspace can effectively suspend the > > device. > > > > This is a must before get virtqueue indexes (base) for live migration, > > since the device could modify them after userland gets them. There are > > individual ways to perform that action for some devices > > (VHOST_NET_SET_BACKEND, VHOST_VSOCK_SET_RUNNING, ...) but there was no > > way to perform it for any vhost device (and, in particular, vhost-vdpa). > > > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > --- > > drivers/vdpa/vdpa_sim/vdpa_sim.c | 21 +++++++++++++++++++++ > > drivers/vdpa/vdpa_sim/vdpa_sim.h | 1 + > > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 3 +++ > > drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 3 +++ > > 4 files changed, 28 insertions(+) > > > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c > > index 0f2865899647..213883487f9b 100644 > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c > > @@ -107,6 +107,7 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim) > > for (i = 0; i < vdpasim->dev_attr.nas; i++) > > vhost_iotlb_reset(&vdpasim->iommu[i]); > > > > + vdpasim->running = true; > > spin_unlock(&vdpasim->iommu_lock); > > > > vdpasim->features = 0; > > @@ -505,6 +506,24 @@ static int vdpasim_reset(struct vdpa_device *vdpa) > > return 0; > > } > > > > +static int vdpasim_suspend(struct vdpa_device *vdpa) > > +{ > > + struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > + int i; > > + > > + spin_lock(&vdpasim->lock); > > + vdpasim->running = false; > > + if (vdpasim->running) { > > + /* Check for missed buffers */ > > + for (i = 0; i < vdpasim->dev_attr.nvqs; ++i) > > + vdpasim_kick_vq(vdpa, i); > > This seems only valid if we allow resuming? > Right, deleting for v7. Thanks! > Thanks > > > + > > + } > > + spin_unlock(&vdpasim->lock); > > + > > + return 0; > > +} > > + > > static size_t vdpasim_get_config_size(struct vdpa_device *vdpa) > > { > > struct vdpasim *vdpasim = vdpa_to_sim(vdpa); > > @@ -694,6 +713,7 @@ static const struct vdpa_config_ops vdpasim_config_ops = { > > .get_status = vdpasim_get_status, > > .set_status = vdpasim_set_status, > > .reset = vdpasim_reset, > > + .suspend = vdpasim_suspend, > > .get_config_size = vdpasim_get_config_size, > > .get_config = vdpasim_get_config, > > .set_config = vdpasim_set_config, > > @@ -726,6 +746,7 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = { > > .get_status = vdpasim_get_status, > > .set_status = vdpasim_set_status, > > .reset = vdpasim_reset, > > + .suspend = vdpasim_suspend, > > .get_config_size = vdpasim_get_config_size, > > .get_config = vdpasim_get_config, > > .set_config = vdpasim_set_config, > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h > > index 622782e92239..061986f30911 100644 > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h > > @@ -66,6 +66,7 @@ struct vdpasim { > > u32 generation; > > u64 features; > > u32 groups; > > + bool running; > > /* spinlock to synchronize iommu table */ > > spinlock_t iommu_lock; > > }; > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > > index 42d401d43911..bcdb1982c378 100644 > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > > @@ -204,6 +204,9 @@ static void vdpasim_blk_work(struct work_struct *work) > > if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) > > goto out; > > > > + if (!vdpasim->running) > > + goto out; > > + > > for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { > > struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; > > > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c > > index 5125976a4df8..886449e88502 100644 > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c > > @@ -154,6 +154,9 @@ static void vdpasim_net_work(struct work_struct *work) > > > > spin_lock(&vdpasim->lock); > > > > + if (!vdpasim->running) > > + goto out; > > + > > if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) > > goto out; > > > > -- > > 2.31.1 > > >
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c index 0f2865899647..213883487f9b 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c @@ -107,6 +107,7 @@ static void vdpasim_do_reset(struct vdpasim *vdpasim) for (i = 0; i < vdpasim->dev_attr.nas; i++) vhost_iotlb_reset(&vdpasim->iommu[i]); + vdpasim->running = true; spin_unlock(&vdpasim->iommu_lock); vdpasim->features = 0; @@ -505,6 +506,24 @@ static int vdpasim_reset(struct vdpa_device *vdpa) return 0; } +static int vdpasim_suspend(struct vdpa_device *vdpa) +{ + struct vdpasim *vdpasim = vdpa_to_sim(vdpa); + int i; + + spin_lock(&vdpasim->lock); + vdpasim->running = false; + if (vdpasim->running) { + /* Check for missed buffers */ + for (i = 0; i < vdpasim->dev_attr.nvqs; ++i) + vdpasim_kick_vq(vdpa, i); + + } + spin_unlock(&vdpasim->lock); + + return 0; +} + static size_t vdpasim_get_config_size(struct vdpa_device *vdpa) { struct vdpasim *vdpasim = vdpa_to_sim(vdpa); @@ -694,6 +713,7 @@ static const struct vdpa_config_ops vdpasim_config_ops = { .get_status = vdpasim_get_status, .set_status = vdpasim_set_status, .reset = vdpasim_reset, + .suspend = vdpasim_suspend, .get_config_size = vdpasim_get_config_size, .get_config = vdpasim_get_config, .set_config = vdpasim_set_config, @@ -726,6 +746,7 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = { .get_status = vdpasim_get_status, .set_status = vdpasim_set_status, .reset = vdpasim_reset, + .suspend = vdpasim_suspend, .get_config_size = vdpasim_get_config_size, .get_config = vdpasim_get_config, .set_config = vdpasim_set_config, diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h index 622782e92239..061986f30911 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h @@ -66,6 +66,7 @@ struct vdpasim { u32 generation; u64 features; u32 groups; + bool running; /* spinlock to synchronize iommu table */ spinlock_t iommu_lock; }; diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c index 42d401d43911..bcdb1982c378 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c @@ -204,6 +204,9 @@ static void vdpasim_blk_work(struct work_struct *work) if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) goto out; + if (!vdpasim->running) + goto out; + for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c index 5125976a4df8..886449e88502 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c @@ -154,6 +154,9 @@ static void vdpasim_net_work(struct work_struct *work) spin_lock(&vdpasim->lock); + if (!vdpasim->running) + goto out; + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK)) goto out;