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

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

[Chen Hanxiao]

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

For now, when qemu receives an error from host aer report
by vfio pci passthough devices, qemu just terminate the guest.
Usually user want to know what error occurred
rather than stop the guest.

This patches add aer capability support for vfio device,
then pass the error to guest, and let guest driver to recover
from the error.
Turning on SERR# for error forwording in bridge control register
patch in seabios has been merged as commit 32ec3ee.

notes:
  this series patches enable aer support single/multi-function,
  for multi-function, require all the function of the slot assigned to
  VM and on the same slot.

Chen Fan (15):
  vfio: extract vfio_get_hot_reset_info as a single function
  vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
  pcie: modify the capability size assert
  vfio: make the 4 bytes aligned for capability size
  vfio: add pcie extanded capability support
  aer: impove pcie_aer_init to support vfio device
  vfio: add aer support for vfio device
  vfio: add check host bus reset is support or not
  pci: add bus reset_notifiers callbacks for host bus reset
  vfio: add sec_bus_reset notifier to notify physical bus reset is
    needed
  vfio: modify vfio_pci_hot_reset to support bus reset
  vfio: do hot bus reset when do virtual secondary bus reset
  pcie_aer: expose pcie_aer_msg() interface
  vfio-pci: pass the aer error to guest
  vfio: add 'aer' property to expose aercap

 hw/pci-bridge/ioh3420.c            |   2 +-
 hw/pci-bridge/xio3130_downstream.c |   2 +-
 hw/pci-bridge/xio3130_upstream.c   |   2 +-
 hw/pci/pci.c                       |  16 +
 hw/pci/pci_bridge.c                |   6 +
 hw/pci/pcie.c                      |   2 +-
 hw/pci/pcie_aer.c                  |   6 +-
 hw/vfio/pci.c                      | 636 +++++++++++++++++++++++++++++++++----
 include/hw/pci/pci.h               |   4 +
 include/hw/pci/pci_bus.h           |   2 +
 include/hw/pci/pcie_aer.h          |   3 +-
 11 files changed, 609 insertions(+), 72 deletions(-)

-- 
2.1.0

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

[Chen Hanxiao]

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>
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 hw/vfio/pci.c | 65 ++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 47 insertions(+), 18 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 2ed877f..b693ee7 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2669,6 +2669,50 @@ 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;
+
+    return 0;
+error:
+    g_free(info);
+    return ret;
+}
+
 static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
 {
     PCIDevice *pdev = &vdev->pdev;
@@ -2808,7 +2852,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;
@@ -2820,12 +2864,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,
@@ -2834,18 +2874,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 */
-- 
2.1.0

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

[Chen Hanxiao]

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>
Signed-off-by: Chen Hanxiao <chenhanxiao@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 b693ee7..f9ff9c4 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2713,6 +2713,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;
@@ -2854,9 +2896,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");
@@ -2935,34 +2975,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");
-- 
2.1.0

[Qemu-devel] [PATCH v12 03/15] pcie: modify the capability size assert

[Chen Hanxiao]

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

 Device's Offset and size can reach PCIE_CONFIG_SPACE_SIZE,
 fix the corresponding assert.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 hw/pci/pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 6e28985..d66fb10 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -594,7 +594,7 @@ void pcie_add_capability(PCIDevice *dev,
 
     assert(offset >= PCI_CONFIG_SPACE_SIZE);
     assert(offset < offset + size);
-    assert(offset + size < PCIE_CONFIG_SPACE_SIZE);
+    assert(offset + size <= PCIE_CONFIG_SPACE_SIZE);
     assert(size >= 8);
     assert(pci_is_express(dev));
 
-- 
2.1.0

[Qemu-devel] [PATCH v12 04/15] vfio: make the 4 bytes aligned for capability size

[Chen Hanxiao]

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

this function search the capability from the end, the last
size should 0x100 - pos, not 0xff - pos.

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

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index f9ff9c4..cae45a0 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2509,7 +2509,8 @@ static void vfio_unmap_bars(VFIOPCIDevice *vdev)
  */
 static uint8_t vfio_std_cap_max_size(PCIDevice *pdev, uint8_t pos)
 {
-    uint8_t tmp, next = 0xff;
+    uint8_t tmp;
+    uint16_t next = PCI_CONFIG_SPACE_SIZE;
 
     for (tmp = pdev->config[PCI_CAPABILITY_LIST]; tmp;
          tmp = pdev->config[tmp + 1]) {
-- 
2.1.0

[Qemu-devel] [PATCH v12 05/15] vfio: add pcie extanded capability support

[Chen Hanxiao]

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

For vfio pcie device, we could expose the extended capability on
PCIE bus. in order to avoid config space broken, 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>
Signed-off-by: Chen Hanxiao <chenhanxiao@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 cae45a0..d7dc8a1 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2522,6 +2522,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);
@@ -2831,16 +2846,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;
+
+    /*
+     * In order to avoid breaking config space, create a copy to
+     * use for parsing 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);
+        if (next == PCI_CONFIG_SPACE_SIZE) {
+            /* Begin the rebuild, we should set the next offset zero. */
+            pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
+        }
+
+        /* Use emulated header pointer to allow dropping extended caps */
+        pci_set_long(vdev->emulated_config_bits + next, 0xffffffff);
+    }
+
+    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)
-- 
2.1.0

