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

[Chen Fan <chen.fan.fnst@cn.fujitsu.com>]

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 <chen.fan.fnst@cn.fujitsu.com>
>> ---
>>   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;
>
>
> .
>