Re: [PATCH 2/4] vhost: convert byte order on SVQ used event write

[Michael S. Tsirkin]

On Mon, Oct 31, 2022 at 09:54:34AM +0100, Eugenio Perez Martin wrote:
> On Sat, Oct 29, 2022 at 12:48 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
> > >
> > > Fixes: 01f8beacea2a ("vhost: toggle device callbacks using used event 
> > > idx")
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > ---
> > >   hw/virtio/vhost-shadow-virtqueue.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
> > > b/hw/virtio/vhost-shadow-virtqueue.c
> > > index 70766ea740..467099f5d9 100644
> > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > @@ -382,7 +382,7 @@ static bool 
> > > vhost_svq_enable_notification(VhostShadowVirtqueue *svq)
> > >   {
> > >       if (virtio_vdev_has_feature(svq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
> > >           uint16_t *used_event = &svq->vring.avail->ring[svq->vring.num];
> > > -        *used_event = svq->shadow_used_idx;
> > > +        *used_event = cpu_to_le16(svq->shadow_used_idx);
> >
> > This looks correct, but what about:
> >
> >             virtio_stw_p(svq->vdev, used_event, svq->shadow_used_idx);
> >
> 
> Hi Philippe,
> 
> I think this has the same answer as [1], the endianness conversion
> from the guest to the host may not be the same as the one needed from
> qemu SVQ to the vdpa device. Please let me know if it is not the case.
> 
> Thanks!
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg06081.html

So considering legacy, i do not belive you can make a legacy
device on top of modern one using SVQ alone.

So I'd say SVQ should follow virtio endian-ness, not LE.


> > >       } else {
> > >           svq->vring.avail->flags &= 
> > > ~cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT);
> > >       }
> >