Skip to contentSkip to conversation

virtio: add modern config accessors

PATCH In review Michael S. Tsirkin 3 participants 9 messages Mar 2, 2015 – Mar 5, 2015 virtio core

lore
Share & tools
lore

Michael S. Tsirkin

author maintainer
#0 lore raw

PATCH 1/2 virtio: add modern config accessors

virtio 1.0 defines config space as LE,
as opposed to pre-1.0 which was native endian.

Add API for transports to execute word/dword accesses in little endian format - will be useful for mmio and pci (byte access is also wrapped, for completeness).

For simplicity, we still keep config in host native endian format, byteswap to LE on guest access.

---
 include/hw/virtio/virtio.h |  6 +++
 hw/virtio/virtio.c         | 93 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)
include/hw/virtio/virtio.h +6 -0
index df09993..7a6a9d1 100644
@@ -227,6 +227,12 @@ uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr);
 void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data);
 void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data);
 void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data);
+uint32_t virtio_config_modern_readb(VirtIODevice *vdev, uint32_t addr);
+uint32_t virtio_config_modern_readw(VirtIODevice *vdev, uint32_t addr);
+uint32_t virtio_config_modern_readl(VirtIODevice *vdev, uint32_t addr);
+void virtio_config_modern_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data);
+void virtio_config_modern_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data);
+void virtio_config_modern_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data);
 void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
 hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
 void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
hw/virtio/virtio.c +93 -0

99 lines not on this page · raw message

Signature · 1 line
MST
author maintainer
#1 lore raw

PATCH 2/2 virtio-pci: switch to modern accessors for 1.0

virtio 1.0 config space is in LE format for all devices, use modern wrappers when accessed through the 1.0 BAR.

---
 hw/virtio/virtio-pci.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