[Qemu-devel] [PATCH v12 06/15] aer: impove pcie_aer_init to support vfio device

[Chen Hanxiao]

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

pcie_aer_init was used to emulate an aer capability for pcie device,
but for vfio device, the aer config space size is mutable and is not
always equal to PCI_ERR_SIZEOF(0x48). it depends on where the TLP Prefix
register required, so here we add a size argument.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 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                  | 4 ++--
 include/hw/pci/pcie_aer.h          | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index cce2fdd..4d9cd3f 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -129,7 +129,7 @@ static int ioh3420_initfn(PCIDevice *d)
         goto err_pcie_cap;
     }
     pcie_cap_root_init(d);
-    rc = pcie_aer_init(d, IOH_EP_AER_OFFSET);
+    rc = pcie_aer_init(d, 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 b3a6479..9737041 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -92,7 +92,7 @@ static int xio3130_downstream_initfn(PCIDevice *d)
         goto err_pcie_cap;
     }
     pcie_cap_arifwd_init(d);
-    rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
+    rc = pcie_aer_init(d, 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 eada582..4d7f894 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -81,7 +81,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);
+    rc = pcie_aer_init(d, 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 f1847ac..786f25d 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -94,12 +94,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)
+int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size)
 {
     PCIExpressDevice *exp;
 
     pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
-                        offset, PCI_ERR_SIZEOF);
+                        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 2fb8388..156acb0 100644
--- a/include/hw/pci/pcie_aer.h
+++ b/include/hw/pci/pcie_aer.h
@@ -87,7 +87,7 @@ struct PCIEAERErr {
 
 extern const VMStateDescription vmstate_pcie_aer_log;
 
-int pcie_aer_init(PCIDevice *dev, uint16_t offset);
+int pcie_aer_init(PCIDevice *dev, 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);
-- 
2.1.0

[Qemu-devel] [PATCH v12 07/15] vfio: add aer support for vfio device

[Chen Hanxiao]

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>
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 hw/vfio/pci.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 87 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d7dc8a1..5e24e9d 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -32,6 +32,7 @@
 #include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "qemu-common.h"
 #include "qemu/error-report.h"
 #include "qemu/event_notifier.h"
@@ -160,6 +161,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;
@@ -2846,6 +2849,74 @@ 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;
+    uint8_t *exp_cap = pdev->config + pdev->exp.exp_cap;
+    PCIDevice *dev_iter;
+    uint8_t type;
+    uint32_t severity, errcap;
+    int ret;
+
+    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, pdev->exp.aer_cap + 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);
+    ret = pcie_aer_init(pdev, pos, size);
+    if (ret) {
+        return ret;
+    }
+
+    /* load physical registers */
+    severity = vfio_pci_read_config(pdev,
+                   pdev->exp.aer_cap + PCI_ERR_UNCOR_SEVER, 4);
+    pci_long_test_and_clear_mask(exp_cap + PCI_ERR_UNCOR_SEVER, ~severity);
+
+    return 0;
+
+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;
@@ -2853,6 +2924,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;
 
     /*
      * In order to avoid breaking config space, create a copy to
@@ -2874,7 +2946,19 @@ 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);
+        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;
+        }
+
         if (next == PCI_CONFIG_SPACE_SIZE) {
             /* Begin the rebuild, we should set the next offset zero. */
             pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
@@ -2884,8 +2968,9 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
         pci_set_long(vdev->emulated_config_bits + next, 0xffffffff);
     }
 
+out:
     g_free(config);
-    return 0;
+    return ret;
 }
 
 static int vfio_add_capabilities(VFIOPCIDevice *vdev)
-- 
2.1.0

[Qemu-devel] [PATCH v12 08/15] vfio: add check host bus reset is support or not

[Chen Hanxiao]

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

