Re: [PATCH 2/4] vhost: toggle device callbacks using used event idx

[Eugenio Perez Martin]

On Mon, Oct 24, 2022 at 4:06 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Oct 24, 2022 at 04:00:37PM +0200, Eugenio Perez Martin wrote:
> > > > It's generally a waste that we don't use endian-ness annotations
> > > > the way linux does.
> > >
> > > Yes, it's worth doing something similar sometime.
> > >
> >
> > Maybe we could wrap them in some struct like virtio_le16 or virtio_16,
> > avoiding at least integer direct assignment? Wrappers like
> > cpu_to_virtio16 could return these structs and I think all compilers
> > should emit the same code as direct assignment.
> >
> > Thanks!
> >
>
> This will break bitwise operations such as | and &.
> Generally Linux has solved the problem and I don't think
> we should go look for another solution.
>

That's right, we would need to do it with functions like we do with
the Int128 type. The idea is the same, do not mix operations with
bitwise integers and cpu type ones.

But I totally agree a sparse tag or similar is way more clean and convenient.


>
> >
> >
> > > Thanks
> > >
> > > >
> > > >
> > > > > > Thanks
> > > > > >
> > > > > > > +    } else {
> > > > > > > +        svq->vring.avail->flags &= 
> > > > > > > ~cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT);
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    /* Make sure the event is enabled before the read of 
> > > > > > > used_idx */
> > > > > > >      smp_mb();
> > > > > > >      return !vhost_svq_more_used(svq);
> > > > > > >  }
> > > > > > >
> > > > > > >  static void vhost_svq_disable_notification(VhostShadowVirtqueue 
> > > > > > > *svq)
> > > > > > >  {
> > > > > > > -    svq->vring.avail->flags |= 
> > > > > > > cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT);
> > > > > > > +    /*
> > > > > > > +     * No need to disable notification in the event idx case, 
> > > > > > > since used event
> > > > > > > +     * index is already an index too far away.
> > > > > > > +     */
> > > > > > > +    if (!virtio_vdev_has_feature(svq->vdev, 
> > > > > > > VIRTIO_RING_F_EVENT_IDX)) {
> > > > > > > +        svq->vring.avail->flags |= 
> > > > > > > cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT);
> > > > > > > +    }
> > > > > > >  }
> > > > > > >
> > > > > > >  static uint16_t vhost_svq_last_desc_of_chain(const 
> > > > > > > VhostShadowVirtqueue *svq,
> > > > > > > --
> > > > > > > 2.31.1
> > > > > > >
> > > > > >
> > > >
> > >
>