[Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash with virtio-scsi-pci device

Thomas Huth posted 1 patch 6 years, 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1493126327-13162-1-git-send-email-thuth@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
hw/s390x/ipl.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash with virtio-scsi-pci device
Posted by Thomas Huth 6 years, 12 months ago
qemu-system-s390x currently crashes when it is started with a
virtio-scsi-pci device, e.g.:

 qemu-system-s390x -nographic -enable-kvm -device virtio-scsi-pci \
                   -drive file=/tmp/disk.dat,if=none,id=d1,format=raw \
                   -device scsi-cd,drive=d1,bootindex=1

The problem is that the code in s390_gen_initial_iplb() currently assumes
that all SCSI devices are also CCW devices, which is not the case for
virtio-scsi-pci of course. Fix it by adding an appropriate check for
TYPE_CCW_DEVICE here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/s390x/ipl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 7978c7d..f674d50 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -248,7 +248,13 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
             SCSIBus *bus = scsi_bus_from_device(sd);
             VirtIOSCSI *vdev = container_of(bus, VirtIOSCSI, bus);
             VirtIOSCSICcw *scsi_ccw = container_of(vdev, VirtIOSCSICcw, vdev);
-            CcwDevice *ccw_dev = CCW_DEVICE(scsi_ccw);
+            CcwDevice *ccw_dev;
+
+            ccw_dev = (CcwDevice *)object_dynamic_cast(OBJECT(scsi_ccw),
+                                                       TYPE_CCW_DEVICE);
+            if (!ccw_dev) {       /* It might be a PCI device instead */
+                return false;
+            }
 
             ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN);
             ipl->iplb.blk0_len =
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash with virtio-scsi-pci device
Posted by Christian Borntraeger 6 years, 12 months ago
On 04/25/2017 03:18 PM, Thomas Huth wrote:
> qemu-system-s390x currently crashes when it is started with a
> virtio-scsi-pci device, e.g.:
> 
>  qemu-system-s390x -nographic -enable-kvm -device virtio-scsi-pci \
>                    -drive file=/tmp/disk.dat,if=none,id=d1,format=raw \
>                    -device scsi-cd,drive=d1,bootindex=1
> 
> The problem is that the code in s390_gen_initial_iplb() currently assumes
> that all SCSI devices are also CCW devices, which is not the case for
> virtio-scsi-pci of course. Fix it by adding an appropriate check for
> TYPE_CCW_DEVICE here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

applied thanks.

> ---
>  hw/s390x/ipl.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 7978c7d..f674d50 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -248,7 +248,13 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
>              SCSIBus *bus = scsi_bus_from_device(sd);
>              VirtIOSCSI *vdev = container_of(bus, VirtIOSCSI, bus);
>              VirtIOSCSICcw *scsi_ccw = container_of(vdev, VirtIOSCSICcw, vdev);
> -            CcwDevice *ccw_dev = CCW_DEVICE(scsi_ccw);
> +            CcwDevice *ccw_dev;
> +
> +            ccw_dev = (CcwDevice *)object_dynamic_cast(OBJECT(scsi_ccw),
> +                                                       TYPE_CCW_DEVICE);
> +            if (!ccw_dev) {       /* It might be a PCI device instead */
> +                return false;
> +            }
> 
>              ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN);
>              ipl->iplb.blk0_len =
> 


Re: [Qemu-devel] [PATCH] hw/s390x/ipl: Fix crash with virtio-scsi-pci device
Posted by Cornelia Huck 6 years, 12 months ago
On Tue, 25 Apr 2017 15:18:47 +0200
Thomas Huth <thuth@redhat.com> wrote:

> qemu-system-s390x currently crashes when it is started with a
> virtio-scsi-pci device, e.g.:
> 
>  qemu-system-s390x -nographic -enable-kvm -device virtio-scsi-pci \
>                    -drive file=/tmp/disk.dat,if=none,id=d1,format=raw \
>                    -device scsi-cd,drive=d1,bootindex=1
> 
> The problem is that the code in s390_gen_initial_iplb() currently assumes
> that all SCSI devices are also CCW devices, which is not the case for
> virtio-scsi-pci of course. Fix it by adding an appropriate check for
> TYPE_CCW_DEVICE here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/s390x/ipl.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 7978c7d..f674d50 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -248,7 +248,13 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl)
>              SCSIBus *bus = scsi_bus_from_device(sd);
>              VirtIOSCSI *vdev = container_of(bus, VirtIOSCSI, bus);
>              VirtIOSCSICcw *scsi_ccw = container_of(vdev, VirtIOSCSICcw, vdev);
> -            CcwDevice *ccw_dev = CCW_DEVICE(scsi_ccw);
> +            CcwDevice *ccw_dev;
> +
> +            ccw_dev = (CcwDevice *)object_dynamic_cast(OBJECT(scsi_ccw),
> +                                                       TYPE_CCW_DEVICE);
> +            if (!ccw_dev) {       /* It might be a PCI device instead */
> +                return false;
> +            }
> 
>              ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN);
>              ipl->iplb.blk0_len =

This patch made me look at what we do if we have a non-ccw virtio-net
device for ipl. We'll set ->netboot (which implies that we load the
netboot fw later), but leave the iplb as invalid (so setting the start
address in the iplb won't have any effect as we ignore an invalid iplb
later on). The s390-ccw bios cannot deal with non-ccw boot devices
anyway.

In the end, it's just a bit odd, but no harm is done.