when init vfio devices done, we should test all the devices supported
aer whether conflict with others. For each one, get the hot reset
info for the affected device list.  For each affected device, all
should attach to the VM and on the same slot. also, we should test
all of the non-AER supporting vfio-pci devices on or below the target
bus to verify they have a reset mechanism.

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

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 5e24e9d..ed4d87e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -32,6 +32,7 @@
 #include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
 #include "hw/pci/pci_bridge.h"
 #include "qemu-common.h"
 #include "qemu/error-report.h"
@@ -2849,6 +2850,133 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
     return 0;
 }
 
+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_check_host_bus_reset(VFIOPCIDevice *vdev)
+{
+    PCIDevice *pdev = &vdev->pdev;
+    struct vfio_pci_hot_reset_info *info = NULL;
+    struct vfio_pci_dependent_device *devices;
+    VFIOGroup *group;
+    int ret, i, k;
+
+    ret = vfio_get_hot_reset_info(vdev, &info);
+    if (ret) {
+        error_report("vfio: Cannot get hot reset info");
+        return ret;
+    }
+
+    /* 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;
+        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) {
+            /* Print all the device associated with that
+             * unfound group_id in vfio_group_list.
+             */
+            for (k = 0; k< info->count; k++) {
+                if (devices[k].group_id == devices[i].group_id)
+                    error_report("vfio: Cannot enable AER for device %s "
+                               "depends on group %d which is not owned.",
+                               vdev->vbasedev.name,
+                               devices[k].group_id);
+            }
+
+            ret = -1;
+            goto out;
+        }
+
+        /* Ensure affected devices for reset in the same slot */
+        QLIST_FOREACH(vbasedev, &group->device_list, next) {
+            if (vbasedev->type != VFIO_DEVICE_TYPE_PCI) {
+                continue;
+            }
+            tmp = container_of(vbasedev, VFIOPCIDevice, vbasedev);
+            if (vfio_pci_host_match(&host, &tmp->host)) {
+                PCIDevice *pci = PCI_DEVICE(tmp);
+
+                if (pci->bus == pdev->bus &&
+                    PCI_SLOT(pci->devfn) == PCI_SLOT(pdev->devfn)) {
+                    found = true;
+                    break;
+                }
+                error_report("vfio: Cannot enable AER for device %s, "
+                             "the dependent device %s is not in the same slot",
+                             vdev->vbasedev.name, tmp->vbasedev.name);
+                ret = -1;
+                goto out;
+            }
+        }
+
+        /* 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);
+            ret = -1;
+            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 whether all of 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_host_bus_reset(vdev)) {
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
+
 static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
                           int pos, uint16_t size)
 {
@@ -2885,6 +3013,14 @@ static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
         dev_iter = pci_bridge_get_device(dev_iter->bus);
     }
 
+    if (DEVICE(vdev)->hotplugged) {
+        /* Make sure this device does not conflict the existing aer topology */
+        ret = vfio_check_devices_host_bus_reset();
+        if (ret) {
+            return ret;
+        }
+    }
+
     errcap = vfio_pci_read_config(pdev, pdev->exp.aer_cap + PCI_ERR_CAP, 4);
     /*
      * The ability to record multiple headers is depending on
@@ -3040,13 +3176,6 @@ static void vfio_pci_post_reset(VFIOPCIDevice *vdev)
     vfio_enable_intx(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;
@@ -3708,6 +3837,20 @@ static void vfio_setup_resetfn(VFIOPCIDevice *vdev)
     }
 }
 
+static void vfio_pci_machine_done_notify(Notifier *notifier, void *unused)
+{
+    int ret;
+
+    ret = vfio_check_devices_host_bus_reset();
+    if (ret) {
+        hw_error("Some host bus did not support reset capability");
+    }
+}
+
+static Notifier machine_notifier = {
+    .notify = vfio_pci_machine_done_notify,
+};
+
 static int vfio_initfn(PCIDevice *pdev)
 {
     VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
@@ -3993,6 +4136,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)
-- 
2.1.0

[Qemu-devel] [PATCH v12 09/15] pci: add bus reset_notifiers callbacks for host bus reset

[Chen Hanxiao]

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

Particularly, For vfio devices, Once need to recovery devices
by bus reset such as AER, we always need to reset the host bus
to recovery the devices under the bus, so we need to add pci bus
callbacks to specify to do host bus reset.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 hw/pci/pci.c             | 16 ++++++++++++++++
 hw/pci/pci_bridge.c      |  6 ++++++
 include/hw/pci/pci.h     |  4 ++++
 include/hw/pci/pci_bus.h |  2 ++
 4 files changed, 28 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 442f822..2784eee 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -75,11 +75,27 @@ static const VMStateDescription vmstate_pcibus = {
     }
 };
 
+void pci_bus_add_reset_notifier(PCIBus *bus, Notifier *notify)
+{
+    notifier_list_add(&bus->reset_notifiers, notify);
+}
+
+void pci_bus_remove_reset_notifier(Notifier *notify)
+{
+    notifier_remove(notify);
+}
+
+void pci_bus_run_reset_notifier(PCIBus *bus, void *opaque)
+{
+    notifier_list_notify(&bus->reset_notifiers, opaque);
+}
+
 static void pci_bus_realize(BusState *qbus, Error **errp)
 {
     PCIBus *bus = PCI_BUS(qbus);
 
     vmstate_register(NULL, -1, &vmstate_pcibus, bus);
+    notifier_list_init(&bus->reset_notifiers);
 }
 
 static void pci_bus_unrealize(BusState *qbus, Error **errp)
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 40c97b1..a8e3f57 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -267,6 +267,12 @@ void pci_bridge_write_config(PCIDevice *d,
 
     newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
     if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
+        /*
+         * Notify all vfio-pci devices under the bus
+         * to do physical bus reset.
+         */
+        pci_for_each_bus(&s->sec_bus,
+                pci_bus_run_reset_notifier, d);
         /* Trigger hot reset on 0->1 transition. */
         qbus_reset_all(&s->sec_bus.qbus);
     }
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 551cb3d..fee04ff 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -7,6 +7,7 @@
 #include "exec/memory.h"
 #include "sysemu/dma.h"
 #include "qapi/error.h"
