Xen PCI Passthrough security fixes

---
 hw/xen/xen_pt.c             |   25 +++++++++++++++++++++----
 hw/xen/xen_pt.h             |    2 --
 hw/xen/xen_pt_config_init.c |    4 ----
 3 files changed, 21 insertions(+), 10 deletions(-)

hw/xen/xen_pt.c +21 -4

index d095c08..8923582 100644
@@ -234,7 +234,7 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
     int index = 0;
     XenPTRegGroup *reg_grp_entry = NULL;
     int rc = 0;
-    uint32_t read_val = 0;
+    uint32_t read_val = 0, wb_mask;
     int emul_len = 0;
     XenPTReg *reg_entry = NULL;
     uint32_t find_addr = addr;

@@ -271,6 +271,9 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
     if (rc < 0) {
         XEN_PT_ERR(d, "pci_read_block failed. return value: %d.\n", rc);
         memset(&read_val, 0xff, len);
+        wb_mask = 0;
+    } else {
+        wb_mask = 0xFFFFFFFF >> ((4 - len) << 3);
     }
 
     /* pass directly to the real device for passthrough type register group */

@@ -298,6 +301,11 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
 
             valid_mask <<= (find_addr - real_offset) << 3;
             ptr_val = (uint8_t *)&val + (real_offset & 3);
