| Message ID | 20220628131032.213986-5-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390-ccw bios fixes and clean-ups | expand |
On Tue, 2022-06-28 at 15:10 +0200, Thomas Huth wrote: > The s390-ccw bios fails to boot if the boot disk is a virtio-blk > disk with a sector size of 4096. For example: > > dasdfmt -b 4096 -d cdl -y -p -M quick /dev/dasdX > fdasd -a /dev/dasdX > install a guest onto /dev/dasdX1 using virtio-blk > qemu-system-s390x -nographic -hda /dev/dasdX1 > > The bios then bails out with: > > ! Cannot read block 0 ! > > Looking at virtio_ipl_disk_is_valid() and especially the function > virtio_disk_is_scsi(), it does not really make sense that we expect > only such a limited disk geometry (like a block size of 512) for > our boot disks. Let's relax the check and allow everything that > remotely looks like a sane disk. This indeed corrects that problem (thank you!), and I really don't understand why we'd want to limit the geometry anyhow. I think this is good. Reviewed-by: Eric Farman <farman@linux.ibm.com> > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > pc-bios/s390-ccw/virtio.h | 2 -- > pc-bios/s390-ccw/virtio-blkdev.c | 41 ++++++---------------------- > ---- > 2 files changed, 7 insertions(+), 36 deletions(-) > > diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h > index c2c17c29ca..8f917d47a9 100644 > --- a/pc-bios/s390-ccw/virtio.h > +++ b/pc-bios/s390-ccw/virtio.h > @@ -186,8 +186,6 @@ void virtio_assume_scsi(void); > void virtio_assume_eckd(void); > void virtio_assume_iso9660(void); > > -extern bool virtio_disk_is_scsi(void); > -extern bool virtio_disk_is_eckd(void); > extern bool virtio_ipl_disk_is_valid(void); > extern int virtio_get_block_size(void); > extern uint8_t virtio_get_heads(void); > diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390- > ccw/virtio-blkdev.c > index 49ed2b4bee..b14cbc3d9e 100644 > --- a/pc-bios/s390-ccw/virtio-blkdev.c > +++ b/pc-bios/s390-ccw/virtio-blkdev.c > @@ -166,46 +166,19 @@ void virtio_assume_eckd(void) > virtio_eckd_sectors_for_block_size(vdev- > >config.blk.blk_size); > } > > -bool virtio_disk_is_scsi(void) > -{ > - VDev *vdev = virtio_get_device(); > - > - if (vdev->guessed_disk_nature == VIRTIO_GDN_SCSI) { > - return true; > - } > - switch (vdev->senseid.cu_model) { > - case VIRTIO_ID_BLOCK: > - return (vdev->config.blk.geometry.heads == 255) > - && (vdev->config.blk.geometry.sectors == 63) > - && (virtio_get_block_size() == VIRTIO_SCSI_BLOCK_SIZE); > - case VIRTIO_ID_SCSI: > - return true; > - } > - return false; > -} > - > -bool virtio_disk_is_eckd(void) > +bool virtio_ipl_disk_is_valid(void) > { > + int blksize = virtio_get_block_size(); > VDev *vdev = virtio_get_device(); > - const int block_size = virtio_get_block_size(); > > - if (vdev->guessed_disk_nature == VIRTIO_GDN_DASD) { > + if (vdev->guessed_disk_nature == VIRTIO_GDN_SCSI || > + vdev->guessed_disk_nature == VIRTIO_GDN_DASD) { > return true; > } > - switch (vdev->senseid.cu_model) { > - case VIRTIO_ID_BLOCK: > - return (vdev->config.blk.geometry.heads == 15) > - && (vdev->config.blk.geometry.sectors == > - virtio_eckd_sectors_for_block_size(block_size)); > - case VIRTIO_ID_SCSI: > - return false; > - } > - return false; > -} > > -bool virtio_ipl_disk_is_valid(void) > -{ > - return virtio_disk_is_scsi() || virtio_disk_is_eckd(); > + return (vdev->senseid.cu_model == VIRTIO_ID_BLOCK || > + vdev->senseid.cu_model == VIRTIO_ID_SCSI) && > + blksize >= 512 && blksize <= 4096; > } > > int virtio_get_block_size(void)
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h index c2c17c29ca..8f917d47a9 100644 --- a/pc-bios/s390-ccw/virtio.h +++ b/pc-bios/s390-ccw/virtio.h @@ -186,8 +186,6 @@ void virtio_assume_scsi(void); void virtio_assume_eckd(void); void virtio_assume_iso9660(void); -extern bool virtio_disk_is_scsi(void); -extern bool virtio_disk_is_eckd(void); extern bool virtio_ipl_disk_is_valid(void); extern int virtio_get_block_size(void); extern uint8_t virtio_get_heads(void); diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c index 49ed2b4bee..b14cbc3d9e 100644 --- a/pc-bios/s390-ccw/virtio-blkdev.c +++ b/pc-bios/s390-ccw/virtio-blkdev.c @@ -166,46 +166,19 @@ void virtio_assume_eckd(void) virtio_eckd_sectors_for_block_size(vdev->config.blk.blk_size); } -bool virtio_disk_is_scsi(void) -{ - VDev *vdev = virtio_get_device(); - - if (vdev->guessed_disk_nature == VIRTIO_GDN_SCSI) { - return true; - } - switch (vdev->senseid.cu_model) { - case VIRTIO_ID_BLOCK: - return (vdev->config.blk.geometry.heads == 255) - && (vdev->config.blk.geometry.sectors == 63) - && (virtio_get_block_size() == VIRTIO_SCSI_BLOCK_SIZE); - case VIRTIO_ID_SCSI: - return true; - } - return false; -} - -bool virtio_disk_is_eckd(void) +bool virtio_ipl_disk_is_valid(void) { + int blksize = virtio_get_block_size(); VDev *vdev = virtio_get_device(); - const int block_size = virtio_get_block_size(); - if (vdev->guessed_disk_nature == VIRTIO_GDN_DASD) { + if (vdev->guessed_disk_nature == VIRTIO_GDN_SCSI || + vdev->guessed_disk_nature == VIRTIO_GDN_DASD) { return true; } - switch (vdev->senseid.cu_model) { - case VIRTIO_ID_BLOCK: - return (vdev->config.blk.geometry.heads == 15) - && (vdev->config.blk.geometry.sectors == - virtio_eckd_sectors_for_block_size(block_size)); - case VIRTIO_ID_SCSI: - return false; - } - return false; -} -bool virtio_ipl_disk_is_valid(void) -{ - return virtio_disk_is_scsi() || virtio_disk_is_eckd(); + return (vdev->senseid.cu_model == VIRTIO_ID_BLOCK || + vdev->senseid.cu_model == VIRTIO_ID_SCSI) && + blksize >= 512 && blksize <= 4096; } int virtio_get_block_size(void)
The s390-ccw bios fails to boot if the boot disk is a virtio-blk disk with a sector size of 4096. For example: dasdfmt -b 4096 -d cdl -y -p -M quick /dev/dasdX fdasd -a /dev/dasdX install a guest onto /dev/dasdX1 using virtio-blk qemu-system-s390x -nographic -hda /dev/dasdX1 The bios then bails out with: ! Cannot read block 0 ! Looking at virtio_ipl_disk_is_valid() and especially the function virtio_disk_is_scsi(), it does not really make sense that we expect only such a limited disk geometry (like a block size of 512) for our boot disks. Let's relax the check and allow everything that remotely looks like a sane disk. Signed-off-by: Thomas Huth <thuth@redhat.com> --- pc-bios/s390-ccw/virtio.h | 2 -- pc-bios/s390-ccw/virtio-blkdev.c | 41 ++++++-------------------------- 2 files changed, 7 insertions(+), 36 deletions(-)