+#include "qemu/notify.h"
 
 /* PCI includes legacy ISA access.  */
 #include "hw/isa/isa.h"
@@ -381,6 +382,9 @@ void pci_bus_fire_intx_routing_notifier(PCIBus *bus);
 void pci_device_set_intx_routing_notifier(PCIDevice *dev,
                                           PCIINTxRoutingNotifier notifier);
 void pci_device_reset(PCIDevice *dev);
+void pci_bus_add_reset_notifier(PCIBus *bus, Notifier *notify);
+void pci_bus_remove_reset_notifier(Notifier *notify);
+void pci_bus_run_reset_notifier(PCIBus *bus, void *opaque);
 
 PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
                                const char *default_model,
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 403fec6..a529890 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -39,6 +39,8 @@ struct PCIBus {
        Keep a count of the number of devices with raised IRQs.  */
     int nirq;
     int *irq_count;
+
+    NotifierList reset_notifiers;
 };
 
 typedef struct PCIBridgeWindows PCIBridgeWindows;
-- 
2.1.0

[Qemu-devel] [PATCH v12 10/15] vfio: add sec_bus_reset notifier to notify physical bus reset is needed

[Chen Hanxiao]

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

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

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index ed4d87e..bab72a4 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -156,6 +156,7 @@ typedef struct VFIOPCIDevice {
     PCIHostDeviceAddress host;
     EventNotifier err_notifier;
     EventNotifier req_notifier;
+    Notifier sec_bus_reset_notifier;
     int (*resetfn)(struct VFIOPCIDevice *);
     uint32_t features;
 #define VFIO_FEATURE_ENABLE_VGA_BIT 0
@@ -171,6 +172,7 @@ typedef struct VFIOPCIDevice {
     bool req_enabled;
     bool has_flr;
     bool has_pm_reset;
+    bool needs_bus_reset;
     bool rom_read_failed;
 } VFIOPCIDevice;
 
@@ -2977,6 +2979,81 @@ static int vfio_check_devices_host_bus_reset(void)
     return 0;
 }
 
+/* Update all the reset state of the affected devices in VM */
+static void vfio_pci_host_needs_bus_reset(Notifier *n, void *opaque)
+{
+    VFIOPCIDevice *vdev = container_of(n, VFIOPCIDevice, sec_bus_reset_notifier);
+    VFIODevice *vbasedev = &vdev->vbasedev;
+    PCIDevice *parent = opaque;
+    VFIOGroup *group;
+    struct vfio_pci_hot_reset_info *info = NULL;
+    struct vfio_pci_dependent_device *devices;
+    int i, ret;
+
+    ret = vfio_get_hot_reset_info(vdev, &info);
+    if (ret) {
+        goto out;
+    }
+
+    vdev->needs_bus_reset = true;
+    /*
+     * qbus_reset_all always reset the devices from the depth level,
+     * here only need to do the reset for the device encounter the aer,
+     * in order to avoid reset the affected device first below the bus,
+     * we only set the top level devices as needs_reset.
+     */
+    if (parent == pci_bridge_get_device(vdev->pdev.bus)) {
+        vbasedev->needs_reset = true;
+    }
+
+    /* 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;
+
+        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) {
+            goto out;
+        }
+
+        /* update all affected device the bus reset status */
+        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)) {
+                tmp->needs_bus_reset = true;
+                tmp->vbasedev.needs_reset = false;
+                break;
+            }
+        }
+    }
+
+out:
+    g_free(info);
+}
+
 static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
                           int pos, uint16_t size)
 {
@@ -3045,6 +3122,9 @@ static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
                    pdev->exp.aer_cap + PCI_ERR_UNCOR_SEVER, 4);
     pci_long_test_and_clear_mask(exp_cap + PCI_ERR_UNCOR_SEVER, ~severity);
 