hw/virtio/virtio-pci.c +6 -6
index 4c9a0b8..49ea7fc 100644
@@ -1198,13 +1198,13 @@ static uint64_t virtio_pci_device_read(void *opaque, hwaddr addr,
 
     switch (size) {
     case 1:
-        val = virtio_config_readb(vdev, addr);
+        val = virtio_config_modern_readb(vdev, addr);
         break;
     case 2:
-        val = virtio_config_readw(vdev, addr);
+        val = virtio_config_modern_readw(vdev, addr);
         break;
     case 4:
-        val = virtio_config_readl(vdev, addr);
+        val = virtio_config_modern_readl(vdev, addr);
         break;
     }
     return val;
@@ -1216,13 +1216,13 @@ static void virtio_pci_device_write(void *opaque, hwaddr addr,
     VirtIODevice *vdev = opaque;
     switch (size) {
     case 1:
-        virtio_config_writeb(vdev, addr, val);
+        virtio_config_modern_writeb(vdev, addr, val);
         break;
     case 2:
-        virtio_config_writew(vdev, addr, val);
+        virtio_config_modern_writew(vdev, addr, val);
         break;
     case 4:
-        virtio_config_writel(vdev, addr, val);
+        virtio_config_modern_writel(vdev, addr, val);
         break;
     }
 }
Signature · 1 line
MST
#2 lore raw

Re: [Qemu-devel] [PATCH 2/2] virtio-pci: switch to modern accessors for 1.0

Quoted 9 lines · On Mon, 2 Mar 2015 12:40:28 +0100 "Michael S. Tsirkin" <[email protected]> wrote:

virtio 1.0 config space is in LE format for all devices, use modern wrappers when accessed through the 1.0 BAR.

---
 hw/virtio/virtio-pci.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Not that I'm deeply familiar with pci :), but this looks good to me.

(This is on top of your pci branch, btw?)

author maintainer
#3 lore raw

Re: [Qemu-devel] [PATCH 2/2] virtio-pci: switch to modern accessors for 1.0

Quoted 13 lines · Cornelia Huck
Quoted 9 lines · On Mon, 2 Mar 2015 12:40:28 +0100 "Michael S. Tsirkin" <[email protected]> wrote:

virtio 1.0 config space is in LE format for all devices, use modern wrappers when accessed through the 1.0 BAR.

---
 hw/virtio/virtio-pci.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
Quoted 3 lines

Not that I'm deeply familiar with pci :), but this looks good to me.

(This is on top of your pci branch, btw?)

virtio 1.0 branch

#4 lore raw

Re: [Qemu-devel] [PATCH 2/2] virtio-pci: switch to modern accessors for 1.0

In reply to Michael S. Tsirkin
Quoted 3 lines · Michael S. Tsirkin

virtio 1.0 config space is in LE format for all devices, use modern wrappers when accessed through the 1.0 BAR.

Hmm, I'm not so sure about these patches. It's easy to miss the existence of the _modern variants, and they're 90% the same as the legacy variants.

But as long as it's fixed...

Thanks,
Rusty.
PS. rng, block, net and 9p virtio 1.0 seem to work OK on BE guests (LE host).

author maintainer
#5 lore raw

Re: [Qemu-devel] [PATCH 2/2] virtio-pci: switch to modern accessors for 1.0

Quoted 9 lines · Rusty Russell
Quoted 3 lines · Michael S. Tsirkin

virtio 1.0 config space is in LE format for all devices, use modern wrappers when accessed through the 1.0 BAR.

Hmm, I'm not so sure about these patches. It's easy to miss the existence of the _modern variants, and they're 90% the same as the legacy variants.

Quoted 5 lines

But as long as it's fixed...

Thanks,
Rusty.
PS. rng, block, net and 9p virtio 1.0 seem to work OK on BE guests (LE host).

Hmm good point. What if I'll rework this to get bool legacy parameter?

#6 lore raw

Re: [PATCH 2/2] virtio-pci: switch to modern accessors for 1.0

Quoted 10 lines · Michael S. Tsirkin
Quoted 9 lines · Rusty Russell
Quoted 3 lines · Michael S. Tsirkin

virtio 1.0 config space is in LE format for all devices, use modern wrappers when accessed through the 1.0 BAR.

Hmm, I'm not so sure about these patches. It's easy to miss the existence of the _modern variants, and they're 90% the same as the legacy variants.

Quoted 7 lines
Quoted 5 lines

But as long as it's fixed...

Thanks,
Rusty.
PS. rng, block, net and 9p virtio 1.0 seem to work OK on BE guests (LE host).

Hmm good point. What if I'll rework this to get bool legacy parameter?

The functions have access to vdev, so they can determine that already.

Simply renaming the old version to _legacy would be enough.

Thanks,
Rusty.

#7 lore raw

Re: [Qemu-devel] [PATCH 2/2] virtio-pci: switch to modern accessors for 1.0

In reply to Michael S. Tsirkin
Quoted 10 lines · Michael S. Tsirkin
Quoted 9 lines · Rusty Russell
Quoted 3 lines · Michael S. Tsirkin

virtio 1.0 config space is in LE format for all devices, use modern wrappers when accessed through the 1.0 BAR.

Hmm, I'm not so sure about these patches. It's easy to miss the existence of the _modern variants, and they're 90% the same as the legacy variants.

Quoted 7 lines
Quoted 5 lines

But as long as it's fixed...

Thanks,
Rusty.
PS. rng, block, net and 9p virtio 1.0 seem to work OK on BE guests (LE host).

Hmm good point. What if I'll rework this to get bool legacy parameter?

The functions have access to vdev, so they can determine that already.

Simply renaming the old version to _legacy would be enough.

Thanks,
Rusty.

Reviewed-by on v1 1/2
#8 lore raw
In reply to Michael S. Tsirkin
Quoted 7 lines · On Mon, 2 Mar 2015 12:40:25 +0100 "Michael S. Tsirkin" <[email protected]> wrote:

virtio 1.0 defines config space as LE,
as opposed to pre-1.0 which was native endian.

Add API for transports to execute word/dword accesses in little endian format - will be useful for mmio and pci (byte access is also wrapped, for completeness).

Quoted 8 lines

For simplicity, we still keep config in host native endian format, byteswap to LE on guest access.

---
 include/hw/virtio/virtio.h |  6 +++
 hw/virtio/virtio.c         | 93 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)

One could also imagine making the accessors dependant on whether the provided virtio device is standard compliant, but as mmio will probably register different MemoryRegionOps for v1.0 devices as well, this should work out nicer this way.