Re: [PATCH] pc-bios/s390-ccw: Fix booting with logical block size < physical block size

[Eric Farman]

On Fri, 2022-08-05 at 12:14 +0200, Cornelia Huck wrote:
> On Fri, Aug 05 2022, Thomas Huth <thuth@redhat.com> wrote:
> 
> > For accessing single blocks during boot, it's the logical block
> > size that
> > matters. (Physical block sizes are rather interesting e.g. for
> > creating
> > file systems with the correct alignment for speed reasons etc.).
> > So the s390-ccw bios has to use the logical block size for
> > calculating
> > sector numbers during the boot phase, the "physical_block_exp"
> > shift
> > value must not be taken into account. This change fixes the boot
> > process
> > when the guest hast been installed on a disk where the logical
> > block size
> > differs from the physical one, e.g. if the guest has been installed
> > like this:
> > 
> >  qemu-system-s390x -nographic -accel kvm -m 2G \
> >   -drive if=none,id=d1,file=fedora.iso,format=raw,media=cdrom \
> >   -device virtio-scsi -device scsi-cd,drive=d1 \
> >   -drive if=none,id=d2,file=test.qcow2,format=qcow2
> >   -device virtio-
> > blk,drive=d2,physical_block_size=4096,logical_block_size=512
> > 
> > Linux correctly uses the logical block size of 512 for the
> > installation,
> > but the s390-ccw bios tries to boot from a disk with 4096 block
> > size so
> > far, as long as this patch has not been applied yet (well, it used
> > to work
> > by accident in the past due to the virtio_assume_scsi() hack that
> > used to
> > enforce 512 byte sectors on all virtio-block disks, but that hack
> > has been
> > well removed in commit 5447de2619050a0a4d to fix other scenarios).
> 
> I wonder whether there's more stuff lurking in there; the old code
> seems
> to have "worked" in many cases by accident, and cleaning up things
> might
> expose more odd code. Generally, reading ccw bios code gives me a
> headache :)

Same here. But Thomas' recent changes are good medicine. :)

> 
> > Fixes: 5447de2619 ("pc-bios/s390-ccw/virtio-blkdev: Remove
> > virtio_assume_scsi()")
> > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2112303
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >  pc-bios/s390-ccw/virtio-blkdev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Looks sane to me.
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 

Reviewed-by: Eric Farman <farman@linux.ibm.com>