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

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

[Cao jin]

After such a long time silence, finally we have a really workable version.
It has quite a few difference from previous, so does kernel driver parts.
I will try to describe the big changes(and reason) as following. Also still
have some unsure points need comments, and left some debug lines in the patch,
so it is also a RFC version.

FYI:
1. v9 triggered a bug of igb driver which causes guest kernel oops, actually
   it doesn't have any relationship with aer patchset, we just trigger it.
   Now oops issue has been fixed: http://patchwork.ozlabs.org/patch/692171/

v10 changelog:
1. Many comments & commit message rephrase, and minor changes,
   not list them here.

2. Rebase on Eric Auger's "Convert VFIO-PCI to realize" patchset.

3. Introduce a new patch 3 to support configurable AER cap version.

4. Drop vfio_pci_host_match_slot() in patch 6, because host_match
   implies slot match

5. Change what eventfd signals(check vfio driver patch). Before, vfio-pci
   driver add 1 to the counter, which doesn't have meaning, just for
   notification. Now, vfio-pci's error detected handler read the uncorrectable
   error status and signal it to qemu.
   Why? When testing previous version(host aer driver still reset link), found
   that there is quite possibility that register reading returns the invalid
   value 0xFFFFFFFF, which results in all(2 in my environment) qemu's vfio-pci
   function send AER to root port while I only inject error into one function.
   This is unreasonable, and I think it is the result of reading during device
   reset. Previous patch does considered to find the real function who has
   error happened, but won't work under the situation I found. So move the
   register reading as early as possible would be the nature thoughts, and it
   is moved into vfio-pci driver. Although now reset_link is disabled in aer
   driver, get the uncor error status as early as possible still make sense,
   for example: if user space driver does device reset once receiving the
   error notification, and then read register.

6. Drop the ugly patch which does "wait 3s to do guest reset". There are
   several consideration in it.

   1st, previous patch use a flag in function 0 to indicate that the functions
   under the virtual bus got error notification, then do bus reset rashly, as
   the response to guest reset_link. This is unreasonable. In physical world,
   set bridge's secondary bus reset would send hot-reset TLP to all functions
   below, trigger every device's reset separately. Emulated device should
   behave the same, means using DeviceClass->reset without any logical change.

   2nd, in vfio-pci driver, we use pci_cfg_access_trylock() to prevent config
   access. When signal aer to guest, guest's aer driver will do config space
   access[*], which results in qemu process being scheduled out. Unlock the
   config space in vfio-pci resume handler, then there is no need to wait
   anything.
   [*] find_source_device->find_device_iter->is_error_source

7. For description integrality, I put the changelog of kernel part here.
   The *key issue* of whole functionality is on the reset_link in host
   error recovery.
   On one hand, device-specific driver in guest often will do register
   read/write during its error recovery, but reset_link in host will make the
   result of read/write unpredictable, the guest kernel oops issue is one of
   the results.
   On the other hand, link reset is one big step in whole error recovery. error
   recovery is very device specific thing, so, vfio-pci as the meta driver,
   won't know how to recover naturally. So it is user space driver, or device
   specific driver in guest, who should take the whole responsibility to do the
   error recovery, include link reset.

Unsure points on vfio-pci driver:
1. Previous patch disabled irq notification in its error detected handler, but
   without enabling it again in resume handler, which is wrong. I am not sure
   whether we need to disable it, so I just removed the code.

2. Two new fields: aer_recovering & aer_error_completion of struct
   vfio_pci_device, and the related operation on them, I am not sure we still
   need them, for now, I think maybe we can drop them.

Test:
I test it with intel 82576 NIC, which has 2 functions, function 1 has cable
connected to gateway, function 0 has no link.

1. Linux: guest works well with this patchset. Robustness is good.
   Steps:
   * start the VM with 2 assigned functions
   * assign ip to function 1, add route info, and ping the gateway
   * on host, injecting error to the assigned devices in any combination:
     inject fatal/nonfatal error to func 0/func 1.  Or, repeat injecting
     certain error to certain function quickly. The result meets the expectation

2. Windows: only test on Win7, but recovery don't work, when injecting error,
   Win7 will show the blue screen, and then reboot. I think it is the bug of
   Windows driver, and not this patch's scope to fix it.
   On previous version, Win7 shows blue screen, then reboot, then blue again,
   reboot again, repeatedly, won't stop.


Chen Fan (9):
  vfio: extract vfio_get_hot_reset_info as a single function
  vfio: squeeze out vfio_pci_do_hot_reset to support bus reset
  vfio: new function to init aer cap for vfio device
  vfio: refine function vfio_pci_host_match
  vfio: add check host bus reset is support or not
  pci: introduce function validation check during hotplug
  vfio: check aer functionality for hotplugged device
  vfio-pci: pass the aer error to guest
  vfio: add 'aer' property to expose aercap

