[PATCH 0/4] Endianess and coding style fixes for SVQ event idx support

[PATCH 0/4] Endianess and coding style fixes for SVQ event idx support

[Eugenio Pérez]

Some fixes that did not get in time for the last net pull request.

Eugenio Pérez (4):
  vhost: Delete useless casting
  vhost: convert byte order on SVQ used event write
  vhost: Fix lines over 80 characters
  vhost: convert byte order on avail_event read

 hw/virtio/vhost-shadow-virtqueue.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
2.31.1

[PATCH 1/4] vhost: Delete useless casting

[Eugenio Pérez]

The used event is already an uint16_t pointer

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 5bd14cad96..70766ea740 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -381,7 +381,7 @@ static bool vhost_svq_more_used(VhostShadowVirtqueue *svq)
 static bool vhost_svq_enable_notification(VhostShadowVirtqueue *svq)
 {
     if (virtio_vdev_has_feature(svq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
-        uint16_t *used_event = (uint16_t *)&svq->vring.avail->ring[svq->vring.num];
+        uint16_t *used_event = &svq->vring.avail->ring[svq->vring.num];
         *used_event = svq->shadow_used_idx;
     } else {
         svq->vring.avail->flags &= ~cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT);
-- 
2.31.1

Re: [PATCH 1/4] vhost: Delete useless casting

[Philippe Mathieu-Daudé]

On 28/10/22 18:02, Eugenio Pérez wrote:
> The used event is already an uint16_t pointer
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>   hw/virtio/vhost-shadow-virtqueue.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

[Eugenio Pérez]

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);
     } else {
         svq->vring.avail->flags &= ~cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT);
     }
-- 
2.31.1

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

[Philippe Mathieu-Daudé]

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

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

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

[Michael S. Tsirkin]

On Sat, Oct 29, 2022 at 12:48:43AM +0200, Philippe Mathieu-Daudé 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);


Philippe thanks for review but this comment isn't all that clear.
I think you meant something like:
	this won't handle endian-ness for legacy virtio devices
	on BE correctly. I think  virtio_stw_p would be better.

which would make sense.

Yes contributors should document motivation for changes but so
should reviewers.
	

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

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

[Philippe Mathieu-Daudé]

On 31/10/22 09:17, Michael S. Tsirkin wrote:
> On Sat, Oct 29, 2022 at 12:48:43AM +0200, Philippe Mathieu-Daudé 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);
> 
> 
> Philippe thanks for review but this comment isn't all that clear.
> I think you meant something like:
> 	this won't handle endian-ness for legacy virtio devices
> 	on BE correctly. I think  virtio_stw_p would be better.
> 
> which would make sense.
> 
> Yes contributors should document motivation for changes but so
> should reviewers.

Yes, fair enough :)

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

[Eugenio Perez Martin]

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

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

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);
> > >       }
> >

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

[Eugenio Perez Martin]

On Mon, Oct 31, 2022 at 1:33 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> 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.
>

Right, more work is needed. For example, config r/w conversions. But
it's a valid use case where SVQ helps too.

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

At this moment both the device that the guest sees and the vdpa device
must be modern ones to enable SVQ. So the event idx must be stored in
the vring in LE. Similar access functions as virtio_ld* and virtio_st*
are needed if SVQ supports legacy vdpa devices in the future.

The point is that svq->shadow_avail_idx is decoupled from the guest's
avail ring, event idx, etc. It will always be in the host's CPU
endianness, regardless of the guest's one. And, for the moment, the
event idx write must be in LE.

There is a more fundamental problem about using virtio_{st,ld}* here:
These read from and write to guest's memory, but neither
svq->shadow_used_idx or shadow vring are in guest's memory but only in
qemu's VA. To start the support of legacy vdpa devices would involve a
deeper change here, since all shadow vring writes and reads are
written this way.

Thanks!

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

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

[Michael S. Tsirkin]

On Mon, Oct 31, 2022 at 02:02:16PM +0100, Eugenio Perez Martin wrote:
> On Mon, Oct 31, 2022 at 1:33 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > 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.
> >
> 
> Right, more work is needed. For example, config r/w conversions. But
> it's a valid use case where SVQ helps too.

I am not sure why it's valid frankly.

