[Qemu-devel] [PATCH 3/3] propagate errors from ioport registering up to pci level

[Glauber de Oliveira Costa <gcosta@redhat.com>]

From: Glauber Costa <gcosta@redhat.com>

In situations like pci-passthrough, the ioport registering can
fail, because another device is already present and in charge for
an io address. The current state would crash qemu, but we can propagate
the errors up to the pci layer, avoiding it.

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 qemu/hw/pci-passthrough.c |   28 ++++++++++++++++++++++++----
 qemu/hw/pci.c             |   30 ++++++++++++++++++++----------
 qemu/hw/pci.h             |    2 +-
 3 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/qemu/hw/pci-passthrough.c b/qemu/hw/pci-passthrough.c
index 7ffcc7b..3912447 100644
--- a/qemu/hw/pci-passthrough.c
+++ b/qemu/hw/pci-passthrough.c
@@ -127,7 +127,7 @@ pt_ioport_read(b)
 pt_ioport_read(w)
 pt_ioport_read(l)
 
-static void pt_iomem_map(PCIDevice * d, int region_num,
+static int pt_iomem_map(PCIDevice * d, int region_num,
 			 uint32_t e_phys, uint32_t e_size, int type)
 {
 	pt_dev_t *r_dev = (pt_dev_t *) d;
@@ -141,6 +141,7 @@ static void pt_iomem_map(PCIDevice * d, int region_num,
 	cpu_register_physical_memory(e_phys,
 				     r_dev->dev.io_regions[region_num].size,
 				     r_dev->v_addrs[region_num].memory_index);
+	return 0;
 }
 
 
@@ -148,7 +149,8 @@ static void pt_ioport_map(PCIDevice * pci_dev, int region_num,
 			  uint32_t addr, uint32_t size, int type)
 {
 	pt_dev_t *r_dev = (pt_dev_t *) pci_dev;
-	int i;
+	int i, err;
+
 	uint32_t ((*rf[])(void *, uint32_t)) =  { pt_ioport_readb,
 						  pt_ioport_readw,
 						  pt_ioport_readl
@@ -163,10 +165,14 @@ static void pt_ioport_map(PCIDevice * pci_dev, int region_num,
 	      "region_num=%d \n", addr, type, size, region_num);
 
 	for (i = 0; i < 3; i++) {
-		register_ioport_write(addr, size, 1<<i, wf[i],
+		err = register_ioport_write(addr, size, 1<<i, wf[i],
 				      (void *) (r_dev->v_addrs + region_num));
-		register_ioport_read(addr, size, 1<<i, rf[i],
+		if (err < 0)
+			return err;
+		err = register_ioport_read(addr, size, 1<<i, rf[i],
 				     (void *) (r_dev->v_addrs + region_num));
+		if (err < 0)
+			return err;
 	}
 }
 
@@ -455,6 +461,18 @@ struct {
 int nptdevs;
 extern int piix_get_irq(int);
 
+static int pt_pci_unregister(PCIDevice *pci_dev)
+{
+	pt_dev_t *pt = (pt_dev_t *)pci_dev;
+	int i;
+	for (i = 0; i < MAX_PTDEVS ; i++) {
+		if (ptdevs[i].ptdev == pt)
+			ptdevs[i].ptdev = NULL;
+	}
+	return 0;
+}
+
+
 /* The pci config space got updated. Check if irq numbers have changed
  * for our devices
  */
@@ -572,6 +590,8 @@ int pt_init(PCIBus * bus)
 			ret = -1;
 		}
 		ptdevs[i].ptdev = dev;
+		/* FIXME: Can the unregister callback be ever called before this point? */
+		dev->dev.unregister = pt_pci_unregister;
 	}
 
 	if (kvm_enabled() && !qemu_kvm_irqchip_in_kernel())
diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c
index 7e4ce2d..5265b81 100644
--- a/qemu/hw/pci.c
+++ b/qemu/hw/pci.c
@@ -48,7 +48,7 @@ struct PCIBus {
     int irq_count[];
 };
 
-static void pci_update_mappings(PCIDevice *d);
+static int pci_update_mappings(PCIDevice *d);
 static void pci_set_irq(void *opaque, int irq_num, int level);
 void pci_pt_update_irq(PCIDevice *d);
 
@@ -133,13 +133,14 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
 int pci_device_load(PCIDevice *s, QEMUFile *f)
 {
     uint32_t version_id;
-    int i;
+    int i, err;
 
     version_id = qemu_get_be32(f);
     if (version_id > 2)
         return -EINVAL;
     qemu_get_buffer(f, s->config, 256);
-    pci_update_mappings(s);
+    if ((err = pci_update_mappings(s)) < 0)
+	return err;
 
     if (version_id >= 2)
         for (i = 0; i < 4; i ++)
@@ -192,7 +193,7 @@ static target_phys_addr_t pci_to_cpu_addr(target_phys_addr_t addr)
     return addr + pci_mem_base;
 }
 
-static void pci_unregister_io_regions(PCIDevice *pci_dev)
+void pci_unregister_io_regions(PCIDevice *pci_dev)
 {
     PCIIORegion *r;
     int i;
@@ -256,11 +257,22 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num,
     *(uint32_t *)(pci_dev->config + addr) = cpu_to_le32(type);
 }
 
+static int map_pci_region(PCIDevice *d, int i, PCIIORegion *r)
+{
+	int err = 0;
 
-static void pci_update_mappings(PCIDevice *d)
+	if ((err = r->map_func(d, i, r->addr, r->size, r->type)) < 0) {
+		fprintf(stderr, "Could not map pci device %s\n", d->name);
+		pci_unregister_device(d);
+	}
+	r->status = PCI_STATUS_REGISTERED;
+	return err;
+}
+
+static int pci_update_mappings(PCIDevice *d)
 {
     PCIIORegion *r;
-    int cmd, i;
+    int cmd, i, err;
     uint32_t last_addr, new_addr, config_ofs;
 
     cmd = le16_to_cpu(*(uint16_t *)(d->config + PCI_COMMAND));
@@ -328,10 +340,8 @@ static void pci_update_mappings(PCIDevice *d)
                     }
                 }
                 r->addr = new_addr;
-                if (r->addr != -1) {
-                    r->map_func(d, i, r->addr, r->size, r->type);
-                    r->status = PCI_STATUS_REGISTERED;
-                }
+                if ((r->addr != -1) && ((err = map_pci_region(d, i, r)) < 0))
+			return err;
             }
         }
     }
diff --git a/qemu/hw/pci.h b/qemu/hw/pci.h
index 6350ad2..7eeff2e 100644
--- a/qemu/hw/pci.h
+++ b/qemu/hw/pci.h
@@ -15,7 +15,7 @@ typedef void PCIConfigWriteFunc(PCIDevice *pci_dev,
                                 uint32_t address, uint32_t data, int len);
 typedef uint32_t PCIConfigReadFunc(PCIDevice *pci_dev,
                                    uint32_t address, int len);
-typedef void PCIMapIORegionFunc(PCIDevice *pci_dev, int region_num,
+typedef int PCIMapIORegionFunc(PCIDevice *pci_dev, int region_num,
                                 uint32_t addr, uint32_t size, int type);
 typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
 
-- 
1.5.5