gmane.comp.emulators.kvm.devel

[ kvm-Bugs-2386363 ] virtio-net doesn't set vc->info_str

SourceForge.net — 2008-12-04T03:43:34

Bugs item #2386363, was opened at 2008-12-03 19:43 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=893831&aid=2386363&group_id=180599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: qemu Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Nolan (sf-nolan) Assigned to: Nobody/Anonymous (nobody) Summary: virtio-net doesn't set vc->info_str Initial Comment: Which makes "info network" not show the devices, which makes it difficult to figure out what NIC model a running VM is using via the monitor. The fix is simple, just add the following to virtio-net.c::virtio_net_init right after the call to qemu_new_vlan_client: snprintf(n->vc->info_str, sizeof(n->vc->info_str), "%s macaddr=%02x:%02x:%02x:%02x:%02x:%02x", "virtio", n->nd->maca

RE: powerpc kvm-userspace build fixes

Zhang, Xiantao — 2008-12-04T02:12:56

IA64 still needs it since the upstream doesn't include tcg fixes. :( Xiantao -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: Virtio network performance problem

Anthony Liguori — 2008-12-03T20:35:32

On an older kernel, you'll likely see packet corruption with virtio-net because of an uninitialized variable. Please try the patch I just posted (Fix uninitialized offset in virtio-net receive_header) and let me know if it helps. Regards, Anthony Liguori -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH] Use guards in virtio-net for easier upstream merging

Anthony Liguori — 2008-12-03T20:32:10

This gets virtio-net into an upstream acceptable state. This includes introducing USE_KVM guards for IO thread notification (where did qemu_service_io() go?). It also includes introducing TAP_VNET_HDR which is for code that relies on the tap vnet support that is not currently in upstream QEMU. Finally, it drops packets if not ready to receive. This is unnecessary in kvm-userspace but necessary in QEMU. Signed-off-by: Anthony Liguori us.ibm.com> diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c index 6493ff6..b53feb3 100644 --- a/qemu/hw/virtio-net.c +++ b/qemu/hw/virtio-net.c < at >< at > -15,7 +15,11 < at >< at > #include "net.h" #include "qemu-timer.h" #include "virtio-net.h" +#ifdef USE_KVM #include "qemu-kvm.h" +#endif + +//#define TAP_VNET_HDR typedef struct VirtIONet { < at >< at > -49,9 +53,10 < at >< at > static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config) static uint32_t virtio_net_get_features(VirtIODevice *vdev) { + uint32_t features = (1 << VIRTIO_NET_F_MAC); +#ifdef TAP_VNET_

[PATCH] Fix unitialized offset in virtio-net receive_header

Anthony Liguori — 2008-12-03T20:27:06

If vnet support is not available on the tap device, offset is uninitialized and badness ensues. Signed-off-by: Anthony Liguori us.ibm.com> diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c index b5d5f9e..4d7e6ad 100644 --- a/qemu/hw/virtio-net.c +++ b/qemu/hw/virtio-net.c < at >< at > -219,7 +219,7 < at >< at > static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt, const void *buf, int size, int hdr_len) { struct virtio_net_hdr *hdr = iov[0].iov_base; - int offset; + int offset = 0; hdr->flags = 0; hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: Virtio network performance problem

Chris Wedgwood — 2008-12-03T19:21:33

Actually, worded badly. A UP guest on an SMP host with unsync'ed TSCs would show this too obviously. As would changes in the TSC speed. The first I guess we can avoid by pinning that latter I'm not so sure about. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0 of 6] PowerPC KVM patches for 2.6.29

Hollis Blanchard — 2008-12-03T19:31:17

Well, I suppose we could save the timestamp to the vcpu, and call kvmtrace code later using that data. I'll stop now and let Christian comment when he's back. :)

Re: Virtio network performance problem

Chris Wedgwood — 2008-12-03T19:20:08

TSC instability? Is this an SMP guest? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

RE: Virtio network performance problem

Adrian Schmitz — 2008-12-03T19:27:15

I don't know enough to tell whether there's a TSC problem or not, to be honest. The host has 2X AMD quad-core CPUs (let me know if you want the /proc/cpuinfo). The guest has 2 VCPUs. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0 of 6] PowerPC KVM patches for 2.6.29

