[Qemu-devel] [PATCHv2 1/4] qemu/qdev: type safety in reset handler

[Qemu-devel] [PATCHv2 1/4] qemu/qdev: type safety in reset handler

[Michael S. Tsirkin]

Add type safety to qdev reset handlers, by declaring them as
DeviceState * rather than void *.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/qdev.c    |   13 +++++++++----
 hw/qdev.h    |    3 ++-
 hw/rtl8139.c |   10 +++++-----
 hw/tcx.c     |    6 +++---
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 43b1beb..79a5bf5 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -209,6 +209,13 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     return qdev;
 }
 
+static void qdev_reset(void *opaque)
+{
+    DeviceState *dev = opaque;
+    if (dev->info->reset)
+        dev->info->reset(dev);
+}
+
 /* Initialize a device.  Device properties should be set before calling
    this function.  IRQs and MMIO regions should be connected/mapped after
    calling this function.  */
@@ -219,8 +226,7 @@ int qdev_init(DeviceState *dev)
     rc = dev->info->init(dev, dev->info);
     if (rc < 0)
         return rc;
-    if (dev->info->reset)
-        qemu_register_reset(dev->info->reset, dev);
+    qemu_register_reset(qdev_reset, dev);
     if (dev->info->vmsd)
         vmstate_register(-1, dev->info->vmsd, dev);
     return 0;
@@ -233,8 +239,7 @@ void qdev_free(DeviceState *dev)
     if (dev->info->vmsd)
         vmstate_unregister(dev->info->vmsd, dev);
 #endif
-    if (dev->info->reset)
-        qemu_unregister_reset(dev->info->reset, dev);
+    qemu_unregister_reset(qdev_reset, dev);
     QLIST_REMOVE(dev, sibling);
     qemu_free(dev);
 }
diff --git a/hw/qdev.h b/hw/qdev.h
index 623ded5..61a252c 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -100,6 +100,7 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 /*** Device API.  ***/
 
 typedef int (*qdev_initfn)(DeviceState *dev, DeviceInfo *info);
+typedef void (*qdev_resetfn)(DeviceState *dev);
 
 struct DeviceInfo {
     const char *name;
@@ -110,7 +111,7 @@ struct DeviceInfo {
     int no_user;
 
     /* callbacks */
-    QEMUResetHandler *reset;
+    qdev_resetfn reset;
 
     /* device state */
     const VMStateDescription *vmsd;
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 83cb1ff..59c4b6f 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -1173,9 +1173,9 @@ static void rtl8139_reset_rxring(RTL8139State *s, uint32_t bufferSize)
     s->RxBufAddr = 0;
 }
 
-static void rtl8139_reset(void *opaque)
+static void rtl8139_reset(DeviceState *d)
 {
-    RTL8139State *s = opaque;
+    RTL8139State *s = container_of(d, RTL8139State, dev.qdev);
     int i;
 
     /* restore MAC address */
@@ -1371,7 +1371,7 @@ static void rtl8139_ChipCmd_write(RTL8139State *s, uint32_t val)
     if (val & CmdReset)
     {
         DEBUG_PRINT(("RTL8139: ChipCmd reset\n"));
-        rtl8139_reset(s);
+        rtl8139_reset(&s->dev.qdev);
     }
     if (val & CmdRxEnb)
     {
@@ -1544,7 +1544,7 @@ static void rtl8139_Cfg9346_write(RTL8139State *s, uint32_t val)
     } else if (opmode == 0x40) {
         /* Reset.  */
         val = 0;
-        rtl8139_reset(s);
+        rtl8139_reset(&s->dev.qdev);
     }
 
     s->Cfg9346 = val;
@@ -3466,7 +3466,7 @@ static int pci_rtl8139_init(PCIDevice *dev)
                            PCI_ADDRESS_SPACE_MEM, rtl8139_mmio_map);
 
     qdev_get_macaddr(&dev->qdev, s->macaddr);
-    rtl8139_reset(s);
+    rtl8139_reset(&s->dev.qdev);
     s->vc = qdev_get_vlan_client(&dev->qdev,
                                  rtl8139_can_receive, rtl8139_receive, NULL,
                                  rtl8139_cleanup, s);
diff --git a/hw/tcx.c b/hw/tcx.c
index 012d01b..e996973 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -411,9 +411,9 @@ static const VMStateDescription vmstate_tcx = {
     }
 };
 
-static void tcx_reset(void *opaque)
+static void tcx_reset(DeviceState *d)
 {
-    TCXState *s = opaque;
+    TCXState *s = container_of(d, TCXState, busdev.qdev);
 
     /* Initialize palette */
     memset(s->r, 0, 256);
@@ -560,7 +560,7 @@ static int tcx_init1(SysBusDevice *dev)
                                      tcx_screen_dump, NULL, s);
     }
 
