[PATCH 0/2] Fix booting tcm_vhost + seabios

[Asias He]

Asias He (2):
  virtio-scsi: Set _DRIVER_OK flag before scsi target scanning
  virtio-scsi: Pack struct virtio_scsi_{req_cmd,resp_cmd}

src/virtio-scsi.c | 5 +++--
 src/virtio-scsi.h | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

[PATCH 1/2] virtio-scsi: Set _DRIVER_OK flag before scsi target scanning

[Asias He]

Before we start scsi target scanning, we need to set the
VIRTIO_CONFIG_S_DRIVER_OK flag so the device can do setup properly.

This fix a bug when booting tcm_vhost with seabios.

Signed-off-by: Asias He 
Acked-by: Paolo Bonzini 
---
 src/virtio-scsi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c
index 879ddfb..4de1255 100644
--- a/src/virtio-scsi.c
+++ b/src/virtio-scsi.c
@@ -147,6 +147,9 @@ init_virtio_scsi(struct pci_device *pci)
         goto fail;
     }

+    vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
+                  VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK);
+
     int i, tot;
     for (tot = 0, i = 0; i < 256; i++)
         tot += virtio_scsi_scan_target(pci, ioaddr, vq, i);
@@ -154,8 +157,6 @@ init_virtio_scsi(struct pci_device *pci)
     if (!tot)
         goto fail;

-    vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
-                  VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK);
     return;

fail:

[PATCH 1/2] virtio-scsi: Set _DRIVER_OK flag before scsi target scanning

[Michael S. Tsirkin]

On Fri, Mar 15, 2013 at 09:45:15AM +0800, Asias He wrote:
> Before we start scsi target scanning, we need to set the
> VIRTIO_CONFIG_S_DRIVER_OK flag so the device can do setup properly.
>
> This fix a bug when booting tcm_vhost with seabios.
>
> Signed-off-by: Asias He 
> Acked-by: Paolo Bonzini

Acked-by: Michael S. Tsirkin
> src/virtio-scsi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c
> index 879ddfb..4de1255 100644
> --- a/src/virtio-scsi.c
> +++ b/src/virtio-scsi.c
> @@ -147,6 +147,9 @@ init_virtio_scsi(struct pci_device *pci)
>          goto fail;
>      }
> vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
>              VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK);
> int i, tot;
>    for (tot = 0, i = 0; i < 256; i++)
>        tot += virtio_scsi_scan_target(pci, ioaddr, vq, i);
>
> @@ -154,8 +157,6 @@ init_virtio_scsi(struct pci_device *pci)
>      if (!tot)
>          goto fail;
> vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
>              VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK);
> return;
> fail:
>
> 1.8.1.4

[PATCH 2/2] virtio-scsi: Pack struct virtio_scsi_{req_cmd, resp_cmd}

[Asias He]

Device needs the exact size of these data structure. Prevent padding.

This fixes guest hang when booting seabios + tcm_vhost.

Signed-off-by: Asias He 
---
 src/virtio-scsi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/virtio-scsi.h b/src/virtio-scsi.h
index bbfbf30..96c3701 100644
--- a/src/virtio-scsi.h
+++ b/src/virtio-scsi.h
@@ -26,7 +26,7 @@ struct virtio_scsi_req_cmd {
     u8 prio;
     u8 crn;
     char cdb[VIRTIO_SCSI_CDB_SIZE];
-};
+} __attribute__((packed));

/* This is the first element of the "in" scatter-gather list. */
 struct virtio_scsi_resp_cmd {
@@ -36,7 +36,7 @@ struct virtio_scsi_resp_cmd {
     u8 status;
     u8 response;
     u8 sense[VIRTIO_SCSI_SENSE_SIZE];
-};
+} __attribute__((packed));

#define VIRTIO_SCSI_S_OK            0

[PATCH 2/2] virtio-scsi: Pack struct virtio_scsi_{req_cmd, resp_cmd}

[Paolo Bonzini]

