| 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
| ||
|
Message-ID: <4b32d0b4-b794-cc1c-25f7-50b5ea6ac25e@redhat.com>
Date: Wed, 9 Mar 2022 14:46:01 +0800
From: Jason Wang <jasowang@...hat.com>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org
Cc: Jeff Dike <jdike@...toit.com>, Richard Weinberger <richard@....at>,
Anton Ivanov <anton.ivanov@...bridgegreys.com>,
"Michael S. Tsirkin" <mst@...hat.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Hans de Goede <hdegoede@...hat.com>,
Mark Gross <markgross@...nel.org>,
Vadim Pasternak <vadimp@...dia.com>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Cornelia Huck <cohuck@...hat.com>,
Halil Pasic <pasic@...ux.ibm.com>,
Heiko Carstens <hca@...ux.ibm.com>,
Vasily Gorbik <gor@...ux.ibm.com>,
Christian Borntraeger <borntraeger@...ux.ibm.com>,
Alexander Gordeev <agordeev@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
Johannes Berg <johannes.berg@...el.com>,
Vincent Whitchurch <vincent.whitchurch@...s.com>,
linux-um@...ts.infradead.org, platform-driver-x86@...r.kernel.org,
linux-remoteproc@...r.kernel.org, linux-s390@...r.kernel.org,
kvm@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH v7 04/26] virtio_ring: split: extract the logic of
creating vring
在 2022/3/8 下午8:34, Xuan Zhuo 写道:
> Separate the logic of split to create vring queue.
>
> For the convenience of passing parameters, add a structure
> vring_split.
>
> This feature is required for subsequent virtuqueue reset vring.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> ---
> drivers/virtio/virtio_ring.c | 74 +++++++++++++++++++++++++-----------
> 1 file changed, 51 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index b87130c8f312..d32793615451 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -85,6 +85,13 @@ struct vring_desc_extra {
> u16 next; /* The next desc state in a list. */
> };
>
> +struct vring_split {
> + void *queue;
> + dma_addr_t dma_addr;
> + size_t queue_size_in_bytes;
> + struct vring vring;
> +};
So this structure will be only used in vring_create_vring_split() which
seems not that useful.
More see below.
> +
> struct vring_virtqueue {
> struct virtqueue vq;
>
> @@ -915,28 +922,21 @@ static void *virtqueue_detach_unused_buf_split(struct virtqueue *_vq)
> return NULL;
> }
>
> -static struct virtqueue *vring_create_virtqueue_split(
> - unsigned int index,
> - unsigned int num,
> - unsigned int vring_align,
> - struct virtio_device *vdev,
> - bool weak_barriers,
> - bool may_reduce_num,
> - bool context,
> - bool (*notify)(struct virtqueue *),
> - void (*callback)(struct virtqueue *),
> - const char *name)
> +static int vring_create_vring_split(struct vring_split *vring,
> + struct virtio_device *vdev,
> + unsigned int vring_align,
> + bool weak_barriers,
> + bool may_reduce_num,
> + u32 num)
I'd rename this as vring_alloc_queue_split() and let it simply return
the address of queue like vring_alloc_queue().
And let it simple accept dma_addr_t *dma_adder instead of vring_split.
> {
> - struct virtqueue *vq;
> void *queue = NULL;
> dma_addr_t dma_addr;
> size_t queue_size_in_bytes;
> - struct vring vring;
>
> /* We assume num is a power of 2. */
> if (num & (num - 1)) {
> dev_warn(&vdev->dev, "Bad virtqueue length %u\n", num);
> - return NULL;
> + return -EINVAL;
> }
>
> /* TODO: allocate each queue chunk individually */
> @@ -947,11 +947,11 @@ static struct virtqueue *vring_create_virtqueue_split(
> if (queue)
> break;
> if (!may_reduce_num)
> - return NULL;
> + return -ENOMEM;
> }
>
> if (!num)
> - return NULL;
> + return -ENOMEM;
>
> if (!queue) {
> /* Try to get a single page. You are my only hope! */
> @@ -959,21 +959,49 @@ static struct virtqueue *vring_create_virtqueue_split(
> &dma_addr, GFP_KERNEL|__GFP_ZERO);
> }
> if (!queue)
> - return NULL;
> + return -ENOMEM;
>
> queue_size_in_bytes = vring_size(num, vring_align);
> - vring_init(&vring, num, queue, vring_align);
> + vring_init(&vring->vring, num, queue, vring_align);
It's better to move this to its caller (vring_create_virtqueue_split),
so we have rather simple logic below:
> +
> + vring->dma_addr = dma_addr;
> + vring->queue = queue;
> + vring->queue_size_in_bytes = queue_size_in_bytes;
> +
> + return 0;
> +}
> +
> +static struct virtqueue *vring_create_virtqueue_split(
> + unsigned int index,
> + unsigned int num,
> + unsigned int vring_align,
> + struct virtio_device *vdev,
> + bool weak_barriers,
> + bool may_reduce_num,
> + bool context,
> + bool (*notify)(struct virtqueue *),
> + void (*callback)(struct virtqueue *),
> + const char *name)
> +{
> + struct vring_split vring;
> + struct virtqueue *vq;
> + int err;
> +
> + err = vring_create_vring_split(&vring, vdev, vring_align, weak_barriers,
> + may_reduce_num, num);
> + if (err)
> + return NULL;
queue = vring_alloc_queue_split(vdev, &dma_addr, ...);
if (!queue)
return -ENOMEM;
vring_init();
...
Thanks
>
> - vq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
> + vq = __vring_new_virtqueue(index, vring.vring, vdev, weak_barriers, context,
> notify, callback, name);
> if (!vq) {
> - vring_free_queue(vdev, queue_size_in_bytes, queue,
> - dma_addr);
> + vring_free_queue(vdev, vring.queue_size_in_bytes, vring.queue,
> + vring.dma_addr);
> return NULL;
> }
>
> - to_vvq(vq)->split.queue_dma_addr = dma_addr;
> - to_vvq(vq)->split.queue_size_in_bytes = queue_size_in_bytes;
> + to_vvq(vq)->split.queue_dma_addr = vring.dma_addr;
> + to_vvq(vq)->split.queue_size_in_bytes = vring.queue_size_in_bytes;
> to_vvq(vq)->we_own_ring = true;
>
> return vq;
Powered by blists - more mailing lists