Re: [Qemu-devel] [RFC v8.1 10/13] vfio: do hot bus reset when do virtual secondary bus reset

[Alex Williamson <alex.williamson@redhat.com>]

On Wed, 2015-05-27 at 10:46 +0800, Chen Fan wrote:
> 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>
> ---
>  hw/vfio/pci.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 75 insertions(+)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index a8c5988..b05ccdf 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3803,6 +3803,64 @@ out:
>      g_free(info);
>  }
>  
> +/* Check when device reset the affected devices is single or multi,
> + * because there was differentiate the hot reset of mulitple in-use
> + * devices and hot reset of a single in-use device. */
> +static int vfio_pci_affect_devices_is_multi(VFIOPCIDevice *vdev)
> +{
> +    VFIOGroup *group;
> +    struct vfio_pci_hot_reset_info *info = NULL;
> +    struct vfio_pci_dependent_device *devices;
> +    int ret, i, count = 0;
> +
> +    ret = vfio_get_hot_reset_info(vdev, &info);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    devices = &info->devices[0];
> +
> +    /* Verify that we have all the groups required */
> +    for (i = 0; i < info->count; i++) {
> +        PCIHostDeviceAddress host;
> +        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) {
> +            ret = -1;
> +            goto out;
> +        }
> +
> +        QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
> +            if (vbasedev_iter->type != VFIO_DEVICE_TYPE_PCI) {
> +                continue;
> +            }
> +            count++;
> +        }
> +    }
> +
> +    ret = count;
> +out:
> +    g_free(info);
> +    return ret;
> +}

This basically does almost all the work of the hot reset function just
to figure out whether it's a single or multi-reset to call the hot reset
function with the right parameter.  Why not just do a hot reset?

> +
>  static void vfio_pci_machine_done_notify(Notifier *notifier, void *unused)
>  {
>      VFIOGroup *group;
> @@ -4039,6 +4097,23 @@ static void vfio_pci_reset(DeviceState *dev)
>  
>      trace_vfio_pci_reset(vdev->vbasedev.name);
>  
> +    if (vdev->needs_bus_reset) {
> +        vdev->needs_bus_reset = false;

A comment would be nice to signal that this secondary test avoids
duplicate bus resets.

> +        if (vdev->vbasedev.needs_reset) {
> +            int ret;
> +
> +            ret = vfio_pci_affect_devices_is_multi(vdev);
> +            if (ret < 0) {
> +                error_report("vfio: Cannot reset device %04x:%02x:%02x.%x, "
> +                             "get hot reset info failed.", vdev->host.domain,
> +                             vdev->host.bus, vdev->host.slot, vdev->host.function);
> +            } else {
> +                vfio_pci_hot_reset(vdev, ret ? true : false);
> +            }
> +        }
> +        return;
> +    }
> +
>      vfio_pci_pre_reset(vdev);
>  
>      if (vdev->resetfn && !vdev->resetfn(vdev)) {