Hollis Blanchard — 2008-12-03T19:22:13

However, we grab timestamps extremely early and late in the exit handlers, in contexts from which it is not safe to call C code. This is really important because we need to be able to measure the time spent in the interrupt handler assembly. For x86 that may be confined to a simple inline asm statement, but the code in arch/powerpc/kvm/booke_interrupts.S is non-trivial and worth measuring.

Virtio network performance problem

Adrian Schmitz — 2008-12-03T19:11:45

I'm having a problem with virtio networking. I mentioned this in a previous thread, but I'm starting a new one since Dor solved the problem I reported in the original thread. This appears to be a new problem. Iperf shows throughput of ~120 Mb/s between a Windows Server 2003 x64 guest and the br0 interface on the host. Also, a ping between the same two interfaces looks pretty bad: Pinging 10.10.10.128 with 32 bytes of data: Reply from 10.10.10.128: bytes=32 time=-389ms TTL=64 Reply from 10.10.10.128: bytes=32 time<1ms TTL=64 Reply from 10.10.10.128: bytes=32 time=392ms TTL=64 Reply from 10.10.10.128: bytes=32 time=392ms TTL=64 Reply from 10.10.10.128: bytes=32 time=377ms TTL=64 Reply from 10.10.10.128: bytes=32 time=392ms TTL=64 Reply from 10.10.10.128: bytes=32 time<1ms TTL=64 Reply from 10.10.10.128: bytes=32 time=401ms TTL=64 Reply from 10.10.10.128: bytes=32 time=1ms TTL=64 Using e1000 driver on this guest, iperf shows 320+ Mb/s throughput, and latency looks like it should. Host kernel:2.6.18 KVM V

kvm-userspace ppc fixes

Hollis Blanchard — 2008-12-03T19:07:26

Hi Avi, among these patches are fixes for a number of bugs in kvm-userspace, including the qemu build break and bugs in the "user" directory. Please apply. -Hollis -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] user: ppc: Add "hello world" test for libcflat

Hollis Blanchard — 2008-12-03T19:07:31

From: Deepa Srinivasan us.ibm.com> Add a test program ("hello world") to exercise printf() in libcflat. Signed-off-by: Deepa Srinivasan us.ibm.com> Signed-off-by: Hollis Blanchard us.ibm.com> --- user/config-powerpc.mak | 3 ++- user/test/powerpc/helloworld.c | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletions(-) create mode 100644 user/test/powerpc/helloworld.c diff --git a/user/config-powerpc.mak b/user/config-powerpc.mak index 79af245..dd7ef54 100644 --- a/user/config-powerpc.mak +++ b/user/config-powerpc.mak < at >< at > -17,7 +17,8 < at >< at > simpletests := \ # theses tests use cstart.o, libcflat, and libgcc tests := \ -test/powerpc/exit.bin +test/powerpc/exit.bin \ +test/powerpc/helloworld.bin include config-powerpc-$(PROCESSOR).mak diff --git a/user/test/powerpc/helloworld.c b/user/test/powerpc/helloworld.c new file mode 100644 index 0000000..f8630f7 --- /dev/null +++ b/user/test/powerpc/helloworld.c < at >< at > -0,0 +1,27 < at >< at > +/* + * This program

Re: powerpc kvm-userspace build fixes

Hollis Blanchard — 2008-12-03T19:03:36

I'll send the fix in a separate mail. Looks like we missed the addition of a dependency from upstream, which we probably didn't get because it conflicted with the --disable-cpu-emulation patch in kvm-userspace. PowerPC stopped using that once upstream fixed PowerPC host builds. If IA64 doesn't use it (and I don't think they do), we can probably drop that patch completely, which should make future merges a little easier... IA64 people?

Re: Hangs

2008-12-03T17:49:18