Dou Liyang (1):
  pcie_aer: support configurable AER capa version

 hw/net/e1000e.c                    |   3 +-
 hw/pci-bridge/ioh3420.c            |   2 +-
 hw/pci-bridge/xio3130_downstream.c |   2 +-
 hw/pci-bridge/xio3130_upstream.c   |   2 +-
 hw/pci/pci.c                       |  29 ++
 hw/pci/pcie_aer.c                  |   5 +-
 hw/vfio/pci.c                      | 543 ++++++++++++++++++++++++++++++++-----
 hw/vfio/pci.h                      |   4 +
 include/hw/pci/pci.h               |   1 +
 include/hw/pci/pcie_aer.h          |   3 +-
 10 files changed, 516 insertions(+), 78 deletions(-)

-- 
1.8.3.1

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

[Cao jin]

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

This function is used to get affected devices by bus reset.
It can be used for different purpose, like the coming aer
functionality.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.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 d7dbe0e..171fd2a 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1777,6 +1777,49 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, uint8_t pos)
     }
 }
 
+/* return -errno on failure, return 0 on success. */
+static int vfio_get_hot_reset_info(VFIOPCIDevice *vdev,
+                                   struct vfio_pci_hot_reset_info **ret_info,
+                                   Error **errp)
+{
+    struct vfio_pci_hot_reset_info *info;
+    int ret, count;
+
+    assert(!(*ret_info));
+
+    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;
+        error_setg(errp, "vfio-pci: fail to get pci hot reset info: %m");
+        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_setg(errp, "vfio-pci: fail to get pci hot reset info: %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, Error **errp)
 {
     PCIDevice *pdev = &vdev->pdev;
@@ -2021,12 +2064,13 @@ static bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
 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;
     int ret, i, count;
     bool multi = false;
+    Error *err = NULL;
 
     trace_vfio_pci_hot_reset(vdev->vbasedev.name, single ? "one" : "multi");
 
@@ -2035,12 +2079,9 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
     }
     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, &err);
+    if (ret) {
+        error_report_err(err);
         if (!vdev->has_pm_reset) {
             error_report("vfio: Cannot reset device %s, "
                          "no available reset mechanism.", vdev->vbasedev.name);
@@ -2048,18 +2089,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.8.3.1

[Qemu-devel] [PATCH v10 02/10] vfio: squeeze out vfio_pci_do_hot_reset to 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 during
guest AER recovery.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 76 +++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 31 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 171fd2a..94d25a1 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1820,6 +1820,49 @@ error:
     return ret;
 }
 
+/* Do host bus reset */
+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, Error **errp)
 {
     PCIDevice *pdev = &vdev->pdev;
@@ -2066,9 +2109,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;
     Error *err = NULL;
 
@@ -2150,34 +2191,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.8.3.1

[Qemu-devel] [PATCH v10 03/10] pcie_aer: support configurable AER capa version

[Cao jin]

From: Dou Liyang <douly.fnst@cn.fujitsu.com>

Now, AER capa version is fixed to v2, if assigned device is actually
v1, then this value will inconsistent between guest and host

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/net/e1000e.c                    | 3 ++-
 hw/pci-bridge/ioh3420.c            | 2 +-
 hw/pci-bridge/xio3130_downstream.c | 2 +-
 hw/pci-bridge/xio3130_upstream.c   | 2 +-
 hw/pci/pcie_aer.c                  | 5 +++--
 include/hw/pci/pcie_aer.h          | 3 ++-
 6 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 4994e1c..66de849 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -472,7 +472,8 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp)
         hw_error("Failed to initialize PM capability");
     }
 
-    if (pcie_aer_init(pci_dev, e1000e_aer_offset, PCI_ERR_SIZEOF) < 0) {
+    if (pcie_aer_init(pci_dev, PCI_ERR_VER, e1000e_aer_offset,
+                              PCI_ERR_SIZEOF) < 0) {
         hw_error("Failed to initialize AER capability");
     }
 
diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index c8b5ac4..d70784c 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -135,7 +135,7 @@ static int ioh3420_initfn(PCIDevice *d)
         goto err_pcie_cap;
     }
 
-    rc = pcie_aer_init(d, IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
+    rc = pcie_aer_init(d, PCI_ERR_VER, IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
     if (rc < 0) {
         goto err;
     }
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index cef6e13..5d1ce01 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -97,7 +97,7 @@ static int xio3130_downstream_initfn(PCIDevice *d)
         goto err_pcie_cap;
     }
 
-    rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
+    rc = pcie_aer_init(d, PCI_ERR_VER, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
     if (rc < 0) {
         goto err;
     }
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index 4ad0440..3819964 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -85,7 +85,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
     pcie_cap_flr_init(d);
     pcie_cap_deverr_init(d);
 
-    rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
+    rc = pcie_aer_init(d, PCI_ERR_VER, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
     if (rc < 0) {
         goto err;
     }
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 048ce6a..ac47f34 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -96,11 +96,12 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log)
     aer_log->log_num = 0;
 }
 
-int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size)
+int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, uint16_t offset,
+                  uint16_t size)
 {
     PCIExpressDevice *exp;
 
-    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
+    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, cap_ver,
                         offset, size);
     exp = &dev->exp;
     exp->aer_cap = offset;
diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
index c2ee4e2..c373591 100644
--- a/include/hw/pci/pcie_aer.h
+++ b/include/hw/pci/pcie_aer.h
@@ -87,7 +87,8 @@ struct PCIEAERErr {
 
 extern const VMStateDescription vmstate_pcie_aer_log;
 
-int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size);
+int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, uint16_t offset,
+                  uint16_t size);
 void pcie_aer_exit(PCIDevice *dev);
 void pcie_aer_write_config(PCIDevice *dev,
                            uint32_t addr, uint32_t val, int len);
-- 
1.8.3.1

[Qemu-devel] [PATCH v10 04/10] vfio: new function to init aer cap for vfio device

[Cao jin]

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

Introduce new function to initilize AER capability registers
for vfio-pci device.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 hw/vfio/pci.h |  3 +++
 2 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 94d25a1..4ff6626 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1937,18 +1937,81 @@ out:
     return 0;
 }
 
-static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
+static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
+                          int pos, uint16_t size, Error **errp)
+{
+    PCIDevice *pdev = &vdev->pdev;
+    PCIDevice *dev_iter;
+    uint8_t type;
+    uint32_t errcap;
+
+    /* In case the physical device has AER cap while user doesn't enable AER,
+     * still allocate the config space in the emulated device for AER */
+    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) {
+        if (!pci_is_express(dev_iter)) {
+            goto error;
+        }
+
+        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, cap_ver, pos, size);
+
+error:
+    error_setg(errp, "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, Error **errp)
 {
     PCIDevice *pdev = &vdev->pdev;
     uint32_t header;
     uint16_t cap_id, next, size;
     uint8_t cap_ver;
     uint8_t *config;
+    int ret = 0;
 
     /* Only add extended caps if we have them and the guest can see them */
     if (!pci_is_express(pdev) || !pci_bus_is_express(pdev->bus) ||
         !pci_get_long(pdev->config + PCI_CONFIG_SPACE_SIZE)) {
-        return;
+        return 0;
     }
 
     /*
@@ -1997,6 +2060,9 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
                                    PCI_EXT_CAP_NEXT_MASK);
 
         switch (cap_id) {
+        case PCI_EXT_CAP_ID_ERR:
+            ret = vfio_setup_aer(vdev, cap_ver, next, size, errp);
+            break;
         case PCI_EXT_CAP_ID_SRIOV: /* Read-only VF BARs confuse OVMF */
         case PCI_EXT_CAP_ID_ARI: /* XXX Needs next function virtualization */
             trace_vfio_add_ext_cap_dropped(vdev->vbasedev.name, cap_id, next);
@@ -2005,6 +2071,9 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
             pcie_add_capability(pdev, cap_id, cap_ver, next, size);
         }
 
+        if (ret) {
+            goto out;
+        }
     }
 
     /* Cleanup chain head ID if necessary */
@@ -2012,8 +2081,9 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
         pci_set_word(pdev->config + PCI_CONFIG_SPACE_SIZE, 0);
     }
 
+out:
     g_free(config);
-    return;
+    return ret;
 }
 
 static int vfio_add_capabilities(VFIOPCIDevice *vdev, Error **errp)
@@ -2031,8 +2101,8 @@ static int vfio_add_capabilities(VFIOPCIDevice *vdev, Error **errp)
         return ret;
     }
 
-    vfio_add_ext_cap(vdev);
-    return 0;
+    ret = vfio_add_ext_cap(vdev, errp);
+    return ret;
 }
 
 static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
@@ -2813,6 +2883,13 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
         goto out_teardown;
     }
 
+    if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+        !pdev->exp.aer_cap) {
+        error_setg(errp, "vfio: Unable to enable AER for device %s, device "
+                   "does not support AER signaling", vdev->vbasedev.name);
+        return;
+    }
+
     if (vdev->vga) {
         vfio_vga_quirk_setup(vdev);
     }
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index a8366bb..64701c4 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"
@@ -132,6 +133,8 @@ typedef struct VFIOPCIDevice {
 #define VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT 2
 #define VFIO_FEATURE_ENABLE_IGD_OPREGION \
                                 (1 << VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT)
+#define VFIO_FEATURE_ENABLE_AER_BIT 3
+#define VFIO_FEATURE_ENABLE_AER (1 << VFIO_FEATURE_ENABLE_AER_BIT)
     int32_t bootindex;
     uint32_t igd_gms;
     uint8_t pm_cap;
-- 
1.8.3.1

[Qemu-devel] [PATCH v10 05/10] vfio: refine function vfio_pci_host_match

[Cao jin]

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

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 4ff6626..95cb3c2 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2164,14 +2164,27 @@ static void vfio_pci_post_reset(VFIOPCIDevice *vdev)
     }
 }
 