+            if (reg->emu_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) {
+                wb_mask &= ~((reg->emu_mask
+                              >> ((find_addr - real_offset) << 3))
+                             << ((len - emul_len) << 3));
+            }
 
             /* do emulation based on register size */
             switch (reg->size) {

@@ -350,10 +358,19 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
     memory_region_transaction_commit();
 
 out:
-    if (!(reg && reg->no_wb)) {
+    for (index = 0; wb_mask; index += len) {
         /* unknown regs are passed through */
-        rc = xen_host_pci_set_block(&s->real_device, addr,
-                                    (uint8_t *)&val, len);
+        while (!(wb_mask & 0xff)) {
+            index++;
+            wb_mask >>= 8;
+        }
+        len = 0;
+        do {
+            len++;
+            wb_mask >>= 8;
+        } while (wb_mask & 0xff);
+        rc = xen_host_pci_set_block(&s->real_device, addr + index,
+                                    (uint8_t *)&val + index, len);
 
         if (rc < 0) {
             XEN_PT_ERR(d, "pci_write_block failed. return value: %d.\n", rc);

hw/xen/xen_pt.h +0 -2

index 942dc60..52ceb85 100644
@@ -105,8 +105,6 @@ struct XenPTRegInfo {
     uint32_t ro_mask;
     /* reg emulate field mask (ON:emu, OFF:passthrough) */
     uint32_t emu_mask;
-    /* no write back allowed */
-    uint32_t no_wb;
     xen_pt_conf_reg_init init;
     /* read/write function pointer
      * for double_word/word/byte size */

hw/xen/xen_pt_config_init.c +0 -4

index 95a51db..dae0519 100644
@@ -1279,7 +1279,6 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
         .init_val   = 0x00000000,
         .ro_mask    = 0x00000003,
         .emu_mask   = 0xFFFFFFFF,
-        .no_wb      = 1,
         .init       = xen_pt_common_reg_init,
         .u.dw.read  = xen_pt_long_reg_read,
         .u.dw.write = xen_pt_msgaddr32_reg_write,

@@ -1291,7 +1290,6 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
         .init_val   = 0x00000000,
         .ro_mask    = 0x00000000,
         .emu_mask   = 0xFFFFFFFF,
-        .no_wb      = 1,
         .init       = xen_pt_msgaddr64_reg_init,
         .u.dw.read  = xen_pt_long_reg_read,
         .u.dw.write = xen_pt_msgaddr64_reg_write,

@@ -1303,7 +1301,6 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
         .init_val   = 0x0000,
         .ro_mask    = 0x0000,
         .emu_mask   = 0xFFFF,
-        .no_wb      = 1,
         .init       = xen_pt_msgdata_reg_init,
         .u.w.read   = xen_pt_word_reg_read,
         .u.w.write  = xen_pt_msgdata_reg_write,

@@ -1315,7 +1312,6 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
         .init_val   = 0x0000,
         .ro_mask    = 0x0000,
         .emu_mask   = 0xFFFF,
-        .no_wb      = 1,
         .init       = xen_pt_msgdata_reg_init,
         .u.w.read   = xen_pt_word_reg_read,
         .u.w.write  = xen_pt_msgdata_reg_write,

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #2 lore raw

PATCH 2/11 xen: don't allow guest to control MSI mask register

From: Jan Beulich <[email protected]>

It's being used by the hypervisor. For now simply mimic a device not capable of masking, and fully emulate any accesses a guest may issue nevertheless as simple reads/writes without side effects.

This is XSA-129.

Signed-off-by Jan Beulich <[email protected]>

---
 hw/pci/msi.c                |    4 --
 hw/xen/xen_pt_config_init.c |   98 ++++++++++++++++++++++++++++++++++++++-----
 include/hw/pci/pci_regs.h   |    2 +
 3 files changed, 90 insertions(+), 14 deletions(-)

hw/pci/msi.c +0 -4

index c111dba..f9c0484 100644
@@ -21,10 +21,6 @@
 #include "hw/pci/msi.h"
 #include "qemu/range.h"
 
-/* Eventually those constants should go to Linux pci_regs.h */
-#define PCI_MSI_PENDING_32      0x10
-#define PCI_MSI_PENDING_64      0x14
-
 /* PCI_MSI_ADDRESS_LO */
 #define PCI_MSI_ADDRESS_LO_MASK         (~0x3)

hw/xen/xen_pt_config_init.c +88 ~~-10~~

128 lines not on this page · raw message

include/hw/pci/pci_regs.h +2 -0

index 56a404b..57e8c80 100644
@@ -298,8 +298,10 @@
 #define PCI_MSI_ADDRESS_HI	8	/* Upper 32 bits (if PCI_MSI_FLAGS_64BIT set) */
 #define PCI_MSI_DATA_32		8	/* 16 bits of data for 32-bit devices */
 #define PCI_MSI_MASK_32		12	/* Mask bits register for 32-bit devices */
+#define PCI_MSI_PENDING_32	16	/* Pending bits register for 32-bit devices */
 #define PCI_MSI_DATA_64		12	/* 16 bits of data for 64-bit devices */
 #define PCI_MSI_MASK_64		16	/* Mask bits register for 64-bit devices */
+#define PCI_MSI_PENDING_64	20	/* Pending bits register for 32-bit devices */
 
 /* MSI-X registers */
 #define PCI_MSIX_FLAGS		2

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #3 lore raw

PATCH 3/11 xen/MSI-X: limit error messages

From: Jan Beulich <[email protected]>

Limit error messages resulting from bad guest behavior to avoid allowing the guest to cause the control domain's disk to fill.

The first message in pci_msix_write() can simply be deleted, as this is indeed bad guest behavior, but such out of bounds writes don't really need to be logged.

The second one is more problematic, as there guest behavior may only appear to be wrong: For one, the old logic didn't take the mask-all bit into account. And then this shouldn't depend on host device state (i.e. the host may have masked the entry without the guest having done so). Plus these writes shouldn't be dropped even when an entry is unmasked. Instead, if they can't be made take effect right away, they should take effect on the next unmasking or enabling operation - the specification explicitly describes such caching behavior. Until we can validly drop the message (implementing such caching/latching behavior), issue the message just once per MSI-X table entry.

Note that the log message in pci_msix_read() similar to the one being removed here is not an issue: "addr" being of unsigned type, and the maximum size of the MSI-X table being 32k, entry_nr simply can't be negative and hence the conditonal guarding issuing of the message will never be true.

This is XSA-130.

Signed-off-by Jan Beulich <[email protected]>

---
 hw/xen/xen_pt.h     |    1 +
 hw/xen/xen_pt_msi.c |   12 +++++++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

hw/xen/xen_pt.h +1 -0

index 52ceb85..8c9b6c2 100644
@@ -175,6 +175,7 @@ typedef struct XenPTMSIXEntry {
     uint32_t data;
     uint32_t vector_ctrl;
     bool updated; /* indicate whether MSI ADDR or DATA is updated */
+    bool warned;  /* avoid issuing (bogus) warning more than once */
 } XenPTMSIXEntry;
 typedef struct XenPTMSIX {
     uint32_t ctrl_offset;

hw/xen/xen_pt_msi.c +7 -5

index 9ed9321..68db623 100644
@@ -434,11 +434,10 @@ static void pci_msix_write(void *opaque, hwaddr addr,
     XenPCIPassthroughState *s = opaque;
     XenPTMSIX *msix = s->msix;
     XenPTMSIXEntry *entry;
-    int entry_nr, offset;
+    unsigned int entry_nr, offset;
 
     entry_nr = addr / PCI_MSIX_ENTRY_SIZE;
-    if (entry_nr < 0 || entry_nr >= msix->total_entries) {
-        XEN_PT_ERR(&s->dev, "asked MSI-X entry '%i' invalid!\n", entry_nr);
+    if (entry_nr >= msix->total_entries) {
         return;
     }
     entry = &msix->msix_entry[entry_nr];

@@ -460,8 +459,11 @@ static void pci_msix_write(void *opaque, hwaddr addr,
             + PCI_MSIX_ENTRY_VECTOR_CTRL;
 
         if (msix->enabled && !(*vec_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT)) {
-            XEN_PT_ERR(&s->dev, "Can't update msix entry %d since MSI-X is"
-                       " already enabled.\n", entry_nr);
+            if (!entry->warned) {
+                entry->warned = true;
+                XEN_PT_ERR(&s->dev, "Can't update msix entry %d since MSI-X is"
+                           " already enabled.\n", entry_nr);
+            }
             return;
         }

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #4 lore raw

PATCH 4/11 xen/MSI: don't open-code pass-through of enable bit modifications

From: Jan Beulich <[email protected]>

Without this the actual XSA-131 fix would cause the enable bit to not get set anymore (due to the write back getting suppressed there based on the OR of emu_mask, ro_mask, and res_mask).

Note that the fiddling with the enable bit shouldn't really be done by qemu, but making this work right (via libxc and the hypervisor) will require more extensive changes, which can be postponed until after the security issue got addressed.

This is a preparatory patch for XSA-131.

Signed-off-by Jan Beulich <[email protected]>

---
 hw/xen/xen_pt_config_init.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

hw/xen/xen_pt_config_init.c +2 -8

index 68b8f22..436d0fd 100644
@@ -1053,7 +1053,6 @@ static int xen_pt_msgctrl_reg_write(XenPCIPassthroughState *s,
     XenPTMSI *msi = s->msi;
     uint16_t writable_mask = 0;
     uint16_t throughable_mask = 0;
-    uint16_t raw_val;
 
     /* Currently no support for multi-vector */
     if (*val & PCI_MSI_FLAGS_QSIZE) {

@@ -1066,12 +1065,11 @@ static int xen_pt_msgctrl_reg_write(XenPCIPassthroughState *s,
     msi->flags |= cfg_entry->data & ~PCI_MSI_FLAGS_ENABLE;
 
     /* create value for writing to I/O device register */
-    raw_val = *val;
     throughable_mask = ~reg->emu_mask & valid_mask;
     *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
 
     /* update MSI */
-    if (raw_val & PCI_MSI_FLAGS_ENABLE) {
+    if (*val & PCI_MSI_FLAGS_ENABLE) {
         /* setup MSI pirq for the first time */
         if (!msi->initialized) {
             /* Init physical one */

@@ -1099,10 +1097,6 @@ static int xen_pt_msgctrl_reg_write(XenPCIPassthroughState *s,
         xen_pt_msi_disable(s);
     }
 
-    /* pass through MSI_ENABLE bit */
-    *val &= ~PCI_MSI_FLAGS_ENABLE;
-    *val |= raw_val & PCI_MSI_FLAGS_ENABLE;
-
     return 0;
 }

@@ -1301,7 +1295,7 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
         .size       = 2,
         .init_val   = 0x0000,
         .ro_mask    = 0xFF8E,
-        .emu_mask   = 0x017F,
+        .emu_mask   = 0x017E,
         .init       = xen_pt_msgctrl_reg_init,
         .u.w.read   = xen_pt_word_reg_read,
         .u.w.write  = xen_pt_msgctrl_reg_write,

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #5 lore raw

PATCH 5/11 xen/pt: consolidate PM capability emu_mask

From: Jan Beulich <[email protected]>

There's no point in xen_pt_pmcsr_reg_{read,write}() each ORing PCI_PM_CTRL_STATE_MASK and PCI_PM_CTRL_NO_SOFT_RESET into a local emu_mask variable - we can have the same effect by setting the field descriptor's emu_mask member suitably right away. Note that xen_pt_pmcsr_reg_write() is being retained in order to allow later patches to be less intrusive.

This is a preparatory patch for XSA-131.

Signed-off-by Jan Beulich <[email protected]>

Acked-by Ian Campbell <[email protected]>

---
 hw/xen/xen_pt_config_init.c |   25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

hw/xen/xen_pt_config_init.c +4 ~~-21~~

index 436d0fd..516236a 100644
@@ -933,38 +933,21 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[] = {
  * Power Management Capability
  */
 
-/* read Power Management Control/Status register */
-static int xen_pt_pmcsr_reg_read(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
-                                 uint16_t *value, uint16_t valid_mask)
-{
-    XenPTRegInfo *reg = cfg_entry->reg;
-    uint16_t valid_emu_mask = reg->emu_mask;
-
-    valid_emu_mask |= PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_NO_SOFT_RESET;
-
-    valid_emu_mask = valid_emu_mask & valid_mask;
-    *value = XEN_PT_MERGE_VALUE(*value, cfg_entry->data, ~valid_emu_mask);
-
-    return 0;
-}
 /* write Power Management Control/Status register */
 static int xen_pt_pmcsr_reg_write(XenPCIPassthroughState *s,
                                   XenPTReg *cfg_entry, uint16_t *val,
                                   uint16_t dev_value, uint16_t valid_mask)
 {
     XenPTRegInfo *reg = cfg_entry->reg;
-    uint16_t emu_mask = reg->emu_mask;
     uint16_t writable_mask = 0;
     uint16_t throughable_mask = 0;
 
-    emu_mask |= PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_NO_SOFT_RESET;
-
     /* modify emulate register */
-    writable_mask = emu_mask & ~reg->ro_mask & valid_mask;
+    writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
     cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~emu_mask & valid_mask;
+    throughable_mask = ~reg->emu_mask & valid_mask;
     *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
 
     return 0;

@@ -1000,9 +983,9 @@ static XenPTRegInfo xen_pt_emu_reg_pm[] = {
         .size       = 2,
         .init_val   = 0x0008,
         .ro_mask    = 0xE1FC,
-        .emu_mask   = 0x8100,
+        .emu_mask   = 0x810B,
         .init       = xen_pt_common_reg_init,
-        .u.w.read   = xen_pt_pmcsr_reg_read,
+        .u.w.read   = xen_pt_word_reg_read,
         .u.w.write  = xen_pt_pmcsr_reg_write,
     },
     {

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #6 lore raw

PATCH 6/11 xen/pt: correctly handle PM status bit

From: Jan Beulich <[email protected]>

xen_pt_pmcsr_reg_write() needs an adjustment to deal with the RW1C nature of the not passed through bit 15 (PCI_PM_CTRL_PME_STATUS).

This is a preparatory patch for XSA-131.

Signed-off-by Jan Beulich <[email protected]>

---
 hw/xen/xen_pt_config_init.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

hw/xen/xen_pt_config_init.c +2 -1

index 516236a..027ac32 100644
@@ -948,7 +948,8 @@ static int xen_pt_pmcsr_reg_write(XenPCIPassthroughState *s,
 
     /* create value for writing to I/O device register */
     throughable_mask = ~reg->emu_mask & valid_mask;
-    *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
+    *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~PCI_PM_CTRL_PME_STATUS,
+                              throughable_mask);
 
     return 0;
 }

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #7 lore raw

PATCH 7/11 xen/pt: split out calculation of throughable mask in PCI config space handling

From: Jan Beulich <[email protected]>

This is just to avoid having to adjust that calculation later in multiple places.

Note that including ->ro_mask in get_throughable_mask()'s calculation is only an apparent (i.e. benign) behavioral change: For r/o fields it doesn't matter > whether they get passed through - either the same flag is also set in emu_mask (then there's no change at all) or the field is r/o in hardware (and hence a write won't change it anyway).

This is a preparatory patch for XSA-131.

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Anthony PERARD <[email protected]>

---
 hw/xen/xen_pt_config_init.c |   51 +++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 31 deletions(-)

hw/xen/xen_pt_config_init.c +20 ~~-31~~

index 027ac32..3833b9e 100644
@@ -95,6 +95,14 @@ XenPTReg *xen_pt_find_reg(XenPTRegGroup *reg_grp, uint32_t address)
     return NULL;
 }
 
+static uint32_t get_throughable_mask(const XenPCIPassthroughState *s,
+                                     const XenPTRegInfo *reg,
+                                     uint32_t valid_mask)
+{
+    uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask);
+
+    return throughable_mask & valid_mask;
+}
 
 /****************
  * general register functions

@@ -157,14 +165,13 @@ static int xen_pt_byte_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
 {
     XenPTRegInfo *reg = cfg_entry->reg;
     uint8_t writable_mask = 0;
-    uint8_t throughable_mask = 0;
+    uint8_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
 
     /* modify emulate register */
     writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
     cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
     *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
 
     return 0;

@@ -175,14 +182,13 @@ static int xen_pt_word_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
 {
     XenPTRegInfo *reg = cfg_entry->reg;
     uint16_t writable_mask = 0;
-    uint16_t throughable_mask = 0;
+    uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
 
     /* modify emulate register */
     writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
     cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
     *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
 
     return 0;

@@ -193,14 +199,13 @@ static int xen_pt_long_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
 {
     XenPTRegInfo *reg = cfg_entry->reg;
     uint32_t writable_mask = 0;
-    uint32_t throughable_mask = 0;
+    uint32_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
 
     /* modify emulate register */
     writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
     cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
     *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
 
     return 0;

@@ -292,15 +297,13 @@ static int xen_pt_cmd_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
 {
     XenPTRegInfo *reg = cfg_entry->reg;
     uint16_t writable_mask = 0;
-    uint16_t throughable_mask = 0;
+    uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
 
     /* modify emulate register */
     writable_mask = ~reg->ro_mask & valid_mask;
     cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
-
     if (*val & PCI_COMMAND_INTX_DISABLE) {
         throughable_mask |= PCI_COMMAND_INTX_DISABLE;
     } else {

@@ -454,7 +457,6 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
     PCIDevice *d = &s->dev;
     const PCIIORegion *r;
     uint32_t writable_mask = 0;
-    uint32_t throughable_mask = 0;
     uint32_t bar_emu_mask = 0;
     uint32_t bar_ro_mask = 0;
     uint32_t r_size = 0;

@@ -511,8 +513,7 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
     }
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~bar_emu_mask & valid_mask;
-    *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
+    *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0);
 
     return 0;
 }

@@ -526,9 +527,8 @@ static int xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
     XenPTRegion *base = NULL;
     PCIDevice *d = (PCIDevice *)&s->dev;
     uint32_t writable_mask = 0;
-    uint32_t throughable_mask = 0;
+    uint32_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
     pcibus_t r_size = 0;
-    uint32_t bar_emu_mask = 0;
     uint32_t bar_ro_mask = 0;
 
     r_size = d->io_regions[PCI_ROM_SLOT].size;

@@ -537,7 +537,6 @@ static int xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
     r_size = xen_pt_get_emul_size(base->bar_flag, r_size);
 
     /* set emulate mask and read-only mask */
-    bar_emu_mask = reg->emu_mask;
     bar_ro_mask = (reg->ro_mask | (r_size - 1)) & ~PCI_ROM_ADDRESS_ENABLE;
 
     /* modify emulate register */

@@ -545,7 +544,6 @@ static int xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s,
     cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~bar_emu_mask & valid_mask;
     *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
 
     return 0;

@@ -940,14 +938,13 @@ static int xen_pt_pmcsr_reg_write(XenPCIPassthroughState *s,
 {
     XenPTRegInfo *reg = cfg_entry->reg;
     uint16_t writable_mask = 0;
-    uint16_t throughable_mask = 0;
+    uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
 
     /* modify emulate register */
     writable_mask = reg->emu_mask & ~reg->ro_mask & valid_mask;
     cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
     *val = XEN_PT_MERGE_VALUE(*val, dev_value & ~PCI_PM_CTRL_PME_STATUS,
                               throughable_mask);

@@ -1036,7 +1033,7 @@ static int xen_pt_msgctrl_reg_write(XenPCIPassthroughState *s,
     XenPTRegInfo *reg = cfg_entry->reg;
     XenPTMSI *msi = s->msi;
     uint16_t writable_mask = 0;
-    uint16_t throughable_mask = 0;
+    uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
 
     /* Currently no support for multi-vector */
     if (*val & PCI_MSI_FLAGS_QSIZE) {

@@ -1049,7 +1046,6 @@ static int xen_pt_msgctrl_reg_write(XenPCIPassthroughState *s,
     msi->flags |= cfg_entry->data & ~PCI_MSI_FLAGS_ENABLE;
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
     *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
 
     /* update MSI */

@@ -1161,7 +1157,6 @@ static int xen_pt_msgaddr32_reg_write(XenPCIPassthroughState *s,
 {
     XenPTRegInfo *reg = cfg_entry->reg;
     uint32_t writable_mask = 0;
-    uint32_t throughable_mask = 0;
     uint32_t old_addr = cfg_entry->data;
 
     /* modify emulate register */

@@ -1170,8 +1165,7 @@ static int xen_pt_msgaddr32_reg_write(XenPCIPassthroughState *s,
     s->msi->addr_lo = cfg_entry->data;
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
-    *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
+    *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0);
 
     /* update MSI */
     if (cfg_entry->data != old_addr) {

@@ -1189,7 +1183,6 @@ static int xen_pt_msgaddr64_reg_write(XenPCIPassthroughState *s,
 {
     XenPTRegInfo *reg = cfg_entry->reg;
     uint32_t writable_mask = 0;
-    uint32_t throughable_mask = 0;
     uint32_t old_addr = cfg_entry->data;
 
     /* check whether the type is 64 bit or not */

@@ -1206,8 +1199,7 @@ static int xen_pt_msgaddr64_reg_write(XenPCIPassthroughState *s,
     s->msi->addr_hi = cfg_entry->data;
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
-    *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
+    *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0);
 
     /* update MSI */
     if (cfg_entry->data != old_addr) {

@@ -1229,7 +1221,6 @@ static int xen_pt_msgdata_reg_write(XenPCIPassthroughState *s,
     XenPTRegInfo *reg = cfg_entry->reg;
     XenPTMSI *msi = s->msi;
     uint16_t writable_mask = 0;
-    uint16_t throughable_mask = 0;
     uint16_t old_data = cfg_entry->data;
     uint32_t offset = reg->offset;

@@ -1247,8 +1238,7 @@ static int xen_pt_msgdata_reg_write(XenPCIPassthroughState *s,
     msi->data = cfg_entry->data;
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
-    *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
+    *val = XEN_PT_MERGE_VALUE(*val, dev_value, 0);
 
     /* update MSI */
     if (cfg_entry->data != old_data) {

@@ -1410,7 +1400,7 @@ static int xen_pt_msixctrl_reg_write(XenPCIPassthroughState *s,
 {
     XenPTRegInfo *reg = cfg_entry->reg;
     uint16_t writable_mask = 0;
-    uint16_t throughable_mask = 0;
+    uint16_t throughable_mask = get_throughable_mask(s, reg, valid_mask);
     int debug_msix_enabled_old;
 
     /* modify emulate register */

@@ -1418,7 +1408,6 @@ static int xen_pt_msixctrl_reg_write(XenPCIPassthroughState *s,
     cfg_entry->data = XEN_PT_MERGE_VALUE(*val, cfg_entry->data, writable_mask);
 
     /* create value for writing to I/O device register */
-    throughable_mask = ~reg->emu_mask & valid_mask;
     *val = XEN_PT_MERGE_VALUE(*val, dev_value, throughable_mask);
 
     /* update MSI-X */

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #8 lore raw

PATCH 8/11 xen/pt: mark all PCIe capability bits read-only

From: Jan Beulich <[email protected]>

xen_pt_emu_reg_pcie[]'s PCI_EXP_DEVCAP needs to cover all bits as read- only to avoid unintended write-back (just a precaution, the field ought to be read-only in hardware).

This is a preparatory patch for XSA-131.

Signed-off-by Jan Beulich <[email protected]>

---
 hw/xen/xen_pt_config_init.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

hw/xen/xen_pt_config_init.c +1 -1

index 3833b9e..9f6c00e 100644
@@ -871,7 +871,7 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[] = {
         .offset     = PCI_EXP_DEVCAP,
         .size       = 4,
         .init_val   = 0x00000000,
-        .ro_mask    = 0x1FFCFFFF,
+        .ro_mask    = 0xFFFFFFFF,
         .emu_mask   = 0x10000000,
         .init       = xen_pt_common_reg_init,
         .u.dw.read  = xen_pt_long_reg_read,

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #9 lore raw

PATCH 9/11 xen/pt: mark reserved bits in PCI config space fields

From: Jan Beulich <[email protected]>

The adjustments are solely to make the subsequent patches work right (and hence make the patch set consistent), namely if permissive mode (introduced by the last patch) gets used (as both reserved registers and reserved fields must be similarly protected from guest access in default mode, but the guest should be allowed access to them in permissive mode).

This is a preparatory patch for XSA-131.

Signed-off-by Jan Beulich <[email protected]>

---
 hw/xen/xen_pt.h             |    2 ++
 hw/xen/xen_pt_config_init.c |   14 +++++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

hw/xen/xen_pt.h +2 -0

index 8c9b6c2..f9795eb 100644
@@ -101,6 +101,8 @@ struct XenPTRegInfo {
     uint32_t offset;
     uint32_t size;
     uint32_t init_val;
+    /* reg reserved field mask (ON:reserved, OFF:defined) */
+    uint32_t res_mask;
     /* reg read only field mask (ON:RO/ROS, OFF:other) */
     uint32_t ro_mask;
     /* reg emulate field mask (ON:emu, OFF:passthrough) */

hw/xen/xen_pt_config_init.c +9 -5

index 9f6c00e..efd8bac 100644
@@ -578,7 +578,7 @@ static XenPTRegInfo xen_pt_emu_reg_header0[] = {
         .offset     = PCI_COMMAND,
         .size       = 2,
         .init_val   = 0x0000,
-        .ro_mask    = 0xF880,
+        .res_mask   = 0xF880,
         .emu_mask   = 0x0743,
         .init       = xen_pt_common_reg_init,
         .u.w.read   = xen_pt_word_reg_read,

@@ -603,7 +603,8 @@ static XenPTRegInfo xen_pt_emu_reg_header0[] = {
         .offset     = PCI_STATUS,
         .size       = 2,
         .init_val   = 0x0000,
-        .ro_mask    = 0x06FF,
+        .res_mask   = 0x0007,
+        .ro_mask    = 0x06F8,
         .emu_mask   = 0x0010,
         .init       = xen_pt_status_reg_init,
         .u.w.read   = xen_pt_word_reg_read,

@@ -980,7 +981,8 @@ static XenPTRegInfo xen_pt_emu_reg_pm[] = {
         .offset     = PCI_PM_CTRL,
         .size       = 2,
         .init_val   = 0x0008,
-        .ro_mask    = 0xE1FC,
+        .res_mask   = 0x00F0,
+        .ro_mask    = 0xE10C,
         .emu_mask   = 0x810B,
         .init       = xen_pt_common_reg_init,
         .u.w.read   = xen_pt_word_reg_read,

@@ -1268,7 +1270,8 @@ static XenPTRegInfo xen_pt_emu_reg_msi[] = {
         .offset     = PCI_MSI_FLAGS,
         .size       = 2,
         .init_val   = 0x0000,
-        .ro_mask    = 0xFF8E,
+        .res_mask   = 0xFE00,
+        .ro_mask    = 0x018E,
         .emu_mask   = 0x017E,
         .init       = xen_pt_msgctrl_reg_init,
         .u.w.read   = xen_pt_word_reg_read,

@@ -1446,7 +1449,8 @@ static XenPTRegInfo xen_pt_emu_reg_msix[] = {
         .offset     = PCI_MSI_FLAGS,
         .size       = 2,
         .init_val   = 0x0000,
-        .ro_mask    = 0x3FFF,
+        .res_mask   = 0x3800,
+        .ro_mask    = 0x07FF,
         .emu_mask   = 0x0000,
         .init       = xen_pt_msixctrl_reg_init,
         .u.w.read   = xen_pt_word_reg_read,

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #10 lore raw

PATCH 10/11 xen/pt: add a few PCI config space field descriptions

From: Jan Beulich <[email protected]>

Since the next patch will turn all not explicitly described fields read-only by default, those fields that have guest writable bits need to be given explicit descriptors.

This is a preparatory patch for XSA-131.

Signed-off-by Jan Beulich <[email protected]>

---
 hw/xen/xen_pt_config_init.c |   28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

hw/xen/xen_pt_config_init.c +28 -0

index efd8bac..19f926b 100644
@@ -754,6 +754,15 @@ static XenPTRegInfo xen_pt_emu_reg_vpd[] = {
         .u.b.write  = xen_pt_byte_reg_write,
     },
     {
+        .offset     = PCI_VPD_ADDR,
+        .size       = 2,
+        .ro_mask    = 0x0003,
+        .emu_mask   = 0x0003,
+        .init       = xen_pt_common_reg_init,
+        .u.w.read   = xen_pt_word_reg_read,
+        .u.w.write  = xen_pt_word_reg_write,
+    },
+    {
         .size = 0,
     },
 };

@@ -889,6 +898,16 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[] = {
         .u.w.read   = xen_pt_word_reg_read,
         .u.w.write  = xen_pt_word_reg_write,
     },
+    /* Device Status reg */
+    {
+        .offset     = PCI_EXP_DEVSTA,
+        .size       = 2,
+        .res_mask   = 0xFFC0,
+        .ro_mask    = 0x0030,
+        .init       = xen_pt_common_reg_init,
+        .u.w.read   = xen_pt_word_reg_read,
+        .u.w.write  = xen_pt_word_reg_write,
+    },
     /* Link Control reg */
     {
         .offset     = PCI_EXP_LNKCTL,

@@ -900,6 +919,15 @@ static XenPTRegInfo xen_pt_emu_reg_pcie[] = {
         .u.w.read   = xen_pt_word_reg_read,
         .u.w.write  = xen_pt_word_reg_write,
     },
+    /* Link Status reg */
+    {
+        .offset     = PCI_EXP_LNKSTA,
+        .size       = 2,
+        .ro_mask    = 0x3FFF,
+        .init       = xen_pt_common_reg_init,
+        .u.w.read   = xen_pt_word_reg_read,
+        .u.w.write  = xen_pt_word_reg_write,
+    },
     /* Device Control 2 reg */
     {
         .offset     = 0x28,

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:10 UTC #11 lore raw

PATCH 11/11 xen/pt: unknown PCI config space fields should be read-only

From: Jan Beulich <[email protected]>

... by default. Add a per-device "permissive" mode similar to pciback's to allow restoring previous behavior (and hence break security again, i.e. should be used only for trusted guests).

This is part of XSA-131.

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Anthony PERARD <[email protected]>)

---
 hw/xen/xen_pt.c             |   32 +++++++++++++++++++++++++++++---
 hw/xen/xen_pt.h             |    2 ++
 hw/xen/xen_pt_config_init.c |    4 ++++
 3 files changed, 35 insertions(+), 3 deletions(-)

hw/xen/xen_pt.c +29 -3

index 8923582..9afcda8 100644
@@ -239,6 +239,7 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
     XenPTReg *reg_entry = NULL;
     uint32_t find_addr = addr;
     XenPTRegInfo *reg = NULL;
+    bool wp_flag = false;
 
     if (xen_pt_pci_config_access_check(d, addr, len)) {
         return;

@@ -278,6 +279,10 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
 
     /* pass directly to the real device for passthrough type register group */
     if (reg_grp_entry == NULL) {
+        if (!s->permissive) {
+            wb_mask = 0;
+            wp_flag = true;
+        }
         goto out;
     }

@@ -298,12 +303,15 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
             uint32_t real_offset = reg_grp_entry->base_offset + reg->offset;
             uint32_t valid_mask = 0xFFFFFFFF >> ((4 - emul_len) << 3);
             uint8_t *ptr_val = NULL;
+            uint32_t wp_mask = reg->emu_mask | reg->ro_mask;
 
             valid_mask <<= (find_addr - real_offset) << 3;
             ptr_val = (uint8_t *)&val + (real_offset & 3);
-            if (reg->emu_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) {
-                wb_mask &= ~((reg->emu_mask
-                              >> ((find_addr - real_offset) << 3))
+            if (!s->permissive) {
+                wp_mask |= reg->res_mask;
+            }
+            if (wp_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) {
+                wb_mask &= ~((wp_mask >> ((find_addr - real_offset) << 3))
                              << ((len - emul_len) << 3));
             }

@@ -347,6 +355,16 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
         } else {
             /* nothing to do with passthrough type register,
              * continue to find next byte */
+            if (!s->permissive) {
+                wb_mask &= ~(0xff << ((len - emul_len) << 3));
+                /* Unused BARs will make it here, but we don't want to issue
+                 * warnings for writes to them (bogus writes get dealt with
+                 * above).
+                 */
+                if (index < 0) {
+                    wp_flag = true;
+                }
+            }
             emul_len--;
             find_addr++;
         }

@@ -358,6 +376,13 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
     memory_region_transaction_commit();
 
 out:
+    if (wp_flag && !s->permissive_warned) {
+        s->permissive_warned = true;
+        xen_pt_log(d, "Write-back to unknown field 0x%02x (partially) inhibited (0x%0*x)\n",
+                   addr, len * 2, wb_mask);
+        xen_pt_log(d, "If the device doesn't work, try enabling permissive mode\n");
+        xen_pt_log(d, "(unsafe) and if it helps report the problem to xen-devel\n");
+    }
     for (index = 0; wb_mask; index += len) {
         /* unknown regs are passed through */
         while (!(wb_mask & 0xff)) {

@@ -824,6 +849,7 @@ static void xen_pt_unregister_device(PCIDevice *d)
 
 static Property xen_pci_passthrough_properties[] = {
     DEFINE_PROP_PCI_HOST_DEVADDR("hostaddr", XenPCIPassthroughState, hostaddr),
+    DEFINE_PROP_BOOL("permissive", XenPCIPassthroughState, permissive, false),
     DEFINE_PROP_END_OF_LIST(),
 };

hw/xen/xen_pt.h +2 -0

index f9795eb..4bba559 100644
@@ -197,6 +197,8 @@ struct XenPCIPassthroughState {
 
     PCIHostDeviceAddress hostaddr;
     bool is_virtfn;
+    bool permissive;
+    bool permissive_warned;
     XenHostPCIDevice real_device;
     XenPTRegion bases[PCI_NUM_REGIONS]; /* Access regions */
     QLIST_HEAD(, XenPTRegGroup) reg_grps;

hw/xen/xen_pt_config_init.c +4 -0

index 19f926b..f3cf069 100644
@@ -101,6 +101,10 @@ static uint32_t get_throughable_mask(const XenPCIPassthroughState *s,
 {
     uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask);
 
+    if (!s->permissive) {
+        throughable_mask &= ~reg->res_mask;
+    }
+
     return throughable_mask & valid_mask;
 }

Signature · 1 line

1.7.10.4

Stefano Stabellini

author

2015-06-02 15:32 UTC #12 lore raw

Quoted 8 lines · Stefano Stabellini

Hi all,
the following is a collection of QEMU security fixes for PCI Passthrough on Xen. Non-Xen usages of QEMU are unaffected.
Although the CVEs have already been made public, given the large amount of changes, I decided not to send a pull request without giving a chance to the QEMU community to comment on the patches first.

Peter convinced me to send out a pull request immediately. If anybody has any comments on the patches, we can still fix them up later or even revert them if that becomes necessary.

I'll also apply the patches to all qemu-xen stable trees now.

Quoted 31 lines

Each patch has a detail description of what is trying to fix. You can also cross-reference the CVE numbers.
Jan Beulich (11):
xen: properly gate host writes of modified PCI CFG contents
xen: don't allow guest to control MSI mask register
xen/MSI-X: limit error messages
xen/MSI: don't open-code pass-through of enable bit modifications xen/pt: consolidate PM capability emu_mask
xen/pt: correctly handle PM status bit
xen/pt: split out calculation of throughable mask in PCI config space handling xen/pt: mark all PCIe capability bits read-only
xen/pt: mark reserved bits in PCI config space fields
xen/pt: add a few PCI config space field descriptions
xen/pt: unknown PCI config space fields should be read-only
 hw/pci/msi.c                |    4 -
 hw/xen/xen_pt.c             |   51 +++++++++-
 hw/xen/xen_pt.h             |    7 +-
 hw/xen/xen_pt_config_init.c |  235 ++++++++++++++++++++++++++++---------------
 hw/xen/xen_pt_msi.c         |   12 ++-
 include/hw/pci/pci_regs.h   |    2 +
 6 files changed, 217 insertions(+), 94 deletions(-)
Cheers,
Stefano

Peter Maydell

2015-06-02 15:51 UTC #13 lore raw

Quoted 6 lines · Stefano Stabellini

Quoted 5 lines · Stefano Stabellini
Hi all,
the following is a collection of QEMU security fixes for PCI Passthrough on Xen. Non-Xen usages of QEMU are unaffected.

Quoted 7 lines

Quoted 3 lines
Although the CVEs have already been made public, given the large amount of changes, I decided not to send a pull request without giving a chance to the QEMU community to comment on the patches first.
Peter convinced me to send out a pull request immediately. If anybody has any comments on the patches, we can still fix them up later or even revert them if that becomes necessary.

For the record, the rationale is:
* fixes for CVEs will have been reviewed during the nondisclosure period
* getting security fixes into master in a timely fashion is important
* having the patches in upstream master be different from the ones advertised with the CVE can cause confusion about which are "correct"
* if there are any problems with the CVE fixes (stylistic or otherwise) we can correct them with followup patches

Our workflow/process for handling security issues is not set in stone (indeed it's evolving a bit at the moment), so comments/suggestions welcome.

thanks
-- PMM

Ian Campbell

2015-06-02 15:47 UTC #14 lore raw

Quoted 2 lines · Stefano Stabellini

the following is a collection of QEMU security fixes for PCI Passthrough on Xen.

Part of this locks down the PCI cfg space emulation, which means we now need a way for people to request the old "permissive" behaviour for devices which need it. Per the xl docs:
It is recommended to enable this option only for trusted VMs under administrator control.

The toolstack (libxl, xl etc) already support a permissive flag in the domain cfg, and this series adds a new device property. All we need to do is tie them together.

The simple version is below. I also have an incremental update which uses the QMP device-list-properties command to probe for the presence of this property (so things can automatically work with unpatches qemu). I think it's not really necessary in this case.

Ian.

-----8>---------

Quoted 1 line

From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001

From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.

EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)

Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.

Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.

qemu-xen-traditional already supports the permissive mode setting via xenstore.

Signed-off-by Ian Campbell <[email protected]>

Cc Stefano Stabellini <[email protected]>

Cc Anthony PERARD <[email protected]>

---
v2: Only set argument if permissive==true.

---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

tools/libxl/libxl_qmp.c +12 -0

index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn), PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
     rc = qmp_synchronous_send(qmp, "device_add", args,
                               NULL, NULL, qmp->timeout);

Signature · 1 line

1.7.10.4

Ian Campbell

2015-06-17 12:38 UTC #15 lore raw

On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:

ping?

Quoted 75 lines · Stefano Stabellini

Quoted 2 lines
the following is a collection of QEMU security fixes for PCI Passthrough on Xen.
Part of this locks down the PCI cfg space emulation, which means we now need a way for people to request the old "permissive" behaviour for devices which need it. Per the xl docs:
It is recommended to enable this option only for trusted VMs under administrator control.
The toolstack (libxl, xl etc) already support a permissive flag in the domain cfg, and this series adds a new device property. All we need to do is tie them together.
The simple version is below. I also have an incremental update which uses the QMP device-list-properties command to probe for the presence of this property (so things can automatically work with unpatches qemu). I think it's not really necessary in this case.
Ian.
-----8>---------
From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>
---
v2: Only set argument if permissive==true.
---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
tools/libxl/libxl_qmp.c +12 -0
index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn), PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
     rc = qmp_synchronous_send(qmp, "device_add", args,
                               NULL, NULL, qmp->timeout);

Stefano Stabellini

author

2015-06-17 13:52 UTC #16 lore raw

Quoted 3 lines · Ian Campbell

On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:
ping?

The QEMU patch series is already upstream in QEMU and qemu-xen, so I think we can apply this patch to libxl now.

Quoted 77 lines

Quoted 75 lines · Stefano Stabellini
Quoted 2 lines
the following is a collection of QEMU security fixes for PCI Passthrough on Xen.
Part of this locks down the PCI cfg space emulation, which means we now need a way for people to request the old "permissive" behaviour for devices which need it. Per the xl docs:
It is recommended to enable this option only for trusted VMs under administrator control.
The toolstack (libxl, xl etc) already support a permissive flag in the domain cfg, and this series adds a new device property. All we need to do is tie them together.
The simple version is below. I also have an incremental update which uses the QMP device-list-properties command to probe for the presence of this property (so things can automatically work with unpatches qemu). I think it's not really necessary in this case.
Ian.
-----8>---------
From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>
---
v2: Only set argument if permissive==true.
---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
tools/libxl/libxl_qmp.c +12 -0
index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn), PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
     rc = qmp_synchronous_send(qmp, "device_add", args,
                               NULL, NULL, qmp->timeout);

Ian Campbell

2015-06-17 13:54 UTC #17 lore raw

Quoted 7 lines · Stefano Stabellini

Quoted 3 lines · Ian Campbell
On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:
ping?
The QEMU patch series is already upstream in QEMU and qemu-xen, so I think we can apply this patch to libxl now.

Yes, which is why somebody should Ack or Reviewed-by it.

Quoted 78 lines

Quoted 78 lines
Quoted 76 lines · Stefano Stabellini
Quoted 2 lines
the following is a collection of QEMU security fixes for PCI Passthrough on Xen.
Part of this locks down the PCI cfg space emulation, which means we now need a way for people to request the old "permissive" behaviour for devices which need it. Per the xl docs:
It is recommended to enable this option only for trusted VMs under administrator control.
The toolstack (libxl, xl etc) already support a permissive flag in the domain cfg, and this series adds a new device property. All we need to do is tie them together.
The simple version is below. I also have an incremental update which uses the QMP device-list-properties command to probe for the presence of this property (so things can automatically work with unpatches qemu). I think it's not really necessary in this case.
Ian.
-----8>---------
From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>
---
v2: Only set argument if permissive==true.
---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
tools/libxl/libxl_qmp.c +12 -0
index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn), PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
     rc = qmp_synchronous_send(qmp, "device_add", args,
                               NULL, NULL, qmp->timeout);

Ian Campbell

2015-07-03 14:49 UTC #18 lore raw

PATCH 1/1 tools: libxl: allow permissive qemu-upstream pci passthrough.

On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:

Anyone fancy (n)acking this followup to XSA-131? I think we probably want this for 4.6.

Quoted 52 lines

From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>
---
v2: Only set argument if permissive==true.
---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
tools/libxl/libxl_qmp.c +12 -0
index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn), PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
     rc = qmp_synchronous_send(qmp, "device_add", args,
                               NULL, NULL, qmp->timeout);

George Dunlap

Acked-by on v1 1/1

2015-07-06 12:20 UTC #19 lore raw

Re: [PATCH] tools: libxl: allow permissive qemu-upstream pci passthrough.

Quoted 4 lines · Ian Campbell

On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:
Anyone fancy (n)acking this followup to XSA-131? I think we probably want this for 4.6.

As someone who has had dealings with the libxl side of hte "permissive" flag, I'll certainly Ack the idea of the change:

Acked-by George Dunlap <[email protected]>

Not qualified to say whether there's anyone on qmp listening, or if the flag will be ignored properly on older versions of qemu.

-George

Quoted 60 lines

Quoted 43 lines
From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>
---
v2: Only set argument if permissive==true.
---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
tools/libxl/libxl_qmp.c +5 -0
index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn), PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
tools/libxl/libxl_qmp.c int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev) · line 866
Quoted 10 lines
tools/libxl/libxl_qmp.c +7 -0
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
     rc = qmp_synchronous_send(qmp, "device_add", args,
                               NULL, NULL, qmp->timeout);
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Anthony PERARD

Acked-by on v1 1/1

2015-07-06 12:59 UTC #20 lore raw

Re: [PATCH] tools: libxl: allow permissive qemu-upstream pci passthrough.

Quoted 28 lines · Ian Campbell

On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:
Anyone fancy (n)acking this followup to XSA-131? I think we probably want this for 4.6.
Quoted 23 lines
From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>

Acked-by Anthony PERARD <[email protected]>

Quoted 29 lines

---
v2: Only set argument if permissive==true.

---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

tools/libxl/libxl_qmp.c +12 -0

index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn), PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
     rc = qmp_synchronous_send(qmp, "device_add", args,
                               NULL, NULL, qmp->timeout);

Signature · 1 line

Anthony PERARD

Wei Liu

Acked-by on v1 1/1

2015-07-07 13:40 UTC #21 lore raw

Re: [PATCH] tools: libxl: allow permissive qemu-upstream pci passthrough.

Quoted 28 lines · Ian Campbell

On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:
Anyone fancy (n)acking this followup to XSA-131? I think we probably want this for 4.6.
Quoted 23 lines
From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>

Acked-by Wei Liu <[email protected]>

Quoted 30 lines

Quoted 29 lines

---
v2: Only set argument if permissive==true.

---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

tools/libxl/libxl_qmp.c +12 -0

index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid, libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn), PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
     rc = qmp_synchronous_send(qmp, "device_add", args,
                               NULL, NULL, qmp->timeout);

Ian Campbell

Applied

2015-07-07 15:41 UTC #22 lore raw

Re: [PATCH] tools: libxl: allow permissive qemu-upstream pci passthrough.

Quoted 23 lines · Wei Liu

Quoted 22 lines · Ian Campbell
On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:
Anyone fancy (n)acking this followup to XSA-131? I think we probably want this for 4.6.
Quoted 17 lines
From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.

Quoted 8 lines

Quoted 6 lines
Quoted 6 lines
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>
Acked-by Wei Liu <[email protected]>

Applied, thanks,

Ian.

Ian Campbell

2015-10-06 08:36 UTC #23 lore raw

Re: [PATCH] tools: libxl: allow permissive qemu-upstream pci passthrough.

Ian, Can you backport this to 4.5 and earlier please.

Quoted 66 lines · Ian Campbell

On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:
Anyone fancy (n)acking this followup to XSA-131? I think we probably want this for 4.6.
Quoted 52 lines
From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>
---
v2: Only set argument if permissive==true.
---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
tools/libxl/libxl_qmp.c +12 -0
index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid,
 libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn),
 PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
Quoted 2 lines
rc = qmp_synchronous_send(qmp, "device_add", args,
NULL, NULL, qmp->timeout);
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Stefano Stabellini

author

2015-10-06 13:07 UTC #24 lore raw

Re: [PATCH] tools: libxl: allow permissive qemu-upstream pci passthrough.

All the QEMU side XSA-131 patches are already in the qemu-xen 4.3, 4.4 and 4.5 trees.

Quoted 70 lines · Ian Campbell

Ian, Can you backport this to 4.5 and earlier please.
Quoted 66 lines · Ian Campbell
On Tue, 2015-06-02 at 16:47 +0100, Ian Campbell wrote:
Anyone fancy (n)acking this followup to XSA-131? I think we probably want this for 4.6.
Quoted 52 lines
From c395657b03a1e2b7616d987e7078694874981979 Mon Sep 17 00:00:00 2001
From: Ian Campbell <[email protected]>
Date: Mon, 1 Jun 2015 11:32:23 +0100
Subject: [PATCH] tools: libxl: allow permissive qemu-upstream pci
passthrough.
EMBARGOED UNTIL 2015-06-02 12:00 (WITH XSA-131 ET AL)
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In order to allow local configuration of the existing libxl_device_pci "permissive" flag needs to be plumbed through via the new QMP property added by the XSA-131 patches.
Versions of QEMU prior to XSA-131 did not support this permissive property, so we only pass it if it is true. Older versions only supported permissive mode.
qemu-xen-traditional already supports the permissive mode setting via xenstore.
Signed-off-by Ian Campbell <[email protected]>
Cc Stefano Stabellini <[email protected]>
Cc Anthony PERARD <[email protected]>
---
v2: Only set argument if permissive==true.
---
 tools/libxl/libxl_qmp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
tools/libxl/libxl_qmp.c +12 -0
index 9aa7e2e..6484f5e 100644
@@ -849,6 +849,18 @@ int libxl__qmp_pci_add(libxl__gc *gc, int domid,
 libxl_device_pci *pcidev)
         QMP_PARAMETERS_SPRINTF(&args, "addr", "%x.%x",
                                PCI_SLOT(pcidev->vdevfn),
 PCI_FUNC(pcidev->vdevfn));
     }
+    /*
+     * Version of QEMU prior to the XSA-131 fix did not support this
+     * property and were effectively always in permissive mode. The
+     * fix for XSA-131 switched the default to be restricted by
+     * default and added the permissive property.
+     *
+     * Therefore in order to support both old and new QEMU we only set
+     * the permissive flag if it is true. Users of older QEMU have no
+     * reason to set the flag so this is ok.
+     */
+    if (pcidev->permissive)
+        qmp_parameters_add_bool(gc, &args, "permissive", true);
 
Quoted 2 lines
rc = qmp_synchronous_send(qmp, "device_add", args,
NULL, NULL, qmp->timeout);
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Ian Jackson

2015-10-06 15:18 UTC #25 lore raw

Re: [PATCH] tools: libxl: allow permissive qemu-upstream pci passthrough.

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] tools: libxl: allow permissive qemu-upstream pci passthrough."):

Quoted 5 lines

All the QEMU side XSA-131 patches are already in the qemu-xen 4.3, 4.4 and 4.5 trees.
Quoted 1 line · Ian Campbell
Ian, Can you backport this to 4.5 and earlier please.

Thanks. I have added this to my queue.

Ian.

Ian Jackson

2015-10-20 17:09 UTC #26 lore raw

Re: [PATCH] tools: libxl: allow permissive qemu-upstream pci passthrough. [and 1 more messages]