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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Apr 2022 15:04:10 +0800 From: Xuan Zhuo <xuanzhuo@...ux.alibaba.com> To: Jason Wang <jasowang@...hat.com> 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, netdev@...r.kernel.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, virtualization@...ts.linux-foundation.org Subject: Re: [PATCH v9 09/32] virtio_ring: split: extract the logic of vq init On Tue, 12 Apr 2022 11:42:25 +0800, Jason Wang <jasowang@...hat.com> wrote: > > 在 2022/4/6 上午11:43, Xuan Zhuo 写道: > > Separate the logic of initializing vq, and subsequent patches will call > > it separately. > > > > The feature of this part is that it does not depend on the information > > passed by the upper layer and can be called repeatedly. > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com> > > --- > > drivers/virtio/virtio_ring.c | 68 ++++++++++++++++++++---------------- > > 1 file changed, 38 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > > index 083f2992ba0d..874f878087a3 100644 > > --- a/drivers/virtio/virtio_ring.c > > +++ b/drivers/virtio/virtio_ring.c > > @@ -916,6 +916,43 @@ static void *virtqueue_detach_unused_buf_split(struct virtqueue *_vq) > > return NULL; > > } > > > > +static void vring_virtqueue_init_split(struct vring_virtqueue *vq, > > + struct virtio_device *vdev, > > + bool own_ring) > > +{ > > + vq->packed_ring = false; > > + vq->vq.num_free = vq->split.vring.num; > > + vq->we_own_ring = own_ring; > > + vq->broken = false; > > + vq->last_used_idx = 0; > > + vq->event_triggered = false; > > + vq->num_added = 0; > > + vq->use_dma_api = vring_use_dma_api(vdev); > > +#ifdef DEBUG > > + vq->in_use = false; > > + vq->last_add_time_valid = false; > > +#endif > > + > > + vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX); > > + > > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM)) > > + vq->weak_barriers = false; > > + > > + vq->split.avail_flags_shadow = 0; > > + vq->split.avail_idx_shadow = 0; > > + > > + /* No callback? Tell other side not to bother us. */ > > + if (!vq->vq.callback) { > > + vq->split.avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT; > > + if (!vq->event) > > + vq->split.vring.avail->flags = cpu_to_virtio16(vdev, > > + vq->split.avail_flags_shadow); > > + } > > + > > + /* Put everything in free lists. */ > > + vq->free_head = 0; > > > It's not clear what kind of initialization that we want to do here. E.g > it mixes split specific setups with some general setups which is kind of > duplication of vring_virtqueue_init_packed(). > > I wonder if it's better to only do split specific setups here and have a > common helper to do the setup that is irrelevant to ring layout. Yes, you are right, I didn't notice this situation before. Thanks. > > Thanks > > > > +} > > + > > static void vring_virtqueue_attach_split(struct vring_virtqueue *vq, > > struct vring vring, > > struct vring_desc_state_split *desc_state, > > @@ -2249,42 +2286,15 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index, > > if (!vq) > > return NULL; > > > > - vq->packed_ring = false; > > vq->vq.callback = callback; > > vq->vq.vdev = vdev; > > vq->vq.name = name; > > - vq->vq.num_free = vring.num; > > vq->vq.index = index; > > - vq->we_own_ring = false; > > vq->notify = notify; > > vq->weak_barriers = weak_barriers; > > - vq->broken = false; > > - vq->last_used_idx = 0; > > - vq->event_triggered = false; > > - vq->num_added = 0; > > - vq->use_dma_api = vring_use_dma_api(vdev); > > -#ifdef DEBUG > > - vq->in_use = false; > > - vq->last_add_time_valid = false; > > -#endif > > > > vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC) && > > !context; > > - vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX); > > - > > - if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM)) > > - vq->weak_barriers = false; > > - > > - vq->split.avail_flags_shadow = 0; > > - vq->split.avail_idx_shadow = 0; > > - > > - /* No callback? Tell other side not to bother us. */ > > - if (!callback) { > > - vq->split.avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT; > > - if (!vq->event) > > - vq->split.vring.avail->flags = cpu_to_virtio16(vdev, > > - vq->split.avail_flags_shadow); > > - } > > > > err = vring_alloc_state_extra_split(vring.num, &state, &extra); > > if (err) { > > @@ -2293,9 +2303,7 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index, > > } > > > > vring_virtqueue_attach_split(vq, vring, state, extra); > > - > > - /* Put everything in free lists. */ > > - vq->free_head = 0; > > + vring_virtqueue_init_split(vq, vdev, false); > > > > spin_lock(&vdev->vqs_list_lock); > > list_add_tail(&vq->vq.list, &vdev->vqs); >
Powered by blists - more mailing lists