Re: [PATCH] s390-bios: Skip writing iplb location to low core for ccw ipl

[Christian Borntraeger]

On 03.11.20 13:32, Thomas Huth wrote:
> On 30/10/2020 13.28, Christian Borntraeger wrote:
>> From: "Jason J. Herne" <jjherne@linux.ibm.com>
>>
>> The architecture states that the iplb location is only written to low
>> core for list directed ipl and not for traditional ccw ipl. If we don't
>> skip this then operating systems that load by reading into low core
>> memory may fail to start.
> 
> Just double-checking: But doing write_subsystem_identification()
> unconditionally is ok, right?

Yes, for any channel device IPL the subsystem ID is stored in absolute locations
184-187 so this should be good as virtio is a channel device.
> 
>> We should also not write the iplb pointer for network boot as it might
>> overwrite content that we got via network.
> 
> FWIW, write_iplb_location() is already just a dummy function in netmain.c,
> so this should not have been an issue in the network bootloader, I hope.

OK. I think we can keep the check for !VIRTIO_ID_NET anyway.

> 
>> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
>> Fixes: 9bfc04f9ef68 ("pc-bios: s390x: Save iplb location in lowcore")
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  pc-bios/s390-ccw/main.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
>> index 43c792cf9509..fc4bfaa45529 100644
>> --- a/pc-bios/s390-ccw/main.c
>> +++ b/pc-bios/s390-ccw/main.c
>> @@ -43,7 +43,9 @@ void write_subsystem_identification(void)
>>  
>>  void write_iplb_location(void)
>>  {
>> -    lowcore->ptr_iplb = ptr2u32(&iplb);
>> +    if (cutype == CU_TYPE_VIRTIO && virtio_get_device_type() != 
>> VIRTIO_ID_NET) {
>> +        lowcore->ptr_iplb = ptr2u32(&iplb);
>> +    }
>>  }
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> 
> Christian, Cornelia, could you please pick up the patch? I'm not sure
> whether I can do another PR this week for the RC...
>