[Qemu-devel] [PATCH 00/10] vfio-pci: pass the aer error to guest, part2

[Qemu-devel] [PATCH 00/10] vfio-pci: pass the aer error to guest, part2

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

this is the part 2 of supporting vfio device with aer enabled, the part 1
has been sent out. the code based on v16 series patchset.

v16 -> this:
   main code changed on the 7/10 and 8/10 patches. others patches only
   fix some trivial comments or codes sugguested by Marcel.

Chen Fan (10):
  vfio: extract vfio_get_hot_reset_info as a single function
  vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
  vfio: add pcie extended capability support
  vfio: add aer support for vfio device
  vfio: add check host bus reset is support or not
  add check reset mechanism when hotplug vfio device
  pci: introduce pci bus pre reset
  vfio: introduce last reset sequence id
  vfio-pci: pass the aer error to guest
  vfio: add 'aer' property to expose aercap

 hw/core/qdev.c           |   4 +-
 hw/pci/pci.c             |  54 +++++
 hw/pci/pci_bridge.c      |   5 +-
 hw/vfio/pci.c            | 610 ++++++++++++++++++++++++++++++++++++++++++-----
 hw/vfio/pci.h            |   9 +
 include/hw/pci/pci_bus.h |   9 +
 include/hw/qdev-core.h   |   3 +
 7 files changed, 628 insertions(+), 66 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 01/10] vfio: extract vfio_get_hot_reset_info as a single function

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

the function is used to get affected devices by bus reset.
so here extract it, and can used for aer soon.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 66 +++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 18 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e671506..9ddaa99 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1691,6 +1691,51 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, uint8_t pos)
     }
 }
 
