From 71ebd6dcf9dc4d733923ba798154aba8893c28ca Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Thu, 17 Jun 2010 15:15:45 +0900 Subject: [PATCH 1/8] pci: fix pci_device_reset Clear interrupt disable bit on reset, according to PCI spec. Fix pci_device_reset() with 64bit BAR. Signed-off-by: Isaku Yamahata Signed-off-by: Michael S. Tsirkin --- hw/pci.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index a3c28738c6..645506aa83 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -155,15 +155,24 @@ static void pci_device_reset(PCIDevice *dev) dev->irq_state = 0; pci_update_irq_status(dev); - dev->config[PCI_COMMAND] &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | - PCI_COMMAND_MASTER); + /* Clear all writeable bits */ + pci_set_word(dev->config + PCI_COMMAND, + pci_get_word(dev->config + PCI_COMMAND) & + ~pci_get_word(dev->wmask + PCI_COMMAND)); dev->config[PCI_CACHE_LINE_SIZE] = 0x0; dev->config[PCI_INTERRUPT_LINE] = 0x0; for (r = 0; r < PCI_NUM_REGIONS; ++r) { - if (!dev->io_regions[r].size) { + PCIIORegion *region = &dev->io_regions[r]; + if (!region->size) { continue; } - pci_set_long(dev->config + pci_bar(dev, r), dev->io_regions[r].type); + + if (!(region->type & PCI_BASE_ADDRESS_SPACE_IO) && + region->type & PCI_BASE_ADDRESS_MEM_TYPE_64) { + pci_set_quad(dev->config + pci_bar(dev, r), region->type); + } else { + pci_set_long(dev->config + pci_bar(dev, r), region->type); + } } pci_update_mappings(dev); } From 9dd749aace247d7ac669ce23d038b912a1f85112 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 17 Jun 2010 14:08:24 +0300 Subject: [PATCH 2/8] pcnet: address TODOs pcnet enables memory/io on init, which does not make sense as BAR values are wrong. Signed-off-by: Michael S. Tsirkin Tested-by: Jan Kiszka --- hw/pcnet.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/hw/pcnet.c b/hw/pcnet.c index 5e7593050e..b52935adf1 100644 --- a/hw/pcnet.c +++ b/hw/pcnet.c @@ -1981,25 +1981,14 @@ static int pci_pcnet_init(PCIDevice *pci_dev) pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_AMD); pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_AMD_LANCE); - /* TODO: value should be 0 at RST# */ - pci_set_word(pci_conf + PCI_COMMAND, - PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER); pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_FAST_BACK | PCI_STATUS_DEVSEL_MEDIUM); pci_conf[PCI_REVISION_ID] = 0x10; - /* TODO: 0 is the default anyway, no need to set it. */ - pci_conf[PCI_CLASS_PROG] = 0x00; pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET); - /* TODO: not necessary, is set when BAR is registered. */ - pci_set_long(pci_conf + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_SPACE_IO); - pci_set_long(pci_conf + PCI_BASE_ADDRESS_0 + 4, - PCI_BASE_ADDRESS_SPACE_MEMORY); - pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID, 0x0); pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, 0x0); - /* TODO: value must be 0 at RST# */ pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0 pci_conf[PCI_MIN_GNT] = 0x06; pci_conf[PCI_MAX_LAT] = 0xff; @@ -2008,11 +1997,10 @@ static int pci_pcnet_init(PCIDevice *pci_dev) s->mmio_index = cpu_register_io_memory(pcnet_mmio_read, pcnet_mmio_write, &d->state); - /* TODO: use pci_dev, avoid cast below. */ - pci_register_bar((PCIDevice *)d, 0, PCNET_IOPORT_SIZE, + pci_register_bar(pci_dev, 0, PCNET_IOPORT_SIZE, PCI_BASE_ADDRESS_SPACE_IO, pcnet_ioport_map); - pci_register_bar((PCIDevice *)d, 1, PCNET_PNPMMIO_SIZE, + pci_register_bar(pci_dev, 1, PCNET_PNPMMIO_SIZE, PCI_BASE_ADDRESS_SPACE_MEMORY, pcnet_mmio_map); s->irq = pci_dev->irq[0]; From f2b07c92a457be0cdd2ec5d848cded2505b9c350 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Tue, 15 Jun 2010 12:48:36 +0900 Subject: [PATCH 3/8] pci hotplug: make pci_device_hot_remove() static Signed-off-by: Isaku Yamahata Acked-by: Gerd Hoffmann Signed-off-by: Michael S. Tsirkin --- hw/pci-hotplug.c | 2 +- sysemu.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c index fe468d646e..c38f47fbf1 100644 --- a/hw/pci-hotplug.c +++ b/hw/pci-hotplug.c @@ -265,7 +265,7 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict) } #endif -int pci_device_hot_remove(Monitor *mon, const char *pci_addr) +static int pci_device_hot_remove(Monitor *mon, const char *pci_addr) { PCIDevice *d; int dom, bus; diff --git a/sysemu.h b/sysemu.h index c758243ca1..9c988bb2a3 100644 --- a/sysemu.h +++ b/sysemu.h @@ -149,7 +149,6 @@ extern unsigned int nb_prom_envs; /* pci-hotplug */ void pci_device_hot_add(Monitor *mon, const QDict *qdict); void drive_hot_add(Monitor *mon, const QDict *qdict); -int pci_device_hot_remove(Monitor *mon, const char *pci_addr); void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict); /* serial ports */ From fdac1d99c4f6078cb3b08f9d21db5f89368bad4b Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 17 Jun 2010 14:17:59 +0300 Subject: [PATCH 4/8] rtl8139: address TODOs Make rtl8139 spec compliant, fixing reset values for command register. Signed-off-by: Michael S. Tsirkin --- hw/rtl8139.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/hw/rtl8139.c b/hw/rtl8139.c index 441f0a9146..d92981dc0d 100644 --- a/hw/rtl8139.c +++ b/hw/rtl8139.c @@ -3357,11 +3357,8 @@ static int pci_rtl8139_init(PCIDevice *dev) pci_conf = s->dev.config; pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_REALTEK); pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_REALTEK_8139); - /* TODO: value should be 0 at RST#. */ - pci_conf[PCI_COMMAND] = PCI_COMMAND_IO | PCI_COMMAND_MASTER; pci_conf[PCI_REVISION_ID] = RTL8139_PCI_REVID; /* >=0x20 is for 8139C+ */ pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET); - /* TODO: value should be 0 at RST# */ pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin 0 */ /* TODO: start of capability list, but no capability * list bit in status register, and offset 0xdc seems unused. */ From bd57ce8c7b3ab2c74335947e69f1eabc73fae438 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 17 Jun 2010 14:01:38 +0300 Subject: [PATCH 5/8] vmware_vga: fix reset value for command register Make init value for this register match the spec. BAR address is 0 at init, so enabling it only works by chance. Signed-off-by: Michael S. Tsirkin --- hw/vmware_vga.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index 8c7224968b..12bff480eb 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -1240,9 +1240,6 @@ static int pci_vmsvga_initfn(PCIDevice *dev) pci_config_set_vendor_id(s->card.config, PCI_VENDOR_ID_VMWARE); pci_config_set_device_id(s->card.config, SVGA_PCI_DEVICE_ID); - s->card.config[PCI_COMMAND] = PCI_COMMAND_IO | - PCI_COMMAND_MEMORY | - PCI_COMMAND_MASTER; /* I/O + Memory */ pci_config_set_class(s->card.config, PCI_CLASS_DISPLAY_VGA); s->card.config[PCI_CACHE_LINE_SIZE] = 0x08; /* Cache line size */ s->card.config[PCI_LATENCY_TIMER] = 0x40; /* Latency timer */ From a213ff63ead450c3bf3b47497681a1498570fcdd Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Tue, 22 Jun 2010 11:55:35 +0900 Subject: [PATCH 6/8] pci hotplug: make pci hotplug return value to caller make pci hotplug callback return value to caller. And when returning error, allocated resources are freed. Signed-off-by: Isaku Yamahata Signed-off-by: Michael S. Tsirkin --- hw/pci.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 645506aa83..fbba6e38fe 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1693,8 +1693,14 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base) pci_dev->romfile = qemu_strdup(info->romfile); pci_add_option_rom(pci_dev); - if (qdev->hotplugged) - bus->hotplug(bus->hotplug_qdev, pci_dev, 1); + if (qdev->hotplugged) { + rc = bus->hotplug(bus->hotplug_qdev, pci_dev, 1); + if (rc != 0) { + int r = pci_unregister_device(&pci_dev->qdev); + assert(!r); + return rc; + } + } return 0; } @@ -1702,8 +1708,7 @@ static int pci_unplug_device(DeviceState *qdev) { PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev); - dev->bus->hotplug(dev->bus->hotplug_qdev, dev, 0); - return 0; + return dev->bus->hotplug(dev->bus->hotplug_qdev, dev, 0); } void pci_qdev_register(PCIDeviceInfo *info) From 279a42535dc977c495bdbda8c8831016e05a0a5d Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 22 Jun 2010 16:22:49 +0300 Subject: [PATCH 7/8] virtio-net: correct packet length math We were requesting too much when checking buffer length: size already includes host header length. Further, we should not exit if we get a packet that is too long, since this might not be under control of the guest. Just drop the packet. Red Hat bz 591494 Signed-off-by: Michael S. Tsirkin --- hw/virtio-net.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index f41db45b00..075f72df2d 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -528,17 +528,18 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_ { VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque; struct virtio_net_hdr_mrg_rxbuf *mhdr = NULL; - size_t hdr_len, offset, i; + size_t guest_hdr_len, offset, i, host_hdr_len; if (!virtio_net_can_receive(&n->nic->nc)) return -1; /* hdr_len refers to the header we supply to the guest */ - hdr_len = n->mergeable_rx_bufs ? + guest_hdr_len = n->mergeable_rx_bufs ? sizeof(struct virtio_net_hdr_mrg_rxbuf) : sizeof(struct virtio_net_hdr); - if (!virtio_net_has_buffers(n, size + hdr_len)) + host_hdr_len = n->has_vnet_hdr ? sizeof(struct virtio_net_hdr) : 0; + if (!virtio_net_has_buffers(n, size + guest_hdr_len - host_hdr_len)) return 0; if (!receive_filter(n, buf, size)) @@ -553,13 +554,14 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_ total = 0; - if ((i != 0 && !n->mergeable_rx_bufs) || - virtqueue_pop(n->rx_vq, &elem) == 0) { + if (virtqueue_pop(n->rx_vq, &elem) == 0) { if (i == 0) return -1; - fprintf(stderr, "virtio-net truncating packet: " - "offset %zd, size %zd, hdr_len %zd\n", - offset, size, hdr_len); + fprintf(stderr, "virtio-net unexpected empty queue: " + "i %zd mergeable %d offset %zd, size %zd, " + "guest hdr len %zd, host hdr len %zd guest features 0x%x\n", + i, n->mergeable_rx_bufs, offset, size, + guest_hdr_len, host_hdr_len, n->vdev.guest_features); exit(1); } @@ -568,7 +570,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_ exit(1); } - if (!n->mergeable_rx_bufs && elem.in_sg[0].iov_len != hdr_len) { + if (!n->mergeable_rx_bufs && elem.in_sg[0].iov_len != guest_hdr_len) { fprintf(stderr, "virtio-net header not in first element\n"); exit(1); } @@ -580,19 +582,32 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_ mhdr = (struct virtio_net_hdr_mrg_rxbuf *)sg[0].iov_base; offset += receive_header(n, sg, elem.in_num, - buf + offset, size - offset, hdr_len); - total += hdr_len; + buf + offset, size - offset, guest_hdr_len); + total += guest_hdr_len; } /* copy in packet. ugh */ len = iov_from_buf(sg, elem.in_num, buf + offset, size - offset); total += len; + offset += len; + /* If buffers can't be merged, at this point we + * must have consumed the complete packet. + * Otherwise, drop it. */ + if (!n->mergeable_rx_bufs && offset < size) { +#if 0 + fprintf(stderr, "virtio-net truncated non-mergeable packet: " + + "i %zd mergeable %d offset %zd, size %zd, " + "guest hdr len %zd, host hdr len %zd\n", + i, n->mergeable_rx_bufs, + offset, size, guest_hdr_len, host_hdr_len); +#endif + return size; + } /* signal other side */ virtqueue_fill(n->rx_vq, &elem, total, i++); - - offset += len; } if (mhdr) From eb0557dbd1c045b57f1fa1ed5c2d22fbfd667583 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 6 Jul 2010 14:17:51 +0300 Subject: [PATCH 8/8] pci: fix bridge update bridge config write should trigger updates on the secondary bus. never on the primary bus. Signed-off-by: Michael S. Tsirkin --- hw/pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/pci.c b/hw/pci.c index fbba6e38fe..a98d6f3ad1 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1568,7 +1568,9 @@ static void pci_bridge_write_config(PCIDevice *d, /* memory base/limit, prefetchable base/limit and io base/limit upper 16 */ ranges_overlap(address, len, PCI_MEMORY_BASE, 20)) { - pci_bridge_update_mappings(d->bus); + PCIBridge *s = container_of(d, PCIBridge, dev); + PCIBus *secondary_bus = &s->bus; + pci_bridge_update_mappings(secondary_bus); } }