+    vdev->sec_bus_reset_notifier.notify = vfio_pci_host_needs_bus_reset;
+    pci_bus_add_reset_notifier(pdev->bus, &vdev->sec_bus_reset_notifier);
+
     return 0;
 
 error:
@@ -4029,6 +4109,9 @@ static void vfio_exitfn(PCIDevice *pdev)
 
     vfio_unregister_req_notifier(vdev);
     vfio_unregister_err_notifier(vdev);
+    if (vdev->features & VFIO_FEATURE_ENABLE_AER) {
+        pci_bus_remove_reset_notifier(&vdev->sec_bus_reset_notifier);
+    }
     pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
     vfio_disable_interrupts(vdev);
     if (vdev->intx.mmap_timer) {
-- 
2.1.0

[Qemu-devel] [PATCH v12 11/15] vfio: modify vfio_pci_hot_reset to support bus reset

[Chen Hanxiao]

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

the vfio_pci_hot_reset differentiate the single and multi in-used
devices for reset. but in multi case, when some dependent devices
are not assigned to VM, the devices can not be recovered by driver.

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

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index bab72a4..c52456b 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3287,6 +3287,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
         PCIHostDeviceAddress host;
         VFIOPCIDevice *tmp;
         VFIODevice *vbasedev_iter;
+        bool found;
 
         host.domain = devices[i].segment;
         host.bus = devices[i].bus;
@@ -3316,6 +3317,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
             goto out;
         }
 
+        found = false;
         /* Prep dependent devices for reset and clear our marker. */
         QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
             if (vbasedev_iter->type != VFIO_DEVICE_TYPE_PCI) {
@@ -3327,12 +3329,23 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
                     ret = -EINVAL;
                     goto out_single;
                 }
+                found = true;
                 vfio_pci_pre_reset(tmp);
                 tmp->vbasedev.needs_reset = false;
                 multi = true;
                 break;
             }
         }
+
+        /*
+         * If the dependent device is not assigned to VM, we should
+         * not call hot bus reset, otherwise, the dependent device
+         * can not be used in the future.
+         */
+        if (!single && !found) {
+            ret = -EINVAL;
+            goto out_single;
+        }
     }
 
     if (!single && !multi) {
-- 
2.1.0

[Qemu-devel] [PATCH v12 12/15] vfio: do hot bus reset when do virtual secondary bus reset

[Chen Hanxiao]

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

when do virtual secondary bus reset, the vfio device under
this bus need to do host bus reset to reset the device.
so add this case.

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

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index c52456b..bd67608 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -4134,6 +4134,27 @@ static void vfio_exitfn(PCIDevice *pdev)
     vfio_unregister_bars(vdev);
 }
 
