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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJaqyWeiNWnWUzEUEo8HeuuF8XMPtKw9SapxLxLJECWJ0zNTUA@mail.gmail.com>
Date:   Tue, 24 May 2022 09:42:06 +0200
From:   Eugenio Perez Martin <eperezma@...hat.com>
To:     Stefano Garzarella <sgarzare@...hat.com>
Cc:     Si-Wei Liu <si-wei.liu@...cle.com>,
        virtualization <virtualization@...ts.linux-foundation.org>,
        Jason Wang <jasowang@...hat.com>,
        kvm list <kvm@...r.kernel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Longpeng <longpeng2@...wei.com>,
        Zhu Lingshan <lingshan.zhu@...el.com>,
        Martin Petrus Hubertus Habets <martinh@...inx.com>,
        Harpreet Singh Anand <hanand@...inx.com>, dinang@...inx.com,
        Eli Cohen <elic@...dia.com>,
        Laurent Vivier <lvivier@...hat.com>, pabloc@...inx.com,
        "Dawar, Gautam" <gautam.dawar@....com>,
        Xie Yongji <xieyongji@...edance.com>, habetsm.xilinx@...il.com,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        tanuj.kamde@....com, Wu Zongyong <wuzongyong@...ux.alibaba.com>,
        martinpo@...inx.com, Cindy Lu <lulu@...hat.com>,
        ecree.xilinx@...il.com, Parav Pandit <parav@...dia.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Zhang Min <zhang.min9@....com.cn>
Subject: Re: [PATCH 1/4] vdpa: Add stop operation

On Tue, May 24, 2022 at 9:09 AM Stefano Garzarella <sgarzare@...hat.com> wrote:
>
> On Mon, May 23, 2022 at 09:20:14PM +0200, Eugenio Perez Martin wrote:
> >On Sat, May 21, 2022 at 12:13 PM Si-Wei Liu <si-wei.liu@...cle.com> wrote:
> >>
> >>
> >>
> >> On 5/20/2022 10:23 AM, Eugenio Pérez wrote:
> >> > This operation is optional: It it's not implemented, backend feature bit
> >> > will not be exposed.
> >> >
> >> > Signed-off-by: Eugenio Pérez <eperezma@...hat.com>
> >> > ---
> >> >   include/linux/vdpa.h | 6 ++++++
> >> >   1 file changed, 6 insertions(+)
> >> >
> >> > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> >> > index 15af802d41c4..ddfebc4e1e01 100644
> >> > --- a/include/linux/vdpa.h
> >> > +++ b/include/linux/vdpa.h
> >> > @@ -215,6 +215,11 @@ struct vdpa_map_file {
> >> >    * @reset:                  Reset device
> >> >    *                          @vdev: vdpa device
> >> >    *                          Returns integer: success (0) or error (< 0)
> >> > + * @stop:                    Stop or resume the device (optional, but it must
> >> > + *                           be implemented if require device stop)
> >> > + *                           @vdev: vdpa device
> >> > + *                           @stop: stop (true), not stop (false)
> >> > + *                           Returns integer: success (0) or error (< 0)
> >> Is this uAPI meant to address all use cases described in the full blown
> >> _F_STOP virtio spec proposal, such as:
> >>
> >> --------------%<--------------
> >>
> >> ...... the device MUST finish any in flight
> >> operations after the driver writes STOP.  Depending on the device, it
> >> can do it
> >> in many ways as long as the driver can recover its normal operation
> >> if it
> >> resumes the device without the need of resetting it:
> >>
> >> - Drain and wait for the completion of all pending requests until a
> >>    convenient avail descriptor. Ignore any other posterior descriptor.
> >> - Return a device-specific failure for these descriptors, so the driver
> >>    can choose to retry or to cancel them.
> >> - Mark them as done even if they are not, if the kind of device can
> >>    assume to lose them.
> >> --------------%<--------------
> >>
> >
> >Right, this is totally underspecified in this series.
> >
> >I'll expand on it in the next version, but that text proposed to
> >virtio-comment was complicated and misleading. I find better to get
> >the previous version description. Would the next description work?
> >
> >```
> >After the return of ioctl, the device MUST finish any pending operations like
> >in flight requests. It must also preserve all the necessary state (the
> >virtqueue vring base plus the possible device specific states) that is required
> >for restoring in the future.
>
> For block devices wait for all in-flight requests could take several
> time.
>
> Could this be a problem if the caller gets stuck on this ioctl?
>
> If it could be a problem, maybe we should use an eventfd to signal that
> the device is successfully stopped.
>

For that particular problem I'd very much prefer to add directly an
ioctl to get the inflight descriptors. We know for sure we will need
them, and it will be cleaner in the long run.

As I understand the vdpa block simulator, there is no need to return
the inflight descriptors since all of the requests are processed in a
synchronous way. So, for this iteration, we could offer the stop
feature to qemu.

Other non-simulated devices would need it. Could it be delayed to
future development?

Thanks!

> Thanks,
> Stefano
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