> > So I'd say SVQ should follow virtio endian-ness, not LE.
> 
> At this moment both the device that the guest sees and the vdpa device
> must be modern ones to enable SVQ. So the event idx must be stored in
> the vring in LE. Similar access functions as virtio_ld* and virtio_st*
> are needed if SVQ supports legacy vdpa devices in the future.
> 
> The point is that svq->shadow_avail_idx is decoupled from the guest's
> avail ring, event idx, etc. It will always be in the host's CPU
> endianness, regardless of the guest's one. And, for the moment, the
> event idx write must be in LE.
> 
> There is a more fundamental problem about using virtio_{st,ld}* here:
> These read from and write to guest's memory, but neither
> svq->shadow_used_idx or shadow vring are in guest's memory but only in
> qemu's VA. To start the support of legacy vdpa devices would involve a
> deeper change here, since all shadow vring writes and reads are
> written this way.
> 
> Thanks!

Yea generally, I don't know how it can work given legacy
will never attach a PASID to a VQ.

But maybe given we add yet another variant of endian-ness
it is time to actually use sparse tags for this stuff.

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

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

[Eugenio Perez Martin]

On Mon, Oct 31, 2022 at 4:09 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Oct 31, 2022 at 02:02:16PM +0100, Eugenio Perez Martin wrote:
> > On Mon, Oct 31, 2022 at 1:33 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > 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.
> > >
> >
> > Right, more work is needed. For example, config r/w conversions. But
> > it's a valid use case where SVQ helps too.
>
> I am not sure why it's valid frankly.
>
> > > So I'd say SVQ should follow virtio endian-ness, not LE.
> >
> > At this moment both the device that the guest sees and the vdpa device
> > must be modern ones to enable SVQ. So the event idx must be stored in
> > the vring in LE. Similar access functions as virtio_ld* and virtio_st*
> > are needed if SVQ supports legacy vdpa devices in the future.
> >
> > The point is that svq->shadow_avail_idx is decoupled from the guest's
> > avail ring, event idx, etc. It will always be in the host's CPU
> > endianness, regardless of the guest's one. And, for the moment, the
> > event idx write must be in LE.
> >
> > There is a more fundamental problem about using virtio_{st,ld}* here:
> > These read from and write to guest's memory, but neither
> > svq->shadow_used_idx or shadow vring are in guest's memory but only in
> > qemu's VA. To start the support of legacy vdpa devices would involve a
> > deeper change here, since all shadow vring writes and reads are
> > written this way.
> >
> > Thanks!
>
> Yea generally, I don't know how it can work given legacy
> will never attach a PASID to a VQ.
>

The conversion I tried to put here was legacy guests communicating in
big endian with qemu, and then qemu communicating in little endian
with modern devices. For this to work SVQ should be enabled for all
the queues all the time.

Then the simplest conversion function here should be cpu_to_leNN,
isn't it? The only device we support here is a modern, little endian,

But maybe my example just added more noise. My point is that this
write and all the writes and loads added on these patches, have
nothing to do with the guest's endianness. They are only from the SVQ
vring to the device. And they are not forwarding the used_event of the
guest, but another decoupled one that may or may not match. That's why
the endianness we should take into account is not the vdev one, but
only the CPU and little endian.

> But maybe given we add yet another variant of endian-ness
> it is time to actually use sparse tags for this stuff.
>

I agree with this. I can try to do a fast POC.

Thanks!


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

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

[Michael S. Tsirkin]

On Mon, Oct 31, 2022 at 05:05:42PM +0100, Eugenio Perez Martin wrote:
> On Mon, Oct 31, 2022 at 4:09 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, Oct 31, 2022 at 02:02:16PM +0100, Eugenio Perez Martin wrote:
> > > On Mon, Oct 31, 2022 at 1:33 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > 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.
> > > >
> > >
> > > Right, more work is needed. For example, config r/w conversions. But
> > > it's a valid use case where SVQ helps too.
> >
> > I am not sure why it's valid frankly.
> >
> > > > So I'd say SVQ should follow virtio endian-ness, not LE.
> > >
> > > At this moment both the device that the guest sees and the vdpa device
> > > must be modern ones to enable SVQ. So the event idx must be stored in
> > > the vring in LE. Similar access functions as virtio_ld* and virtio_st*
> > > are needed if SVQ supports legacy vdpa devices in the future.
> > >
> > > The point is that svq->shadow_avail_idx is decoupled from the guest's
> > > avail ring, event idx, etc. It will always be in the host's CPU
> > > endianness, regardless of the guest's one. And, for the moment, the
> > > event idx write must be in LE.
> > >
> > > There is a more fundamental problem about using virtio_{st,ld}* here:
> > > These read from and write to guest's memory, but neither
> > > svq->shadow_used_idx or shadow vring are in guest's memory but only in
> > > qemu's VA. To start the support of legacy vdpa devices would involve a
> > > deeper change here, since all shadow vring writes and reads are
> > > written this way.
> > >
> > > Thanks!
> >
> > Yea generally, I don't know how it can work given legacy
> > will never attach a PASID to a VQ.
> >
> 
> The conversion I tried to put here was legacy guests communicating in
> big endian with qemu, and then qemu communicating in little endian
> with modern devices. For this to work SVQ should be enabled for all
> the queues all the time.

