From 8059feee004111534c4c0652e2f0715e9b4e0754 Mon Sep 17 00:00:00 2001 From: Michael S. Tsirkin Date: Tue, 27 Oct 2015 10:01:44 +0200 Subject: virtio: introduce virtio_map virtio_map_sg currently fails if one of the entries it's mapping is contigious in GPA but not HVA address space. Introduce virtio_map which handles this by splitting sg entries. This new API generally turns out to be a good idea since it's harder to misuse: at least in one case the existing one was used incorrectly. This will still fail if there's no space left in the sg, but luckily max queue size in use is currently 256, while max sg size is 1024, so we should be OK even is all entries happen to cross a single DIMM boundary. Won't work well with very small DIMM sizes, unfortunately: e.g. this will fail with 4K DIMMs where a single request might span a large number of DIMMs. Let's hope these are uncommon - at least we are not breaking things. Note: virtio-scsi calls virtio_map_sg on data loaded from network, and validates input, asserting on failure. Copy the validating code here - it will be dropped from virtio-scsi in a follow-up patch. Reported-by: Igor Mammedov Signed-off-by: Michael S. Tsirkin Reviewed-by: Stefan Hajnoczi Reviewed-by: Igor Mammedov --- include/hw/virtio/virtio.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 9d09115fab..9d9abb4689 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -153,6 +153,7 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem, void virtqueue_map_sg(struct iovec *sg, hwaddr *addr, size_t num_sg, int is_write); +void virtqueue_map(VirtQueueElement *elem); int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem); int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes, unsigned int out_bytes); -- cgit v1.2.3-55-g7522 From 3945ecf1ec4f6e6aa28d0c396a7f5d983c6810d8 Mon Sep 17 00:00:00 2001 From: Michael S. Tsirkin Date: Tue, 27 Oct 2015 10:22:59 +0200 Subject: virtio: drop virtqueue_map_sg Deprecated in favor of virtqueue_map. Signed-off-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov --- hw/virtio/virtio.c | 7 ------- include/hw/virtio/virtio.h | 2 -- 2 files changed, 9 deletions(-) (limited to 'include') diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 462157044c..939f802110 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -491,13 +491,6 @@ static void virtqueue_map_iovec(struct iovec *sg, hwaddr *addr, } } -/* Deprecated: don't use in new code */ -void virtqueue_map_sg(struct iovec *sg, hwaddr *addr, - size_t num_sg, int is_write) -{ - virtqueue_map_iovec(sg, addr, &num_sg, num_sg, is_write); -} - void virtqueue_map(VirtQueueElement *elem) { virtqueue_map_iovec(elem->in_sg, elem->in_addr, &elem->in_num, diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 9d9abb4689..205fadf234 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -151,8 +151,6 @@ void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem, void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem, unsigned int len, unsigned int idx); -void virtqueue_map_sg(struct iovec *sg, hwaddr *addr, - size_t num_sg, int is_write); void virtqueue_map(VirtQueueElement *elem); int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem); int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes, -- cgit v1.2.3-55-g7522 From 340065e5a11a515382c8b1112424c97e86ad2a3f Mon Sep 17 00:00:00 2001 From: Michael S. Tsirkin Date: Wed, 28 Oct 2015 18:54:05 +0200 Subject: Revert "pc: memhp: force gaps between DIMM's GPA" This reverts commit aa8580cddf011e8cedcf87f7a0fdea7549fc4704. As described in http://article.gmane.org/gmane.comp.emulators.qemu/371432 that commit causes linux guests to crash on memory hot-unplug. The original problem it's trying to solve has now been addressed within virtio. Signed-off-by: Michael S. Tsirkin --- hw/i386/pc.c | 6 ++---- hw/i386/pc_piix.c | 1 - hw/i386/pc_q35.c | 1 - include/hw/i386/pc.h | 1 - 4 files changed, 2 insertions(+), 7 deletions(-) (limited to 'include') diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 3d958bae5b..d234caebbb 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1616,7 +1616,6 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, HotplugHandlerClass *hhc; Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); - PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); PCDIMMDevice *dimm = PC_DIMM(dev); PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); MemoryRegion *mr = ddc->get_memory_region(dimm); @@ -1632,8 +1631,8 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, goto out; } - pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, - pcmc->inter_dimm_gap, &local_err); + pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, false, + &local_err); if (local_err) { goto out; } @@ -1953,7 +1952,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) PCMachineClass *pcmc = PC_MACHINE_CLASS(oc); HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc); - pcmc->inter_dimm_gap = true; pcmc->get_hotplug_handler = mc->get_hotplug_handler; mc->get_hotplug_handler = pc_get_hotpug_handler; mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 9d4425a5b9..393dcc4544 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -487,7 +487,6 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m) m->alias = NULL; m->is_default = 0; pcmc->broken_reserved_end = true; - pcmc->inter_dimm_gap = false; SET_MACHINE_COMPAT(m, PC_COMPAT_2_4); } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 3744abd397..2f8f3963c4 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -385,7 +385,6 @@ static void pc_q35_2_4_machine_options(MachineClass *m) pc_q35_2_5_machine_options(m); m->alias = NULL; pcmc->broken_reserved_end = true; - pcmc->inter_dimm_gap = false; SET_MACHINE_COMPAT(m, PC_COMPAT_2_4); } diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 7037de044d..606dbc2854 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -60,7 +60,6 @@ struct PCMachineClass { /*< public >*/ bool broken_reserved_end; - bool inter_dimm_gap; HotplugHandler *(*get_hotplug_handler)(MachineState *machine, DeviceState *dev); }; -- cgit v1.2.3-55-g7522 From d6a9b0b89d27e0a688f37c1732d4dec40613669e Mon Sep 17 00:00:00 2001 From: Michael S. Tsirkin Date: Wed, 28 Oct 2015 18:55:06 +0200 Subject: Revert "memhp: extend address auto assignment to support gaps" This reverts commit df0acded19ec4b826aa095cfc19d341bd66fafd3. There's no point to it now that the only user has been reverted. Signed-off-by: Michael S. Tsirkin --- hw/i386/pc.c | 3 +-- hw/mem/pc-dimm.c | 15 ++++++--------- hw/ppc/spapr.c | 2 +- include/hw/mem/pc-dimm.h | 7 +++---- 4 files changed, 11 insertions(+), 16 deletions(-) (limited to 'include') diff --git a/hw/i386/pc.c b/hw/i386/pc.c index d234caebbb..0cb8afd2c2 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1631,8 +1631,7 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, goto out; } - pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, false, - &local_err); + pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, &local_err); if (local_err) { goto out; } diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 2bae994667..80f424b442 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -33,8 +33,7 @@ typedef struct pc_dimms_capacity { } pc_dimms_capacity; void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms, - MemoryRegion *mr, uint64_t align, bool gap, - Error **errp) + MemoryRegion *mr, uint64_t align, Error **errp) { int slot; MachineState *machine = MACHINE(qdev_get_machine()); @@ -50,7 +49,7 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms, addr = pc_dimm_get_free_addr(hpms->base, memory_region_size(&hpms->mr), - !addr ? NULL : &addr, align, gap, + !addr ? NULL : &addr, align, memory_region_size(mr), &local_err); if (local_err) { goto out; @@ -295,8 +294,8 @@ static int pc_dimm_built_list(Object *obj, void *opaque) uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, uint64_t address_space_size, - uint64_t *hint, uint64_t align, bool gap, - uint64_t size, Error **errp) + uint64_t *hint, uint64_t align, uint64_t size, + Error **errp) { GSList *list = NULL, *item; uint64_t new_addr, ret = 0; @@ -341,15 +340,13 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, goto out; } - if (ranges_overlap(dimm->addr, dimm_size, new_addr, - size + (gap ? 1 : 0))) { + if (ranges_overlap(dimm->addr, dimm_size, new_addr, size)) { if (hint) { DeviceState *d = DEVICE(dimm); error_setg(errp, "address range conflicts with '%s'", d->id); goto out; } - new_addr = QEMU_ALIGN_UP(dimm->addr + dimm_size + (gap ? 1 : 0), - align); + new_addr = QEMU_ALIGN_UP(dimm->addr + dimm_size, align); } } ret = new_addr; diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e1202cec9f..288b57e24f 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2157,7 +2157,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, goto out; } - pc_dimm_memory_plug(dev, &ms->hotplug_memory, mr, align, false, &local_err); + pc_dimm_memory_plug(dev, &ms->hotplug_memory, mr, align, &local_err); if (local_err) { goto out; } diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h index c1ee7b0408..d83bf30ea9 100644 --- a/include/hw/mem/pc-dimm.h +++ b/include/hw/mem/pc-dimm.h @@ -83,16 +83,15 @@ typedef struct MemoryHotplugState { uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, uint64_t address_space_size, - uint64_t *hint, uint64_t align, bool gap, - uint64_t size, Error **errp); + uint64_t *hint, uint64_t align, uint64_t size, + Error **errp); int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp); int qmp_pc_dimm_device_list(Object *obj, void *opaque); uint64_t pc_existing_dimms_capacity(Error **errp); void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms, - MemoryRegion *mr, uint64_t align, bool gap, - Error **errp); + MemoryRegion *mr, uint64_t align, Error **errp); void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms, MemoryRegion *mr); #endif -- cgit v1.2.3-55-g7522 From 3f1e1478db2d67098d98f2c3acf5a4946b7fb643 Mon Sep 17 00:00:00 2001 From: Cao jin Date: Wed, 28 Oct 2015 14:20:31 +0800 Subject: enable multi-function hot-add Enable PCIe device multi-function hot-add, just ensure function 0 is added last, then driver will get the notification to scan the slot. Signed-off-by: Cao jin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pci.c | 40 +++++++++++++++++++++++++++++++++++++++- hw/pci/pci_host.c | 15 +++++++++++++++ hw/pci/pcie.c | 18 +++++++++--------- include/hw/pci/pci.h | 1 + 4 files changed, 64 insertions(+), 10 deletions(-) (limited to 'include') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index b0bf54061f..168b9cc56b 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -847,6 +847,9 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, PCIConfigWriteFunc *config_write = pc->config_write; Error *local_err = NULL; AddressSpace *dma_as; + DeviceState *dev = DEVICE(pci_dev); + + pci_dev->bus = bus; if (devfn < 0) { for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices); @@ -864,9 +867,17 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name); return NULL; + } else if (dev->hotplugged && + pci_get_function_0(pci_dev)) { + error_setg(errp, "PCI: slot %d function 0 already ocuppied by %s," + " new func %s cannot be exposed to guest.", + PCI_SLOT(devfn), + bus->devices[PCI_DEVFN(PCI_SLOT(devfn), 0)]->name, + name); + + return NULL; } - pci_dev->bus = bus; pci_dev->devfn = devfn; dma_as = pci_device_iommu_address_space(pci_dev); @@ -2454,6 +2465,33 @@ void pci_bus_get_w64_range(PCIBus *bus, Range *range) pci_for_each_device_under_bus(bus, pci_dev_get_w64, range); } +static bool pcie_has_upstream_port(PCIDevice *dev) +{ + PCIDevice *parent_dev = pci_bridge_get_device(dev->bus); + + /* Device associated with an upstream port. + * As there are several types of these, it's easier to check the + * parent device: upstream ports are always connected to + * root or downstream ports. + */ + return parent_dev && + pci_is_express(parent_dev) && + parent_dev->exp.exp_cap && + (pcie_cap_get_type(parent_dev) == PCI_EXP_TYPE_ROOT_PORT || + pcie_cap_get_type(parent_dev) == PCI_EXP_TYPE_DOWNSTREAM); +} + +PCIDevice *pci_get_function_0(PCIDevice *pci_dev) +{ + if(pcie_has_upstream_port(pci_dev)) { + /* With an upstream PCIe port, we only support 1 device at slot 0 */ + return pci_dev->bus->devices[0]; + } else { + /* Other bus types might support multiple devices at slots 0-31 */ + return pci_dev->bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]; + } +} + static const TypeInfo pci_device_type_info = { .name = TYPE_PCI_DEVICE, .parent = TYPE_DEVICE, diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c index 3e26f9256c..49f59a5dbc 100644 --- a/hw/pci/pci_host.c +++ b/hw/pci/pci_host.c @@ -20,6 +20,7 @@ #include "hw/pci/pci.h" #include "hw/pci/pci_host.h" +#include "hw/pci/pci_bus.h" #include "trace.h" /* debug PCI */ @@ -52,6 +53,13 @@ void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr, uint32_t limit, uint32_t val, uint32_t len) { assert(len <= 4); + /* non-zero functions are only exposed when function 0 is present, + * allowing direct removal of unexposed functions. + */ + if (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev)) { + return; + } + trace_pci_cfg_write(pci_dev->name, PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn), addr, val); pci_dev->config_write(pci_dev, addr, val, MIN(len, limit - addr)); @@ -63,6 +71,13 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr, uint32_t ret; assert(len <= 4); + /* non-zero functions are only exposed when function 0 is present, + * allowing direct removal of unexposed functions. + */ + if (pci_dev->qdev.hotplugged && !pci_get_function_0(pci_dev)) { + return ~0x0; + } + ret = pci_dev->config_read(pci_dev, addr, MIN(len, limit - addr)); trace_pci_cfg_read(pci_dev->name, PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn), addr, ret); diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index a72d516e4f..32c65c27a4 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -249,16 +249,16 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, return; } - /* TODO: multifunction hot-plug. - * Right now, only a device of function = 0 is allowed to be - * hot plugged/unplugged. + /* To enable multifunction hot-plug, we just ensure the function + * 0 added last. When function 0 is added, we set the sltsta and + * inform OS via event notification. */ - assert(PCI_FUNC(pci_dev->devfn) == 0); - - pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA, - PCI_EXP_SLTSTA_PDS); - pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), - PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP); + if (pci_get_function_0(pci_dev)) { + pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA, + PCI_EXP_SLTSTA_PDS); + pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), + PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP); + } } static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index f5e7fd818a..379b6e1a45 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -397,6 +397,7 @@ void pci_for_each_bus_depth_first(PCIBus *bus, void *(*begin)(PCIBus *bus, void *parent_state), void (*end)(PCIBus *bus, void *state), void *parent_state); +PCIDevice *pci_get_function_0(PCIDevice *pci_dev); /* Use this wrapper when specific scan order is not required. */ static inline -- cgit v1.2.3-55-g7522