linux-kernel - Re: [PATCH V7 19/19] virtio

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] [day] [month] [year] [list]

Message-ID: <CACGkMEtDgs-JXvuTAbAJX2qcx5ZrkS=gK_4aw6bmxhQ8pLY4vQ@mail.gmail.com>
Date: Wed, 15 Oct 2025 12:42:17 +0800
From: Jason Wang <jasowang@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: xuanzhuo@...ux.alibaba.com, eperezma@...hat.com, 
	virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V7 19/19] virtio_ring: add in order support

On Tue, Oct 14, 2025 at 5:13 PM Michael S. Tsirkin <mst@...hat.com> wrote:
>
> On Thu, Sep 25, 2025 at 06:37:08PM +0800, Jason Wang wrote:
> > @@ -683,7 +707,12 @@ static inline int virtqueue_add_split(struct vring_virtqueue *vq,
> >       vq->vq.num_free -= descs_used;
> >
> >       /* Update free pointer */
> > -     if (indirect)
> > +     if (virtqueue_is_in_order(vq)) {
> > +             vq->free_head += descs_used;
> > +             if (vq->free_head >= vq->split.vring.num)
> > +                     vq->free_head -= vq->split.vring.num;
> > +             vq->split.desc_state[head].total_len = total_len;;
> > +     } else if (indirect)
> >               vq->free_head = vq->split.desc_extra[head].next;
> >       else
> >               vq->free_head = i;
>
> So in order is clearly doing something funky with the free_head.
> It's no longer a head of a linked list of free descriptors, is it?
> what is it doing and why?

Since descriptors were consumed in order, it's not yet a list. But
it's still the free head.

How about something like:

        /*
         * Without IN_ORDER it's the head of free buffer list. With
         * IN_ORDER and SPLIT, it's the next available buffer
         * index. With IN_ORDER and PACKED, it's unused.
         */
        unsigned int free_head;

>  Please add code comments to explain
> both where free_list is defined and where it's used.
> For example, virtqueue_vring_attach_packed only inits free_list
> if not in order. So who will init it for in order? And so on.

Packed virtqueue reuses the next_avail_idx, so free_head is unused there.

How about:

static void virtqueue_vring_attach_packed(struct vring_virtqueue *vq,
                                          struct
vring_virtqueue_packed *vring_packed)
{
        vq->packed = *vring_packed;

        if (virtqueue_is_in_order(vq))
                vq->batch_last.id = vq->packed.vring.num;
        else {
                /*
                 * Put everything in free lists. Note that
                 * next_avail_idx is sufficient with IN_ORDER so
                 * free_head is unused.
                 */
                vq->free_head = 0 ;
        }
}

Thanks

>
> --
> MST
>