+/*
+ * return negative with errno, return 0 on success.
+ * if success, the point of ret_info fill with the affected device reset info.
+ *
+ */
+static int vfio_get_hot_reset_info(VFIOPCIDevice *vdev,
+                                   struct vfio_pci_hot_reset_info **ret_info)
+{
+    struct vfio_pci_hot_reset_info *info;
+    int ret, count;
+
+    *ret_info = NULL;
+
+    info = g_malloc0(sizeof(*info));
+    info->argsz = sizeof(*info);
+
+    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
+    if (ret && errno != ENOSPC) {
+        ret = -errno;
+        goto error;
+    }
+
+    count = info->count;
+
+    info = g_realloc(info, sizeof(*info) +
+                     (count * sizeof(struct vfio_pci_dependent_device)));
+    info->argsz = sizeof(*info) +
+                  (count * sizeof(struct vfio_pci_dependent_device));
+
+    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
+    if (ret) {
+        ret = -errno;
+        error_report("vfio: hot reset info failed: %m");
+        goto error;
+    }
+
+    *ret_info = info;
+    info = NULL;
+
+    return 0;
+error:
+    g_free(info);
+    return ret;
+}
+
 static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
 {
     PCIDevice *pdev = &vdev->pdev;
@@ -1830,7 +1875,7 @@ static bool vfio_pci_host_match(PCIHostDeviceAddress *host1,
 static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
 {
     VFIOGroup *group;
-    struct vfio_pci_hot_reset_info *info;
+    struct vfio_pci_hot_reset_info *info = NULL;
     struct vfio_pci_dependent_device *devices;
     struct vfio_pci_hot_reset *reset;
     int32_t *fds;
@@ -1842,12 +1887,8 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
     vfio_pci_pre_reset(vdev);
     vdev->vbasedev.needs_reset = false;
 
-    info = g_malloc0(sizeof(*info));
-    info->argsz = sizeof(*info);
-
-    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
-    if (ret && errno != ENOSPC) {
-        ret = -errno;
+    ret = vfio_get_hot_reset_info(vdev, &info);
+    if (ret) {
         if (!vdev->has_pm_reset) {
             error_report("vfio: Cannot reset device %04x:%02x:%02x.%x, "
                          "no available reset mechanism.", vdev->host.domain,
@@ -1856,18 +1897,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
         goto out_single;
     }
 
-    count = info->count;
-    info = g_realloc(info, sizeof(*info) + (count * sizeof(*devices)));
-    info->argsz = sizeof(*info) + (count * sizeof(*devices));
     devices = &info->devices[0];
-
-    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
-    if (ret) {
-        ret = -errno;
-        error_report("vfio: hot reset info failed: %m");
-        goto out_single;
-    }
-
     trace_vfio_pci_hot_reset_has_dep_devices(vdev->vbasedev.name);
 
     /* Verify that we have all the groups required */
-- 
1.9.3

[Qemu-devel] [PATCH 02/10] vfio: squeeze out vfio_pci_do_hot_reset for support bus reset

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

squeeze out vfio_pci_do_hot_reset to do host bus reset when AER recovery.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 75 +++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 9ddaa99..b03c394 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1736,6 +1736,48 @@ error:
     return ret;
 }
 
+static int vfio_pci_do_hot_reset(VFIOPCIDevice *vdev,
+                                 struct vfio_pci_hot_reset_info *info)
+{
+    VFIOGroup *group;
+    struct vfio_pci_hot_reset *reset;
+    int32_t *fds;
+    int ret, i, count;
+    struct vfio_pci_dependent_device *devices;
+
+    /* Determine how many group fds need to be passed */
+    count = 0;
+    devices = &info->devices[0];
+    QLIST_FOREACH(group, &vfio_group_list, next) {
+        for (i = 0; i < info->count; i++) {
+            if (group->groupid == devices[i].group_id) {
+                count++;
+                break;
+            }
+        }
+    }
+
+    reset = g_malloc0(sizeof(*reset) + (count * sizeof(*fds)));
+    reset->argsz = sizeof(*reset) + (count * sizeof(*fds));
+    fds = &reset->group_fds[0];
+
+    /* Fill in group fds */
+    QLIST_FOREACH(group, &vfio_group_list, next) {
+        for (i = 0; i < info->count; i++) {
+            if (group->groupid == devices[i].group_id) {
+                fds[reset->count++] = group->fd;
+                break;
+            }
+        }
+    }
+
+    /* Bus reset! */
+    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_PCI_HOT_RESET, reset);
+    g_free(reset);
+
+    return ret;
+}
+
 static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
 {
     PCIDevice *pdev = &vdev->pdev;
@@ -1877,9 +1919,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
     VFIOGroup *group;
     struct vfio_pci_hot_reset_info *info = NULL;
     struct vfio_pci_dependent_device *devices;
-    struct vfio_pci_hot_reset *reset;
-    int32_t *fds;
-    int ret, i, count;
+    int ret, i;
     bool multi = false;
 
     trace_vfio_pci_hot_reset(vdev->vbasedev.name, single ? "one" : "multi");
@@ -1958,34 +1998,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
         goto out_single;
     }
 
-    /* Determine how many group fds need to be passed */
-    count = 0;
-    QLIST_FOREACH(group, &vfio_group_list, next) {
-        for (i = 0; i < info->count; i++) {
-            if (group->groupid == devices[i].group_id) {
-                count++;
-                break;
-            }
-        }
-    }
-
-    reset = g_malloc0(sizeof(*reset) + (count * sizeof(*fds)));
-    reset->argsz = sizeof(*reset) + (count * sizeof(*fds));
-    fds = &reset->group_fds[0];
-
-    /* Fill in group fds */
-    QLIST_FOREACH(group, &vfio_group_list, next) {
-        for (i = 0; i < info->count; i++) {
-            if (group->groupid == devices[i].group_id) {
-                fds[reset->count++] = group->fd;
-                break;
-            }
-        }
-    }
-
-    /* Bus reset! */
-    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_PCI_HOT_RESET, reset);
-    g_free(reset);
+    ret = vfio_pci_do_hot_reset(vdev, info);
 
     trace_vfio_pci_hot_reset_result(vdev->vbasedev.name,
                                     ret ? "%m" : "Success");
-- 
1.9.3

[Qemu-devel] [PATCH 03/10] vfio: add pcie extended capability support

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

For vfio pcie device, we could expose the extended capability on
PCIE bus. due to add a new pcie capability at the tail of the chain,
in order to avoid config space overwritten, we introduce a copy config
for parsing extended caps. and rebuild the pcie extended config space.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index b03c394..e64cce3 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1518,6 +1518,21 @@ static uint8_t vfio_std_cap_max_size(PCIDevice *pdev, uint8_t pos)
     return next - pos;
 }
 
+
+static uint16_t vfio_ext_cap_max_size(const uint8_t *config, uint16_t pos)
+{
+    uint16_t tmp, next = PCIE_CONFIG_SPACE_SIZE;
+
+    for (tmp = PCI_CONFIG_SPACE_SIZE; tmp;
+        tmp = PCI_EXT_CAP_NEXT(pci_get_long(config + tmp))) {
+        if (tmp > pos && tmp < next) {
+            next = tmp;
+        }
+    }
+
+    return next - pos;
+}
+
 static void vfio_set_word_bits(uint8_t *buf, uint16_t val, uint16_t mask)
 {
     pci_set_word(buf, (pci_get_word(buf) & ~mask) | val);
@@ -1853,16 +1868,71 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
     return 0;
 }
 
+static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
+{
+    PCIDevice *pdev = &vdev->pdev;
+    uint32_t header;
+    uint16_t cap_id, next, size;
+    uint8_t cap_ver;
+    uint8_t *config;
+
+    /*
+     * pcie_add_capability always inserts the new capability at the tail
+     * of the chain.  Therefore to end up with a chain that matches the
+     * physical device, we cache the config space to avoid overwriting
+     * the original config space when we parse the extended capabilities.
+     */
+    config = g_memdup(pdev->config, vdev->config_size);
+
+    for (next = PCI_CONFIG_SPACE_SIZE; next;
+         next = PCI_EXT_CAP_NEXT(pci_get_long(config + next))) {
+        header = pci_get_long(config + next);
+        cap_id = PCI_EXT_CAP_ID(header);
+        cap_ver = PCI_EXT_CAP_VER(header);
+
+        /*
+         * If it becomes important to configure extended capabilities to their
+         * actual size, use this as the default when it's something we don't
+         * recognize. Since QEMU doesn't actually handle many of the config
+         * accesses, exact size doesn't seem worthwhile.
+         */
+        size = vfio_ext_cap_max_size(config, next);
+
+        pcie_add_capability(pdev, cap_id, cap_ver, next, size);
+        pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
+
+        /* Use emulated next pointer to allow dropping extended caps */
+        pci_long_test_and_set_mask(vdev->emulated_config_bits + next,
+                                   PCI_EXT_CAP_NEXT_MASK);
+    }
+
+    g_free(config);
+    return 0;
+}
+
 static int vfio_add_capabilities(VFIOPCIDevice *vdev)
 {
     PCIDevice *pdev = &vdev->pdev;
+    int ret;
 
     if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST) ||
         !pdev->config[PCI_CAPABILITY_LIST]) {
         return 0; /* Nothing to add */
     }
 
-    return vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
+    ret = vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
+    if (ret) {
+        return ret;
+    }
+
+    /* on PCI bus, it doesn't make sense to expose extended capabilities. */
+    if (!pci_is_express(pdev) ||
+        !pci_bus_is_express(pdev->bus) ||
+        !pci_get_long(pdev->config + PCI_CONFIG_SPACE_SIZE)) {
+        return 0;
+    }
+
+    return vfio_add_ext_cap(vdev);
 }
 
 static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
-- 
1.9.3

[Qemu-devel] [PATCH 04/10] vfio: add aer support for vfio device

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

Calling pcie_aer_init to initilize aer related registers for
vfio device, then reload physical related registers to expose
device capability.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/vfio/pci.h |  3 +++
 2 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e64cce3..8ec9b25 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1868,6 +1868,62 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
     return 0;
 }
 
