On Tue, 2022-06-28 at 15:10 +0200, Thomas Huth wrote: > The logic of trying an final ISO or ECKD boot on virtio-block devices > is > very weird: Since the geometry hardly ever matches in > virtio_disk_is_scsi(), > virtio_blk_setup_device() always sets a "guessed" disk geometry via > virtio_assume_scsi() (which is certainly also wrong in a lot of > cases). > > zipl_load_vblk() then sees that there's been a > "virtio_guessed_disk_nature" > and tries to fix up the geometry again via virtio_assume_iso9660() > before > always trying to do ipl_iso_el_torito(). That's a very brain-twisting > way of attempting to boot from ISO images, which won't work anymore > after > the following patches that will clean up the virtio_assume_scsi() > mess > (and thus get rid of the "virtio_guessed_disk_nature" here). > > Let's try a better approach instead: ISO files always have a magic > string "CD001" at offset 0x8001 (see e.g. the ECMA-119 specification) > which we can use to decide whether we should try to boot in ISO 9660 > mode (which we should also try if we see a sector size of 2048). > > And if we were not able to boot in ISO mode here, the final boot > attempt > before panicking is to boot in ECKD mode. Since this is our last boot > attempt anyway, simply always assume the ECKD geometry here (if the > sector > size was not 4096 yet), so that we also do not depend on the guessed > disk > geometry from virtio_blk_setup_device() here anymore. > > Signed-off-by: Thomas Huth <th...@redhat.com> > --- > pc-bios/s390-ccw/bootmap.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c > index 56411ab3b6..3181b05382 100644 > --- a/pc-bios/s390-ccw/bootmap.c > +++ b/pc-bios/s390-ccw/bootmap.c > @@ -780,18 +780,35 @@ static void ipl_iso_el_torito(void) > } > } > > +/** > + * Detect whether we're trying to boot from an .ISO image. > + * These always have a signature string "CD001" at offset 0x8001. > + */ > +static bool has_iso_signature(void) > +{ > + if (virtio_read(0x8000 / virtio_get_block_size(), sec)) {
Certainly unlikely, but virtio_get_block_size is able to return zero. > + return false; > + } > + > + return !memcmp("CD001", &sec[1], 5); > +} > + > /******************************************************************* > **** > * Bus specific IPL sequences > */ > > static void zipl_load_vblk(void) > { > - if (virtio_guessed_disk_nature()) { > - virtio_assume_iso9660(); > + int blksize = virtio_get_block_size(); > + > + if (blksize == VIRTIO_ISO_BLOCK_SIZE || has_iso_signature()) { > + if (blksize != VIRTIO_ISO_BLOCK_SIZE) { > + virtio_assume_iso9660(); > + } > + ipl_iso_el_torito(); > } > - ipl_iso_el_torito(); > > - if (virtio_guessed_disk_nature()) { > + if (blksize != VIRTIO_DASD_BLOCK_SIZE) { > sclp_print("Using guessed DASD geometry.\n"); > virtio_assume_eckd(); > }