Yes I got that.  This won't work so easily just because e.g. network
header is slightly different, so it's more than just descriptor
translations even just on data path.

> Then the simplest conversion function here should be cpu_to_leNN,
> isn't it? The only device we support here is a modern, little endian,

At the moment vdpa only properly works with modern. But really
another way to support legacy is if a device has support, and
to fix vdpa to support legacy.

Will we ever do that? Will anyone bother? I don't know.


> But maybe my example just added more noise. My point is that this
> write and all the writes and loads added on these patches, have
> nothing to do with the guest's endianness.

Device might support programmable endian-ness. If it does,
then yes we could thinkably have yet another type of
endian-ness "device endian" but practically setting it to
anything except guest endian is just inviting pain.

> They are only from the SVQ
> vring to the device. And they are not forwarding the used_event of the
> guest, but another decoupled one that may or may not match. That's why
> the endianness we should take into account is not the vdev one, but
> only the CPU and little endian.
> 
> > But maybe given we add yet another variant of endian-ness
> > it is time to actually use sparse tags for this stuff.
> >
> 
> I agree with this. I can try to do a fast POC.
> 
> Thanks!
> 
> 
> > > >
> > > >
> > > > > > >       } else {
> > > > > > >           svq->vring.avail->flags &= ~cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT);
> > > > > > >       }
> > > > > >
> > > >
> >

[PATCH 3/4] vhost: Fix lines over 80 characters

[Eugenio Pérez]