+static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
+                          int pos, uint16_t size)
+{
+    PCIDevice *pdev = &vdev->pdev;
+    PCIDevice *dev_iter;
+    uint8_t type;
+    uint32_t errcap;
+
+    if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) {
+        pcie_add_capability(pdev, PCI_EXT_CAP_ID_ERR,
+                            cap_ver, pos, size);
+        return 0;
+    }
+
+    dev_iter = pci_bridge_get_device(pdev->bus);
+    if (!dev_iter) {
+        goto error;
+    }
+
+    while (dev_iter) {
+        type = pcie_cap_get_type(dev_iter);
+        if ((type != PCI_EXP_TYPE_ROOT_PORT &&
+             type != PCI_EXP_TYPE_UPSTREAM &&
+             type != PCI_EXP_TYPE_DOWNSTREAM)) {
+            goto error;
+        }
+
+        if (!dev_iter->exp.aer_cap) {
+            goto error;
+        }
+
+        dev_iter = pci_bridge_get_device(dev_iter->bus);
+    }
+
+    errcap = vfio_pci_read_config(pdev, pos + PCI_ERR_CAP, 4);
+    /*
+     * The ability to record multiple headers is depending on
+     * the state of the Multiple Header Recording Capable bit and
+     * enabled by the Multiple Header Recording Enable bit.
+     */
+    if ((errcap & PCI_ERR_CAP_MHRC) &&
+        (errcap & PCI_ERR_CAP_MHRE)) {
+        pdev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
+    } else {
+        pdev->exp.aer_log.log_max = 0;
+    }
+
+    pcie_cap_deverr_init(pdev);
+    return pcie_aer_init(pdev, pos, size);
+
+error:
+    error_report("vfio: Unable to enable AER for device %s, parent bus "
+                 "does not support AER signaling", vdev->vbasedev.name);
+    return -1;
+}
+
 static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
 {
     PCIDevice *pdev = &vdev->pdev;
@@ -1875,6 +1931,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
     uint16_t cap_id, next, size;
     uint8_t cap_ver;
     uint8_t *config;
+    int ret = 0;
 
     /*
      * pcie_add_capability always inserts the new capability at the tail
@@ -1898,16 +1955,29 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
          */
         size = vfio_ext_cap_max_size(config, next);
 
-        pcie_add_capability(pdev, cap_id, cap_ver, next, size);
-        pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
+        switch (cap_id) {
+        case PCI_EXT_CAP_ID_ERR:
+            ret = vfio_setup_aer(vdev, cap_ver, next, size);
+            break;
+        default:
+            pcie_add_capability(pdev, cap_id, cap_ver, next, size);
+            break;
+        }
+
+        if (ret) {
+            goto out;
+        }
+
+        pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
 
         /* Use emulated next pointer to allow dropping extended caps */
         pci_long_test_and_set_mask(vdev->emulated_config_bits + next,
                                    PCI_EXT_CAP_NEXT_MASK);
     }
 
+out:
     g_free(config);
-    return 0;
+    return ret;
 }
 
 static int vfio_add_capabilities(VFIOPCIDevice *vdev)
@@ -2662,6 +2732,11 @@ static int vfio_initfn(PCIDevice *pdev)
         goto out_teardown;
     }
 
