kvm(ppc)-userspace: initialize virtio-block

From: Christian Ehrhardt <[email protected]>

virtio-block support is atm initialized in qemu/hw/pc.c which is only called for the "standart pc" platform.
In our case we need to call the virtio-block initialization from our platform init. It might be worth moving this initialization into a funvtion in virtio-block.c and only call that initializer from any platform init that supports virtio - any suggestions/preferences if we should do it now or when another architecture needs that - Anthony ?

This patch adds that initialization for the kvmppc bamboo platform.

Signed-off-by Christian Ehrhardt <[email protected]>

---

[diffstat]

 ppc440_bamboo.c |   13 +++++++++++++
 1 files changed, 13 insertions(+)

Quoted 25 lines · Christian Ehrhardt

FYI some output using virtio-block (still buggy on kvmppc)

Initialization when booting guest (VIO = virtio drvier in guest, U-VIO = virtio emulation in host kvm userspace)

PCI: Enabling device 0000:00:01.0 (0000 -> 0001)
PCI: Enabling device 0000:00:02.0 (0000 -> 0001)
U-VIO virtio_blk_update_config - vdev 0x10d0b008
VIO virtblk_probe - vdev 0xc8925200
U-VIO virtio_blk_get_features - vdev 0x10d0b008
U-VIO virtio_blk_get_features - vdev 0x10d0b008
U-VIO virtio_blk_get_features - vdev 0x10d0b008
blk_queue_max_hw_segments: set to minimum 1
vda:<3>VIO do_virtblk_request - request_queue 0xc8936000
VIO do_req - request_queue 0xc8936000 request 0xc8937320 virtio_blk 0xc880c000 (8secs@0x0 - <NULL> Maj -929934324) U-VIO virtio_blk_handle_output - vdev 0x10d0b008 vq 0x10d0b20c
VIO blk_done - vq 0xc88bcc00
VIO do_virtblk_request - request_queue 0xc8936000

We have no dynamic /dev so anyone who wants to test that will need to create /dev/vda for his needs.
This is easy to check using /proc/partitions.
Well I think that block size is broken here and using fdisk to check the partitions on that (unformatted) disk killed the guest kernel. This needs more debugging, but to enable it for our platform is the first step - comments are welcome.

Quoted 5 lines

bash-3.00# cat /proc/partitions
major minor #blocks name
[...]
254 0 22517998136852480 vda <- ?broken?

My guess is this is run-of-the-mill endianness mismatch. 22517998136852480 = 0x00500000_00000000, which 64-bit byteswapped would be 0x5000, and that's probably a reasonable number of 512-byte blocks. Is your disk image 10MB?

Why would we have a problem, since both guest and host are big-endian? Because virtio is a PCI device, and PCI MMIO are LE, so __virtio_config_val() in the guest is (correctly) using le64_to_cpu().

Why didn't we have problems with virtio-net? Because virtio-net doesn't seem to have anything interesting in PCI config space. virtio-blk's config space contains the capacity and a few other pieces of information.

The fix needs to be in qemu, and given the lack of qemu endianness
infrastructure, I'm afraid it will be a hack. See
http://svn.savannah.nongnu.org/viewvc/trunk/hw/e1000.c?root=qemu&r1@46&r2@45&pathrev@46 for reference. We all know that TARGET_WORDS_BIGENDIAN is totally wrong, but unfortunately it also seems to be the only (accidentally) working solution in qemu without major IO system rework. :(

Quoted 16 lines · Hollis Blanchard

Quoted 5 lines · Christian Ehrhardt

bash-3.00# cat /proc/partitions
major minor #blocks name
[...]
254 0 22517998136852480 vda <- ?broken?

My guess is this is run-of-the-mill endianness mismatch. 22517998136852480 = 0x00500000_00000000, which 64-bit byteswapped would be 0x5000, and that's probably a reasonable number of 512-byte blocks. Is your disk image 10MB?

Why would we have a problem, since both guest and host are big-endian? Because virtio is a PCI device, and PCI MMIO are LE, so __virtio_config_val() in the guest is (correctly) using le64_to_cpu().

Quoted 8 lines

Why didn't we have problems with virtio-net? Because virtio-net doesn't seem to have anything interesting in PCI config space. virtio-blk's config space contains the capacity and a few other pieces of information.

The fix needs to be in qemu, and given the lack of qemu endianness
infrastructure, I'm afraid it will be a hack. See
http://svn.savannah.nongnu.org/viewvc/trunk/hw/e1000.c?root=qemu&r1=4046&r2=4045&pathrev=4046 for reference. We all know that TARGET_WORDS_BIGENDIAN is totally wrong, but unfortunately it also seems to be the only (accidentally) working solution in qemu without major IO system rework. :(

It's actually not so bad since the virtio config space is already read one byte at a time. The following should help.

qemu/hw/virtio-blk.c +0 -0

index 0f55d2a..492bd7f 100644

On Tue, 2008-04-01 at 09:46 -0500, Anthony Liguori wrote:

Thanks Anthony, you've saved me a lot of debug time! Rusty, doing 64-bit PCI config space accesses with ioread8() definitely violates the principle of least surprises, and would have taken me a long time to track down. :(

Attached is a boot log of a PowerPC guest booting from virtio-blk root.

"ramdisk_image" is the standard ~4MB image provided with DENX Embedded Linux Development Kit. Booting is also *way* faster than NFS root (a few seconds to get to a shell :) .

Thanks Anthony, you've saved me a lot of debug time! Rusty, doing 64-bit PCI config space accesses with ioread8() definitely violates the principle of least surprises, and would have taken me a long time to track down. :(

