Re: [PATCH kvmtool] kvmtool: virtio: fix endian for big endian hosts

[Andrew Jones <ajones@xxxxxxxxxxxxxxxx>]

On Wed, Jan 15, 2025 at 10:11:25AM +0000, Ben Dooks wrote:
> When running on a big endian host, the virtio mmio-modern.c correctly
> sets all reads to return little endian values. However the header uses
> a 4 byte char for the magic value, which is always going to be in the
> correct endian regardless of host endian.
> 
> To make the simplest change, simply avoid endian convresion of the
> read of the magic value. This fixes the following bug from the guest:
> 
> [    0.592838] virtio-mmio 10020000.virtio: Wrong magic value 0x76697274!
> 
> Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>
> ---
>  virtio/mmio-modern.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/virtio/mmio-modern.c b/virtio/mmio-modern.c
> index 6c0bb38..fd9c0cb 100644
> --- a/virtio/mmio-modern.c
> +++ b/virtio/mmio-modern.c
> @@ -66,7 +66,10 @@ static void virtio_mmio_config_in(struct kvm_cpu *vcpu,
>  		return;
>  	}
>  
> -	*data = cpu_to_le32(val);
> +	if (addr != VIRTIO_MMIO_MAGIC_VALUE)
> +		*data = cpu_to_le32(val);
> +	else
> +		*data = val;
>  }
>  
>  static void virtio_mmio_config_out(struct kvm_cpu *vcpu,
> -- 
> 2.37.2.352.g3c44437643
>

I think vendor_id should also have the same issue, but drivers don't
notice because they all use VIRTIO_DEV_ANY_ID. So how about the
change below instead?

Thanks,
drew

diff --git a/include/kvm/virtio-mmio.h b/include/kvm/virtio-mmio.h
index b428b8d32f48..133817c1dc44 100644
--- a/include/kvm/virtio-mmio.h
+++ b/include/kvm/virtio-mmio.h
@@ -18,7 +18,7 @@ struct virtio_mmio_ioevent_param {
 };

 struct virtio_mmio_hdr {
-       char    magic[4];
+       u32     magic;
        u32     version;
        u32     device_id;
        u32     vendor_id;
diff --git a/virtio/mmio.c b/virtio/mmio.c
index fae73b52dae0..782268e8f842 100644
--- a/virtio/mmio.c
+++ b/virtio/mmio.c
@@ -6,6 +6,7 @@
 #include "kvm/irq.h"
 #include "kvm/fdt.h"

+#include <linux/byteorder.h>
 #include <linux/virtio_mmio.h>
 #include <string.h>

@@ -168,10 +169,10 @@ int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
                return r;

        vmmio->hdr = (struct virtio_mmio_hdr) {
-               .magic          = {'v', 'i', 'r', 't'},
+               .magic          = le32_to_cpu(0x74726976), /* 'virt' */
                .version        = legacy ? 1 : 2,
                .device_id      = subsys_id,
-               .vendor_id      = 0x4d564b4c , /* 'LKVM' */
+               .vendor_id      = le32_to_cpu(0x4d564b4c), /* 'LKVM' */
                .queue_num_max  = 256,
        };