From 950fe0aa3f55ad6bb135fc9cde9ebf4df05f62fc Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 15 May 2015 13:46:11 +0100 Subject: [PATCH 1/5] xen/pass-through: fold host PCI command register writes The code introduced to address XSA-126 allows simplification of other code in xen_pt_initfn(): All we need to do is update "cmd" suitably, as it'll be written back to the host register near the end of the function anyway. Signed-off-by: Stefano Stabellini Signed-off-by: Jan Beulich --- hw/xen/xen_pt.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 9afcda8e21..8d47a4502c 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -740,10 +740,7 @@ static int xen_pt_initfn(PCIDevice *d) machine_irq, pirq, rc); /* Disable PCI intx assertion (turn on bit10 of devctl) */ - xen_host_pci_set_word(&s->real_device, - PCI_COMMAND, - pci_get_word(s->dev.config + PCI_COMMAND) - | PCI_COMMAND_INTX_DISABLE); + cmd |= PCI_COMMAND_INTX_DISABLE; machine_irq = 0; s->machine_irq = 0; } else { @@ -765,9 +762,7 @@ static int xen_pt_initfn(PCIDevice *d) e_intx, rc); /* Disable PCI intx assertion (turn on bit10 of devctl) */ - xen_host_pci_set_word(&s->real_device, PCI_COMMAND, - *(uint16_t *)(&s->dev.config[PCI_COMMAND]) - | PCI_COMMAND_INTX_DISABLE); + cmd |= PCI_COMMAND_INTX_DISABLE; xen_pt_mapped_machine_irq[machine_irq]--; if (xen_pt_mapped_machine_irq[machine_irq] == 0) { From 69976894c1d91c4b0c985fa05936cb6b8d01382b Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Mon, 8 Jun 2015 14:11:51 +0100 Subject: [PATCH 2/5] xen/pass-through: ROM BAR handling adjustments Expecting the ROM BAR to be written with an all ones value when sizing the region is wrong - the low bit has another meaning (enable/disable) and bits 1..10 are reserved. The PCI spec also mandates writing all ones to just the address portion of the register. Use suitable constants also for initializing the ROM BAR register field description. Signed-off-by: Stefano Stabellini Signed-off-by: Jan Beulich --- hw/xen/xen_pt.c | 16 ++++++++++++---- hw/xen/xen_pt_config_init.c | 4 ++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 8d47a4502c..7b8a31e4b7 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -249,10 +249,18 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr, /* check unused BAR register */ index = xen_pt_bar_offset_to_index(addr); - if ((index >= 0) && (val > 0 && val < XEN_PT_BAR_ALLF) && - (s->bases[index].bar_flag == XEN_PT_BAR_FLAG_UNUSED)) { - XEN_PT_WARN(d, "Guest attempt to set address to unused Base Address " - "Register. (addr: 0x%02x, len: %d)\n", addr, len); + if ((index >= 0) && (val != 0)) { + uint32_t chk = val; + + if (index == PCI_ROM_SLOT) + chk |= (uint32_t)~PCI_ROM_ADDRESS_MASK; + + if ((chk != XEN_PT_BAR_ALLF) && + (s->bases[index].bar_flag == XEN_PT_BAR_FLAG_UNUSED)) { + XEN_PT_WARN(d, "Guest attempt to set address to unused " + "Base Address Register. (addr: 0x%02x, len: %d)\n", + addr, len); + } } /* find register group entry */ diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c index f3cf069b60..f3730928d7 100644 --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -729,8 +729,8 @@ static XenPTRegInfo xen_pt_emu_reg_header0[] = { .offset = PCI_ROM_ADDRESS, .size = 4, .init_val = 0x00000000, - .ro_mask = 0x000007FE, - .emu_mask = 0xFFFFF800, + .ro_mask = ~PCI_ROM_ADDRESS_MASK & ~PCI_ROM_ADDRESS_ENABLE, + .emu_mask = (uint32_t)PCI_ROM_ADDRESS_MASK, .init = xen_pt_bar_reg_init, .u.dw.read = xen_pt_long_reg_read, .u.dw.write = xen_pt_exp_rom_bar_reg_write, From 3782f60d2047cb86567889307ce78baacf518635 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 5 Jun 2015 13:04:18 +0100 Subject: [PATCH 3/5] xen/pass-through: log errno values rather than function return ones Functions setting errno commonly return just -1, which is of no particular use in the log file. Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini --- hw/xen/xen_pt.c | 26 +++++++++++++------------- hw/xen/xen_pt_msi.c | 24 ++++++++++++------------ 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 7b8a31e4b7..329dbd2794 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -615,8 +615,8 @@ static void xen_pt_region_update(XenPCIPassthroughState *s, guest_port, machine_port, size, op); if (rc) { - XEN_PT_ERR(d, "%s ioport mapping failed! (rc: %i)\n", - adding ? "create new" : "remove old", rc); + XEN_PT_ERR(d, "%s ioport mapping failed! (err: %i)\n", + adding ? "create new" : "remove old", errno); } } else { pcibus_t guest_addr = sec->offset_within_address_space; @@ -629,8 +629,8 @@ static void xen_pt_region_update(XenPCIPassthroughState *s, XEN_PFN(size + XC_PAGE_SIZE - 1), op); if (rc) { - XEN_PT_ERR(d, "%s mem mapping failed! (rc: %i)\n", - adding ? "create new" : "remove old", rc); + XEN_PT_ERR(d, "%s mem mapping failed! (err: %i)\n", + adding ? "create new" : "remove old", errno); } } } @@ -744,8 +744,8 @@ static int xen_pt_initfn(PCIDevice *d) rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq); if (rc < 0) { - XEN_PT_ERR(d, "Mapping machine irq %u to pirq %i failed, (rc: %d)\n", - machine_irq, pirq, rc); + XEN_PT_ERR(d, "Mapping machine irq %u to pirq %i failed, (err: %d)\n", + machine_irq, pirq, errno); /* Disable PCI intx assertion (turn on bit10 of devctl) */ cmd |= PCI_COMMAND_INTX_DISABLE; @@ -766,8 +766,8 @@ static int xen_pt_initfn(PCIDevice *d) PCI_SLOT(d->devfn), e_intx); if (rc < 0) { - XEN_PT_ERR(d, "Binding of interrupt %i failed! (rc: %d)\n", - e_intx, rc); + XEN_PT_ERR(d, "Binding of interrupt %i failed! (err: %d)\n", + e_intx, errno); /* Disable PCI intx assertion (turn on bit10 of devctl) */ cmd |= PCI_COMMAND_INTX_DISABLE; @@ -776,7 +776,7 @@ static int xen_pt_initfn(PCIDevice *d) if (xen_pt_mapped_machine_irq[machine_irq] == 0) { if (xc_physdev_unmap_pirq(xen_xc, xen_domid, machine_irq)) { XEN_PT_ERR(d, "Unmapping of machine interrupt %i failed!" - " (rc: %d)\n", machine_irq, rc); + " (err: %d)\n", machine_irq, errno); } } s->machine_irq = 0; @@ -814,9 +814,9 @@ static void xen_pt_unregister_device(PCIDevice *d) 0 /* isa_irq */); if (rc < 0) { XEN_PT_ERR(d, "unbinding of interrupt INT%c failed." - " (machine irq: %i, rc: %d)" + " (machine irq: %i, err: %d)" " But bravely continuing on..\n", - 'a' + intx, machine_irq, rc); + 'a' + intx, machine_irq, errno); } } @@ -834,9 +834,9 @@ static void xen_pt_unregister_device(PCIDevice *d) rc = xc_physdev_unmap_pirq(xen_xc, xen_domid, machine_irq); if (rc < 0) { - XEN_PT_ERR(d, "unmapping of interrupt %i failed. (rc: %d)" + XEN_PT_ERR(d, "unmapping of interrupt %i failed. (err: %d)" " But bravely continuing on..\n", - machine_irq, rc); + machine_irq, errno); } } } diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c index 68db6233dc..263e0514a2 100644 --- a/hw/xen/xen_pt_msi.c +++ b/hw/xen/xen_pt_msi.c @@ -132,8 +132,8 @@ static int msi_msix_setup(XenPCIPassthroughState *s, msix_entry, table_base); if (rc) { XEN_PT_ERR(&s->dev, - "Mapping of MSI%s (rc: %i, vec: %#x, entry %#x)\n", - is_msix ? "-X" : "", rc, gvec, msix_entry); + "Mapping of MSI%s (err: %i, vec: %#x, entry %#x)\n", + is_msix ? "-X" : "", errno, gvec, msix_entry); return rc; } } @@ -166,12 +166,12 @@ static int msi_msix_update(XenPCIPassthroughState *s, pirq, gflags, table_addr); if (rc) { - XEN_PT_ERR(d, "Updating of MSI%s failed. (rc: %d)\n", - is_msix ? "-X" : "", rc); + XEN_PT_ERR(d, "Updating of MSI%s failed. (err: %d)\n", + is_msix ? "-X" : "", errno); if (xc_physdev_unmap_pirq(xen_xc, xen_domid, *old_pirq)) { - XEN_PT_ERR(d, "Unmapping of MSI%s pirq %d failed.\n", - is_msix ? "-X" : "", *old_pirq); + XEN_PT_ERR(d, "Unmapping of MSI%s pirq %d failed. (err: %d)\n", + is_msix ? "-X" : "", *old_pirq, errno); } *old_pirq = XEN_PT_UNASSIGNED_PIRQ; } @@ -199,8 +199,8 @@ static int msi_msix_disable(XenPCIPassthroughState *s, is_msix ? "-X" : "", pirq, gvec); rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, gflags); if (rc) { - XEN_PT_ERR(d, "Unbinding of MSI%s failed. (pirq: %d, gvec: %#x)\n", - is_msix ? "-X" : "", pirq, gvec); + XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: %d, gvec: %#x)\n", + is_msix ? "-X" : "", errno, pirq, gvec); return rc; } } @@ -208,8 +208,8 @@ static int msi_msix_disable(XenPCIPassthroughState *s, XEN_PT_LOG(d, "Unmap MSI%s pirq %d\n", is_msix ? "-X" : "", pirq); rc = xc_physdev_unmap_pirq(xen_xc, xen_domid, pirq); if (rc) { - XEN_PT_ERR(d, "Unmapping of MSI%s pirq %d failed. (rc: %i)\n", - is_msix ? "-X" : "", pirq, rc); + XEN_PT_ERR(d, "Unmapping of MSI%s pirq %d failed. (err: %i)\n", + is_msix ? "-X" : "", pirq, errno); return rc; } @@ -385,8 +385,8 @@ int xen_pt_msix_update_remap(XenPCIPassthroughState *s, int bar_index) ret = xc_domain_unbind_pt_irq(xen_xc, xen_domid, entry->pirq, PT_IRQ_TYPE_MSI, 0, 0, 0, 0); if (ret) { - XEN_PT_ERR(&s->dev, "unbind MSI-X entry %d failed\n", - entry->pirq); + XEN_PT_ERR(&s->dev, "unbind MSI-X entry %d failed (err: %d)\n", + entry->pirq, errno); } entry->updated = true; } From 74526eb01886ca45774c1e9c736f61536fa2bda1 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 5 Jun 2015 13:04:55 +0100 Subject: [PATCH 4/5] xen/pass-through: constify some static data This is done indirectly by adjusting two typedefs and helps emphasizing that the respective tables aren't supposed to be modified at runtime (as they may be shared between devices). Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini --- hw/xen/xen_pt.h | 8 ++++---- hw/xen/xen_pt_config_init.c | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index 4bba559763..232165a7d1 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -31,7 +31,7 @@ void xen_pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3); /* Helper */ #define XEN_PFN(x) ((x) >> XC_PAGE_SHIFT) -typedef struct XenPTRegInfo XenPTRegInfo; +typedef const struct XenPTRegInfo XenPTRegInfo; typedef struct XenPTReg XenPTReg; typedef struct XenPCIPassthroughState XenPCIPassthroughState; @@ -133,11 +133,11 @@ struct XenPTReg { uint32_t data; /* emulated value */ }; -typedef struct XenPTRegGroupInfo XenPTRegGroupInfo; +typedef const struct XenPTRegGroupInfo XenPTRegGroupInfo; /* emul reg group size initialize method */ typedef int (*xen_pt_reg_size_init_fn) - (XenPCIPassthroughState *, const XenPTRegGroupInfo *, + (XenPCIPassthroughState *, XenPTRegGroupInfo *, uint32_t base_offset, uint8_t *size); /* emulated register group information */ @@ -152,7 +152,7 @@ struct XenPTRegGroupInfo { /* emul register group management table */ typedef struct XenPTRegGroup { QLIST_ENTRY(XenPTRegGroup) entries; - const XenPTRegGroupInfo *reg_grp; + XenPTRegGroupInfo *reg_grp; uint32_t base_offset; uint8_t size; QLIST_HEAD(, XenPTReg) reg_tbl_list; diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c index f3730928d7..dd37be38a4 100644 --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -96,8 +96,7 @@ XenPTReg *xen_pt_find_reg(XenPTRegGroup *reg_grp, uint32_t address) } static uint32_t get_throughable_mask(const XenPCIPassthroughState *s, - const XenPTRegInfo *reg, - uint32_t valid_mask) + XenPTRegInfo *reg, uint32_t valid_mask) { uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask); From ffffbb369f3ed9bca5ff2867143f76d0c6e069c0 Mon Sep 17 00:00:00 2001 From: Stefano Stabellini Date: Mon, 22 Jun 2015 13:00:42 +0000 Subject: [PATCH 5/5] Revert "xen-hvm: increase maxmem before calling xc_domain_populate_physmap" This reverts commit c1d322e6048796296555dd36fdd102d7fa2f50bf. The original commit fixes a bug when assigning a large number of devices which require option roms to a guest. (One known configuration that needs extra memory is having more than 4 emulated NICs assigned. Three or fewer NICs seems to work without this functionality.) However, by unilaterally increasing maxmem, it introduces two problems. First, now libxl's calculation of the required maxmem during migration is broken -- any guest which exercised this functionality will fail on migration. (Guests which have the default number of devices are not affected.) Secondly, it makes it impossible for a higher-level toolstack or administer to predict how much memory a VM will actually use, making it much more difficult to effectively use all of the memory on a machine. Signed-off-by: George Dunlap Signed-off-by: Stefano Stabellini --- xen-hvm.c | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/xen-hvm.c b/xen-hvm.c index 315864ca70..31cb3cac61 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -87,12 +87,6 @@ static inline ioreq_t *xen_vcpu_ioreq(shared_iopage_t *shared_page, int vcpu) #endif #define BUFFER_IO_MAX_DELAY 100 -/* Leave some slack so that hvmloader does not complain about lack of - * memory at boot time ("Could not allocate order=0 extent"). - * Once hvmloader is modified to cope with that situation without - * printing warning messages, QEMU_SPARE_PAGES can be removed. - */ -#define QEMU_SPARE_PAGES 16 typedef struct XenPhysmap { hwaddr start_addr; @@ -250,8 +244,6 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr) unsigned long nr_pfn; xen_pfn_t *pfn_list; int i; - xc_domaininfo_t info; - unsigned long free_pages; if (runstate_check(RUN_STATE_INMIGRATE)) { /* RAM already populated in Xen */ @@ -274,22 +266,6 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr) pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i; } - if ((xc_domain_getinfolist(xen_xc, xen_domid, 1, &info) != 1) || - (info.domain != xen_domid)) { - hw_error("xc_domain_getinfolist failed"); - } - free_pages = info.max_pages - info.tot_pages; - if (free_pages > QEMU_SPARE_PAGES) { - free_pages -= QEMU_SPARE_PAGES; - } else { - free_pages = 0; - } - if ((free_pages < nr_pfn) && - (xc_domain_setmaxmem(xen_xc, xen_domid, - ((info.max_pages + nr_pfn - free_pages) - << (XC_PAGE_SHIFT - 10))) < 0)) { - hw_error("xc_domain_setmaxmem failed"); - } if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, pfn_list)) { hw_error("xen: failed to populate ram at " RAM_ADDR_FMT, ram_addr); }