Re: [PATCH v5 00/19] s390x: Add Full Boot Order Support

[Jared Rossi <jrossi@linux.ibm.com>]

On 11/6/24 6:10 AM, Thomas Huth wrote:
> On 05/11/2024 17.42, Jared Rossi wrote:
>> Hi Thomas, Sebastian,
>>
>> It looks like this is simply caused by the "is_cdrom" value only ever 
>> being set
>> to true.  I think it is a one-line fix that just makes sure to 
>> initialize the
>> value to false each time we try a new device:
>>
>> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
>> index a4d1c05aac..3fdba0bedc 100644
>> --- a/pc-bios/s390-ccw/main.c
>> +++ b/pc-bios/s390-ccw/main.c
>> @@ -214,6 +214,7 @@ static void boot_setup(void)
>>   static bool find_boot_device(void)
>>   {
>>       VDev *vdev = virtio_get_device();
>> +    vdev->is_cdrom = false;
>>       bool found = false;
>>
>>       switch (iplb.pbt) {
>>
>> I tested it with the two scenarios you mention and with the existing 
>> qtests,
>> and it seems to work correctly now.
>
> Agreed, this seems to fix the issue when all devices are properly 
> marked with bootindex properties. But it still persists in case the 
> BIOS has to scan for the boot device, e.g.:
>
> qemu-system-s390x -accel kvm -m 2G -nographic \
>  -drive if=none,id=d1,file=/tmp/bad.dat,format=raw,media=cdrom \
>  -device virtio-scsi -device scsi-cd,drive=d1 \
>  -drive if=none,id=d2,file=good.qcow2 -device virtio-blk,drive=d2
>
> Isn't there a better place to set the is_cdrom variable?
>
>  Thomas
>

Hi Thomas,

What I found is that the original issue with clearing the "is_cdrom" 
value is
just as easily fixed for both indexed devices and probed devices by moving
where "is_cdrom" is set to false:

diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index a4d1c05aac..7509755e36 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -242,6 +242,7 @@ static bool find_boot_device(void)
  static int virtio_setup(void)
  {
      VDev *vdev = virtio_get_device();
+    vdev->is_cdrom = false;
      int ret;

      switch (vdev->senseid.cu_model) {

Unfortunately when verifying the fix I found another unrelated issue with
probing, which is that only the first device attached to the scsi controller
will be found.  This is because virtio_scsi_setup() itself contains a 
probing
loop to find a LUN when none is specified, and, unsurprisingly, it returns
the first thing it finds attached to the controller.  As a result, the main
device probing loop will move on after trying only one LUN per controller.

Fixing this won't be a simple one-liner because it will require 
implementation
of nested probing for scsi devices, such that all LUNS on the controller are
probed before moving to the next device.

Regards,
  Jared Rossi