+    if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+        !pdev->exp.aer_cap) {
+        goto out_teardown;
+    }
+
     /* QEMU emulates all of MSI & MSIX */
     if (pdev->cap_present & QEMU_PCI_CAP_MSIX) {
         memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff,
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 6256587..e0c53f2 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -15,6 +15,7 @@
 #include "qemu-common.h"
 #include "exec/memory.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "hw/vfio/vfio-common.h"
 #include "qemu/event_notifier.h"
 #include "qemu/queue.h"
@@ -128,6 +129,8 @@ typedef struct VFIOPCIDevice {
 #define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT)
 #define VFIO_FEATURE_ENABLE_REQ_BIT 1
 #define VFIO_FEATURE_ENABLE_REQ (1 << VFIO_FEATURE_ENABLE_REQ_BIT)
+#define VFIO_FEATURE_ENABLE_AER_BIT 2
+#define VFIO_FEATURE_ENABLE_AER (1 << VFIO_FEATURE_ENABLE_AER_BIT)
     int32_t bootindex;
     uint8_t pm_cap;
     bool has_vga;
-- 
1.9.3

[Qemu-devel] [PATCH 05/10] vfio: add check host bus reset is support or not

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

when boot up a VM that assigning vfio devices with aer enabled, we
must check the vfio device whether support host bus reset. because
when one error occur. OS driver always recover the device by do a
bus reset, in order to recover the vfio device, qemu must to do a
host bus reset to reset the device to default status. and for all
affected devices by the bus reset. we must check them whether all
are assigned to the VM.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 hw/vfio/pci.h |   1 +
 2 files changed, 228 insertions(+), 7 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 8ec9b25..54032d0 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1868,6 +1868,214 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
     return 0;
 }
 
+static bool vfio_pci_host_slot_match(PCIHostDeviceAddress *host1,
+                                     PCIHostDeviceAddress *host2)
+{
+    return (host1->domain == host2->domain && host1->bus == host2->bus &&
+            host1->slot == host2->slot);
+}
+
+static bool vfio_pci_host_match(PCIHostDeviceAddress *host1,
+                                PCIHostDeviceAddress *host2)
+{
+    return (vfio_pci_host_slot_match(host1, host2) &&
+            host1->function == host2->function);
+}
+
+struct VFIODeviceFind {
+    PCIDevice *pdev;
+    bool found;
+};
+
+static void vfio_check_device_noreset(PCIBus *bus, PCIDevice *pdev,
+                                      void *opaque)
+{
+    DeviceState *dev = DEVICE(pdev);
+    DeviceClass *dc = DEVICE_GET_CLASS(dev);
+    VFIOPCIDevice *vdev;
+    struct VFIODeviceFind *find = opaque;
+
+    if (find->found) {
+        return;
+    }
+
+    if (!object_dynamic_cast(OBJECT(dev), "vfio-pci")) {
+        if (!dc->reset) {
+            goto found;
+        }
+        return;
+    }
+    vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+    if (!(vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+        !vdev->vbasedev.reset_works) {
+        goto found;
+    }
+
+    return;
+found:
+    find->pdev = pdev;
+    find->found = true;
+}
+
+static void device_find(PCIBus *bus, PCIDevice *pdev, void *opaque)
+{
+    struct VFIODeviceFind *find = opaque;
+
+    if (find->found) {
+        return;
+    }
+
+    if (pdev == find->pdev) {
+        find->found = true;
+    }
+}
+
+static int vfio_check_host_bus_reset(VFIOPCIDevice *vdev)
+{
+    PCIBus *bus = vdev->pdev.bus;
+    struct vfio_pci_hot_reset_info *info = NULL;
+    struct vfio_pci_dependent_device *devices;
+    VFIOGroup *group;
+    struct VFIODeviceFind find;
+    int ret, i;
+
+    ret = vfio_get_hot_reset_info(vdev, &info);
+    if (ret) {
+        error_report("vfio: Cannot enable AER for device %s,"
+                     " device does not support hot reset.",
+                     vdev->vbasedev.name);
+        return ret;
+    }
+
+    ret = -1;
+    /* List all affected devices by bus reset */
+    devices = &info->devices[0];
+
+    /* Verify that we have all the groups required */
+    for (i = 0; i < info->count; i++) {
+        PCIHostDeviceAddress host;
+        VFIOPCIDevice *tmp;
+        VFIODevice *vbasedev_iter;
+        bool found = false;
+
+        host.domain = devices[i].segment;
+        host.bus = devices[i].bus;
+        host.slot = PCI_SLOT(devices[i].devfn);
+        host.function = PCI_FUNC(devices[i].devfn);
+
+        /* Skip the current device */
+        if (vfio_pci_host_match(&host, &vdev->host)) {
+            continue;
+        }
+
+        /* Ensure we own the group of the affected device */
+        QLIST_FOREACH(group, &vfio_group_list, next) {
+            if (group->groupid == devices[i].group_id) {
+                break;
+            }
+        }
+
+        if (!group) {
+            error_report("vfio: Cannot enable AER for device %s, "
+                         "depends on group %d which is not owned.",
+                         vdev->vbasedev.name, devices[i].group_id);
+            goto out;
+        }
+
+        /* Ensure affected devices for reset behind the bus */
+        QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
+            if (vbasedev_iter->type != VFIO_DEVICE_TYPE_PCI) {
+                continue;
+            }
+            tmp = container_of(vbasedev_iter, VFIOPCIDevice, vbasedev);
+            if (vfio_pci_host_match(&host, &tmp->host)) {
+                PCIDevice *pci = PCI_DEVICE(tmp);
+
+                /*
+                 * AER errors may be broadcast to all functions of a multi-
+                 * function endpoint.  If any of those sibling functions are
+                 * also assigned, they need to have AER enabled or else an
+                 * error may continue to cause a vm_stop condition.  IOW,
+                 * AER setup of this function would be pointless.
+                 */
+                if (vfio_pci_host_slot_match(&vdev->host, &tmp->host) &&
+                    !(tmp->features & VFIO_FEATURE_ENABLE_AER)) {
+                    error_report("vfio: Cannot enable AER for device %s, on same slot"
+                                 " the dependent device %s which does not enable AER.",
+                                 vdev->vbasedev.name, tmp->vbasedev.name);
+                    goto out;
+                }
+
+                find.pdev = pci;
+                find.found = false;
+                pci_for_each_device(bus, pci_bus_num(bus),
+                                    device_find, &find);
+                if (!find.found) {
+                    error_report("vfio: Cannot enable AER for device %s, "
+                                 "the dependent device %s is not under the same bus",
+                                 vdev->vbasedev.name, tmp->vbasedev.name);
+                    goto out;
+                }
+                found = true;
+                break;
+            }
+        }
+
+        /* Ensure all affected devices assigned to VM */
+        if (!found) {
+            error_report("vfio: Cannot enable AER for device %s, "
+                         "the dependent device %04x:%02x:%02x.%x "
+                         "is not assigned to VM.",
+                         vdev->vbasedev.name, host.domain, host.bus,
+                         host.slot, host.function);
+            goto out;
+        }
+    }
+
+    /*
+     * Check the all pci devices on or below the target bus
+     * have a reset mechanism at least.
+     */
+    find.pdev = NULL;
+    find.found = false;
+    pci_for_each_device(bus, pci_bus_num(bus),
+                        vfio_check_device_noreset, &find);
+    if (find.found) {
+        error_report("vfio: Cannot enable AER for device %s, "
+                     "the affected device %s does not have a reset mechanism.",
+                     vdev->vbasedev.name, find.pdev->name);
+        goto out;
+    }
+
+    ret = 0;
+out:
+    g_free(info);
+    return ret;
+}
+
+static int vfio_check_devices_host_bus_reset(void)
+{
+    VFIOGroup *group;
+    VFIODevice *vbasedev;
+    VFIOPCIDevice *vdev;
+
+    /* Check All vfio-pci devices if have bus reset capability */
+    QLIST_FOREACH(group, &vfio_group_list, next) {
+        QLIST_FOREACH(vbasedev, &group->device_list, next) {
+            if (vbasedev->type != VFIO_DEVICE_TYPE_PCI) {
+                continue;
+            }
+            vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
+            if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+                vfio_check_host_bus_reset(vdev)) {
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
+
 static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
                           int pos, uint16_t size)
 {
@@ -2047,13 +2255,6 @@ static void vfio_pci_post_reset(VFIOPCIDevice *vdev)
     vfio_intx_enable(vdev);
 }
 
-static bool vfio_pci_host_match(PCIHostDeviceAddress *host1,
-                                PCIHostDeviceAddress *host2)
-{
-    return (host1->domain == host2->domain && host1->bus == host2->bus &&
-            host1->slot == host2->slot && host1->function == host2->function);
-}
-
 static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
 {
     VFIOGroup *group;
@@ -2559,6 +2760,20 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
     vdev->req_enabled = false;
 }
 
+static void vfio_pci_machine_done_notify(Notifier *notifier, void *unused)
+{
+    int ret;
+
+    ret = vfio_check_devices_host_bus_reset();
+    if (ret) {
+        exit(1);
+    }
+}
+
+static Notifier machine_notifier = {
+    .notify = vfio_pci_machine_done_notify,
+};
+
 static int vfio_initfn(PCIDevice *pdev)
 {
     VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
@@ -2905,6 +3120,11 @@ static const TypeInfo vfio_pci_dev_info = {
 static void register_vfio_pci_dev_type(void)
 {
     type_register_static(&vfio_pci_dev_info);
+    /*
+     * Register notifier when machine init is done, since we need
+     * check the configration manner after all vfio device are inited.
+     */
+    qemu_add_machine_init_done_notifier(&machine_notifier);
 }
 
 type_init(register_vfio_pci_dev_type)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index e0c53f2..aff46c2 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -15,6 +15,7 @@
 #include "qemu-common.h"
 #include "exec/memory.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/vfio/vfio-common.h"
 #include "qemu/event_notifier.h"
-- 
1.9.3

[Qemu-devel] [PATCH 06/10] add check reset mechanism when hotplug vfio device

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

Since we support multi-function hotplug, the function 0 indicates
the closure of the slot, so we have the chance to do the check.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/pci/pci.c             | 29 +++++++++++++++++++++++++++++
 hw/vfio/pci.c            | 19 +++++++++++++++++++
 hw/vfio/pci.h            |  2 ++
 include/hw/pci/pci_bus.h |  5 +++++
 4 files changed, 55 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index d940f79..e41925e 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -82,6 +82,7 @@ static void pci_bus_realize(BusState *qbus, Error **errp)
     PCIBus *bus = PCI_BUS(qbus);
 
     vmstate_register(NULL, -1, &vmstate_pcibus, bus);
+    notifier_with_return_list_init(&bus->hotplug_notifiers);
 }
 
 static void pci_bus_unrealize(BusState *qbus, Error **errp)
@@ -1836,6 +1837,22 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
     return bus->devices[devfn];
 }
 
+void pci_bus_add_hotplug_notifier(PCIBus *bus, NotifierWithReturn *notify)
+{
+    notifier_with_return_list_add(&bus->hotplug_notifiers, notify);
+}
+
+void pci_bus_remove_hotplug_notifier(NotifierWithReturn *notifier)
+{
+    notifier_with_return_remove(notifier);
+}
+
+static int pci_bus_hotplug_notifier(PCIBus *bus, void *opaque)
+{
+    return notifier_with_return_list_notify(&bus->hotplug_notifiers,
+                                            opaque);
+}
+
 static void pci_qdev_realize(DeviceState *qdev, Error **errp)
 {
     PCIDevice *pci_dev = (PCIDevice *)qdev;
@@ -1878,6 +1895,18 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
         pci_qdev_unrealize(DEVICE(pci_dev), NULL);
         return;
     }
+
+    /*
+     *  If the function is func 0, indicate the closure of the slot.
+     *  signal the callback.
+     */
+    if (DEVICE(pci_dev)->hotplugged &&
+        pci_get_function_0(pci_dev) == pci_dev &&
+        pci_bus_hotplug_notifier(bus, pci_dev)) {
+        error_setg(errp, "failed to hotplug function 0");
+        pci_qdev_unrealize(DEVICE(pci_dev), NULL);
+        return;
+    }
 }
 
 static void pci_default_realize(PCIDevice *dev, Error **errp)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 54032d0..b900f70 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2076,6 +2076,19 @@ static int vfio_check_devices_host_bus_reset(void)
     return 0;
 }
 
+static int vfio_check_bus_reset(NotifierWithReturn *n, void *opaque)
+{
+    VFIOPCIDevice *vdev = container_of(n, VFIOPCIDevice, hotplug_notifier);
+    PCIDevice *pci_dev = PCI_DEVICE(vdev);
+    PCIDevice *pci_func0 = opaque;
+
+    if (pci_get_function_0(pci_dev) != pci_func0) {
+        return 0;
+    }
+
+    return vfio_check_host_bus_reset(vdev);
+}
+
 static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
                           int pos, uint16_t size)
 {
@@ -2123,6 +2136,9 @@ static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
         pdev->exp.aer_log.log_max = 0;
     }
 
