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

[Chen Fan]

On 05/20/2015 03:34 AM, Alex Williamson wrote:
> 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.
Due to we may have the arbitrary configuration manner after hotplug
/unplug vfio-pci devices, the enabled aer feature probably became to
disabled. so shall we consider not to expose the aer feature to user,
and enable/disable it automatically?

Thanks,
Chen



> > +        }
> > +        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;
>
>
> .