qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 06/27] virtio: Add virtio_queue_get_used_notify_split


From: Eugenio Perez Martin
Subject: Re: [RFC PATCH 06/27] virtio: Add virtio_queue_get_used_notify_split
Date: Tue, 12 Jan 2021 19:21:27 +0100

On Mon, Dec 7, 2020 at 5:58 PM Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
> On Fri, Nov 20, 2020 at 07:50:44PM +0100, Eugenio PĂ©rez wrote:
> > This function is just used for a few commits, so SW LM is developed
> > incrementally, and it is deleted after it is useful.
> >
> > For a few commits, only the events (irqfd, eventfd) are forwarded.
>
> s/eventfd/ioeventfd/ (irqfd is also an eventfd)
>

Oops, will fix, thanks!

> > +bool virtio_queue_get_used_notify_split(VirtQueue *vq)
> > +{
> > +    VRingMemoryRegionCaches *caches;
> > +    hwaddr pa = offsetof(VRingUsed, flags);
> > +    uint16_t flags;
> > +
> > +    RCU_READ_LOCK_GUARD();
> > +
> > +    caches = vring_get_region_caches(vq);
> > +    assert(caches);
> > +    flags = virtio_lduw_phys_cached(vq->vdev, &caches->used, pa);
> > +    return !(VRING_USED_F_NO_NOTIFY & flags);
> > +}
>
> QEMU stores the notification status:
>
> void virtio_queue_set_notification(VirtQueue *vq, int enable)
> {
>     vq->notification = enable; <---- here
>
>     if (!vq->vring.desc) {
>         return;
>     }
>
>     if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
>         virtio_queue_packed_set_notification(vq, enable);
>     } else {
>         virtio_queue_split_set_notification(vq, enable);
>
> I'm wondering why it's necessary to fetch from guest RAM instead of
> using vq->notification? It also works for both split and packed
> queues so the code would be simpler.

To use vq->notification makes sense at the end of the series.

However, at this stage (just routing notifications, not descriptors),
vhost device is the one updating that flag, not qemu. Since we cannot
just migrate used ring memory to qemu without migrating descriptors
ring too, qemu needs to check guest's memory looking for vhost device
updates on that flag.

I can see how that deserves better documentation or even a better
name. Also, this function should be in the shadow vq file, not
virtio.c.

Thanks!




reply via email to

[Prev in Thread] Current Thread [Next in Thread]