Re: [Qemu-devel] [qemu-s390x] [PATCH 04/15] s390-bios: Extend find_dev() for non-virtio devices

["Jason J. Herne" <jjherne@linux.ibm.com>]

On 2/11/19 11:38 AM, Thomas Huth wrote:
> On 2019-01-29 14:29, Jason J. Herne wrote:
>> We need a method for finding the subchannel of a dasd device. Let's
>> modify find_dev to handle this since it mostly does what we need. Up to
>> this point find_dev has been specific to only virtio devices.
>>
>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>> Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> ---
>>   pc-bios/s390-ccw/main.c | 16 +++++++++++-----
>>   1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
>> index 67df421..7e3f65e 100644
>> --- a/pc-bios/s390-ccw/main.c
>> +++ b/pc-bios/s390-ccw/main.c
>> @@ -49,6 +49,12 @@ unsigned int get_loadparm_index(void)
>>       return atoui(loadparm_str);
>>   }
>>   
>> +/*
>> + * Find the subchannel connected to the given device (dev_no) and fill in the
>> + * subchannel information block (schib) with the connected subchannel's info.
>> + * NOTE: The global variable blk_schid is updated to contain the subchannel
>> + * information.
>> + */
>>   static bool find_dev(Schib *schib, int dev_no)
>>   {
>>       int i, r;
>> @@ -62,15 +68,15 @@ static bool find_dev(Schib *schib, int dev_no)
>>           if (!schib->pmcw.dnv) {
>>               continue;
>>           }
>> -        if (!virtio_is_supported(blk_schid)) {
>> -            continue;
>> -        }
>> +
>>           /* Skip net devices since no IPLB is created and therefore no
>> -         * no network bootloader has been loaded
>> +         * network bootloader has been loaded
>>            */
>> -        if (virtio_get_device_type() == VIRTIO_ID_NET && dev_no < 0) {
>> +        if (virtio_is_supported(blk_schid) &&
>> +            virtio_get_device_type() == VIRTIO_ID_NET && dev_no < 0) {
>>               continue;
>>           }
>> +
>>           if ((dev_no < 0) || (schib->pmcw.dev == dev_no)) {
>>               return true;
>>           }
>>
> 
> Not sure whether this really works as expected? If dev_no is -1, this
> used to return the first supported virtio device. Now it returns the
> first device that could be found - but how are we sure that we can boot
> from that device?
> 

How could we ever be sure we could boot from the first virtio device? The dev_no=1 case 
means we don't know which device to boot from so we're guessing.  The only thing that is 
changing here is that we're allowing non-virtio devices as well. We do this because we now 
support booting from a device type that is not virtio.


-- 
-- Jason J. Herne (jjherne@linux.ibm.com)