Re: [Qemu-devel] [PATCH v16 07/14] vfio: add aer support for vfio device

[Chen Fan]

On 01/18/2016 05:12 PM, Marcel Apfelbaum wrote:

On 01/12/2016 04:43 AM, Cao jin wrote:

From: Chen Fan <chen.fan.f...@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.f...@cn.fujitsu.com>
---

hw/vfio/pci.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---

  hw/vfio/pci.h |  3 +++
  2 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 64b0867..38b0aa5 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c

@@ -1832,6 +1832,62 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)

      return 0;
  }

+static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
+                          int pos, uint16_t size)
+{
+    PCIDevice *pdev = &vdev->pdev;
+    PCIDevice *dev_iter;
+    uint8_t type;
+    uint32_t errcap;
+
+    if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) {
+        pcie_add_capability(pdev, PCI_EXT_CAP_ID_ERR,
+                            cap_ver, pos, size);
+        return 0;
+    }
+
+    dev_iter = pci_bridge_get_device(pdev->bus);
+    if (!dev_iter) {
+        goto error;
+    }
+
+    while (dev_iter) {
+        type = pcie_cap_get_type(dev_iter);
+        if ((type != PCI_EXP_TYPE_ROOT_PORT &&
+             type != PCI_EXP_TYPE_UPSTREAM &&
+             type != PCI_EXP_TYPE_DOWNSTREAM)) {
+            goto error;
+        }
+
+        if (!dev_iter->exp.aer_cap) {
+            goto error;
+        }
+
+        dev_iter = pci_bridge_get_device(dev_iter->bus);
+    }
+
+    errcap = vfio_pci_read_config(pdev, pos + PCI_ERR_CAP, 4);
+    /*
+     * The ability to record multiple headers is depending on
+     * the state of the Multiple Header Recording Capable bit and
+     * enabled by the Multiple Header Recording Enable bit.
+     */
+    if ((errcap & PCI_ERR_CAP_MHRC) &&
+        (errcap & PCI_ERR_CAP_MHRE)) {
+        pdev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
+    } else {
+        pdev->exp.aer_log.log_max = 0;
+    }
+
+    pcie_cap_deverr_init(pdev);
+    return pcie_aer_init(pdev, pos, size);
+
+error:

+ error_report("vfio: Unable to enable AER for device %s, parent bus " + "does not support AER signaling", vdev->vbasedev.name);

+    return -1;
+}
+
  static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
  {
      PCIDevice *pdev = &vdev->pdev;
@@ -1839,6 +1895,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
@@ -1860,16 +1917,29 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
           */
          size = vfio_ext_cap_max_size(config, next);

-        pcie_add_capability(pdev, cap_id, cap_ver, next, size);

- pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));

+        switch (cap_id) {
+        case PCI_EXT_CAP_ID_ERR:
+            ret = vfio_setup_aer(vdev, cap_ver, next, size);
+            break;
+        default:
+            pcie_add_capability(pdev, cap_id, cap_ver, next, size);
+            break;
+        }
+
+        if (ret) {
+            goto out;
+        }
+

+ pci_set_long(pdev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));

/* Use emulated next pointer to allow dropping extended caps */

pci_long_test_and_set_mask(vdev->emulated_config_bits + next,
                                     PCI_EXT_CAP_NEXT_MASK);
      }

+out:
      g_free(config);
-    return 0;
+    return ret;
  }

  static int vfio_add_capabilities(VFIOPCIDevice *vdev)
@@ -2624,6 +2694,11 @@ static int vfio_initfn(PCIDevice *pdev)
          goto out_teardown;
      }

+    if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+        !pdev->exp.aer_cap) {

Hi,

I think we need an error_report here, otherwise the init
will fail without knowing the reason.

maybe we need to exclude this case. if the device hasn't the aer cap,
the ENABLE_AER would be not affected.

Thanks,
Chen

Thanks,
Marcel

+        goto out_teardown;
+    }
+
      /* QEMU emulates all of MSI & MSIX */
      if (pdev->cap_present & QEMU_PCI_CAP_MSIX) {
          memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff,
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index f004d52..48c1f69 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"
@@ -127,6 +128,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;

.