+static int vfio_pci_is_single_function(VFIOPCIDevice *vdev)
+{
+    struct vfio_pci_hot_reset_info *info = NULL;
+    int ret;
+
+    ret = vfio_get_hot_reset_info(vdev, &info);
+    if (ret) {
+        goto out;
+    }
+
+    if (info->count > 1) {
+        ret = 0;
+        goto out;
+    }
+
+    ret = 1;
+out:
+    g_free(info);
+    return ret;
+}
+
 static void vfio_pci_reset(DeviceState *dev)
 {
     PCIDevice *pdev = DO_UPCAST(PCIDevice, qdev, dev);
@@ -4141,6 +4162,16 @@ static void vfio_pci_reset(DeviceState *dev)
 
     trace_vfio_pci_reset(vdev->vbasedev.name);
 
+    if (vdev->needs_bus_reset) {
+        vdev->needs_bus_reset = false;
+        /* Avoid duplicate bus reset */
+        if (vdev->vbasedev.needs_reset) {
+            vfio_pci_hot_reset(vdev,
+                vfio_pci_is_single_function(vdev) ? true : false);
+        }
+        return;
+    }
+
     vfio_pci_pre_reset(vdev);
 
     if (vdev->resetfn && !vdev->resetfn(vdev)) {
-- 
2.1.0

[Qemu-devel] [PATCH v12 13/15] pcie_aer: expose pcie_aer_msg() interface

[Chen Hanxiao]

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

For vfio device, we need to propagate the aer error to
Guest OS. we use the pcie_aer_msg() to send aer error
to guest.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 hw/pci/pcie_aer.c         | 2 +-
 include/hw/pci/pcie_aer.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 786f25d..7b287cf 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -370,7 +370,7 @@ static void pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg)
  *
  * Walk up the bus tree from the device, propagate the error message.
  */
-static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
+void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
 {
     uint8_t type;
 
diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
index 156acb0..c2ee4e2 100644
--- a/include/hw/pci/pcie_aer.h
+++ b/include/hw/pci/pcie_aer.h
@@ -102,5 +102,6 @@ void pcie_aer_root_write_config(PCIDevice *dev,
 
 /* error injection */
 int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err);
+void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg);
 
 #endif /* QEMU_PCIE_AER_H */
-- 
2.1.0

[Qemu-devel] [PATCH v12 14/15] vfio-pci: pass the aer error to guest

[Chen Hanxiao]

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, the results in the qemu eventfd handler getting
invoked.

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

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

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index bd67608..1e04c1e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3581,18 +3581,51 @@ 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 configration 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);
+
+        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.  "
-- 
2.1.0

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

[Chen Hanxiao]

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

add 'aer' property to let user able to decide whether expose
the aer capability. by default we should disable aer feature,
because it needs configuration restrictions.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@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 1e04c1e..eb69a39 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -4252,6 +4252,8 @@ static Property vfio_pci_dev_properties[] = {
                     VFIO_FEATURE_ENABLE_VGA_BIT, false),
     DEFINE_PROP_BIT("x-req", VFIOPCIDevice, features,
                     VFIO_FEATURE_ENABLE_REQ_BIT, true),