-    tcx_reset(s);
+    tcx_reset(&s->busdev.qdev);
     qemu_console_resize(s->ds, s->width, s->height);
     return 0;
 }
-- 
1.6.2.5

[Qemu-devel] [PATCHv2 2/4] qemu/virtio: fix reset with device removal

[Michael S. Tsirkin]

virtio pci registers its own reset handler, but fails to unregister it,
which will lead to crashes after device removal.  Solve this problem by
switching to qdev reset handler, which is automatically unregistered.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio-pci.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index f7a51ff..e300d28 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -155,9 +155,9 @@ static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
     return 0;
 }
 
-static void virtio_pci_reset(void *opaque)
+static void virtio_pci_reset(DeviceState *d)
 {
-    VirtIOPCIProxy *proxy = opaque;
+    VirtIOPCIProxy *proxy = container_of(d, VirtIOPCIProxy, pci_dev.qdev);
     virtio_reset(proxy->vdev);
     msix_reset(&proxy->pci_dev);
 }
@@ -184,7 +184,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case VIRTIO_PCI_QUEUE_PFN:
         pa = (target_phys_addr_t)val << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
         if (pa == 0)
-            virtio_pci_reset(proxy);
+            virtio_pci_reset(&proxy->pci_dev.qdev);
         else
             virtio_queue_set_addr(vdev, vdev->queue_sel, pa);
         break;
@@ -198,7 +198,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case VIRTIO_PCI_STATUS:
         vdev->status = val & 0xFF;
         if (vdev->status == 0)
-            virtio_pci_reset(proxy);
+            virtio_pci_reset(&proxy->pci_dev.qdev);
         break;
     case VIRTIO_MSI_CONFIG_VECTOR:
         msix_vector_unuse(&proxy->pci_dev, vdev->config_vector);
@@ -429,8 +429,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
     pci_register_bar(&proxy->pci_dev, 0, size, PCI_ADDRESS_SPACE_IO,
                            virtio_map);
 
-    qemu_register_reset(virtio_pci_reset, proxy);
-
     virtio_bind_device(vdev, &virtio_pci_bindings, proxy);
 }
 
@@ -525,6 +523,7 @@ static PCIDeviceInfo virtio_info[] = {
             DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
             DEFINE_PROP_END_OF_LIST(),
         },
+        .qdev.reset = virtio_pci_reset,
     },{
         .qdev.name  = "virtio-net-pci",
         .qdev.size  = sizeof(VirtIOPCIProxy),
@@ -534,6 +533,7 @@ static PCIDeviceInfo virtio_info[] = {
                                NIC_NVECTORS_UNSPECIFIED),
             DEFINE_PROP_END_OF_LIST(),
         },
+        .qdev.reset = virtio_pci_reset,
     },{
         .qdev.name = "virtio-console-pci",
         .qdev.size = sizeof(VirtIOPCIProxy),
@@ -542,10 +542,12 @@ static PCIDeviceInfo virtio_info[] = {
             DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
             DEFINE_PROP_END_OF_LIST(),
         },
+        .qdev.reset = virtio_pci_reset,
     },{
         .qdev.name = "virtio-balloon-pci",
         .qdev.size = sizeof(VirtIOPCIProxy),
         .init      = virtio_balloon_init_pci,
+        .qdev.reset = virtio_pci_reset,
     },{
         /* end of list */
     }
-- 
1.6.2.5

[Qemu-devel] [PATCHv2 3/4] qemu/pci: refactor code/symbolic constants

[Michael S. Tsirkin]

refactor code slightly, adding symbolic constants and functions, and
using macros where possible.  This will also make following reset
patches easier.

No functional changes.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci.c |   41 +++++++++++++++++++++--------------------
 hw/pci.h |    2 ++
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index c12b0be..600df2f 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -83,6 +83,16 @@ static const VMStateDescription vmstate_pcibus = {
     }
 };
 
+static inline int pci_bar(int reg)
+{
+    return reg == PCI_ROM_SLOT ? PCI_ROM_ADDRESS : PCI_BASE_ADDRESS_0 + reg * 4;
+}
+
+static void pci_device_reset(PCIDevice *dev)
+{
+    memset(dev->irq_state, 0, sizeof dev->irq_state);
+}
+
 static void pci_bus_reset(void *opaque)
 {
     PCIBus *bus = opaque;
@@ -91,10 +101,10 @@ static void pci_bus_reset(void *opaque)
     for (i = 0; i < bus->nirq; i++) {
         bus->irq_count[i] = 0;
     }
-    for (i = 0; i < 256; i++) {
-        if (bus->devices[i])
-            memset(bus->devices[i]->irq_state, 0,
-                   sizeof(bus->devices[i]->irq_state));
+    for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
+        if (bus->devices[i]) {
+            pci_device_reset(bus->devices[i]);
+        }
     }
 }
 
@@ -419,12 +429,10 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
     r->map_func = map_func;
 
     wmask = ~(size - 1);