+    vdev->hotplug_notifier.notify = vfio_check_bus_reset;
+    pci_bus_add_hotplug_notifier(pdev->bus, &vdev->hotplug_notifier);
+
     pcie_cap_deverr_init(pdev);
     return pcie_aer_init(pdev, pos, size);
 
@@ -3006,6 +3022,9 @@ static void vfio_exitfn(PCIDevice *pdev)
     vfio_unregister_req_notifier(vdev);
     vfio_unregister_err_notifier(vdev);
     pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
+    if (vdev->features & VFIO_FEATURE_ENABLE_AER) {
+        pci_bus_remove_hotplug_notifier(&vdev->hotplug_notifier);
+    }
     vfio_disable_interrupts(vdev);
     if (vdev->intx.mmap_timer) {
         timer_free(vdev->intx.mmap_timer);
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index aff46c2..bf9e93c 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -143,6 +143,8 @@ typedef struct VFIOPCIDevice {
     bool no_kvm_intx;
     bool no_kvm_msi;
     bool no_kvm_msix;
+
+    NotifierWithReturn hotplug_notifier;
 } VFIOPCIDevice;
 
 uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len);
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 403fec6..7812fa9 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -39,8 +39,13 @@ struct PCIBus {
        Keep a count of the number of devices with raised IRQs.  */
     int nirq;
     int *irq_count;
+
+    NotifierWithReturnList hotplug_notifiers;
 };
 
+void pci_bus_add_hotplug_notifier(PCIBus *bus, NotifierWithReturn *notify);
+void pci_bus_remove_hotplug_notifier(NotifierWithReturn *notify);
+
 typedef struct PCIBridgeWindows PCIBridgeWindows;
 
 /*
-- 
1.9.3

[Qemu-devel] [PATCH 07/10] pci: introduce pci bus pre reset

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

avoid repeat bus reset, here introduce a sequence ID for each time
bus hot reset, so each vfio device could know whether they've already
been reset for that sequence ID.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/core/qdev.c           |  4 ++--
 hw/pci/pci.c             | 25 +++++++++++++++++++++++++
 hw/pci/pci_bridge.c      |  5 ++++-
 include/hw/pci/pci_bus.h |  4 ++++
 include/hw/qdev-core.h   |  3 +++
 5 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 779de2b..e36fa07 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -304,14 +304,14 @@ void qdev_unplug(DeviceState *dev, Error **errp)
     }
 }
 
-static int qdev_reset_one(DeviceState *dev, void *opaque)
+int qdev_reset_one(DeviceState *dev, void *opaque)
 {
     device_reset(dev);
 
     return 0;
 }
 
-static int qbus_reset_one(BusState *bus, void *opaque)
+int qbus_reset_one(BusState *bus, void *opaque)
 {
     BusClass *bc = BUS_GET_CLASS(bus);
     if (bc->reset) {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e41925e..ff978bc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -258,6 +258,29 @@ void pci_device_reset(PCIDevice *dev)
     pci_do_device_reset(dev);
 }
 
+int pcibus_pre_reset(BusState *qbus, void *opaque)
+{
+    BusState *bus = (BusState *)opaque;
+    PCIBus *pci_bus = DO_UPCAST(PCIBus, qbus, qbus);
+    static unsigned int reset_seqid;
+
+    /*
+     * To distinguish the different reset times, we
+     * recalculate the global reset seqid with a given bus.
+     */
+    if (qbus == bus) {
+        reset_seqid++;
+        if (!reset_seqid) {
+            reset_seqid = 1;
+	}
+    }
+
+    pci_bus->bus_in_reset = true;
+    pci_bus->reset_seqid = reset_seqid;
+
+    return 0;
+}
+
 /*
  * Trigger pci bus reset under a given bus.
  * Called via qbus_reset_all on RST# assert, after the devices
@@ -277,6 +300,8 @@ static void pcibus_reset(BusState *qbus)
     for (i = 0; i < bus->nirq; i++) {
         assert(bus->irq_count[i] == 0);
     }
+
+    bus->bus_in_reset = false;
 }
 
 static void pci_host_bus_register(PCIBus *bus, DeviceState *parent)
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 7eab9d5..6c35171 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -269,7 +269,10 @@ void pci_bridge_write_config(PCIDevice *d,
     newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
     if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
         /* Trigger hot reset on 0->1 transition. */
