On Tue, 2015-05-19 at 12:42 +0800, Chen Fan wrote:
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.
But the previous patch already enabled it, so a bisect might get it
enabled then disabled.
Signed-off-by: Chen Fan <address@hidden>
---
hw/i386/pc_q35.c | 4 +++
hw/vfio/pci.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/compat.h | 8 +++++
3 files changed, 96 insertions(+)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index e67f2de..94a3a1c 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -439,6 +439,10 @@ static QEMUMachine pc_q35_machine_v2_3 = {
PC_Q35_2_3_MACHINE_OPTIONS,
.name = "pc-q35-2.3",
.init = pc_q35_init_2_3,
+ .compat_props = (GlobalProperty[]) {
+ HW_COMPAT_2_3,
+ { /* end of list */ }
+ },
};
#define PC_Q35_2_2_MACHINE_OPTIONS PC_Q35_2_3_MACHINE_OPTIONS
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index f2fc5d6..b4b9495 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -42,6 +42,7 @@
#include "trace.h"
#include "hw/vfio/vfio.h"
#include "hw/vfio/vfio-common.h"
+#include "hw/pci/pci_bridge.h"
struct VFIOPCIDevice;
@@ -161,6 +162,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;
@@ -2821,10 +2824,31 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev,
uint8_t pos)
static int vfio_setup_aer(VFIOPCIDevice *vdev, int pos, uint16_t size)
{
PCIDevice *pdev = &vdev->pdev;
+ PCIDevice *dev_iter;
uint8_t *exp_cap = pdev->config + pdev->exp.exp_cap;
uint32_t severity, errcap;
+ uint8_t type;
int ret;
+ if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) {
+ return 0;
+ }
+
+ dev_iter = pci_bridge_get_device(pdev->bus);
+ if (!dev_iter) {
So this is testing that it can't be on the root complex endpoint.
+ 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;
+ }
And this tests that we have PCIe devices up to the root complex, but do
do we need to check whether they support aer?
+ 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
@@ -2850,6 +2874,11 @@ static int vfio_setup_aer(VFIOPCIDevice *vdev, int pos,
uint16_t size)
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 "
+ "do not support signal AER", vdev->vbasedev.name);
+ return -1;
}
static int vfio_add_ext_cap(VFIOPCIDevice *vdev, const uint8_t *config)
@@ -3733,9 +3762,52 @@ static void vfio_check_host_bus_reset(VFIODevice
*vbasedev)
vdev->has_bus_reset = true;
out:
+ if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+ !vdev->has_bus_reset) {
+ error_report("vfio: Cannot enable AER for device %s,"
+ "which is not support host bus reset.",
+ vdev->vbasedev.name);
+ exit(1);
+ }
And this is done via the machine init callback, so again we're not doing
anything to support hotplug.
g_free(info);
}
+static int vfio_pci_validate_aer_devices(DeviceState *dev,
+ void *opaque)
+{
+ VFIOPCIDevice *vdev;
+
+ if (!object_dynamic_cast(OBJECT(dev), "vfio-pci")) {
+ return 0;
+ }
+
+ vdev = DO_UPCAST(VFIOPCIDevice, pdev, PCI_DEVICE(dev));
+ if (!vdev->has_bus_reset) {
+ error_report("vfio: device %s is not support host bus reset,"
+ "but on the same bus has vfio device enable AER.",
+ vdev->vbasedev.name);
+ exit(1);
+ }
+ return 0;
+}
+
+static void vfio_check_virtual_bus_reset(VFIODevice *vbasedev)
+{
+ VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
+
+ if (!(vdev->features & VFIO_FEATURE_ENABLE_AER)) {
+ return;
+ }
+
+ /*
+ * Verify that we have all the vfio devices support host bus reset
+ * on the bus
+ */
+ qbus_walk_children(BUS(vdev->pdev.bus), NULL, NULL,
+ vfio_pci_validate_aer_devices,
+ NULL, NULL);
+}
+
static void vfio_pci_machine_done_notify(Notifier *notifier, void *unused)
{
VFIOGroup *group;
@@ -3746,6 +3818,16 @@ static void vfio_pci_machine_done_notify(Notifier
*notifier, void *unused)
vfio_check_host_bus_reset(vbasedev);
}
}
+
+ /*
+ * All devices need support host bus reset on each virtual bus
+ * if one device enable AER.
+ */
+ QLIST_FOREACH(group, &vfio_group_list, next) {
+ QLIST_FOREACH(vbasedev, &group->device_list, next) {
+ vfio_check_virtual_bus_reset(vbasedev);
+ }
+ }
I'm not sure I see how this call path can catch anything missed by
vfio_check_host_bus_reset().
}
static Notifier machine_notifier = {
@@ -4004,6 +4086,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),
/*
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 313682a..f722919 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,7 +1,15 @@
#ifndef HW_COMPAT_H
#define HW_COMPAT_H
+#define HW_COMPAT_2_3 \
+ {\
+ .driver = "vfio-pci",\
+ .property = "x-aer",\
It's defined as "aer" above, not "x-aer". I'm not sure why we need this
though if the default value is off anyway.
+ .value = "off",\
+ }
+
#define HW_COMPAT_2_1 \
+ HW_COMPAT_2_3, \
{\
.driver = "intel-hda",\
.property = "old_msi_addr",\
.