| Message ID | 1457708548-14093-17-git-send-email-mst@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/11/16 16:08, Michael S. Tsirkin wrote: > From: Gerd Hoffmann <kraxel@redhat.com> > > 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 <kraxel@redhat.com> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > Reviewed-by: Fam Zheng <famz@redhat.com> > Tested-by: Fam Zheng <famz@redhat.com> > --- > 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. Thanks Laszlo
On 03/14/16 02:36, Laszlo Ersek wrote: > On 03/11/16 16:08, Michael S. Tsirkin wrote: >> From: Gerd Hoffmann <kraxel@redhat.com> >> >> 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 <kraxel@redhat.com> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> Reviewed-by: Fam Zheng <famz@redhat.com> >> Tested-by: Fam Zheng <famz@redhat.com> >> --- >> 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
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;