-        qbus_reset_all(&s->sec_bus.qbus);
+        qbus_walk_children(&s->sec_bus.qbus, NULL,
+                           pcibus_pre_reset,
+                           qdev_reset_one,
+                           qbus_reset_one, &s->sec_bus.qbus);
     }
 }
 
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 7812fa9..9dd2d6c 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -40,11 +40,15 @@ struct PCIBus {
     int nirq;
     int *irq_count;
 
+    bool bus_in_reset;
+    uint32_t reset_seqid;
+
     NotifierWithReturnList hotplug_notifiers;
 };
 
 void pci_bus_add_hotplug_notifier(PCIBus *bus, NotifierWithReturn *notify);
 void pci_bus_remove_hotplug_notifier(NotifierWithReturn *notify);
+int pcibus_pre_reset(BusState *qbus, void *opaque);
 
 typedef struct PCIBridgeWindows PCIBridgeWindows;
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index abcdee8..3d71c17 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -401,4 +401,7 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
 void device_listener_register(DeviceListener *listener);
 void device_listener_unregister(DeviceListener *listener);
 
+int qdev_reset_one(DeviceState *dev, void *opaque);
+int qbus_reset_one(BusState *bus, void *opaque);
+
 #endif
-- 
1.9.3

[Qemu-devel] [PATCH 08/10] vfio: introduce last reset sequence id

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

avoid multi-reset host bus, we introduce sequence id to specify which
bus is resetting. and if one of the dependent devices has done a reset.
the others should skip.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 14 ++++++++++++++
 hw/vfio/pci.h |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index b900f70..5b885e7 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1951,6 +1951,8 @@ static int vfio_check_host_bus_reset(VFIOPCIDevice *vdev)
     /* List all affected devices by bus reset */
     devices = &info->devices[0];
 
+    vdev->single_depend_dev = (info->count == 1);
+
     /* Verify that we have all the groups required */
     for (i = 0; i < info->count; i++) {
         PCIHostDeviceAddress host;
@@ -2283,6 +2285,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
 
     vfio_pci_pre_reset(vdev);
     vdev->vbasedev.needs_reset = false;
+    vdev->last_bus_reset_seqid = vdev->pdev.bus->reset_seqid;
 
     ret = vfio_get_hot_reset_info(vdev, &info);
     if (ret) {
@@ -2344,6 +2347,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
                 }
                 vfio_pci_pre_reset(tmp);
                 tmp->vbasedev.needs_reset = false;
+                tmp->last_bus_reset_seqid = vdev->pdev.bus->reset_seqid;
                 multi = true;
                 break;
             }
@@ -3040,6 +3044,16 @@ static void vfio_pci_reset(DeviceState *dev)
 
     trace_vfio_pci_reset(vdev->vbasedev.name);
 