Attached is a boot log of a PowerPC guest booting from virtio-blk root.

"ramdisk_image" is the standard ~4MB image provided with DENX Embedded Linux Development Kit. Booting is also *way* faster than NFS root (a few seconds to get to a shell :) .

It's the unfortunate side-effect of using PCI config space without passing it's semantics through to the virtio devices. Right now, you do a config_get which is basically a memcpy. If we didn't do accesses with ioread8(), you could potentially have a caller than did a config_get() of size 4 that didn't intend on having endian conversion applied.

The other option would have been to provide config_get() and config_get8/16/32/64() the later performing endian conversion.

Quoted 6 lines · Anthony Liguori

It's the unfortunate side-effect of using PCI config space without passing it's semantics through to the virtio devices. Right now, you do a config_get which is basically a memcpy. If we didn't do accesses with ioread8(), you could potentially have a caller than did a config_get() of size 4 that didn't intend on having endian conversion applied.

Quoted 3 lines

The other option would have been to provide config_get() and config_get8/16/32/64() the later performing endian conversion.

Config space should be 8/16/32. Is that ever bridged to real PCI config space anyway ? Or only virtio ? And it should be endian swapped at the low level, either by your HV calls or by the low level kernel. Always. That's how PCI config space is supposed to work.

Ben.

Quoted 6 lines

Config space should be 8/16/32. Is that ever bridged to real PCI config space anyway ? Or only virtio ? And it should be endian swapped at the low level, either by your HV calls or by the low level kernel. Always. That's how PCI config space is supposed to work.

I guess the point is, is that virtio config space is an abstraction with the implementation that is based on PCI converting all accesses to a series of 8-bit accesses. The virtio config space happens to be little endian just like the PCI config space.

Quoted 9 lines · Benjamin Herrenschmidt

Quoted 6 lines · Anthony Liguori

It's the unfortunate side-effect of using PCI config space without passing it's semantics through to the virtio devices. Right now, you do a config_get which is basically a memcpy. If we didn't do accesses with ioread8(), you could potentially have a caller than did a config_get() of size 4 that didn't intend on having endian conversion applied.

Quoted 8 lines

Quoted 3 lines

The other option would have been to provide config_get() and config_get8/16/32/64() the later performing endian conversion.

Config space should be 8/16/32. Is that ever bridged to real PCI config space anyway ? Or only virtio ? And it should be endian swapped at the low level, either by your HV calls or by the low level kernel. Always. That's how PCI config space is supposed to work.

I guess the point is, is that virtio config space is an abstraction with the implementation that is based on PCI converting all accesses to a series of 8-bit accesses. The virtio config space happens to be little endian just like the PCI config space.

The point is that a virtio device appears as a PCI device. Like all other PCI devices, it has config space. Unlike all other PCI devices, its config space is accessed with 1-byte reads.

From: Christian Ehrhardt <[email protected]>

virtio-block support is atm initialized in qemu/hw/pc.c which is only called for the "standart pc" platform.
In our case we need to call the virtio-block initialization from our platform init. It might be worth moving this initialization into a funvtion in virtio-block.c and only call that initializer from any platform init that supports virtio - any suggestions/preferences if we should do it now or when another architecture needs that - Anthony ?

This patch adds that initialization for the kvmppc bamboo platform.

Signed-off-by Christian Ehrhardt <[email protected]>

---

[diffstat]

 ppc440_bamboo.c |   13 +++++++++++++
 1 files changed, 13 insertions(+)

qemu/hw/ppc440_bamboo.c +6 -0

@@ -13,6 +13,7 @@
 #include "net.h"
 #include "hw.h"
 #include "pci.h"
+#include "sysemu.h"
 #include "ppc440.h"
 #include "qemu-kvm.h"
 #include "device_tree.h"

@@ -185,6 +186,18 @@ void bamboo_init(ram_addr_t ram_size, in
 		pci_nic_init(pci->bus, nd, -1);
 	}
 
+	/* Add virtio block devices */
+	if (pci) {
+		int index;
+		int unit_id = 0;
+

FYI some output using virtio-block (still buggy on kvmppc)

Initialization when booting guest (VIO = virtio drvier in guest, U-VIO = virtio emulation in host kvm userspace)

PCI: Enabling device 0000:00:01.0 (0000 -> 0001)
PCI: Enabling device 0000:00:02.0 (0000 -> 0001)
U-VIO virtio_blk_update_config - vdev 0x10d0b008
VIO virtblk_probe - vdev 0xc8925200
U-VIO virtio_blk_get_features - vdev 0x10d0b008
U-VIO virtio_blk_get_features - vdev 0x10d0b008
U-VIO virtio_blk_get_features - vdev 0x10d0b008
blk_queue_max_hw_segments: set to minimum 1
vda:<3>VIO do_virtblk_request - request_queue 0xc8936000
VIO do_req - request_queue 0xc8936000 request 0xc8937320 virtio_blk 0xc880c000 (8secs@0x0 - <NULL> Maj -929934324) U-VIO virtio_blk_handle_output - vdev 0x10d0b008 vq 0x10d0b20c
VIO blk_done - vq 0xc88bcc00
VIO do_virtblk_request - request_queue 0xc8936000

We have no dynamic /dev so anyone who wants to test that will need to create /dev/vda for his needs.
This is easy to check using /proc/partitions.
Well I think that block size is broken here and using fdisk to check the partitions on that (unformatted) disk killed the guest kernel. This needs more debugging, but to enable it for our platform is the first step - comments are welcome.

bash-3.00# cat /proc/partitions
major minor #blocks name
[...]
254 0 22517998136852480 vda <- ?broken?