+    DEFINE_PROP_BIT("aer", VFIOPCIDevice, features,
+                    VFIO_FEATURE_ENABLE_AER_BIT, false),
     DEFINE_PROP_INT32("bootindex", VFIOPCIDevice, bootindex, -1),
     DEFINE_PROP_BOOL("x-mmap", VFIOPCIDevice, vbasedev.allow_mmap, true),
     /*
-- 
2.1.0

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

[Alex Williamson]

On Thu, 2015-07-16 at 12:00 +0800, Chen Hanxiao wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> 
> For now, when qemu receives an error from host aer report
> by vfio pci passthough devices, qemu just terminate the guest.
> Usually user want to know what error occurred
> rather than stop the guest.
> 
> This patches add aer capability support for vfio device,
> then pass the error to guest, and let guest driver to recover
> from the error.
> Turning on SERR# for error forwording in bridge control register
> patch in seabios has been merged as commit 32ec3ee.
> 
> notes:
>   this series patches enable aer support single/multi-function,
>   for multi-function, require all the function of the slot assigned to
>   VM and on the same slot.
> 
> Chen Fan (15):
>   vfio: extract vfio_get_hot_reset_info as a single function
>   vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
>   pcie: modify the capability size assert
>   vfio: make the 4 bytes aligned for capability size
>   vfio: add pcie extanded capability support
>   aer: impove pcie_aer_init to support vfio device
>   vfio: add aer support for vfio device
>   vfio: add check host bus reset is support or not
>   pci: add bus reset_notifiers callbacks for host bus reset
>   vfio: add sec_bus_reset notifier to notify physical bus reset is
>     needed
>   vfio: modify vfio_pci_hot_reset to support bus reset
>   vfio: do hot bus reset when do virtual secondary bus reset
>   pcie_aer: expose pcie_aer_msg() interface
>   vfio-pci: pass the aer error to guest
>   vfio: add 'aer' property to expose aercap
> 
>  hw/pci-bridge/ioh3420.c            |   2 +-
>  hw/pci-bridge/xio3130_downstream.c |   2 +-
>  hw/pci-bridge/xio3130_upstream.c   |   2 +-
>  hw/pci/pci.c                       |  16 +
>  hw/pci/pci_bridge.c                |   6 +
>  hw/pci/pcie.c                      |   2 +-
>  hw/pci/pcie_aer.c                  |   6 +-
>  hw/vfio/pci.c                      | 636 +++++++++++++++++++++++++++++++++----
>  include/hw/pci/pci.h               |   4 +
>  include/hw/pci/pci_bus.h           |   2 +
>  include/hw/pci/pcie_aer.h          |   3 +-
>  11 files changed, 609 insertions(+), 72 deletions(-)


This seems to be pretty much the same as v11 where I commented that I
didn't think it was acceptable to have a feature dependent on having all
the functions assigned without supporting hot-add of multi-function
devices.  Can you summarize what's changed here and whether that comment
was addressed.  It would be a courtesy to reviewers to provide at least
a summary changelog with each new version.  Thanks,

Alex

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

[Chen, Hanxiao]

Hi, Alex

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> To: Chen, Hanxiao
> Cc: qemu-devel@nongnu.org; Chen, Fan
> Subject: Re: [Qemu-devel] [PATCH v12 00/15] vfio-pci: pass the aer error to guest
> 
> On Thu, 2015-07-16 at 12:00 +0800, Chen Hanxiao wrote:
> > From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> >
> > For now, when qemu receives an error from host aer report
> > by vfio pci passthough devices, qemu just terminate the guest.
> > Usually user want to know what error occurred
> > rather than stop the guest.
> >
> > This patches add aer capability support for vfio device,
> > then pass the error to guest, and let guest driver to recover
> > from the error.
> > Turning on SERR# for error forwording in bridge control register
> > patch in seabios has been merged as commit 32ec3ee.
> >
> > notes:
> >   this series patches enable aer support single/multi-function,
> >   for multi-function, require all the function of the slot assigned to
> >   VM and on the same slot.
> >
> > Chen Fan (15):
> >   vfio: extract vfio_get_hot_reset_info as a single function
> >   vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
> >   pcie: modify the capability size assert
> >   vfio: make the 4 bytes aligned for capability size
> >   vfio: add pcie extanded capability support
> >   aer: impove pcie_aer_init to support vfio device
> >   vfio: add aer support for vfio device
> >   vfio: add check host bus reset is support or not
> >   pci: add bus reset_notifiers callbacks for host bus reset
> >   vfio: add sec_bus_reset notifier to notify physical bus reset is
> >     needed
> >   vfio: modify vfio_pci_hot_reset to support bus reset
> >   vfio: do hot bus reset when do virtual secondary bus reset
> >   pcie_aer: expose pcie_aer_msg() interface
> >   vfio-pci: pass the aer error to guest
> >   vfio: add 'aer' property to expose aercap
> >
> >  hw/pci-bridge/ioh3420.c            |   2 +-
> >  hw/pci-bridge/xio3130_downstream.c |   2 +-
> >  hw/pci-bridge/xio3130_upstream.c   |   2 +-
> >  hw/pci/pci.c                       |  16 +
> >  hw/pci/pci_bridge.c                |   6 +
> >  hw/pci/pcie.c                      |   2 +-
> >  hw/pci/pcie_aer.c                  |   6 +-
> >  hw/vfio/pci.c                      | 636 +++++++++++++++++++++++++++++++++----
> >  include/hw/pci/pci.h               |   4 +
> >  include/hw/pci/pci_bus.h           |   2 +
> >  include/hw/pci/pcie_aer.h          |   3 +-
> >  11 files changed, 609 insertions(+), 72 deletions(-)
> 
> 
> This seems to be pretty much the same as v11 where I commented that I
> didn't think it was acceptable to have a feature dependent on having all
> the functions assigned without supporting hot-add of multi-function
> devices.  Can you summarize what's changed here and whether that comment
> was addressed.  It would be a courtesy to reviewers to provide at least
> a summary changelog with each new version.  Thanks,
> 

We could hot-unplug all passthrough devices by device_del,
But currently Qemu could not hot-add multi-function pci device.

See TODO in pcie_cap_slot_hotplug_cb:
/* TODO: multifunction hot-plug.
 * Right now, only a device of function = 0 is allowed to be
 * hot plugged/unplugged.
 */
assert(PCI_FUNC(pci_dev->devfn) == 0);

So we had to limit this as a workaround.

Why can't  we add functions one by one to the same slot by device_add?

If we could add functions one by one,
then we can enable aer for the devices once all dependence functions
were added by setting aer as a dynamic property(such as using object_property_add, qom-set)

How do you think?

Regards,
- Chen

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

[Alex Williamson]

