Re: [Qemu-devel] [RFC v7 05/11] vfio: do hot bus reset when do virtual secondary bus reset

[Alex Williamson]

On Tue, 2015-05-19 at 12:42 +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 <address@hidden>
> ---
>  hw/vfio/pci.c                 | 12 +++++++++++-
>  include/hw/vfio/vfio-common.h |  1 -
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index ff639db..cc169ea 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -169,6 +169,7 @@ typedef struct VFIOPCIDevice {
>      bool has_flr;
>      bool has_pm_reset;
>      bool has_bus_reset;
> +    bool needs_bus_reset;
>      bool rom_read_failed;
>  } VFIOPCIDevice;
>  
> @@ -3531,7 +3532,8 @@ 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;
>  
> -    vbasedev->needs_bus_reset = true;
> +    vdev->needs_bus_reset = true;
> +    vbasedev->needs_reset = true;

The above should have happened in patch 03.

>  }
>  
>  static void vfio_check_host_bus_reset(VFIODevice *vbasedev)
> @@ -3807,6 +3809,14 @@ static void vfio_pci_reset(DeviceState *dev)
>  
>      trace_vfio_pci_reset(vdev->vbasedev.name);
>  
> +    if (vdev->has_bus_reset && vdev->needs_bus_reset) {
> +        vdev->needs_bus_reset = false;
> +        if (vdev->vbasedev.needs_reset) {
> +            vfio_pci_hot_reset(vdev, false);

So we're not even going to check to see whether that worked?  How can we
know that the configuration is still the same as after machine init?  An
affected device could have been hot-unplugged an now in use by the host
(in which case this function would fail).  An affected device could have
been hot unplugged and added back to the VM under a different bus, now
we have an arbitrary second device affected by the bus reset from the VM
perspective.

> +        }
> +        return;
> +    }
> +
>      vfio_pci_pre_reset(vdev);
>  
>      if (vdev->resetfn && !vdev->resetfn(vdev)) {
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 5f5691a..0d1fb80 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -102,7 +102,6 @@ typedef struct VFIODevice {
>      bool reset_works;
>      bool needs_reset;
>      bool allow_mmap;
> -    bool needs_bus_reset;
>      VFIODeviceOps *ops;
>      unsigned int num_irqs;
>      unsigned int num_regions;