Re: [SeaBIOS] [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