[PATCH 1/2] init_virtio_blk, init_virtio_scsi: reset HBA on error

[Eric Northup]

Extend commit 5f2d17d35b2339526f3b3d580b279ea78e406a25: reset on all error
paths, and also for virtio_blk not just virtio_scsi.

Signed-off-by: Eric Northup 
---
 src/hw/virtio-blk.c  | 1 +
 src/hw/virtio-scsi.c | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c
index 7b22bf5..e2dbd3c 100644
--- a/src/hw/virtio-blk.c
+++ b/src/hw/virtio-blk.c
@@ -153,6 +153,7 @@ init_virtio_blk(struct pci_device *pci)
     return;

fail:
+    vp_reset(ioaddr);
     free(vdrive->vq);
     free(vdrive);
 }
diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c
index 48fb3e1..8f96687 100644
--- a/src/hw/virtio-scsi.c
+++ b/src/hw/virtio-scsi.c
@@ -158,14 +158,13 @@ init_virtio_scsi(struct pci_device *pci)
     for (tot = 0, i = 0; i < 256; i++)
         tot += virtio_scsi_scan_target(pci, ioaddr, vq, i);

-    if (!tot) {
-        vp_reset(ioaddr);
+    if (!tot)
         goto fail;
-    }

return;

fail:
+    vp_reset(ioaddr);
     free(vq);
 }

[PATCH 2/2] vp_init_simple: enable PCI bus-mastering before relying on DMA.

[Eric Northup]

An analogous change was made in the LSI scsi driver, commit
7d052575258ad2fc487ca3f9a6b62eff1b767900. Qemu works around guests that don't
correctly enable PCI bus mastering before using virtio queues' DMA (search for
VIRTIO_PCI_BUG_BUS_MASTER / VIRTIO_PCI_FLAG_BUS_MASTER_BUG ), but it'd be
better to be correct.

Signed-off-by: Eric Northup 
---
 src/hw/virtio-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c
index a382504..b9b3ab1 100644
--- a/src/hw/virtio-pci.c
+++ b/src/hw/virtio-pci.c
@@ -90,6 +90,7 @@ u16 vp_init_simple(u16 bdf)
         PCI_BASE_ADDRESS_IO_MASK;

vp_reset(ioaddr);
+    pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER);
     vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
                   VIRTIO_CONFIG_S_DRIVER );
     return ioaddr;

[PATCH 1/2] init_virtio_blk, init_virtio_scsi: reset HBA on error

[Laszlo Ersek]

comments below

On 03/12/14 21:42, Eric Northup wrote:
> Extend commit 5f2d17d35b2339526f3b3d580b279ea78e406a25: reset on all error
> paths, and also for virtio_blk not just virtio_scsi.
> Signed-off-by: Eric Northup
>
> src/hw/virtio-blk.c  | 1 +
>  src/hw/virtio-scsi.c | 5 ++---
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c
> index 7b22bf5..e2dbd3c 100644
> --- a/src/hw/virtio-blk.c
> +++ b/src/hw/virtio-blk.c
> @@ -153,6 +153,7 @@ init_virtio_blk(struct pci_device *pci)
>      return;
>
> fail:
> vp_reset(ioaddr);
> free(vdrive->vq);
> free(vdrive);
>
> }

Seems correct and justified. The first jump to the "fail" label is after
vp_init_simple(), which means both that "ioaddr" is valid, and that
we've changed the device status to (ACK | DRIVER).
> diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c
> index 48fb3e1..8f96687 100644
> --- a/src/hw/virtio-scsi.c
> +++ b/src/hw/virtio-scsi.c
> @@ -158,14 +158,13 @@ init_virtio_scsi(struct pci_device *pci)
>      for (tot = 0, i = 0; i < 256; i++)
>          tot += virtio_scsi_scan_target(pci, ioaddr, vq, i);
> if (!tot) {
>    vp_reset(ioaddr);
> if (!tot)
>     goto fail;
>
> }
>
> return;
>
> fail:
> vp_reset(ioaddr);
> free(vq);
>
> }

Same here. I guess I wanted to be surgical in my patch. (Good excuse,
isn't it? :))

Reviewed-by: Laszlo Ersek

Thanks,
Laszlo

[PATCH 1/2] init_virtio_blk, init_virtio_scsi: reset HBA on error

[Kevin O'Connor]

On Wed, Mar 12, 2014 at 01:42:35PM -0700, Eric Northup wrote:
> Extend commit 5f2d17d35b2339526f3b3d580b279ea78e406a25: reset on all error
> paths, and also for virtio_blk not just virtio_scsi.

Thanks.  I have applied both of your patches.

-Kevin