+    addr = pci_bar(region_num);
     if (region_num == PCI_ROM_SLOT) {
-        addr = 0x30;
         /* ROM enable bit is writeable */
-        wmask |= 1;
-    } else {
-        addr = 0x10 + region_num * 4;
+        wmask |= PCI_ROM_ADDRESS_ENABLE;
     }
     *(uint32_t *)(pci_dev->config + addr) = cpu_to_le32(type);
     *(uint32_t *)(pci_dev->wmask + addr) = cpu_to_le32(wmask);
@@ -435,21 +443,15 @@ static void pci_update_mappings(PCIDevice *d)
 {
     PCIIORegion *r;
     int cmd, i;
-    uint32_t last_addr, new_addr, config_ofs;
+    uint32_t last_addr, new_addr;
 
     cmd = le16_to_cpu(*(uint16_t *)(d->config + PCI_COMMAND));
     for(i = 0; i < PCI_NUM_REGIONS; i++) {
         r = &d->io_regions[i];
-        if (i == PCI_ROM_SLOT) {
-            config_ofs = 0x30;
-        } else {
-            config_ofs = 0x10 + i * 4;
-        }
         if (r->size != 0) {
             if (r->type & PCI_ADDRESS_SPACE_IO) {
                 if (cmd & PCI_COMMAND_IO) {
-                    new_addr = le32_to_cpu(*(uint32_t *)(d->config +
-                                                         config_ofs));
+                    new_addr = pci_get_long(d->config + pci_bar(i));
                     new_addr = new_addr & ~(r->size - 1);
                     last_addr = new_addr + r->size - 1;
                     /* NOTE: we have only 64K ioports on PC */
@@ -462,10 +464,9 @@ static void pci_update_mappings(PCIDevice *d)
                 }
             } else {
                 if (cmd & PCI_COMMAND_MEMORY) {
-                    new_addr = le32_to_cpu(*(uint32_t *)(d->config +
-                                                         config_ofs));
+                    new_addr = pci_get_long(d->config + pci_bar(i));
                     /* the ROM slot has a specific enable bit */
-                    if (i == PCI_ROM_SLOT && !(new_addr & 1))
+                    if (i == PCI_ROM_SLOT && !(new_addr & PCI_ROM_ADDRESS_ENABLE))
                         goto no_mem_map;
                     new_addr = new_addr & ~(r->size - 1);
                     last_addr = new_addr + r->size - 1;
@@ -489,7 +490,7 @@ static void pci_update_mappings(PCIDevice *d)
                         int class;
                         /* NOTE: specific hack for IDE in PC case:
                            only one byte must be mapped. */
-                        class = d->config[0x0a] | (d->config[0x0b] << 8);
+                        class = pci_get_word(d->config + PCI_CLASS_DEVICE);
                         if (class == 0x0101 && r->size == 4) {
                             isa_unassign_ioport(r->addr + 2, 1);
                         } else {
diff --git a/hw/pci.h b/hw/pci.h
index 6196b6a..5481757 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -117,6 +117,8 @@ typedef struct PCIIORegion {
 #define PCI_SEC_STATUS		0x1e	/* Secondary status register, only bit 14 used */
 #define PCI_SUBSYSTEM_VENDOR_ID 0x2c    /* 16 bits */
 #define PCI_SUBSYSTEM_ID        0x2e    /* 16 bits */
+#define PCI_ROM_ADDRESS		0x30	/* Bits 31..11 are address, 10..1 reserved */
+#define  PCI_ROM_ADDRESS_ENABLE	0x01
 #define PCI_CAPABILITY_LIST	0x34	/* Offset of first capability list entry */
 #define PCI_INTERRUPT_LINE	0x3c	/* 8 bits */
 #define PCI_INTERRUPT_PIN	0x3d	/* 8 bits */
-- 
1.6.2.5

[Qemu-devel] [PATCHv2 4/4] qemu/pci: reset device registers on bus reset

[Michael S. Tsirkin]

Reset BARs and a couple of other registers on bus reset, as per PCI
spec.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 600df2f..b489d41 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -90,7 +90,20 @@ static inline int pci_bar(int reg)
 
 static void pci_device_reset(PCIDevice *dev)
 {
+    int r;
+
     memset(dev->irq_state, 0, sizeof dev->irq_state);
+    dev->config[PCI_COMMAND] &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
+                                  PCI_COMMAND_MASTER);
+    dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
+    dev->config[PCI_INTERRUPT_LINE] = 0x0;
+    for (r = 0; r < PCI_NUM_REGIONS; ++r) {
+        if (!dev->io_regions[r].size) {
+            continue;
+        }
+        pci_set_long(dev->config + pci_bar(r), dev->io_regions[r].type);
+    }
+    pci_update_mappings(dev);
 }
 
 static void pci_bus_reset(void *opaque)
-- 
1.6.2.5