The guest is Ubuntu 8.10 server (64-bit version). I also have the same problems with Ubuntu 8.04LTS server. Here's the command line: sudo /usr/local/bin/qemu-system-x86_64 \ -no-kvm-irqchip \ -daemonize \ -hda Imgs/sam_home.img \ -m 512 \ -cdrom ISOs/ubuntu-8.10-server-amd64.iso \ -parallel /dev/lp0 \ -vnc :1 \ -net nic,macaddr=DE:AD:BE:EF:01:01,model=e1000 \ -net tap,ifname=tap1,script=/home/chris/kvm/qemu-ifup.sh \ >>& Logs/sam_run.log Earlier in the mail chain, Marcelo had me run vmstat when it was hung, and it was all zeros. He also asked for a stack trace on the qemu and it showed two threads: (gdb) info threads 2 Thread 0x414f1950 (LWP 422) 0x00007f36f07a03e1 in sigtimedwait () from /lib/libc.so.6 1 Thread 0x7f36f1f306e0 (LWP 414) 0x00007f36f084b482 in select () f

[PATCH 2/7] Create virtio-blk.h header

Anthony Liguori — 2008-12-03T17:26:56

Besides cleanliness, targets other than pc want to use virtio-blk so we need to have to define it somewhere other than pc.h Signed-off-by: Anthony Liguori us.ibm.com> diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c index 6de460c..ee38326 100644 --- a/qemu/hw/pc.c +++ b/qemu/hw/pc.c < at >< at > -34,6 +34,7 < at >< at > #include "console.h" #include "fw_cfg.h" #include "device-assignment.h" +#include "virtio-blk.h" #include "qemu-kvm.h" diff --git a/qemu/hw/pc.h b/qemu/hw/pc.h index 5349a41..a874fbd 100644 --- a/qemu/hw/pc.h +++ b/qemu/hw/pc.h < at >< at > -168,10 +168,6 < at >< at > void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd); PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn); -/* virtio-blk.h */ -void *virtio_blk_init(PCIBus *bus, uint16_t vendor, uint16_t device, - BlockDriverState *bs); - /* extboot.c */ void extboot_init(BlockDriverState *bs, int cmd); diff --git a/qemu/hw/virtio-blk.c b/qemu/hw/virtio-blk.c index c9a91ba..bd965f9 100644 --- a/qemu/hw/virtio-blk.c +++ b/qemu/hw/virtio-bl

[PATCH 4/7] Move virtio_balloon_init to virtio-balloon.h

Anthony Liguori — 2008-12-03T17:26:58

Besides cleanliness, targets other than pc want to use virtio-balloon so we need to have to define it somewhere other than pc.h Signed-off-by: Anthony Liguori us.ibm.com> diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c index ee38326..92a8615 100644 --- a/qemu/hw/pc.c +++ b/qemu/hw/pc.c < at >< at > -35,6 +35,7 < at >< at > #include "fw_cfg.h" #include "device-assignment.h" #include "virtio-blk.h" +#include "virtio-balloon.h" #include "qemu-kvm.h" diff --git a/qemu/hw/pc.h b/qemu/hw/pc.h index 5b90198..8cfa1ca 100644 --- a/qemu/hw/pc.h +++ b/qemu/hw/pc.h < at >< at > -168,7 +168,4 < at >< at > void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd); void extboot_init(BlockDriverState *bs, int cmd); -/* virtio-balloon.h */ -void *virtio_balloon_init(PCIBus *bus); - #endif diff --git a/qemu/hw/virtio-balloon.h b/qemu/hw/virtio-balloon.h index 45e4b03..c71f970 100644 --- a/qemu/hw/virtio-balloon.h +++ b/qemu/hw/virtio-balloon.h < at >< at > -15,6 +15,9 < at >< at > #ifndef _QEMU_VIRTIO_BALLOON_H #define _QEMU_VIRTIO_BALLOON_H +#include "virtio.h"

[PATCH 1/7] Fix whitespace in virtio

Anthony Liguori — 2008-12-03T17:26:55