Il 15/03/2013 02:45, Asias He ha scritto:
> Device needs the exact size of these data structure. Prevent padding.
>
> This fixes guest hang when booting seabios + tcm_vhost.
> Signed-off-by: Asias He
>
> src/virtio-scsi.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/virtio-scsi.h b/src/virtio-scsi.h
> index bbfbf30..96c3701 100644
> --- a/src/virtio-scsi.h
> +++ b/src/virtio-scsi.h
> @@ -26,7 +26,7 @@ struct virtio_scsi_req_cmd {
>      u8 prio;
>      u8 crn;
>      char cdb[VIRTIO_SCSI_CDB_SIZE];
> -};
> +} __attribute__((packed));
>
> /* This is the first element of the "in" scatter-gather list. */
>  struct virtio_scsi_resp_cmd {
> @@ -36,7 +36,7 @@ struct virtio_scsi_resp_cmd {
>      u8 status;
>      u8 response;
>      u8 sense[VIRTIO_SCSI_SENSE_SIZE];
> -};
> +} __attribute__((packed));
>
> #define VIRTIO_SCSI_S_OK            0

Reviewed-by: Paolo Bonzini

[PATCH 2/2] virtio-scsi: Pack struct virtio_scsi_{req_cmd, resp_cmd}

[Michael S. Tsirkin]

On Fri, Mar 15, 2013 at 09:45:16AM +0800, Asias He wrote:
> Device needs the exact size of these data structure. Prevent padding.
>
> This fixes guest hang when booting seabios + tcm_vhost.
>
> Signed-off-by: Asias He

Acked-by: Michael S. Tsirkin
> src/virtio-scsi.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/virtio-scsi.h b/src/virtio-scsi.h
> index bbfbf30..96c3701 100644
> --- a/src/virtio-scsi.h
> +++ b/src/virtio-scsi.h
> @@ -26,7 +26,7 @@ struct virtio_scsi_req_cmd {
>      u8 prio;
>      u8 crn;
>      char cdb[VIRTIO_SCSI_CDB_SIZE];
> -};
> +} __attribute__((packed));
>
> /* This is the first element of the "in" scatter-gather list. */
>  struct virtio_scsi_resp_cmd {
> @@ -36,7 +36,7 @@ struct virtio_scsi_resp_cmd {
>      u8 status;
>      u8 response;
>      u8 sense[VIRTIO_SCSI_SENSE_SIZE];
> -};
> +} __attribute__((packed));
>
> #define VIRTIO_SCSI_S_OK            0

[Kevin O'Connor]

On Fri, Mar 15, 2013 at 09:45:14AM +0800, Asias He wrote:
> Asias He (2):
>   virtio-scsi: Set _DRIVER_OK flag before scsi target scanning
>   virtio-scsi: Pack struct virtio_scsi_{req_cmd,resp_cmd}

Thanks.  I pushed these patches.

-Kevin

[Nicholas A. Bellinger]

On Fri, 2013-03-15 at 09:45 +0800, Asias He wrote:
> Asias He (2):
>   virtio-scsi: Set _DRIVER_OK flag before scsi target scanning
>   virtio-scsi: Pack struct virtio_scsi_{req_cmd,resp_cmd}
>
> src/virtio-scsi.c | 5 +++--
>  src/virtio-scsi.h | 4 ++--
>  2 files changed, 5 insertions(+), 4 deletions(-)

Hi Asias,

Thanks for taking the initiative on this, and nice work tracking both of
these stubborn bugs down.

So with these out of the way, we're good to go for an RFC of Paolo's
vhost-scsi-pci code for upstream QEMU, yes..?

I'll have some extra bandwidth this week to spend time on the RFC if
you'd like, otherwise I'm happy with you making the upstream QEMU push
for Paolo's code.

Whatever works best for you.  :)

Thank you,

--nab

[Asias He]

On Mon, Mar 18, 2013 at 02:26:14PM -0700, Nicholas A. Bellinger wrote:
> On Fri, 2013-03-15 at 09:45 +0800, Asias He wrote:
> > Asias He (2):
> >   virtio-scsi: Set _DRIVER_OK flag before scsi target scanning
> >   virtio-scsi: Pack struct virtio_scsi_{req_cmd,resp_cmd}
> >
> > src/virtio-scsi.c | 5 +++--
> >  src/virtio-scsi.h | 4 ++--
> >  2 files changed, 5 insertions(+), 4 deletions(-)
>
> Hi Asias,
>
> Thanks for taking the initiative on this, and nice work tracking both of
> these stubborn bugs down.
>
> So with these out of the way, we're good to go for an RFC of Paolo's
> vhost-scsi-pci code for upstream QEMU, yes..?
>
> I'll have some extra bandwidth this week to spend time on the RFC if
> you'd like, otherwise I'm happy with you making the upstream QEMU push
> for Paolo's code.
>
> Whatever works best for you.  :)

Nice, Nicholas, go ahead. I have sent out the WIP V3 for you ;-)
> Thank you,
>
> --nab