Re: [PATCH 4/4] vhost: convert byte order on avail_event read

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

On Mon, Oct 31, 2022 at 09:29:53AM +0100, Eugenio Perez Martin wrote:
> On Sat, Oct 29, 2022 at 12:53 AM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
> >
> > On 28/10/22 18:02, Eugenio Pérez wrote:
> > > This causes errors on virtio modern devices on big endian hosts
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > ---
> > >   hw/virtio/vhost-shadow-virtqueue.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > > index 18a49e1ecb..3131903edd 100644
> > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > @@ -231,7 +231,8 @@ static void vhost_svq_kick(VhostShadowVirtqueue *svq)
> > >           size_t num = svq->vring.num;
> > >           uint16_t *avail_event = (uint16_t *)&svq->vring.used->ring[num];
> > >
> >
> >    uint16_t avail_event = virtio_lduw_p(svq->vdev,
> >                                         &svq->vring.used->ring[num]);
> >    needs_kick = vring_need_event(avail_event,
> >                                  svq->shadow_avail_idx,
> >                                  svq->shadow_avail_idx - 1);
> >
> 
> It would work, but just because all vrings must be little endian for
> the moment. If we support legacy drivers on a big endian host and
> guest in the future, it would not work.
> 
> virtio_ld and virtio_st handle the conversions between the guest and
> the emulated device in qemu, but this conversion is between qemu
> shadow vring and the vdpa device (assuming modern, little endian for
> the moment).
> 
> Right now the feature set must be the same, but it could not be that
> way in the future.
> 
> Thanks!


I don't think this works  legacy and virtio data path are similar but
not similar enough to allow switches through svq alone.


> > > -        needs_kick = vring_need_event(*avail_event, svq->shadow_avail_idx,
> > > +        needs_kick = vring_need_event(le16_to_cpu(*avail_event),
> > > +                                      svq->shadow_avail_idx,
> > >                                         svq->shadow_avail_idx - 1);
> > >       } else {
> > >           needs_kick = !(svq->vring.used->flags & VRING_USED_F_NO_NOTIFY);
> >