On Tue, 2015-07-28 at 07:48 +0000, Chen, Hanxiao wrote:
> Hi, Alex
> 
> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > To: Chen, Hanxiao
> > Cc: qemu-devel@nongnu.org; Chen, Fan
> > Subject: Re: [Qemu-devel] [PATCH v12 00/15] vfio-pci: pass the aer error to guest
> > 
> > On Thu, 2015-07-16 at 12:00 +0800, Chen Hanxiao wrote:
> > > From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> > >
> > > For now, when qemu receives an error from host aer report
> > > by vfio pci passthough devices, qemu just terminate the guest.
> > > Usually user want to know what error occurred
> > > rather than stop the guest.
> > >
> > > This patches add aer capability support for vfio device,
> > > then pass the error to guest, and let guest driver to recover
> > > from the error.
> > > Turning on SERR# for error forwording in bridge control register
> > > patch in seabios has been merged as commit 32ec3ee.
> > >
> > > notes:
> > >   this series patches enable aer support single/multi-function,
> > >   for multi-function, require all the function of the slot assigned to
> > >   VM and on the same slot.
> > >
> > > Chen Fan (15):
> > >   vfio: extract vfio_get_hot_reset_info as a single function
> > >   vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
> > >   pcie: modify the capability size assert
> > >   vfio: make the 4 bytes aligned for capability size
> > >   vfio: add pcie extanded capability support
> > >   aer: impove pcie_aer_init to support vfio device
> > >   vfio: add aer support for vfio device
> > >   vfio: add check host bus reset is support or not
> > >   pci: add bus reset_notifiers callbacks for host bus reset
> > >   vfio: add sec_bus_reset notifier to notify physical bus reset is
> > >     needed
> > >   vfio: modify vfio_pci_hot_reset to support bus reset
> > >   vfio: do hot bus reset when do virtual secondary bus reset
> > >   pcie_aer: expose pcie_aer_msg() interface
> > >   vfio-pci: pass the aer error to guest
> > >   vfio: add 'aer' property to expose aercap
> > >
> > >  hw/pci-bridge/ioh3420.c            |   2 +-
> > >  hw/pci-bridge/xio3130_downstream.c |   2 +-
> > >  hw/pci-bridge/xio3130_upstream.c   |   2 +-
> > >  hw/pci/pci.c                       |  16 +
> > >  hw/pci/pci_bridge.c                |   6 +
> > >  hw/pci/pcie.c                      |   2 +-
> > >  hw/pci/pcie_aer.c                  |   6 +-
> > >  hw/vfio/pci.c                      | 636 +++++++++++++++++++++++++++++++++----
> > >  include/hw/pci/pci.h               |   4 +
> > >  include/hw/pci/pci_bus.h           |   2 +
> > >  include/hw/pci/pcie_aer.h          |   3 +-
> > >  11 files changed, 609 insertions(+), 72 deletions(-)
> > 
> > 
> > This seems to be pretty much the same as v11 where I commented that I
> > didn't think it was acceptable to have a feature dependent on having all
> > the functions assigned without supporting hot-add of multi-function
> > devices.  Can you summarize what's changed here and whether that comment
> > was addressed.  It would be a courtesy to reviewers to provide at least
> > a summary changelog with each new version.  Thanks,
> > 
> 
> We could hot-unplug all passthrough devices by device_del,
> But currently Qemu could not hot-add multi-function pci device.
> 
> See TODO in pcie_cap_slot_hotplug_cb:
> /* TODO: multifunction hot-plug.
>  * Right now, only a device of function = 0 is allowed to be
>  * hot plugged/unplugged.
>  */
> assert(PCI_FUNC(pci_dev->devfn) == 0);
> 
> So we had to limit this as a workaround.
> 
> Why can't  we add functions one by one to the same slot by device_add?
> 
> If we could add functions one by one,
> then we can enable aer for the devices once all dependence functions
> were added by setting aer as a dynamic property(such as using object_property_add, qom-set)

Adding functions individually is not supported by PCI, the specification
indicates that function 0 must always be present and if function 0
reports the multi-function bit set, system software is to scan function
1 through 7.  Therefore any hotplug notification to the slot should do
nothing unless a function zero is present and real hardware doesn't
typically have the ability to spawn new functions, so I wouldn't
necessarily expect additional notifications to a slot to cause system
software to re-scan for new functions.

Also, if we add functions one by one, how do we know we'll ever see the
complete set necessary to support the user requested AER feature?  We
cannot predict that the user will add the needed functions in the
future.

Paolo suggested a possible solution to this in a separate internal
thread, that perhaps we could do multi-function hot-add so long as the
zero function is added last, effectively signaling the closure of the
slot.  Modifications to the PCI-core would be needed such that non-zero
functions could be hot added and config space hidden until the
successful instantiation of the zero function (to prevent unsolicited
discovery and use of the functions prior to exposure).  Perhaps a slot
closure callback would give us the same opportunity we have with
machine_init_done to give a final check of the slot and abort if our aer
requirements are not met.

Multi-function hot-add is not currently supported because nothing
requires it.  If aer is to depend on multi-function, then it's going to
need to bring multi-function hot-add up to a supportable level in order
to be accepted.  Thanks,

Alex