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

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Oct 25, 2022 at 10:26:35AM +0800, Jason Wang wrote:
> On Mon, Oct 24, 2022 at 10:05 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.
> 
> Yes, but it should not block this series (we can do that in the future
> if we had bandwidth).
> 
> Thanks

Sorry I don't get what you are saying. Which series?
Making LE tags structs is not going to fly, this is why sparse
implements the __bitwise__ tag and in fact this is where the name comes
from - bitwise operations need to work.



> >
> >
> > >
> > >
> > > > 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
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> >