Whoever originally wrote the virtio code quite rudely didn't configure their editor to use spaces instead of tabs. This patch corrects this for all virtio related files. Signed-off-by: Anthony Liguori us.ibm.com> diff --git a/qemu/hw/virtio-balloon.c b/qemu/hw/virtio-balloon.c index 111f27d..9b34e87 100644 --- a/qemu/hw/virtio-balloon.c +++ b/qemu/hw/virtio-balloon.c < at >< at > -79,26 +79,26 < at >< at > static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq) VirtQueueElement elem; while (virtqueue_pop(vq, &elem)) { -size_t offset = 0; -uint32_t pfn; + size_t offset = 0; + uint32_t pfn; -while (memcpy_from_iovector(&pfn, offset, 4, + while (memcpy_from_iovector(&pfn, offset, 4, elem.out_sg, elem.out_num) == 4) { - ram_addr_t pa; - ram_addr_t addr; + ram_addr_t pa; + ram_addr_t addr; - pa = (ram_addr_t)ldl_p(&pfn) << VIRTIO_BALLOON_PFN_SHIFT; - offset += 4; + pa = (ram_a

[PATCH 5/7] Do not access virtio-rings directly

Anthony Liguori — 2008-12-03T17:26:59

This introduces accessors for all virtio ring members and then uses the appropriate physical memory accessors. We shouldn't access guest physical memory directly and this fixes that. As a consequence, this helps us get closer to working live migration. I've done light performance testing and it seems to not have an impact. If further testing demonstrates an impact, we can optimize the physical memory accessor functions. Signed-off-by: Anthony Liguori us.ibm.com> diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c index f621ff3..b64fb00 100644 --- a/qemu/hw/virtio-net.c +++ b/qemu/hw/virtio-net.c < at >< at > -101,18 +101,18 < at >< at > static int virtio_net_can_receive(void *opaque) { VirtIONet *n = opaque; - if (n->rx_vq->vring.avail == NULL || + if (!virtio_queue_ready(n->rx_vq) || !(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) return 0; - if (n->rx_vq->vring.avail->idx == n->rx_vq->last_avail_idx || + if (virtio_queue_empty(n->rx_vq) || (n->mergeable_rx_b

[PATCH 3/7] Create virtio-net.h header

Anthony Liguori — 2008-12-03T17:26:57

Besides cleanliness, targets other than pc want to use virtio-net so we need to have to define it somewhere other than pc.h Signed-off-by: Anthony Liguori us.ibm.com> diff --git a/qemu/hw/pc.h b/qemu/hw/pc.h index a874fbd..5b90198 100644 --- a/qemu/hw/pc.h +++ b/qemu/hw/pc.h < at >< at > -164,10 +164,6 < at >< at > void pci_piix4_ide_init(PCIBus *bus, BlockDriverState **hd_table, int devfn, void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd); -/* virtio-net.c */ - -PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn); - /* extboot.c */ void extboot_init(BlockDriverState *bs, int cmd); diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c index 75bc9a9..c93758d 100644 --- a/qemu/hw/pci.c +++ b/qemu/hw/pci.c < at >< at > -27,6 +27,7 < at >< at > #include "net.h" #include "pc.h" #include "qemu-kvm.h" +#include "virtio-net.h" //#define DEBUG_PCI diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c index b179fcc..f621ff3 100644 --- a/qemu/hw/virtio-net.c +++ b/qemu/hw/virtio-net.c < at >< at > -14,65 +14,9 < at >< at > #include "vir

[PATCH 6/7] Do not modify VirtQueueElement

Anthony Liguori — 2008-12-03T17:27:00

Right now, virtio-net modifies the iovec structure in VirtQueueElement. This is evil. This creates an impossible situation if we want to bounce iovec buffers in VirtQueueElement as we lose track of the original buffers and the resulting free results in very bad things. I tried to refactor receive_headers() and iov_fill() to be able to skip the header if present but failed miserably. Instead of spending more time trying to get that to work, I simply decided to leave the code as-is and copy the iovec to a temporary buffer. I doubt there will be any performance impact as usually there are only a few elements in the iovec anyway. Signed-off-by: Anthony Liguori us.ibm.com> diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c index b64fb00..6493ff6 100644 --- a/qemu/hw/virtio-net.c +++ b/qemu/hw/virtio-net.c < at >< at > -198,6 +198,7 < at >< at > static void virtio_net_receive(void *opaque, const uint8_t *buf, int size) while (offset < size) { VirtQueueElement elem; int len, total; +