By qemu coding style.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/virtio/vhost-shadow-virtqueue.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 467099f5d9..18a49e1ecb 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -228,8 +228,11 @@ static void vhost_svq_kick(VhostShadowVirtqueue *svq)
     smp_mb();
 
     if (virtio_vdev_has_feature(svq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
-        uint16_t avail_event = *(uint16_t *)(&svq->vring.used->ring[svq->vring.num]);
-        needs_kick = vring_need_event(avail_event, svq->shadow_avail_idx, svq->shadow_avail_idx - 1);
+        size_t num = svq->vring.num;
+        uint16_t *avail_event = (uint16_t *)&svq->vring.used->ring[num];
+
+        needs_kick = vring_need_event(*avail_event, svq->shadow_avail_idx,
+                                      svq->shadow_avail_idx - 1);
     } else {
         needs_kick = !(svq->vring.used->flags & VRING_USED_F_NO_NOTIFY);
     }
-- 
2.31.1

Re: [PATCH 3/4] vhost: Fix lines over 80 characters

[Michael S. Tsirkin]

On Fri, Oct 28, 2022 at 06:02:50PM +0200, Eugenio Pérez wrote:
> By qemu coding style.
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

You wrote this code originally so I don't mind but just to note I don't
want a flurry of patches "fixing" lines over 80 chars.

> ---
>  hw/virtio/vhost-shadow-virtqueue.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index 467099f5d9..18a49e1ecb 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -228,8 +228,11 @@ static void vhost_svq_kick(VhostShadowVirtqueue *svq)
>      smp_mb();
>  
>      if (virtio_vdev_has_feature(svq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
> -        uint16_t avail_event = *(uint16_t *)(&svq->vring.used->ring[svq->vring.num]);
> -        needs_kick = vring_need_event(avail_event, svq->shadow_avail_idx, svq->shadow_avail_idx - 1);
> +        size_t num = svq->vring.num;
> +        uint16_t *avail_event = (uint16_t *)&svq->vring.used->ring[num];
> +
> +        needs_kick = vring_need_event(*avail_event, svq->shadow_avail_idx,
> +                                      svq->shadow_avail_idx - 1);
>      } else {
>          needs_kick = !(svq->vring.used->flags & VRING_USED_F_NO_NOTIFY);
>      }
> -- 
> 2.31.1

Re: [PATCH 3/4] vhost: Fix lines over 80 characters

[Eugenio Perez Martin]

On Fri, Oct 28, 2022 at 6:45 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Oct 28, 2022 at 06:02:50PM +0200, Eugenio Pérez wrote:
> > By qemu coding style.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>
> You wrote this code originally so I don't mind but just to note I don't
> want a flurry of patches "fixing" lines over 80 chars.
>

My bad, I didn't realize it the first time I sent it. I probably
missed to run checkpatch.

This patch can be merged with the next one if it is more convenient.

> > ---
> >  hw/virtio/vhost-shadow-virtqueue.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > index 467099f5d9..18a49e1ecb 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -228,8 +228,11 @@ static void vhost_svq_kick(VhostShadowVirtqueue *svq)
> >      smp_mb();
> >
> >      if (virtio_vdev_has_feature(svq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
> > -        uint16_t avail_event = *(uint16_t *)(&svq->vring.used->ring[svq->vring.num]);
> > -        needs_kick = vring_need_event(avail_event, svq->shadow_avail_idx, svq->shadow_avail_idx - 1);
> > +        size_t num = svq->vring.num;
> > +        uint16_t *avail_event = (uint16_t *)&svq->vring.used->ring[num];
> > +
> > +        needs_kick = vring_need_event(*avail_event, svq->shadow_avail_idx,
> > +                                      svq->shadow_avail_idx - 1);
> >      } else {
> >          needs_kick = !(svq->vring.used->flags & VRING_USED_F_NO_NOTIFY);
> >      }
> > --
> > 2.31.1
>

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

[Eugenio Pérez]

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];
 
-        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);
-- 
2.31.1

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

[Philippe Mathieu-Daudé]

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

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

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

[Eugenio Perez Martin]

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!

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

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

[Michael S. Tsirkin]

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

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

[Jason Wang]

在 2022/10/31 20:35, Michael S. Tsirkin 写道:
> 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.


Then we need full copy in that case, looks more like a normal network 
backend instead of registering it to as vhost backend.

Thanks


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

Re: [PATCH 0/4] Endianess and coding style fixes for SVQ event idx support

[Michael S. Tsirkin]

On Fri, Oct 28, 2022 at 06:02:47PM +0200, Eugenio Pérez wrote:
> Some fixes that did not get in time for the last net pull request.


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Jason's tree since he has some bits this depends on.

> Eugenio Pérez (4):
>   vhost: Delete useless casting
>   vhost: convert byte order on SVQ used event write
>   vhost: Fix lines over 80 characters
>   vhost: convert byte order on avail_event read
> 
>  hw/virtio/vhost-shadow-virtqueue.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> -- 
> 2.31.1
> 

Re: [PATCH 0/4] Endianess and coding style fixes for SVQ event idx support

[Philippe Mathieu-Daudé]

On 29/10/22 10:24, Michael S. Tsirkin wrote:
> On Fri, Oct 28, 2022 at 06:02:47PM +0200, Eugenio Pérez wrote:
>> Some fixes that did not get in time for the last net pull request.
> 
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Jason's tree since he has some bits this depends on.
FYI I made 2 comments/questions whether using the virtio LD/ST API
isn't more appropriate here.

Re: [PATCH 0/4] Endianess and coding style fixes for SVQ event idx support

[Jason Wang]

On Sat, Oct 29, 2022 at 12:02 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Some fixes that did not get in time for the last net pull request.
>
> Eugenio Pérez (4):
>   vhost: Delete useless casting
>   vhost: convert byte order on SVQ used event write
>   vhost: Fix lines over 80 characters
>   vhost: convert byte order on avail_event read

I've queued this for rc1.

Thanks

>
>  hw/virtio/vhost-shadow-virtqueue.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> --
> 2.31.1
>
>

Re: [PATCH 0/4] Endianess and coding style fixes for SVQ event idx support

[Lei Yang]

QE tries to test the parameter "event_idx=on". In both environments
"virtio-vdpa + vp_vdpa" and "vhost_vdpa + vp_vdpa", there is no
network connectivity issue after the guest boot up.

Tested-by: Lei Yang <leiyang@redhat.com>


> From: Jason Wang <jasowang@redhat.com>
> Date: Tue, Nov 1, 2022 at 10:42 AM
> Subject: Re: [PATCH 0/4] Endianess and coding style fixes for SVQ
> event idx support
> To: Eugenio Pérez <eperezma@redhat.com>
> Cc: <qemu-devel@nongnu.org>, Stefan Hajnoczi <stefanha@redhat.com>,
> Michael S. Tsirkin <mst@redhat.com>
>
>
> On Sat, Oct 29, 2022 at 12:02 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > Some fixes that did not get in time for the last net pull request.
> >
> > Eugenio Pérez (4):
> >   vhost: Delete useless casting
> >   vhost: convert byte order on SVQ used event write
> >   vhost: Fix lines over 80 characters
> >   vhost: convert byte order on avail_event read
>
> I've queued this for rc1.
>
> Thanks
>
> >
> >  hw/virtio/vhost-shadow-virtqueue.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > --
> > 2.31.1
> >
> >
>