Re: [Qemu-devel] [PATCH v16 08/14] vfio: add check host bus reset is support or not
On 01/18/2016 06:32 PM, Marcel Apfelbaum wrote:
On 01/12/2016 04:43 AM, Cao jin wrote:
From: Chen Fan
Hi,
I think the subject should be rephrased.
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/below the same bus. 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.
Maybe instead of explaining what this patch does you can simply
say what are the requirements:
Something like: Check there are no AER conflicts by making sure the
devices are behind on/below the same bus... (someone with better
English may help :) )
Signed-off-by: Chen Fan
---
hw/vfio/pci.c | 238
--
hw/vfio/pci.h | 1 +
2 files changed, 232 insertions(+), 7 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 38b0aa5..16ab0e3 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1832,6 +1832,218 @@ static int vfio_add_std_cap(VFIOPCIDevice
*vdev, uint8_t pos)
return 0;
}
+static bool vfio_pci_host_slot_match(PCIHostDeviceAddress *host1,
+ PCIHostDeviceAddress *host2)
+{
+return (host1->domain == host2->domain && host1->bus ==
host2->bus &&
+host1->slot == host2->slot);
+}
+
+static bool vfio_pci_host_match(PCIHostDeviceAddress *host1,
+PCIHostDeviceAddress *host2)
+{
+return (vfio_pci_host_slot_match(host1, host2) &&
+host1->function == host2->function);
+}
+
+struct VFIODeviceFind {
+PCIDevice *pdev;
+bool found;
+};
+
+static void vfio_check_device_noreset(PCIBus *bus, PCIDevice *pdev,
+ void *opaque)
+{
+DeviceState *dev = DEVICE(pdev);
+DeviceClass *dc = DEVICE_GET_CLASS(dev);
+VFIOPCIDevice *vdev;
+struct VFIODeviceFind *find = opaque;
+
+if (find->found) {
+return;
+}
+
+if (!object_dynamic_cast(OBJECT(dev), "vfio-pci")) {
+if (!dc->reset) {
+goto found;
+}
+return;
+}
+vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+if (!(vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+!vdev->vbasedev.reset_works) {
+goto found;
+}
+
+return;
+found:
+find->pdev = pdev;
+find->found = true;
+}
+
+static void device_find(PCIBus *bus, PCIDevice *pdev, void *opaque)
+{
+struct VFIODeviceFind *find = opaque;
+
+if (find->found) {
+return;
+}
+
+if (pdev == find->pdev) {
+find->found = true;
+}
+}
+
+static int vfio_check_host_bus_reset(VFIOPCIDevice *vdev)
+{
+PCIBus *bus = vdev->pdev.bus;
+struct vfio_pci_hot_reset_info *info = NULL;
+struct vfio_pci_dependent_device *devices;
+VFIOGroup *group;
+struct VFIODeviceFind find;
+int ret, i;
+
+ret = vfio_get_hot_reset_info(vdev, &info);
+if (ret) {
+error_report("vfio: Cannot enable AER for device %s,"
+ " device does not support hot reset.",
+ vdev->vbasedev.name);
+goto out;
"return" is enough here in case of an error, info is released inside
vfio_get_hot_reset_info
indeed.
+}
+
+/* List all affected devices by bus reset */
+devices = &info->devices[0];
+
+/* Verify that we have all the groups required */
+for (i = 0; i < info->count; i++) {
+PCIHostDeviceAddress host;
+VFIOPCIDevice *tmp;
+VFIODevice *vbasedev_iter;
+bool found = false;
+
+host.domain = devices[i].segment;
+host.bus = devices[i].bus;
+host.slot = PCI_SLOT(devices[i].devfn);
+host.function = PCI_FUNC(devices[i].devfn);
+
+/* Skip the current device */
+if (vfio_pci_host_match(&host, &vdev->host)) {
+continue;
+}
+
+/* Ensure we own the group of the affected device */
+QLIST_FOREACH(group, &vfio_group_list, next) {
+if (group->groupid == devices[i].group_id) {
+break;
+}
+}
+
+if (!group) {
+error_report("vfio: Cannot enable AER for device %s, "
+ "depends on group %d which is not owned.",
+ vdev->vbasedev.name, devices[i].group_id);
+ret = -1;
You can use error codes on return, or you can init ret to -1 at
declaration,
and delete some code lines.
+goto out;
+}
+
+/* Ensure affected devices for reset on/blow the bus */
I think you meant below, not blow.
+QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
+if (vbasedev_iter->type != VFIO_DEVICE_TYPE_PCI) {
+continue;
+}
+tmp = container_of(vbasedev_iter, VFIOPCIDevice, vbasedev);
+
Re: [Qemu-devel] [PATCH v16 08/14] vfio: add check host bus reset is support or not
On 01/12/2016 04:43 AM, Cao jin wrote:
From: Chen Fan
Hi,
I think the subject should be rephrased.
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/below the same bus. 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.
Maybe instead of explaining what this patch does you can simply
say what are the requirements:
Something like: Check there are no AER conflicts by making sure the
devices are behind on/below the same bus... (someone with better English may
help :) )
Signed-off-by: Chen Fan
---
hw/vfio/pci.c | 238 --
hw/vfio/pci.h | 1 +
2 files changed, 232 insertions(+), 7 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 38b0aa5..16ab0e3 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1832,6 +1832,218 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev,
uint8_t pos)
return 0;
}
+static bool vfio_pci_host_slot_match(PCIHostDeviceAddress *host1,
+ PCIHostDeviceAddress *host2)
+{
+return (host1->domain == host2->domain && host1->bus == host2->bus &&
+host1->slot == host2->slot);
+}
+
+static bool vfio_pci_host_match(PCIHostDeviceAddress *host1,
+PCIHostDeviceAddress *host2)
+{
+return (vfio_pci_host_slot_match(host1, host2) &&
+host1->function == host2->function);
+}
+
+struct VFIODeviceFind {
+PCIDevice *pdev;
+bool found;
+};
+
+static void vfio_check_device_noreset(PCIBus *bus, PCIDevice *pdev,
+ void *opaque)
+{
+DeviceState *dev = DEVICE(pdev);
+DeviceClass *dc = DEVICE_GET_CLASS(dev);
+VFIOPCIDevice *vdev;
+struct VFIODeviceFind *find = opaque;
+
+if (find->found) {
+return;
+}
+
+if (!object_dynamic_cast(OBJECT(dev), "vfio-pci")) {
+if (!dc->reset) {
+goto found;
+}
+return;
+}
+vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
+if (!(vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+!vdev->vbasedev.reset_works) {
+goto found;
+}
+
+return;
+found:
+find->pdev = pdev;
+find->found = true;
+}
+
+static void device_find(PCIBus *bus, PCIDevice *pdev, void *opaque)
+{
+struct VFIODeviceFind *find = opaque;
+
+if (find->found) {
+return;
+}
+
+if (pdev == find->pdev) {
+find->found = true;
+}
+}
+
+static int vfio_check_host_bus_reset(VFIOPCIDevice *vdev)
+{
+PCIBus *bus = vdev->pdev.bus;
+struct vfio_pci_hot_reset_info *info = NULL;
+struct vfio_pci_dependent_device *devices;
+VFIOGroup *group;
+struct VFIODeviceFind find;
+int ret, i;
+
+ret = vfio_get_hot_reset_info(vdev, &info);
+if (ret) {
+error_report("vfio: Cannot enable AER for device %s,"
+ " device does not support hot reset.",
+ vdev->vbasedev.name);
+goto out;
"return" is enough here in case of an error, info is released inside
vfio_get_hot_reset_info
+}
+
+/* List all affected devices by bus reset */
+devices = &info->devices[0];
+
+/* Verify that we have all the groups required */
+for (i = 0; i < info->count; i++) {
+PCIHostDeviceAddress host;
+VFIOPCIDevice *tmp;
+VFIODevice *vbasedev_iter;
+bool found = false;
+
+host.domain = devices[i].segment;
+host.bus = devices[i].bus;
+host.slot = PCI_SLOT(devices[i].devfn);
+host.function = PCI_FUNC(devices[i].devfn);
+
+/* Skip the current device */
+if (vfio_pci_host_match(&host, &vdev->host)) {
+continue;
+}
+
+/* Ensure we own the group of the affected device */
+QLIST_FOREACH(group, &vfio_group_list, next) {
+if (group->groupid == devices[i].group_id) {
+break;
+}
+}
+
+if (!group) {
+error_report("vfio: Cannot enable AER for device %s, "
+ "depends on group %d which is not owned.",
+ vdev->vbasedev.name, devices[i].group_id);
+ret = -1;
You can use error codes on return, or you can init ret to -1 at declaration,
and delete some code lines.
+goto out;
+}
+
+/* Ensure affected devices for reset on/blow the bus */
I think you meant below, not blow.
+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)) {
+
