From 950fe0aa3f55ad6bb135fc9cde9ebf4df05f62fc Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 15 May 2015 13:46:11 +0100 Subject: 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(-) (limited to 'hw/xen') diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 9afcda8..8d47a45 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) { -- cgit v1.1 From 69976894c1d91c4b0c985fa05936cb6b8d01382b Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Mon, 8 Jun 2015 14:11:51 +0100 Subject: 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(-) (limited to 'hw/xen') diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 8d47a45..7b8a31e 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 f3cf069..f373092 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, -- cgit v1.1 From 3782f60d2047cb86567889307ce78baacf518635 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 5 Jun 2015 13:04:18 +0100 Subject: 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(-) (limited to 'hw/xen') diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 7b8a31e..329dbd2 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 68db623..263e051 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; } -- cgit v1.1 From 74526eb01886ca45774c1e9c736f61536fa2bda1 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 5 Jun 2015 13:04:55 +0100 Subject: 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(-) (limited to 'hw/xen') diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index 4bba559..232165a 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 f373092..dd37be3 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); -- cgit v1.1