+    /* if need to do a hot reset */
+    if (pdev->bus->bus_in_reset) {
+        if (vdev->last_bus_reset_seqid == pdev->bus->reset_seqid) {
+            return;
+        } else if ((vdev->features & VFIO_FEATURE_ENABLE_AER)) {
+            vfio_pci_hot_reset(vdev, vdev->single_depend_dev);
+            return;
+        }
+    }
+
     vfio_pci_pre_reset(vdev);
 
     if (vdev->resetfn && !vdev->resetfn(vdev)) {
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index bf9e93c..684931d 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -144,6 +144,9 @@ typedef struct VFIOPCIDevice {
     bool no_kvm_msi;
     bool no_kvm_msix;
 
+    bool single_depend_dev;
+    uint32_t last_bus_reset_seqid;
+
     NotifierWithReturn hotplug_notifier;
 } VFIOPCIDevice;
 
-- 
1.9.3

[Qemu-devel] [PATCH 09/10] vfio-pci: pass the aer error to guest

[Cao jin]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

when the vfio device encounters an uncorrectable error in host,
the vfio_pci driver will signal the eventfd registered by this
vfio device, resulting in the qemu eventfd handler getting
invoked.

this patch is to pass the error to guest and let the guest driver
recover from the error.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 5b885e7..3154656 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2587,18 +2587,59 @@ static void vfio_put_device(VFIOPCIDevice *vdev)
 static void vfio_err_notifier_handler(void *opaque)
 {
     VFIOPCIDevice *vdev = opaque;
+    PCIDevice *dev = &vdev->pdev;
+    PCIEAERMsg msg = {
+        .severity = 0,
+        .source_id = (pci_bus_num(dev->bus) << 8) | dev->devfn,
+    };
 
     if (!event_notifier_test_and_clear(&vdev->err_notifier)) {
         return;
     }
 
     /*
-     * TBD. Retrieve the error details and decide what action
-     * needs to be taken. One of the actions could be to pass
-     * the error to the guest and have the guest driver recover
-     * from the error. This requires that PCIe capabilities be
-     * exposed to the guest. For now, we just terminate the
-     * guest to contain the error.
+     * in case the real hardware configuration has been changed,
+     * here we should recheck the bus reset capability.
+     */
+    if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+        vfio_check_host_bus_reset(vdev)) {
+        goto stop;
+    }
+    /*
+     * we should read the error details from the real hardware
+     * configuration spaces, here we only need to do is signaling
+     * to guest an uncorrectable error has occurred.
+     */
+    if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+        dev->exp.aer_cap) {
+        uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+        uint32_t uncor_status;
+        bool isfatal;
+
+        uncor_status = vfio_pci_read_config(dev,
+                           dev->exp.aer_cap + PCI_ERR_UNCOR_STATUS, 4);
+
+        /*
+         * if the error is not emitted by this device, we can
+         * just ignore it.
+         */
+        if (!(uncor_status & ~0UL)) {
+            return;
+        }
+
+        isfatal = uncor_status & pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER);
+
+        msg.severity = isfatal ? PCI_ERR_ROOT_CMD_FATAL_EN :
+                                 PCI_ERR_ROOT_CMD_NONFATAL_EN;
+
+        pcie_aer_msg(dev, &msg);
+        return;
+    }
+
+stop:
+    /*
+     * If the aer capability is not exposed to the guest. we just
+     * terminate the guest to contain the error.
      */
 
     error_report("%s(%04x:%02x:%02x.%x) Unrecoverable error detected.  "
-- 
1.9.3

Re: [Qemu-devel] [PATCH 07/10] pci: introduce pci bus pre reset

[Michael S. Tsirkin]

On Wed, Feb 17, 2016 at 06:43:51PM +0800, Cao jin wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> 
> avoid repeat bus reset, here introduce a sequence ID for each time
> bus hot reset, so each vfio device could know whether they've already
> been reset for that sequence ID.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

If the assumption that vfio functions are combined
in the same way as on the host, then this is not needed:
just reset when function 0 is reset.

I suggest we do it this way at stage one.
Add more checks when the functionality is in place.

> ---
>  hw/core/qdev.c           |  4 ++--
>  hw/pci/pci.c             | 25 +++++++++++++++++++++++++
>  hw/pci/pci_bridge.c      |  5 ++++-
>  include/hw/pci/pci_bus.h |  4 ++++
>  include/hw/qdev-core.h   |  3 +++
>  5 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 779de2b..e36fa07 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -304,14 +304,14 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>      }
>  }
>  
> -static int qdev_reset_one(DeviceState *dev, void *opaque)
> +int qdev_reset_one(DeviceState *dev, void *opaque)
>  {
>      device_reset(dev);
>  
>      return 0;
>  }
>  
> -static int qbus_reset_one(BusState *bus, void *opaque)
> +int qbus_reset_one(BusState *bus, void *opaque)
>  {
>      BusClass *bc = BUS_GET_CLASS(bus);
>      if (bc->reset) {
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index e41925e..ff978bc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -258,6 +258,29 @@ void pci_device_reset(PCIDevice *dev)
>      pci_do_device_reset(dev);
>  }
>  
> +int pcibus_pre_reset(BusState *qbus, void *opaque)
> +{
> +    BusState *bus = (BusState *)opaque;
> +    PCIBus *pci_bus = DO_UPCAST(PCIBus, qbus, qbus);
> +    static unsigned int reset_seqid;
> +
> +    /*
> +     * To distinguish the different reset times, we
> +     * recalculate the global reset seqid with a given bus.
> +     */
> +    if (qbus == bus) {
> +        reset_seqid++;
> +        if (!reset_seqid) {
> +            reset_seqid = 1;
> +	}
> +    }
> +
> +    pci_bus->bus_in_reset = true;
> +    pci_bus->reset_seqid = reset_seqid;
> +
> +    return 0;
> +}
> +
>  /*
>   * Trigger pci bus reset under a given bus.
>   * Called via qbus_reset_all on RST# assert, after the devices
> @@ -277,6 +300,8 @@ static void pcibus_reset(BusState *qbus)
>      for (i = 0; i < bus->nirq; i++) {
>          assert(bus->irq_count[i] == 0);
>      }
> +
> +    bus->bus_in_reset = false;
>  }
>  
>  static void pci_host_bus_register(PCIBus *bus, DeviceState *parent)
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index 7eab9d5..6c35171 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -269,7 +269,10 @@ void pci_bridge_write_config(PCIDevice *d,
>      newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
>      if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
>          /* Trigger hot reset on 0->1 transition. */
> -        qbus_reset_all(&s->sec_bus.qbus);
> +        qbus_walk_children(&s->sec_bus.qbus, NULL,
> +                           pcibus_pre_reset,
> +                           qdev_reset_one,
> +                           qbus_reset_one, &s->sec_bus.qbus);
>      }
>  }
>  
> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> index 7812fa9..9dd2d6c 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -40,11 +40,15 @@ struct PCIBus {
>      int nirq;
>      int *irq_count;
>  
> +    bool bus_in_reset;
> +    uint32_t reset_seqid;
> +
>      NotifierWithReturnList hotplug_notifiers;
>  };
>  
>  void pci_bus_add_hotplug_notifier(PCIBus *bus, NotifierWithReturn *notify);
>  void pci_bus_remove_hotplug_notifier(NotifierWithReturn *notify);
> +int pcibus_pre_reset(BusState *qbus, void *opaque);
>  
>  typedef struct PCIBridgeWindows PCIBridgeWindows;
>  
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index abcdee8..3d71c17 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -401,4 +401,7 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
>  void device_listener_register(DeviceListener *listener);
>  void device_listener_unregister(DeviceListener *listener);
>  
> +int qdev_reset_one(DeviceState *dev, void *opaque);
> +int qbus_reset_one(BusState *bus, void *opaque);
> +
>  #endif
> -- 
> 1.9.3
> 
> 

Re: [Qemu-devel] [PATCH 06/10] add check reset mechanism when hotplug vfio device

[Michael S. Tsirkin]

On Wed, Feb 17, 2016 at 06:43:50PM +0800, Cao jin wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> 
> Since we support multi-function hotplug, the function 0 indicates
> the closure of the slot, so we have the chance to do the check.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

I think I prefer doing it differently: when function is added, query all
existing functions on same device.  Do it with a new callback, not a
notifier.

In any case, I suggest these checks should come later.
At this stage, complete aer functionality simply assuming setup is correct
and all functions are combined in the same way as on the host.