+static int vfio_pci_name_to_addr(const char *name, PCIHostDeviceAddress *addr)
+{
+    if (strlen(name) != 12 ||
+        sscanf(name, "%04x:%02x:%02x.%1x", &addr->domain,
+               &addr->bus, &addr->slot, &addr->function) != 4) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
 {
-    char tmp[13];
+    PCIHostDeviceAddress tmp;
 
-    sprintf(tmp, "%04x:%02x:%02x.%1x", addr->domain,
-            addr->bus, addr->slot, addr->function);
+    if (vfio_pci_name_to_addr(name, &tmp)) {
+        return false;
+    }
 
-    return (strcmp(tmp, name) == 0);
+    return (tmp.domain == addr->domain && tmp.bus == addr->bus &&
+            tmp.slot == addr->slot && tmp.function == addr->function);
 }
 
 static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
-- 
1.8.3.1

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

[Cao jin]

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

When assigning a vfio device with AER enabled, we must check whether
the device supports host bus reset (ie. hot reset) as this may be
used by the guest OS to recover the device from an AER error.
Therefore, QEMU must have the ability to perform a physical
host bus reset using the existing vfio APIs in response to a virtual
bus reset in the VM.  A physical bus reset affects all devices under
the bus, for easier life, we need a few configuration restriction
on the VM:

  1. All physical functions in a single card must be assigned to the VM
     with AER enabled on each and configured on the same virtual bus.

  2. Don't place other devices under the virtual bus in 1, no matter physical,
     emulated, or paravirtual, even if other device supporting AER signaling.

In other words, users wishing to enable AER on a multifunction device
need to assign all functions of the device to the same virtual bus
and enable AER support for each function. This is should be checked
after machine init done.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 259 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
 hw/vfio/pci.h |   1 +
 2 files changed, 237 insertions(+), 23 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 95cb3c2..8687668 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1777,6 +1777,29 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, uint8_t pos)
     }
 }
 
