From 923e2d98ede7404882656aeb4364c3964a95db3d Mon Sep 17 00:00:00 2001 From: Yuanhan Liu Date: Fri, 13 Nov 2015 15:24:09 +0800 Subject: vhost: let SET_VRING_ENABLE message depends on protocol feature But not depend on PROTOCOL_F_MQ feature bit. So that we could use SET_VRING_ENABLE to sign the backend on stop, even if MQ is disabled. That's reasonable, since we will have one queue pair at least. Signed-off-by: Yuanhan Liu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost-user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index c443602..3404b81 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -338,7 +338,7 @@ static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable) .num = enable, }; - if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) { + if (!virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) { return -1; } -- cgit v1.1 From 12b8cbac3c8243b3dd485aaebb82547aefa06adb Mon Sep 17 00:00:00 2001 From: Yuanhan Liu Date: Fri, 13 Nov 2015 15:24:10 +0800 Subject: vhost: don't send RESET_OWNER at stop First of all, RESET_OWNER message is sent incorrectly, as it's sent before GET_VRING_BASE. And the reset message would let the later call get nothing correct. And, sending SET_VRING_ENABLE at stop, which has already been done, makes more sense than RESET_OWNER. Signed-off-by: Yuanhan Liu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/vhost_net.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'hw') diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index d91b7b1..14337a4 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -292,12 +292,6 @@ static void vhost_net_stop_one(struct vhost_net *net, int r = vhost_ops->vhost_net_set_backend(&net->dev, &file); assert(r >= 0); } - } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) { - for (file.index = 0; file.index < net->dev.nvqs; ++file.index) { - const VhostOps *vhost_ops = net->dev.vhost_ops; - int r = vhost_ops->vhost_reset_device(&net->dev); - assert(r >= 0); - } } if (net->nc->info->poll) { net->nc->info->poll(net->nc, true); -- cgit v1.1 From 5421f318ecc82294ad089fd54924df787b67c971 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 16 Nov 2015 13:55:53 +0200 Subject: vhost-user: print original request on error When we get an unexpected response, print out the original request. Helps debug protocol errors tremendously. Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost-user.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 3404b81..5bc6c45 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -121,8 +121,8 @@ static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg) r = qemu_chr_fe_read_all(chr, p, size); if (r != size) { - error_report("Failed to read msg header. Read %d instead of %d.", r, - size); + error_report("Failed to read msg header. Read %d instead of %d." + " Original request %d.", r, size, msg->request); goto fail; } -- cgit v1.1 From dc3db6adde329548771ab2addc2ef8376b2b8b32 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 16 Nov 2015 18:40:18 +0200 Subject: vhost-user: start/stop all rings We are currently only sending VRING_ENABLE message for the first ring, that's wrong: we must start/stop them all. Reported-by: Victor Kaplansky Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost-user.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 5bc6c45..71c3e16 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -333,18 +333,23 @@ static int vhost_user_set_vring_base(struct vhost_dev *dev, static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable) { - struct vhost_vring_state state = { - .index = dev->vq_index, - .num = enable, - }; + int i; if (!virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) { return -1; } - return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state); -} + for (i = 0; i < dev->nvqs; ++i) { + struct vhost_vring_state state = { + .index = dev->vq_index + i, + .num = enable, + }; + + vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, &state); + } + return 0; +} static int vhost_user_get_vring_base(struct vhost_dev *dev, struct vhost_vring_state *ring) -- cgit v1.1 From 1f8431f42d833e8914f2d16ce4a49b7b72b90db0 Mon Sep 17 00:00:00 2001 From: Bandan Das Date: Fri, 13 Nov 2015 01:55:47 -0500 Subject: q35: Check propery to determine if iommu is set The helper function machine_iommu() isn't necesary. We can directly check for the property. Signed-off-by: Bandan Das Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Signed-off-by: Bandan Das --- hw/core/machine.c | 5 ----- hw/pci-host/q35.c | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/core/machine.c b/hw/core/machine.c index f4db340..acca00d 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -462,11 +462,6 @@ bool machine_usb(MachineState *machine) return machine->usb; } -bool machine_iommu(MachineState *machine) -{ - return machine->iommu; -} - bool machine_kernel_irqchip_allowed(MachineState *machine) { return machine->kernel_irqchip_allowed; diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index c81507d..1fb4707 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -506,7 +506,7 @@ static void mch_realize(PCIDevice *d, Error **errp) PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); } /* Intel IOMMU (VT-d) */ - if (machine_iommu(current_machine)) { + if (object_property_get_bool(qdev_get_machine(), "iommu", NULL)) { mch_init_dmar(mch); } } -- cgit v1.1 From 8d211f622b11ac2877c344f29de284d5a842d9d7 Mon Sep 17 00:00:00 2001 From: Bandan Das Date: Fri, 13 Nov 2015 01:55:48 -0500 Subject: i440fx: print an error message if user tries to enable iommu There's no indication of any sort that i440fx doesn't support "iommu=on" Reviewed-by: Eric Blake Signed-off-by: Bandan Das Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Eric Blake Signed-off-by: Bandan Das --- hw/pci-host/piix.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'hw') diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 7b2fbf9..715208b 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -34,6 +34,7 @@ #include "sysemu/sysemu.h" #include "hw/i386/ioapic.h" #include "qapi/visitor.h" +#include "qemu/error-report.h" /* * I440FX chipset data sheet. @@ -301,6 +302,10 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp) static void i440fx_realize(PCIDevice *dev, Error **errp) { dev->config[I440FX_SMRAM] = 0x02; + + if (object_property_get_bool(qdev_get_machine(), "iommu", NULL)) { + error_report("warning: i440fx doesn't support emulated iommu"); + } } PCIBus *i440fx_init(const char *host_type, const char *pci_type, -- cgit v1.1 From 72018d1e1917a56d05e24aedc9f582b7c8385e19 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 17 Nov 2015 16:55:17 +0200 Subject: vhost-user: ignore qemu-only features Some features (such as ctrl vq) are supported by qemu without need to communicate with the backend. Drop them from the feature mask so we set them unconditionally. Reported-by: Victor Kaplansky Signed-off-by: Michael S. Tsirkin --- hw/net/vhost_net.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'hw') diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 14337a4..318c3e6 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -77,14 +77,8 @@ static const int user_feature_bits[] = { VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_MRG_RXBUF, - VIRTIO_NET_F_STATUS, - VIRTIO_NET_F_CTRL_VQ, - VIRTIO_NET_F_CTRL_RX, - VIRTIO_NET_F_CTRL_VLAN, - VIRTIO_NET_F_CTRL_RX_EXTRA, - VIRTIO_NET_F_CTRL_MAC_ADDR, - VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, + /* This bit implies RARP isn't sent by QEMU out of band */ VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, -- cgit v1.1 From 48854f57ce3e6aa4bd13368559e5c292e1c44e49 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Wed, 18 Nov 2015 16:13:54 +0200 Subject: vhost-user: fix log size commit 2b8819c6eee517c1582983773f8555bb3f9ed645 ("vhost-user: modify SET_LOG_BASE to pass mmap size and offset") passes log size in units of 4 byte chunks instead of the expected size in bytes. Fix this up. Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost-user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 71c3e16..1b6c5ac 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -206,7 +206,7 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base, VhostUserMsg msg = { .request = VHOST_USER_SET_LOG_BASE, .flags = VHOST_USER_VERSION, - .payload.log.mmap_size = log->size, + .payload.log.mmap_size = log->size * sizeof(*(log->log)), .payload.log.mmap_offset = 0, .size = sizeof(msg.payload.log), }; -- cgit v1.1 From d9a3b33d2c9f996537b7f1d0246dee2d0120cefb Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 19 Nov 2015 15:14:07 +0200 Subject: acpi: fix buffer overrun on migration ich calls acpi_gpe_init with length ICH9_PMIO_GPE0_LEN so ICH9_PMIO_GPE0_LEN/2 bytes are allocated, but then the full ICH9_PMIO_GPE0_LEN bytes are migrated. As a quick work-around, allocate twice the memory. We'll probably want to tweak code to avoid migrating the extra ICH9_PMIO_GPE0_LEN/2 bytes, but that is a bit trickier to do without breaking migration compatibility. Tested-by: "Dr. David Alan Gilbert" Reported-by: "Dr. David Alan Gilbert" Cc: qemu-stable@nongnu.org Signed-off-by: Michael S. Tsirkin --- hw/acpi/core.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/acpi/core.c b/hw/acpi/core.c index fe6215a..21e113d 100644 --- a/hw/acpi/core.c +++ b/hw/acpi/core.c @@ -625,8 +625,12 @@ void acpi_pm1_cnt_reset(ACPIREGS *ar) void acpi_gpe_init(ACPIREGS *ar, uint8_t len) { ar->gpe.len = len; - ar->gpe.sts = g_malloc0(len / 2); - ar->gpe.en = g_malloc0(len / 2); + /* Only first len / 2 bytes are ever used, + * but the caller in ich9.c migrates full len bytes. + * TODO: fix ich9.c and drop the extra allocation. + */ + ar->gpe.sts = g_malloc0(len); + ar->gpe.en = g_malloc0(len); } void acpi_gpe_reset(ACPIREGS *ar) -- cgit v1.1