From 456496e22556203f5f9c3ff02faa7d75ef3face9 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 20 Sep 2010 18:08:41 +0200 Subject: net: delay freeing peer host device With -netdev, virtio devices present offload features to guest, depending on the backend used. Thus, removing host netdev peer while guest is active leads to guest-visible inconsistency and/or crashes. As a solution, while guest (NIC) peer device exists, we prevent the host peer from being deleted. This patch does this by adding peer_deleted flag in nic state: if host device is going away while guest device is around, set this flag and keep a shell of the host device around for as long as guest device exists. The link is put down so all packets will get discarded. At the moment, management can detect that device deletion is delayed by doing info net. As a next step, we shall add commands that control hotplug/unplug without removing the device, and an event to report that guest has responded to the hotplug event. Signed-off-by: Michael S. Tsirkin Acked-by: Alex Williamson (cherry picked from commit a083a89d7277f3268a251ce635d9aae5559242bd) --- net.c | 49 ++++++++++++++++++++++++++++++++++++++++++------- net.h | 1 + 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/net.c b/net.c index 8ddf872a6..eb8226b59 100644 --- a/net.c +++ b/net.c @@ -281,29 +281,64 @@ NICState *qemu_new_nic(NetClientInfo *info, return nic; } -void qemu_del_vlan_client(VLANClientState *vc) +static void qemu_cleanup_vlan_client(VLANClientState *vc) { if (vc->vlan) { QTAILQ_REMOVE(&vc->vlan->clients, vc, next); } else { - if (vc->send_queue) { - qemu_del_net_queue(vc->send_queue); - } QTAILQ_REMOVE(&non_vlan_clients, vc, next); - if (vc->peer) { - vc->peer->peer = NULL; - } } if (vc->info->cleanup) { vc->info->cleanup(vc); } +} +static void qemu_free_vlan_client(VLANClientState *vc) +{ + if (!vc->vlan) { + if (vc->send_queue) { + qemu_del_net_queue(vc->send_queue); + } + if (vc->peer) { + vc->peer->peer = NULL; + } + } qemu_free(vc->name); qemu_free(vc->model); qemu_free(vc); } +void qemu_del_vlan_client(VLANClientState *vc) +{ + /* If there is a peer NIC, delete and cleanup client, but do not free. */ + if (!vc->vlan && vc->peer && vc->peer->info->type == NET_CLIENT_TYPE_NIC) { + NICState *nic = DO_UPCAST(NICState, nc, vc->peer); + if (nic->peer_deleted) { + return; + } + nic->peer_deleted = true; + /* Let NIC know peer is gone. */ + vc->peer->link_down = true; + if (vc->peer->info->link_status_changed) { + vc->peer->info->link_status_changed(vc->peer); + } + qemu_cleanup_vlan_client(vc); + return; + } + + /* If this is a peer NIC and peer has already been deleted, free it now. */ + if (!vc->vlan && vc->peer && vc->info->type == NET_CLIENT_TYPE_NIC) { + NICState *nic = DO_UPCAST(NICState, nc, vc); + if (nic->peer_deleted) { + qemu_free_vlan_client(vc->peer); + } + } + + qemu_cleanup_vlan_client(vc); + qemu_free_vlan_client(vc); +} + VLANClientState * qemu_find_vlan_client_by_name(Monitor *mon, int vlan_id, const char *client_str) diff --git a/net.h b/net.h index 518cf9c1f..44c31a9b3 100644 --- a/net.h +++ b/net.h @@ -72,6 +72,7 @@ typedef struct NICState { VLANClientState nc; NICConf *conf; void *opaque; + bool peer_deleted; } NICState; struct VLANState { -- cgit v1.2.3-65-gdbad From 8006040d47555cba456c9cc43374fba9e04e3bcc Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 27 Sep 2010 18:32:52 +0200 Subject: virtio: invoke set_status callback on reset As status is set to 0 on reset, invoke the relevant callback. This makes for a cleaner code in devices as they don't need to duplicate the code in their reset routine, as well as excercises this path a little more. In particular this makes it possible to unify vhost-net handling code with the following patch. Signed-off-by: Michael S. Tsirkin (cherry picked from commit e0c472d8c2795e523b0f9006dbe5bc22545c8300) --- hw/virtio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/virtio.c b/hw/virtio.c index 85312b3eb..f6ad42dcb 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -453,6 +453,8 @@ void virtio_reset(void *opaque) VirtIODevice *vdev = opaque; int i; + virtio_set_status(vdev, 0); + if (vdev->reset) vdev->reset(vdev); -- cgit v1.2.3-65-gdbad From 286409ad631f3c5df737d0693785ef67dcfd8ba7 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 27 Sep 2010 18:41:30 +0200 Subject: virtio-net: unify vhost-net start/stop Move all of vhost-net start/stop logic to a single routine, and call it from everywhere. Additionally, start/stop vhost-net on link up/down: we should not transmit anything if user asked us to put the link down. Signed-off-by: Michael S. Tsirkin Acked-by: Alex Williamson --- hw/virtio-net.c | 89 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 43 insertions(+), 46 deletions(-) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 075f72df2..df5e18128 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -51,6 +51,7 @@ typedef struct VirtIONet uint8_t nouni; uint8_t nobcast; uint8_t vhost_started; + bool vm_running; VMChangeStateEntry *vmstate; struct { int in_use; @@ -95,6 +96,38 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config) } } +static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status) +{ + VirtIONet *n = to_virtio_net(vdev); + if (!n->nic->nc.peer) { + return; + } + if (n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) { + return; + } + + if (!tap_get_vhost_net(n->nic->nc.peer)) { + return; + } + if (!!n->vhost_started == ((status & VIRTIO_CONFIG_S_DRIVER_OK) && + (n->status & VIRTIO_NET_S_LINK_UP) && + n->vm_running)) { + return; + } + if (!n->vhost_started) { + int r = vhost_net_start(tap_get_vhost_net(n->nic->nc.peer), &n->vdev); + if (r < 0) { + fprintf(stderr, "unable to start vhost net: %d: " + "falling back on userspace virtio\n", -r); + } else { + n->vhost_started = 1; + } + } else { + vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), &n->vdev); + n->vhost_started = 0; + } +} + static void virtio_net_set_link_status(VLANClientState *nc) { VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque; @@ -107,6 +140,8 @@ static void virtio_net_set_link_status(VLANClientState *nc) if (n->status != old_status) virtio_notify_config(&n->vdev); + + virtio_net_set_status(&n->vdev, n->vdev.status); } static void virtio_net_reset(VirtIODevice *vdev) @@ -120,10 +155,6 @@ static void virtio_net_reset(VirtIODevice *vdev) n->nomulti = 0; n->nouni = 0; n->nobcast = 0; - if (n->vhost_started) { - vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), vdev); - n->vhost_started = 0; - } /* Flush any MAC and VLAN filter table state */ n->mac_table.in_use = 0; @@ -726,12 +757,9 @@ static void virtio_net_save(QEMUFile *f, void *opaque) { VirtIONet *n = opaque; - if (n->vhost_started) { - /* TODO: should we really stop the backend? - * If we don't, it might keep writing to memory. */ - vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), &n->vdev); - n->vhost_started = 0; - } + /* At this point, backend must be stopped, otherwise + * it might keep writing to memory. */ + assert(!n->vhost_started); virtio_save(&n->vdev, f); qemu_put_buffer(f, n->mac, ETH_ALEN); @@ -863,44 +891,14 @@ static NetClientInfo net_virtio_info = { .link_status_changed = virtio_net_set_link_status, }; -static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status) -{ - VirtIONet *n = to_virtio_net(vdev); - if (!n->nic->nc.peer) { - return; - } - if (n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) { - return; - } - - if (!tap_get_vhost_net(n->nic->nc.peer)) { - return; - } - if (!!n->vhost_started == !!(status & VIRTIO_CONFIG_S_DRIVER_OK)) { - return; - } - if (status & VIRTIO_CONFIG_S_DRIVER_OK) { - int r = vhost_net_start(tap_get_vhost_net(n->nic->nc.peer), vdev); - if (r < 0) { - fprintf(stderr, "unable to start vhost net: %d: " - "falling back on userspace virtio\n", -r); - } else { - n->vhost_started = 1; - } - } else { - vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), vdev); - n->vhost_started = 0; - } -} - static void virtio_net_vmstate_change(void *opaque, int running, int reason) { VirtIONet *n = opaque; - uint8_t status = running ? VIRTIO_CONFIG_S_DRIVER_OK : 0; + n->vm_running = running; /* This is called when vm is started/stopped, - * it will start/stop vhost backend if * appropriate + * it will start/stop vhost backend if appropriate * e.g. after migration. */ - virtio_net_set_status(&n->vdev, n->vdev.status & status); + virtio_net_set_status(&n->vdev, n->vdev.status); } VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf) @@ -951,9 +949,8 @@ void virtio_net_exit(VirtIODevice *vdev) VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev); qemu_del_vm_change_state_handler(n->vmstate); - if (n->vhost_started) { - vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), vdev); - } + /* This will stop vhost backend if appropriate. */ + virtio_net_set_status(vdev, 0); qemu_purge_queued_packets(&n->nic->nc); -- cgit v1.2.3-65-gdbad From e74f86377a6184326be13f1c7a6c7f9c218b1798 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Wed, 29 Sep 2010 21:59:55 +0200 Subject: eepro100: Add support for multiple individual addresses (multiple IA) I reviewed the latest sources of Linux, FreeBSD and NetBSD. They all reset the multiple IA bit (multi_ia in BSD) to zero, but I did not find code which sets this bit to one (like it is done by some routers). Running Windows guests also did not set this bit. Intel's Open Source Software Developer Manual does not give much information on the semantics related to this bit, so I had to guess how it works. The guess was good enough to make the router emulation work. Related changes in this patch: * Update naming and documentation of the internal hash register. It is not limited to multicast, but also used for multiple IA. * Dump complete configuration register when debug traces are enabled. * Debug output when multiple IA bit is set during CmdConfigure. * Debug output when frames are received because multiple IA bit is set, or when they are ignored although it is set. Cc: Michael S. Tsirkin Signed-off-by: Stefan Weil Signed-off-by: Michael S. Tsirkin (cherry picked from commit 010ec6293409f10b88631c36145944b9c3277ce1) --- hw/eepro100.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/hw/eepro100.c b/hw/eepro100.c index 2b75c8f49..5f6dcb6e6 100644 --- a/hw/eepro100.c +++ b/hw/eepro100.c @@ -219,7 +219,8 @@ typedef enum { typedef struct { PCIDevice dev; - uint8_t mult[8]; /* multicast mask array */ + /* Hash register (multicast mask array, multiple individual addresses). */ + uint8_t mult[8]; int mmio_index; NICState *nic; NICConf conf; @@ -599,7 +600,7 @@ static void nic_reset(void *opaque) { EEPRO100State *s = opaque; TRACE(OTHER, logout("%p\n", s)); - /* TODO: Clearing of multicast table for selective reset, too? */ + /* TODO: Clearing of hash register for selective reset, too? */ memset(&s->mult[0], 0, sizeof(s->mult)); nic_selective_reset(s); } @@ -851,7 +852,14 @@ static void action_command(EEPRO100State *s) case CmdConfigure: cpu_physical_memory_read(s->cb_address + 8, &s->configuration[0], sizeof(s->configuration)); - TRACE(OTHER, logout("configuration: %s\n", nic_dump(&s->configuration[0], 16))); + TRACE(OTHER, logout("configuration: %s\n", + nic_dump(&s->configuration[0], 16))); + TRACE(OTHER, logout("configuration: %s\n", + nic_dump(&s->configuration[16], + ARRAY_SIZE(s->configuration) - 16))); + if (s->configuration[20] & BIT(6)) { + TRACE(OTHER, logout("Multiple IA bit\n")); + } break; case CmdMulticastList: set_multicast_list(s); @@ -1647,12 +1655,6 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size static const uint8_t broadcast_macaddr[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; - /* TODO: check multiple IA bit. */ - if (s->configuration[20] & BIT(6)) { - missing("Multiple IA bit"); - return -1; - } - if (s->configuration[8] & 0x80) { /* CSMA is disabled. */ logout("%p received while CSMA is disabled\n", s); @@ -1702,6 +1704,16 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size /* Promiscuous: receive all. */ TRACE(RXTX, logout("%p received frame in promiscuous mode, len=%zu\n", s, size)); rfd_status |= 0x0004; + } else if (s->configuration[20] & BIT(6)) { + /* Multiple IA bit set. */ + unsigned mcast_idx = compute_mcast_idx(buf); + assert(mcast_idx < 64); + if (s->mult[mcast_idx >> 3] & (1 << (mcast_idx & 7))) { + TRACE(RXTX, logout("%p accepted, multiple IA bit set\n", s)); + } else { + TRACE(RXTX, logout("%p frame ignored, multiple IA bit set\n", s)); + return -1; + } } else { TRACE(RXTX, logout("%p received frame, ignored, len=%zu,%s\n", s, size, nic_dump(buf, size))); -- cgit v1.2.3-65-gdbad From 7c763827a58ad8a399ff22b6f9f0e1a825e58bd1 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Wed, 6 Oct 2010 15:20:17 +0200 Subject: virtio: change set guest notifier to per-device When using irqfd with vhost-net to inject interrupts, a single evenfd might inject multiple interrupts. Implementing this is much easier with a single per-device callback to set guest notifiers. Signed-off-by: Michael S. Tsirkin (cherry picked from commit 54dd932128ae08d6a5a6668551508e30520b0f86) --- hw/vhost.c | 52 ++++++++++++++++++++++++++++------------------------ hw/virtio-pci.c | 29 ++++++++++++++++++++++++++++- hw/virtio.h | 2 +- 3 files changed, 57 insertions(+), 26 deletions(-) diff --git a/hw/vhost.c b/hw/vhost.c index 65709d005..f6689a7b9 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -456,11 +456,6 @@ static int vhost_virtqueue_init(struct vhost_dev *dev, }; struct VirtQueue *vvq = virtio_get_queue(vdev, idx); - if (!vdev->binding->set_guest_notifier) { - fprintf(stderr, "binding does not support guest notifiers\n"); - return -ENOSYS; - } - if (!vdev->binding->set_host_notifier) { fprintf(stderr, "binding does not support host notifiers\n"); return -ENOSYS; @@ -513,12 +508,6 @@ static int vhost_virtqueue_init(struct vhost_dev *dev, r = -errno; goto fail_alloc; } - r = vdev->binding->set_guest_notifier(vdev->binding_opaque, idx, true); - if (r < 0) { - fprintf(stderr, "Error binding guest notifier: %d\n", -r); - goto fail_guest_notifier; - } - r = vdev->binding->set_host_notifier(vdev->binding_opaque, idx, true); if (r < 0) { fprintf(stderr, "Error binding host notifier: %d\n", -r); @@ -543,8 +532,6 @@ fail_call: fail_kick: vdev->binding->set_host_notifier(vdev->binding_opaque, idx, false); fail_host_notifier: - vdev->binding->set_guest_notifier(vdev->binding_opaque, idx, false); -fail_guest_notifier: fail_alloc: cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, idx), 0, 0); @@ -570,13 +557,6 @@ static void vhost_virtqueue_cleanup(struct vhost_dev *dev, .index = idx, }; int r; - r = vdev->binding->set_guest_notifier(vdev->binding_opaque, idx, false); - if (r < 0) { - fprintf(stderr, "vhost VQ %d guest cleanup failed: %d\n", idx, r); - fflush(stderr); - } - assert (r >= 0); - r = vdev->binding->set_host_notifier(vdev->binding_opaque, idx, false); if (r < 0) { fprintf(stderr, "vhost VQ %d host cleanup failed: %d\n", idx, r); @@ -649,15 +629,26 @@ void vhost_dev_cleanup(struct vhost_dev *hdev) int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) { int i, r; + if (!vdev->binding->set_guest_notifiers) { + fprintf(stderr, "binding does not support guest notifiers\n"); + r = -ENOSYS; + goto fail; + } + + r = vdev->binding->set_guest_notifiers(vdev->binding_opaque, true); + if (r < 0) { + fprintf(stderr, "Error binding guest notifier: %d\n", -r); + goto fail_notifiers; + } r = vhost_dev_set_features(hdev, hdev->log_enabled); if (r < 0) { - goto fail; + goto fail_features; } r = ioctl(hdev->control, VHOST_SET_MEM_TABLE, hdev->mem); if (r < 0) { r = -errno; - goto fail; + goto fail_mem; } for (i = 0; i < hdev->nvqs; ++i) { r = vhost_virtqueue_init(hdev, @@ -677,13 +668,14 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) (uint64_t)(unsigned long)hdev->log); if (r < 0) { r = -errno; - goto fail_vq; + goto fail_log; } } hdev->started = true; return 0; +fail_log: fail_vq: while (--i >= 0) { vhost_virtqueue_cleanup(hdev, @@ -691,13 +683,18 @@ fail_vq: hdev->vqs + i, i); } +fail_mem: +fail_features: + vdev->binding->set_guest_notifiers(vdev->binding_opaque, false); +fail_notifiers: fail: return r; } void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev) { - int i; + int i, r; + for (i = 0; i < hdev->nvqs; ++i) { vhost_virtqueue_cleanup(hdev, vdev, @@ -706,6 +703,13 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev) } vhost_client_sync_dirty_bitmap(&hdev->client, 0, (target_phys_addr_t)~0x0ull); + r = vdev->binding->set_guest_notifiers(vdev->binding_opaque, false); + if (r < 0) { + fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r); + fflush(stderr); + } + assert (r >= 0); + hdev->started = false; qemu_free(hdev->log); hdev->log_size = 0; diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 17c3d1539..79f4906e3 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -449,6 +449,33 @@ static int virtio_pci_set_guest_notifier(void *opaque, int n, bool assign) return 0; } +static int virtio_pci_set_guest_notifiers(void *opaque, bool assign) +{ + VirtIOPCIProxy *proxy = opaque; + VirtIODevice *vdev = proxy->vdev; + int r, n; + + for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) { + if (!virtio_queue_get_num(vdev, n)) { + break; + } + + r = virtio_pci_set_guest_notifier(opaque, n, assign); + if (r < 0) { + goto assign_error; + } + } + + return 0; + +assign_error: + /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */ + while (--n >= 0) { + virtio_pci_set_guest_notifier(opaque, n, !assign); + } + return r; +} + static int virtio_pci_set_host_notifier(void *opaque, int n, bool assign) { VirtIOPCIProxy *proxy = opaque; @@ -486,7 +513,7 @@ static const VirtIOBindings virtio_pci_bindings = { .load_queue = virtio_pci_load_queue, .get_features = virtio_pci_get_features, .set_host_notifier = virtio_pci_set_host_notifier, - .set_guest_notifier = virtio_pci_set_guest_notifier, + .set_guest_notifiers = virtio_pci_set_guest_notifiers, }; static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, diff --git a/hw/virtio.h b/hw/virtio.h index 764970c3e..f5ee662c1 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -93,7 +93,7 @@ typedef struct { int (*load_config)(void * opaque, QEMUFile *f); int (*load_queue)(void * opaque, int n, QEMUFile *f); unsigned (*get_features)(void * opaque); - int (*set_guest_notifier)(void * opaque, int n, bool assigned); + int (*set_guest_notifiers)(void * opaque, bool assigned); int (*set_host_notifier)(void * opaque, int n, bool assigned); } VirtIOBindings; -- cgit v1.2.3-65-gdbad From da5aeb8aeba90bcf33b9ccb89cfee20e04fd6eca Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Wed, 6 Oct 2010 15:20:28 +0200 Subject: vhost: error code fix up errors returned to include errno, not just -1 Signed-off-by: Michael S. Tsirkin (cherry picked from commit c885212109b0ad79888ced410c2ff0d0e883cb15) --- hw/vhost.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/vhost.c b/hw/vhost.c index f6689a7b9..3dc56d2c9 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -517,12 +517,14 @@ static int vhost_virtqueue_init(struct vhost_dev *dev, file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq)); r = ioctl(dev->control, VHOST_SET_VRING_KICK, &file); if (r) { + r = -errno; goto fail_kick; } file.fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vvq)); r = ioctl(dev->control, VHOST_SET_VRING_CALL, &file); if (r) { + r = -errno; goto fail_call; } -- cgit v1.2.3-65-gdbad From 6ed912999d6ef636a5be5ccb266d7d3c0f0310b4 Mon Sep 17 00:00:00 2001 From: Anthony Liguori Date: Thu, 14 Oct 2010 10:00:59 -0500 Subject: Update for 0.13.0 release Signed-off-by: Anthony Liguori --- Changelog | 2 ++ VERSION | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Changelog b/Changelog index 152feaacb..6c42f13b0 100644 --- a/Changelog +++ b/Changelog @@ -1,3 +1,5 @@ +See git history for Changelogs of recent releases. + version 0.12.0: - Update to SeaBIOS 0.5.0 diff --git a/VERSION b/VERSION index 73b08a130..54d1a4f2a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.12.93 +0.13.0 -- cgit v1.2.3-65-gdbad