[RFC PATCH 1/2] tests/qtest: Fix virtio msix message endianness

Nicholas Piggin posted 2 patches 8 months, 1 week ago
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Akihiko Odaki <akihiko.odaki@daynix.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>, Coiby Xu <Coiby.Xu@gmail.com>, Stefan Hajnoczi <stefanha@redhat.com>
There is a newer version of this series
[RFC PATCH 1/2] tests/qtest: Fix virtio msix message endianness
Posted by Nicholas Piggin 8 months, 1 week ago
msix messages are written to memory in little-endian order, so they
should not be byteswapped depending on target endianness, but read
as le and converted to host endian by the qtest.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 tests/qtest/libqos/virtio-pci-modern.c | 9 +++++++--
 tests/qtest/libqos/virtio-pci.c        | 7 +++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/libqos/virtio-pci-modern.c b/tests/qtest/libqos/virtio-pci-modern.c
index 4e67fcbd5d3..67aa2af0bd7 100644
--- a/tests/qtest/libqos/virtio-pci-modern.c
+++ b/tests/qtest/libqos/virtio-pci-modern.c
@@ -8,6 +8,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/bswap.h"
 #include "standard-headers/linux/pci_regs.h"
 #include "standard-headers/linux/virtio_pci.h"
 #include "standard-headers/linux/virtio_config.h"
@@ -136,12 +137,16 @@ static bool get_msix_status(QVirtioPCIDevice *dev, uint32_t msix_entry,
         return qpci_msix_pending(dev->pdev, msix_entry);
     }
 
-    data = qtest_readl(dev->pdev->bus->qts, msix_addr);
+    qtest_memread(dev->pdev->bus->qts, msix_addr, &data, 4);
+    data = le32_to_cpu(data);
     if (data == msix_data) {
         qtest_writel(dev->pdev->bus->qts, msix_addr, 0);
         return true;
-    } else {
+    } else if (data == 0) {
         return false;
+    } else {
+        /* Must only be either 0 (no interrupt) or the msix data. */
+        g_assert_not_reached();
     }
 }
 
diff --git a/tests/qtest/libqos/virtio-pci.c b/tests/qtest/libqos/virtio-pci.c
index 002bf8b8c2d..6b421a4d859 100644
--- a/tests/qtest/libqos/virtio-pci.c
+++ b/tests/qtest/libqos/virtio-pci.c
@@ -131,12 +131,15 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
             /* No ISR checking should be done if masked, but read anyway */
             return qpci_msix_pending(dev->pdev, vqpci->msix_entry);
         } else {
-            data = qtest_readl(dev->pdev->bus->qts, vqpci->msix_addr);
+            qtest_memread(dev->pdev->bus->qts, vqpci->msix_addr, &data, 4);
+            data = le32_to_cpu(data);
             if (data == vqpci->msix_data) {
                 qtest_writel(dev->pdev->bus->qts, vqpci->msix_addr, 0);
                 return true;
-            } else {
+            } else if (data == 0) {
                 return false;
+            } else {
+                g_assert_not_reached();
             }
         }
     } else {
-- 
2.47.1
Re: [RFC PATCH 1/2] tests/qtest: Fix virtio msix message endianness
Posted by Philippe Mathieu-Daudé 8 months, 1 week ago
Hi Nick,

On 15/4/25 10:19, Nicholas Piggin wrote:
> msix messages are written to memory in little-endian order, so they
> should not be byteswapped depending on target endianness, but read
> as le and converted to host endian by the qtest.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   tests/qtest/libqos/virtio-pci-modern.c | 9 +++++++--
>   tests/qtest/libqos/virtio-pci.c        | 7 +++++--
>   2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qtest/libqos/virtio-pci-modern.c b/tests/qtest/libqos/virtio-pci-modern.c
> index 4e67fcbd5d3..67aa2af0bd7 100644
> --- a/tests/qtest/libqos/virtio-pci-modern.c
> +++ b/tests/qtest/libqos/virtio-pci-modern.c
> @@ -8,6 +8,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/bswap.h"
>   #include "standard-headers/linux/pci_regs.h"
>   #include "standard-headers/linux/virtio_pci.h"
>   #include "standard-headers/linux/virtio_config.h"
> @@ -136,12 +137,16 @@ static bool get_msix_status(QVirtioPCIDevice *dev, uint32_t msix_entry,
>           return qpci_msix_pending(dev->pdev, msix_entry);
>       }
>   
> -    data = qtest_readl(dev->pdev->bus->qts, msix_addr);
> +    qtest_memread(dev->pdev->bus->qts, msix_addr, &data, 4);
> +    data = le32_to_cpu(data);
>       if (data == msix_data) {
>           qtest_writel(dev->pdev->bus->qts, msix_addr, 0);
>           return true;
> -    } else {
> +    } else if (data == 0) {
>           return false;
> +    } else {
> +        /* Must only be either 0 (no interrupt) or the msix data. */
> +        g_assert_not_reached();

This distinct change belong to a different preliminary patch.

>       }
>   }
>   
> diff --git a/tests/qtest/libqos/virtio-pci.c b/tests/qtest/libqos/virtio-pci.c
> index 002bf8b8c2d..6b421a4d859 100644
> --- a/tests/qtest/libqos/virtio-pci.c
> +++ b/tests/qtest/libqos/virtio-pci.c
> @@ -131,12 +131,15 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
>               /* No ISR checking should be done if masked, but read anyway */
>               return qpci_msix_pending(dev->pdev, vqpci->msix_entry);
>           } else {
> -            data = qtest_readl(dev->pdev->bus->qts, vqpci->msix_addr);
> +            qtest_memread(dev->pdev->bus->qts, vqpci->msix_addr, &data, 4);
> +            data = le32_to_cpu(data);
>               if (data == vqpci->msix_data) {
>                   qtest_writel(dev->pdev->bus->qts, vqpci->msix_addr, 0);
>                   return true;
> -            } else {
> +            } else if (data == 0) {
>                   return false;
> +            } else {
> +                g_assert_not_reached();

Ditto.

>               }
>           }

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

>       } else {


Re: [RFC PATCH 1/2] tests/qtest: Fix virtio msix message endianness
Posted by Fabiano Rosas 8 months, 1 week ago
Nicholas Piggin <npiggin@gmail.com> writes:

> msix messages are written to memory in little-endian order, so they
> should not be byteswapped depending on target endianness, but read
> as le and converted to host endian by the qtest.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>