Re: [PATCH v2 4/4] vdpa_sim: Implement resume vdpa op

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Oct 17, 2022 at 11:44:44AM +0000, Boeuf, Sebastien wrote:
> On Mon, 2022-10-17 at 13:05 +0200, Eugenio Perez Martin wrote:
> > On Mon, Oct 17, 2022 at 10:47 AM Boeuf, Sebastien
> > <sebastien.boeuf@intel.com> wrote:
> > > 
> > > On Mon, 2022-10-17 at 15:26 +0800, Jason Wang wrote:
> > > > On Fri, Oct 14, 2022 at 7:15 PM <sebastien.boeuf@intel.com>
> > > > wrote:
> > > > > 
> > > > > From: Sebastien Boeuf <sebastien.boeuf@intel.com>
> > > > > 
> > > > > Implement resume operation for vdpa_sim devices, so vhost-vdpa
> > > > > will
> > > > > offer that backend feature and userspace can effectively resume
> > > > > the
> > > > > device.
> > > > > 
> > > > > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
> > > > > ---
> > > > >  drivers/vdpa/vdpa_sim/vdpa_sim.c | 13 +++++++++++++
> > > > >  1 file changed, 13 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > > > > b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > > > > index b071f0d842fb..195dc60bad3c 100644
> > > > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > > > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > > > > @@ -527,6 +527,17 @@ static int vdpasim_suspend(struct
> > > > > vdpa_device
> > > > > *vdpa)
> > > > >         return 0;
> > > > >  }
> > > > > 
> > > > > +static int vdpasim_resume(struct vdpa_device *vdpa)
> > > > > +{
> > > > > +       struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> > > > > +
> > > > > +       spin_lock(&vdpasim->lock);
> > > > > +       vdpasim->running = true;
> > > > > +       spin_unlock(&vdpasim->lock);
> > > > 
> > > > Do we need to schedule work here? Or the assumption is that the
> > > > individual should take care to make sure no kick is lost during a
> > > > suspend/resume?
> > > 
> > > The guest vCPU should be paused, meaning no kick should come from
> > > the
> > > guest while the backend is suspended. But I don't think it would
> > > cause
> > > any harm to schedule some work when resuming the device as this
> > > would
> > > provide a more permissive implementation.
> > > 
> > > Please let me know what you think about it.
> > > 
> > 
> > We should enable the case where the device is suspended by the VMM
> > but
> > the vCPU continues running in my opinion.
> > 
> > I preemptively scheduled work to accommodate this use case in
> > previous
> > versions of the suspend patch [1].
> > 
> > [1] https://lkml.org/lkml/2022/5/24/768
> > 
> > Thanks!
> > 
> 
> Sounds good, I'm going to update the patch to kick the queues on
> resume.

I am not sure the right thing is to kick on resume, that
will bring all kind of issues around startup.
Save and restore state fully.


> Thanks,
> Sebastien
> ---------------------------------------------------------------------
> Intel Corporation SAS (French simplified joint stock company)
> Registered headquarters: "Les Montalets"- 2, rue de Paris, 
> 92196 Meudon Cedex, France
> Registration Number:  302 456 199 R.C.S. NANTERRE
> Capital: 5 208 026.16 Euros
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization