netdev - Re: [PATCH vhost v4 10/10] virtio_ring: virtqueue_set_dma

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: <CACGkMEs+x8bObJ0Fr0LbkPzWqYSoU8Y8504=bqZtjux2T5-_Vg@mail.gmail.com>
Date: Fri, 22 Mar 2024 13:13:36 +0800
From: Jason Wang <jasowang@...hat.com>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Cc: virtualization@...ts.linux.dev, "Michael S. Tsirkin" <mst@...hat.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Subject: Re: [PATCH vhost v4 10/10] virtio_ring: virtqueue_set_dma_premapped
 support disable

On Thu, Mar 21, 2024 at 4:22 PM Xuan Zhuo <xuanzhuo@...ux.alibaba.com> wrote:
>
> On Thu, 21 Mar 2024 14:02:14 +0800, Jason Wang <jasowang@...hat.com> wrote:
> > On Tue, Mar 12, 2024 at 11:36 AM Xuan Zhuo <xuanzhuo@...ux.alibaba.com> wrote:
> > >
> > > Now, the API virtqueue_set_dma_premapped just support to
> > > enable premapped mode.
> > >
> > > If we allow enabling the premapped dynamically, we should
> > > make this API to support disable the premapped mode.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> > > ---
> > >  drivers/virtio/virtio_ring.c | 34 ++++++++++++++++++++++++++--------
> > >  include/linux/virtio.h       |  2 +-
> > >  2 files changed, 27 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index 34f4b2c0c31e..3bf69cae4965 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -2801,6 +2801,7 @@ EXPORT_SYMBOL_GPL(virtqueue_resize);
> > >  /**
> > >   * virtqueue_set_dma_premapped - set the vring premapped mode
> > >   * @_vq: the struct virtqueue we're talking about.
> > > + * @premapped: enable/disable the premapped mode.
> > >   *
> > >   * Enable the premapped mode of the vq.
> > >   *
> > > @@ -2819,9 +2820,10 @@ EXPORT_SYMBOL_GPL(virtqueue_resize);
> > >   * 0: success.
> > >   * -EINVAL: vring does not use the dma api, so we can not enable premapped mode.
> > >   */
> > > -int virtqueue_set_dma_premapped(struct virtqueue *_vq)
> > > +int virtqueue_set_dma_premapped(struct virtqueue *_vq, bool premapped)
> >
> > I think we need to document the requirement for calling this.
> >
> > Looking at the code, it seems it requires to stop the datapath and
> > detach all the used buffers?
>
>
> YES. The complete document is:
>
> /**
>  * virtqueue_set_dma_premapped - set the vring premapped mode
>  * @_vq: the struct virtqueue we're talking about.
>  *
>  * Enable the premapped mode of the vq.
>  *
>  * The vring in premapped mode does not do dma internally, so the driver must
>  * do dma mapping in advance. The driver must pass the dma_address through
>  * dma_address of scatterlist. When the driver got a used buffer from
>  * the vring, it has to unmap the dma address.
>  *
>  * This function must be called immediately after creating the vq, or after vq
>  * reset, and before adding any buffers to it.

I'm not sure this is a good design but we need at least some guard for
this, probably WARN for num_added or others.

Thanks

>  *
>  * Caller must ensure we don't call this with other virtqueue operations
>  * at the same time (except where noted).
>  *
>  * Returns zero or a negative error.
>  * 0: success.
>  * -EINVAL: vring does not use the dma api, so we can not enable premapped mode.
>  */
>
> Thanks
>
>
> >
> > Thanks
> >
>