Il 17/12/2012 17:48, Michael S. Tsirkin ha scritto:
> On Mon, Dec 17, 2012 at 05:24:48PM +0100, Paolo Bonzini wrote:
>> virtio-pci devices do not perform a full reset when zero is written
>> to the status byte. While PCI-specific status is initialized, the
>> reset does not propagate down the qdev bus hierarchy. Because of
>> this, a virtio reset does not cancel in-flight I/O for virtio-scsi
>> (where the cancellation is handled automatically by the SCSI
>> devices underneath virtio-scsi-pci).
>>
>> Reported-by: Bryan Venteicher <bry...@daemoninthecloset.org>
>> Cc: Michael S. Tsirkin <m...@redhat.com>
>> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>
> I would still prefer this logic to reside in virtio.c instead of being
> duplicated in each bus.
> My idea was to simply call qdev_reset_all on the binding from virtio.c
> but other ideas wellcome.
I think you're confusing "in the common superclass of all virtio
transports" vs "in the common superclass of all virtio devices".
virtio.c only implements a common superclass of all virtio devices; in
fact, there is no common superclass of all virtio transports, and it is
not possible without multiple inheritance or stuff like traits (you're
already inheriting from PCIDevice for virtio-*-pci).
Such common superclass, if it existed, would abstract stuff like "write
zero to the status register" and would call qdev_reset_all. But again,
we don't have this concept.
Paolo
>
>
>> ---
>> hw/virtio-pci.c | 28 ++++++++++------------------
>> 1 file changed, 10 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
>> index a7c75fe..2cf5282 100644
>> --- a/hw/virtio-pci.c
>> +++ b/hw/virtio-pci.c
>> @@ -268,12 +268,10 @@ 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_reset(proxy->vdev);
>> - virtio_pci_stop_ioeventfd(proxy);
>> - msix_unuse_all_vectors(&proxy->pci_dev);
>> - }
>> - else
>> + qdev_reset_all(&proxy->pci_dev.qdev);
>> + } else {
>> virtio_queue_set_addr(vdev, vdev->queue_sel, pa);
>> + }
>> break;
>> case VIRTIO_PCI_QUEUE_SEL:
>> if (val < VIRTIO_PCI_QUEUE_MAX)
>> @@ -285,22 +283,16 @@ static void virtio_ioport_write(void *opaque, uint32_t
>> addr, uint32_t val)
>> }
>> break;
>> case VIRTIO_PCI_STATUS:
>> - if (vdev->status == 0) {
>> - virtio_reset(proxy->vdev);
>> - }
>> -
>> - if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
>> - virtio_pci_stop_ioeventfd(proxy);
>> - }
>> -
>> virtio_set_status(vdev, val & 0xFF);
>>
>> - if (val & VIRTIO_CONFIG_S_DRIVER_OK) {
>> - virtio_pci_start_ioeventfd(proxy);
>> - }
>> -
>> if (vdev->status == 0) {
>> - msix_unuse_all_vectors(&proxy->pci_dev);
>> + qdev_reset_all(&proxy->pci_dev.qdev);
>> + } else {
>> + if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
>> + virtio_pci_stop_ioeventfd(proxy);
>> + } else {
>> + virtio_pci_start_ioeventfd(proxy);
>> + }
>> }
>>
>> /* Linux before 2.6.34 sets the device as OK without enabling
>> --
>> 1.8.0.2
>>
