Re: [Qemu-devel] [PULL 16/53] virtio-pci: call pci reset variant when guest requests reset.

[Laszlo Ersek]

On 03/14/16 02:36, Laszlo Ersek wrote:
> On 03/11/16 16:08, Michael S. Tsirkin wrote:
>> From: Gerd Hoffmann <address@hidden>
>>
>> Actually fixes linux not finding virtio 1.0 device virtqueues after
>> reboot.  Which is new I think, any chance linux kernel virtio code
>> became more strict in 4.3?
>>
>> Signed-off-by: Gerd Hoffmann <address@hidden>
>> Reviewed-by: Michael S. Tsirkin <address@hidden>
>> Signed-off-by: Michael S. Tsirkin <address@hidden>
>> Reviewed-by: Fam Zheng <address@hidden>
>> Tested-by: Fam Zheng <address@hidden>
>> ---
>>  hw/virtio/virtio-pci.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index 440776c..0dadb66 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -47,6 +47,7 @@
>>  
>>  static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
>>                                 VirtIOPCIProxy *dev);
>> +static void virtio_pci_reset(DeviceState *qdev);
>>  
>>  /* virtio device */
>>  /* DeviceState to VirtIOPCIProxy. For use off data-path. TODO: use QOM. */
>> @@ -404,9 +405,7 @@ static void virtio_ioport_write(void *opaque, uint32_t 
>> addr, uint32_t val)
>>      case VIRTIO_PCI_QUEUE_PFN:
>>          pa = (hwaddr)val << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
>>          if (pa == 0) {
>> -            virtio_pci_stop_ioeventfd(proxy);
>> -            virtio_reset(vdev);
>> -            msix_unuse_all_vectors(&proxy->pci_dev);
>> +            virtio_pci_reset(DEVICE(proxy));
>>          }
>>          else
>>              virtio_queue_set_addr(vdev, vdev->queue_sel, pa);
>> @@ -432,8 +431,7 @@ static void virtio_ioport_write(void *opaque, uint32_t 
>> addr, uint32_t val)
>>          }
>>  
>>          if (vdev->status == 0) {
>> -            virtio_reset(vdev);
>> -            msix_unuse_all_vectors(&proxy->pci_dev);
>> +            virtio_pci_reset(DEVICE(proxy));
>>          }
>>  
>>          /* Linux before 2.6.34 drives the device without enabling
>> @@ -1353,8 +1351,7 @@ static void virtio_pci_common_write(void *opaque, 
>> hwaddr addr,
>>          }
>>  
>>          if (vdev->status == 0) {
>> -            virtio_reset(vdev);
>> -            msix_unuse_all_vectors(&proxy->pci_dev);
>> +            virtio_pci_reset(DEVICE(proxy));
>>          }
>>  
>>          break;
>>
> 
> I spent about two days hunting this, and was just about to send a patch.
> Nice to see you've already gotten to it. :)
> 
> The specific problem was that the queue_enable field was not cleared
> (for all possible queues of the device) on device reset, only on system
> reset.

... If you wonder, it is not only needed for after reboot, but also for
when OVMF hands off the devices to the kernel (but resets them first in
the appropriate ExitBootServices() callbacks). It took me so long
because when you work on new guest driver code and things break, you
don't immediately suspect the host! :) Oh the hours I spent
instrumenting the guest kernel. Sigh. :)

Thanks
Laszlo