> ---
>  hw/pci/pci.c             | 29 +++++++++++++++++++++++++++++
>  hw/vfio/pci.c            | 19 +++++++++++++++++++
>  hw/vfio/pci.h            |  2 ++
>  include/hw/pci/pci_bus.h |  5 +++++
>  4 files changed, 55 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d940f79..e41925e 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -82,6 +82,7 @@ static void pci_bus_realize(BusState *qbus, Error **errp)
>      PCIBus *bus = PCI_BUS(qbus);
>  
>      vmstate_register(NULL, -1, &vmstate_pcibus, bus);
> +    notifier_with_return_list_init(&bus->hotplug_notifiers);
>  }
>  
>  static void pci_bus_unrealize(BusState *qbus, Error **errp)
> @@ -1836,6 +1837,22 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
>      return bus->devices[devfn];
>  }
>  
> +void pci_bus_add_hotplug_notifier(PCIBus *bus, NotifierWithReturn *notify)
> +{
> +    notifier_with_return_list_add(&bus->hotplug_notifiers, notify);
> +}
> +
> +void pci_bus_remove_hotplug_notifier(NotifierWithReturn *notifier)
> +{
> +    notifier_with_return_remove(notifier);
> +}
> +
> +static int pci_bus_hotplug_notifier(PCIBus *bus, void *opaque)
> +{
> +    return notifier_with_return_list_notify(&bus->hotplug_notifiers,
> +                                            opaque);
> +}
> +
>  static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>  {
>      PCIDevice *pci_dev = (PCIDevice *)qdev;
> @@ -1878,6 +1895,18 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>          pci_qdev_unrealize(DEVICE(pci_dev), NULL);
>          return;
>      }
> +
> +    /*
> +     *  If the function is func 0, indicate the closure of the slot.
> +     *  signal the callback.
> +     */
> +    if (DEVICE(pci_dev)->hotplugged &&
> +        pci_get_function_0(pci_dev) == pci_dev &&
> +        pci_bus_hotplug_notifier(bus, pci_dev)) {
> +        error_setg(errp, "failed to hotplug function 0");
> +        pci_qdev_unrealize(DEVICE(pci_dev), NULL);
> +        return;
> +    }
>  }
>  
>  static void pci_default_realize(PCIDevice *dev, Error **errp)
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 54032d0..b900f70 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2076,6 +2076,19 @@ static int vfio_check_devices_host_bus_reset(void)
>      return 0;
>  }
>  
> +static int vfio_check_bus_reset(NotifierWithReturn *n, void *opaque)
> +{
> +    VFIOPCIDevice *vdev = container_of(n, VFIOPCIDevice, hotplug_notifier);
> +    PCIDevice *pci_dev = PCI_DEVICE(vdev);
> +    PCIDevice *pci_func0 = opaque;
> +
> +    if (pci_get_function_0(pci_dev) != pci_func0) {
> +        return 0;
> +    }
> +
> +    return vfio_check_host_bus_reset(vdev);
> +}
> +
>  static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
>                            int pos, uint16_t size)
>  {
> @@ -2123,6 +2136,9 @@ static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
>          pdev->exp.aer_log.log_max = 0;
>      }
>  
> +    vdev->hotplug_notifier.notify = vfio_check_bus_reset;
> +    pci_bus_add_hotplug_notifier(pdev->bus, &vdev->hotplug_notifier);
> +
>      pcie_cap_deverr_init(pdev);
>      return pcie_aer_init(pdev, pos, size);
>  
> @@ -3006,6 +3022,9 @@ static void vfio_exitfn(PCIDevice *pdev)
>      vfio_unregister_req_notifier(vdev);
>      vfio_unregister_err_notifier(vdev);
>      pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
> +    if (vdev->features & VFIO_FEATURE_ENABLE_AER) {
> +        pci_bus_remove_hotplug_notifier(&vdev->hotplug_notifier);
> +    }
>      vfio_disable_interrupts(vdev);
>      if (vdev->intx.mmap_timer) {
>          timer_free(vdev->intx.mmap_timer);
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index aff46c2..bf9e93c 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -143,6 +143,8 @@ typedef struct VFIOPCIDevice {
>      bool no_kvm_intx;
>      bool no_kvm_msi;
>      bool no_kvm_msix;
> +
> +    NotifierWithReturn hotplug_notifier;
>  } VFIOPCIDevice;
>  
>  uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len);
> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> index 403fec6..7812fa9 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -39,8 +39,13 @@ struct PCIBus {
>         Keep a count of the number of devices with raised IRQs.  */
>      int nirq;
>      int *irq_count;
> +
> +    NotifierWithReturnList hotplug_notifiers;
>  };
>  
> +void pci_bus_add_hotplug_notifier(PCIBus *bus, NotifierWithReturn *notify);
> +void pci_bus_remove_hotplug_notifier(NotifierWithReturn *notify);
> +
>  typedef struct PCIBridgeWindows PCIBridgeWindows;
>  
>  /*
> -- 
> 1.9.3
> 
> 

Re: [Qemu-devel] [PATCH 08/10] vfio: introduce last reset sequence id

[Michael S. Tsirkin]

On Wed, Feb 17, 2016 at 06:43:52PM +0800, Cao jin wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> 
> avoid multi-reset host bus, we introduce sequence id to specify which
> bus is resetting. and if one of the dependent devices has done a reset.
> the others should skip.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>


I don't see why it's needed. All devices on same bus always
undergo bus reset together. Just select a representative (e.g.
smallest devfn) and reset from that.

> ---
>  hw/vfio/pci.c | 14 ++++++++++++++
>  hw/vfio/pci.h |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index b900f70..5b885e7 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1951,6 +1951,8 @@ static int vfio_check_host_bus_reset(VFIOPCIDevice *vdev)
>      /* List all affected devices by bus reset */
>      devices = &info->devices[0];
>  
> +    vdev->single_depend_dev = (info->count == 1);
> +
>      /* Verify that we have all the groups required */
>      for (i = 0; i < info->count; i++) {
>          PCIHostDeviceAddress host;
> @@ -2283,6 +2285,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
>  
>      vfio_pci_pre_reset(vdev);
>      vdev->vbasedev.needs_reset = false;
> +    vdev->last_bus_reset_seqid = vdev->pdev.bus->reset_seqid;
>  
>      ret = vfio_get_hot_reset_info(vdev, &info);
>      if (ret) {
> @@ -2344,6 +2347,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
>                  }
>                  vfio_pci_pre_reset(tmp);
>                  tmp->vbasedev.needs_reset = false;
> +                tmp->last_bus_reset_seqid = vdev->pdev.bus->reset_seqid;
>                  multi = true;
>                  break;
>              }
> @@ -3040,6 +3044,16 @@ static void vfio_pci_reset(DeviceState *dev)
>  
>      trace_vfio_pci_reset(vdev->vbasedev.name);
>  
> +    /* if need to do a hot reset */
> +    if (pdev->bus->bus_in_reset) {
> +        if (vdev->last_bus_reset_seqid == pdev->bus->reset_seqid) {
> +            return;
> +        } else if ((vdev->features & VFIO_FEATURE_ENABLE_AER)) {
> +            vfio_pci_hot_reset(vdev, vdev->single_depend_dev);
> +            return;
> +        }
> +    }
> +
>      vfio_pci_pre_reset(vdev);
>  
>      if (vdev->resetfn && !vdev->resetfn(vdev)) {
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index bf9e93c..684931d 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -144,6 +144,9 @@ typedef struct VFIOPCIDevice {
>      bool no_kvm_msi;
>      bool no_kvm_msix;
>  
> +    bool single_depend_dev;
> +    uint32_t last_bus_reset_seqid;
> +
>      NotifierWithReturn hotplug_notifier;
>  } VFIOPCIDevice;
>  
> -- 
> 1.9.3
> 
>