+static int vfio_pci_name_to_addr(const char *name, PCIHostDeviceAddress *addr)
+{
+    if (strlen(name) != 12 ||
+        sscanf(name, "%04x:%02x:%02x.%1x", &addr->domain,
+               &addr->bus, &addr->slot, &addr->function) != 4) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
+{
+    PCIHostDeviceAddress tmp;
+
+    if (vfio_pci_name_to_addr(name, &tmp)) {
+        return false;
+    }
+
+    return (tmp.domain == addr->domain && tmp.bus == addr->bus &&
+            tmp.slot == addr->slot && tmp.function == addr->function);
+}
+
 /* return -errno on failure, return 0 on success. */
 static int vfio_get_hot_reset_info(VFIOPCIDevice *vdev,
                                    struct vfio_pci_hot_reset_info **ret_info,
@@ -1937,6 +1960,195 @@ out:
     return 0;
 }
 
+static int vfio_device_range_limit(PCIBus *bus)
+{
+    PCIDevice *br = NULL;
+
+    br = pci_bridge_get_device(bus);
+    if (br &&
+        pcie_cap_is_arifwd_enabled(br)) {
+        return 255;
+    }
+
+    return 8;
+}
+
+static void vfio_check_hot_bus_reset(VFIOPCIDevice *vdev, Error **errp)
+{
+    PCIBus *bus = vdev->pdev.bus;
+    struct vfio_pci_hot_reset_info *info = NULL;
+    struct vfio_pci_dependent_device *devices;
+    VFIOGroup *group;
+    int ret, i, devfn, range_limit;
+
+    ret = vfio_get_hot_reset_info(vdev, &info, errp);
+    if (ret) {
+        return;
+    }
+
+    /* 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->vbasedev.name)) {
+            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_setg(errp, "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 all affected devices are on the same 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->vbasedev.name)) {
+                /*
+                 * AER errors will 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 (!(tmp->features & VFIO_FEATURE_ENABLE_AER)) {
+                    error_setg(errp, "vfio: Cannot enable AER for device %s,"
+                               " which is dependent on device %s on the same"
+                               " slot, which does not enable AER",
+                               vdev->vbasedev.name, tmp->vbasedev.name);
+                    goto out;
+                }
+
+                if (tmp->pdev.bus != bus) {
+                    error_setg(errp, "vfio: Cannot enable AER for device %s,"
+                               " the dependent device %s is not on the same"
+                               " virtual bus",
+                               vdev->vbasedev.name, tmp->vbasedev.name);
+                    goto out;
+                }
+                found = true;
+                break;
+            }
+        }
+
+        /* Ensure all affected devices assigned to VM */
+        if (!found) {
+            error_setg(errp, "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;
+        }
+    }
+
+    /*
+     * The above code verified that all functions in a single device affected
+     * by a bus reset exist on the same bus in the VM.  To further simplify,
+     * we also require that there are no other devices beyond those on the
+     * same VM bus.
+     */
+    range_limit = vfio_device_range_limit(bus);
+    for (devfn = 0; devfn < range_limit; devfn++) {
+        VFIOPCIDevice *tmp;
+        PCIDevice *dev;
+        bool found = false;
+
+        dev = pci_find_device(bus, pci_bus_num(bus),
+                  PCI_DEVFN(PCI_SLOT(vdev->pdev.devfn), devfn));
+
+        if (!dev) {
+            continue;
+        }
+
+        if (!object_dynamic_cast(OBJECT(dev), "vfio-pci")) {
+            error_setg(errp, "vfio: Cannot enable AER for device %s, device"
+                             " %s in slot %d function%d must not be configured"
+                             " on the same virtual bus",
+                             vdev->vbasedev.name, dev->name,
+                             PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
+            goto out;
+        }
+
+        tmp = DO_UPCAST(VFIOPCIDevice, pdev, dev);
+        for (i = 0; i < info->count; i++) {
+            PCIHostDeviceAddress host;
+
+            host.domain = devices[i].segment;
+            host.bus = devices[i].bus;
+            host.slot = PCI_SLOT(devices[i].devfn);
+            host.function = PCI_FUNC(devices[i].devfn);
+
+            if (vfio_pci_host_match(&host, tmp->vbasedev.name)) {
+                found = true;
+                break;
+            }
+        }
+
+        if (!found) {
+            error_setg(errp, "vfio: Cannot enable AER for device %s,"
+                             " affected device %s is not configured"
+                             " on the same virtual bus",
+                       vdev->vbasedev.name, tmp->vbasedev.name);
+            goto out;
+        }
+    }
+
+out:
+    g_free(info);
+    return;
+}
+
+static void vfio_aer_check_host_bus_reset(Error **errp)
+{
+    VFIOGroup *group;
+    VFIODevice *vbasedev;
+    VFIOPCIDevice *vdev;
+    Error *err = NULL;
+
+    /* Check if all vfio-pci devices 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_hot_bus_reset(vdev, &err);
+                if (err) {
+                    error_propagate(errp, err);
+                    return;
+                }
+            }
+        }
+    }
+
+    return;
+}
+
 static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
                           int pos, uint16_t size, Error **errp)
 {
@@ -2164,29 +2376,6 @@ static void vfio_pci_post_reset(VFIOPCIDevice *vdev)
     }
 }
 
-static int vfio_pci_name_to_addr(const char *name, PCIHostDeviceAddress *addr)
-{
-    if (strlen(name) != 12 ||
-        sscanf(name, "%04x:%02x:%02x.%1x", &addr->domain,
-               &addr->bus, &addr->slot, &addr->function) != 4) {
-        return -EINVAL;
-    }
-
-    return 0;
-}
-
-static bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
-{
-    PCIHostDeviceAddress tmp;
-
-    if (vfio_pci_name_to_addr(name, &tmp)) {
-        return false;
-    }
-
-    return (tmp.domain == addr->domain && tmp.bus == addr->bus &&
-            tmp.slot == addr->slot && tmp.function == addr->function);
-}
-
 static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
 {
     VFIOGroup *group;
@@ -2717,6 +2906,21 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
     vdev->req_enabled = false;
 }
 
+static void vfio_pci_machine_done_notify(Notifier *notifier, void *unused)
+{
+    Error *err = NULL;
+
+    vfio_aer_check_host_bus_reset(&err);
+    if (err) {
+        error_report_err(err);
+        exit(1);
+    }
+}
+
+static Notifier machine_notifier = {
+    .notify = vfio_pci_machine_done_notify,
+};
+
 static void vfio_realize(PCIDevice *pdev, Error **errp)
 {
     VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
@@ -3122,6 +3326,15 @@ static const TypeInfo vfio_pci_dev_info = {
 static void register_vfio_pci_dev_type(void)
 {
     type_register_static(&vfio_pci_dev_info);
+
+    /*
+     * The AER configuration may depend on multiple devices, so we cannot
+     * validate consistency after each device is initialized.  We can only
+     * depend on function initialization order (function 0 last) for hotplug
+     * devices, therefore a machine-init-done notifier is used to validate
+     * the configuration after all cold-plug devices are processed.
+     */
+     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 64701c4..4c21a46 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.8.3.1

[Qemu-devel] [PATCH v10 07/10] pci: introduce function validation check during hotplug

[Cao jin]

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

PCI hotplug requires that function 0 is added last to close the
slot.  Since vfio-pci supporting AER, we require that the VM bus
contains the same set of devices as the host bus to support AER,
we can perform an AER validation test whenever the function 0 is
hot-added into VM.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/pci/pci.c         | 29 +++++++++++++++++++++++++++++
 include/hw/pci/pci.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 24fae16..26eaf4c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1942,6 +1942,20 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
     return bus->devices[devfn];
 }
 
+static void pci_function_is_valid(PCIBus *bus, PCIDevice *d, void *opaque)
+{
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(d);
+    Error **errp = opaque;
+
+    if (*errp) {
+        return;
+    }
+
+    if (pc->is_valid_func) {
+        pc->is_valid_func(d, errp);
+    }
+}
+
 static void pci_qdev_realize(DeviceState *qdev, Error **errp)
 {
     PCIDevice *pci_dev = (PCIDevice *)qdev;
@@ -1984,6 +1998,21 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
         pci_qdev_unrealize(DEVICE(pci_dev), NULL);
         return;
     }
+
+    /*
+     *  Hot-added function number 0 indicates the closure of the slot, it is
+     *  time to check whether all functions under the same bus is valid.
+     */
+    if (DEVICE(pci_dev)->hotplugged &&
+        pci_get_function_0(pci_dev) == pci_dev) {
+        pci_for_each_device(bus, pci_bus_num(bus),
+                            pci_function_is_valid, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            pci_qdev_unrealize(DEVICE(pci_dev), NULL);
+            return;
+        }
+    }
 }
 
 static void pci_default_realize(PCIDevice *dev, Error **errp)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 772692f..678f305 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -205,6 +205,7 @@ typedef struct PCIDeviceClass {
 
     void (*realize)(PCIDevice *dev, Error **errp);
     int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
+    void (*is_valid_func)(PCIDevice *dev, Error **errp);
     PCIUnregisterFunc *exit;
     PCIConfigReadFunc *config_read;
     PCIConfigWriteFunc *config_write;
-- 
1.8.3.1

[Qemu-devel] [PATCH v10 08/10] vfio: check aer functionality for hotplugged device

[Cao jin]

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

when vfio-pci device is hot-added as function 0 with aer enabled, check
whether this device support hot bus reset.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 8687668..d9236ed 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3248,6 +3248,19 @@ post_reset:
     vfio_pci_post_reset(vdev);
 }
 
+static void vfio_pci_is_valid(PCIDevice *dev, Error **errp)
+{
+    VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, dev);
+    Error *err = NULL;
+
+    if (vdev->features & VFIO_FEATURE_ENABLE_AER) {
+        vfio_check_hot_bus_reset(vdev, &err);
+        if (err) {
+            error_propagate(errp, err);
+        }
+    }
+}
+
 static void vfio_instance_init(Object *obj)
 {
     PCIDevice *pci_dev = PCI_DEVICE(obj);
@@ -3309,6 +3322,7 @@ static void vfio_pci_dev_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     pdc->realize = vfio_realize;
     pdc->exit = vfio_exitfn;
+    pdc->is_valid_func = vfio_pci_is_valid;
     pdc->config_read = vfio_pci_read_config;
     pdc->config_write = vfio_pci_write_config;
     pdc->is_express = 1; /* We might be */
-- 
1.8.3.1

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

[Cao jin]

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

When physical device has uncorrectable error hanppened, the vfio_pci
driver will signal the corresponding QEMU's vfio-pci device via the eventfd
registered by this device, then, the qemu error eventfd handler will be
invoked in event loop.

Construct and pass the aer message to root port, root port will trigger an
interrupt to signal guest, then, the guest driver will do the recovery.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 52 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d9236ed..8bdd889 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2716,18 +2716,56 @@ static void vfio_put_device(VFIOPCIDevice *vdev)
 static void vfio_err_notifier_handler(void *opaque)
 {
     VFIOPCIDevice *vdev = opaque;
+    PCIDevice *dev = &vdev->pdev;
+    Error *err = NULL;
+    PCIEAERMsg msg = {
+        .severity = 0,
+        .source_id = (pci_bus_num(dev->bus) << 8) | dev->devfn,
+    };
+    int len;
+    uint64_t uncor_status;
+
+    /* Read the uncorrectable error status from vfio-pci driver via the eventfd */
+    len = read(vdev->err_notifier.rfd, &uncor_status, sizeof(uncor_status));
+    if (len != sizeof(uncor_status)) {
+        error_report("vfio-pci: uncor error status reading returns"
+                     " invalid number of bytes: %d", len);
+        return;
+    }
+
+    if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) {
+        goto stop;
+    }
+
+    /*
+     * In case the actual hardware configuration has been changed,
+     * should re-check the bus reset capability here.
+     */
+    vfio_check_hot_bus_reset(vdev, &err);
+    if (err) {
+        error_report_err(err);
+        goto stop;
+    }
+
+    /* Complete the aer msg and send it to root port */
+    if (dev->exp.aer_cap) {
+        uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+        bool isfatal = uncor_status &
+                       pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER);
 
-    if (!event_notifier_test_and_clear(&vdev->err_notifier)) {
+        msg.severity = isfatal ? PCI_ERR_ROOT_CMD_FATAL_EN :
+                                 PCI_ERR_ROOT_CMD_NONFATAL_EN;
+ 
+        error_report("vfio-pci device %d sending AER to root port. uncor"
+                     " status = 0x%"PRIx64, dev->devfn, uncor_status);
+        pcie_aer_msg(dev, &msg);
         return;
     }
 
+stop:
     /*
-     * 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.
+     * If the aer capability is not exposed to the guest. we just
+     * terminate the guest to contain the error.
      */
 
     error_report("%s(%s) Unrecoverable error detected. Please collect any data possible and then kill the guest", __func__, vdev->vbasedev.name);
-- 
1.8.3.1

[Qemu-devel] [PATCH v10 10/10] vfio: add 'aer' property to expose aercap

[Cao jin]

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

Add 'aer' property, let user choose whether expose the aer capability
or not. Should disable aer feature by default, because it needs
configuration restrictions.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/vfio/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 8bdd889..f9d671e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3335,6 +3335,8 @@ static Property vfio_pci_dev_properties[] = {
     DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
                        sub_device_id, PCI_ANY_ID),
     DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
+    DEFINE_PROP_BIT("aer", VFIOPCIDevice, features,
+                    VFIO_FEATURE_ENABLE_AER_BIT, false),
     /*
      * TODO - support passed fds... is this necessary?
      * DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v10 03/10] pcie_aer: support configurable AER capa version

[Michael S. Tsirkin]

On Sun, Nov 27, 2016 at 07:32:26PM +0800, Cao jin wrote:
> From: Dou Liyang <douly.fnst@cn.fujitsu.com>
> 
> Now, AER capa version is fixed to v2, if assigned device is actually
> v1, then this value will inconsistent between guest and host
> 
> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/net/e1000e.c                    | 3 ++-
>  hw/pci-bridge/ioh3420.c            | 2 +-
>  hw/pci-bridge/xio3130_downstream.c | 2 +-
>  hw/pci-bridge/xio3130_upstream.c   | 2 +-
>  hw/pci/pcie_aer.c                  | 5 +++--
>  include/hw/pci/pcie_aer.h          | 3 ++-
>  6 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index 4994e1c..66de849 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -472,7 +472,8 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp)
>          hw_error("Failed to initialize PM capability");
>      }
>  
> -    if (pcie_aer_init(pci_dev, e1000e_aer_offset, PCI_ERR_SIZEOF) < 0) {
> +    if (pcie_aer_init(pci_dev, PCI_ERR_VER, e1000e_aer_offset,
> +                              PCI_ERR_SIZEOF) < 0) {
>          hw_error("Failed to initialize AER capability");
>      }
>  
> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> index c8b5ac4..d70784c 100644
> --- a/hw/pci-bridge/ioh3420.c
> +++ b/hw/pci-bridge/ioh3420.c
> @@ -135,7 +135,7 @@ static int ioh3420_initfn(PCIDevice *d)
>          goto err_pcie_cap;
>      }
>  
> -    rc = pcie_aer_init(d, IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
> +    rc = pcie_aer_init(d, PCI_ERR_VER, IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
>      if (rc < 0) {
>          goto err;
>      }
> diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> index cef6e13..5d1ce01 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -97,7 +97,7 @@ static int xio3130_downstream_initfn(PCIDevice *d)
>          goto err_pcie_cap;
>      }
>  
> -    rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
> +    rc = pcie_aer_init(d, PCI_ERR_VER, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
>      if (rc < 0) {
>          goto err;
>      }
> diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> index 4ad0440..3819964 100644
> --- a/hw/pci-bridge/xio3130_upstream.c
> +++ b/hw/pci-bridge/xio3130_upstream.c
> @@ -85,7 +85,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
>      pcie_cap_flr_init(d);
>      pcie_cap_deverr_init(d);
>  
> -    rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
> +    rc = pcie_aer_init(d, PCI_ERR_VER, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
>      if (rc < 0) {
>          goto err;
>      }
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index 048ce6a..ac47f34 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -96,11 +96,12 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log)
>      aer_log->log_num = 0;
>  }
>  
> -int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size)
> +int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, uint16_t offset,
> +                  uint16_t size)
>  {
>      PCIExpressDevice *exp;
>  
> -    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, cap_ver,
>                          offset, size);
>      exp = &dev->exp;
>      exp->aer_cap = offset;
> diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
> index c2ee4e2..c373591 100644
> --- a/include/hw/pci/pcie_aer.h
> +++ b/include/hw/pci/pcie_aer.h
> @@ -87,7 +87,8 @@ struct PCIEAERErr {
>  
>  extern const VMStateDescription vmstate_pcie_aer_log;
>  
> -int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size);
> +int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, uint16_t offset,
> +                  uint16_t size);
>  void pcie_aer_exit(PCIDevice *dev);
>  void pcie_aer_write_config(PCIDevice *dev,
>                             uint32_t addr, uint32_t val, int len);
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH v10 07/10] pci: introduce function validation check during hotplug

[Michael S. Tsirkin]

On Sun, Nov 27, 2016 at 07:32:30PM +0800, Cao jin wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> 
> PCI hotplug requires that function 0 is added last to close the
> slot.  Since vfio-pci supporting AER, we require that the VM bus
> contains the same set of devices as the host bus to support AER,
> we can perform an AER validation test whenever the function 0 is
> hot-added into VM.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>

I'm fine with this interface.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>



> ---
>  hw/pci/pci.c         | 29 +++++++++++++++++++++++++++++
>  include/hw/pci/pci.h |  1 +
>  2 files changed, 30 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 24fae16..26eaf4c 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1942,6 +1942,20 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn)
>      return bus->devices[devfn];
>  }
>  
> +static void pci_function_is_valid(PCIBus *bus, PCIDevice *d, void *opaque)
> +{
> +    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(d);
> +    Error **errp = opaque;
> +
> +    if (*errp) {
> +        return;
> +    }
> +
> +    if (pc->is_valid_func) {
> +        pc->is_valid_func(d, errp);
> +    }
> +}
> +
>  static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>  {
>      PCIDevice *pci_dev = (PCIDevice *)qdev;
> @@ -1984,6 +1998,21 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>          pci_qdev_unrealize(DEVICE(pci_dev), NULL);
>          return;
>      }
> +
> +    /*
> +     *  Hot-added function number 0 indicates the closure of the slot, it is
> +     *  time to check whether all functions under the same bus is valid.
> +     */
> +    if (DEVICE(pci_dev)->hotplugged &&
> +        pci_get_function_0(pci_dev) == pci_dev) {
> +        pci_for_each_device(bus, pci_bus_num(bus),
> +                            pci_function_is_valid, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            pci_qdev_unrealize(DEVICE(pci_dev), NULL);
> +            return;
> +        }
> +    }
>  }
>  
>  static void pci_default_realize(PCIDevice *dev, Error **errp)
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 772692f..678f305 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -205,6 +205,7 @@ typedef struct PCIDeviceClass {
>  
>      void (*realize)(PCIDevice *dev, Error **errp);
>      int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
> +    void (*is_valid_func)(PCIDevice *dev, Error **errp);
>      PCIUnregisterFunc *exit;
>      PCIConfigReadFunc *config_read;
>      PCIConfigWriteFunc *config_write;
> -- 
> 1.8.3.1
> 
> 

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

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v10 00/10] vfio-pci: pass the aer error to guest
Type: series
Message-id: 1480246353-10297-1-git-send-email-caoj.fnst@cn.fujitsu.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
0bffb84 vfio: add 'aer' property to expose aercap
9b06ac3 vfio-pci: pass the aer error to guest
4e9a3ad vfio: check aer functionality for hotplugged device
666206a pci: introduce function validation check during hotplug
344ab2b vfio: add check host bus reset is support or not
3b4bb2b vfio: refine function vfio_pci_host_match
9527ac2 vfio: new function to init aer cap for vfio device
9a0ae86 pcie_aer: support configurable AER capa version
646d5a3 vfio: squeeze out vfio_pci_do_hot_reset to support bus reset
45d8932 vfio: extract vfio_get_hot_reset_info as a single function

=== OUTPUT BEGIN ===
Checking PATCH 1/10: vfio: extract vfio_get_hot_reset_info as a single function...
Checking PATCH 2/10: vfio: squeeze out vfio_pci_do_hot_reset to support bus reset...
Checking PATCH 3/10: pcie_aer: support configurable AER capa version...
Checking PATCH 4/10: vfio: new function to init aer cap for vfio device...
Checking PATCH 5/10: vfio: refine function vfio_pci_host_match...
Checking PATCH 6/10: vfio: add check host bus reset is support or not...
Checking PATCH 7/10: pci: introduce function validation check during hotplug...
Checking PATCH 8/10: vfio: check aer functionality for hotplugged device...
Checking PATCH 9/10: vfio-pci: pass the aer error to guest...
WARNING: line over 80 characters
#36: FILE: hw/vfio/pci.c:2728:
+    /* Read the uncorrectable error status from vfio-pci driver via the eventfd */

ERROR: trailing whitespace
#67: FILE: hw/vfio/pci.c:2758:
+ $

total: 1 errors, 1 warnings, 63 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 10/10: vfio: add 'aer' property to expose aercap...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org