Re: [PATCH 2/2] pc-bios: s390x: Give precedence to reset PSW

[Eric Farman <farman@linux.ibm.com>]

On 11/19/20 3:20 PM, Thomas Huth wrote:
> On 19/11/2020 17.57, Eric Farman wrote:
>> Let's look at the Reset PSW first instead of the contents of memory.
>> It might be leftover from an earlier system boot when processing
>> a chreipl.
>>
>> Signed-off-by: Eric Farman <farman@linux.ibm.com>
>> ---
>>   pc-bios/s390-ccw/jump2ipl.c | 20 ++++++++++----------
>>   1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>> index fbae45b03c..67b4afb6a0 100644
>> --- a/pc-bios/s390-ccw/jump2ipl.c
>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>> @@ -72,16 +72,6 @@ void jump_to_IPL_code(uint64_t address)
>>   
>>   void jump_to_low_kernel(void)
>>   {
>> -    /*
>> -     * If it looks like a Linux binary, i.e. there is the "S390EP" magic from
>> -     * arch/s390/kernel/head.S here, then let's jump to the well-known Linux
>> -     * kernel start address (when jumping to the PSW-at-zero address instead,
>> -     * the kernel startup code fails when we booted from a network device).
>> -     */
>> -    if (!memcmp((char *)0x10008, "S390EP", 6)) {
>> -        jump_to_IPL_code(KERN_IMAGE_START);
>> -    }
>> -
>>       /* Trying to get PSW at zero address */
>>       if (*((uint64_t *)0) & RESET_PSW_MASK) {
>>           /*
>> @@ -92,6 +82,16 @@ void jump_to_low_kernel(void)
>>           jump_to_IPL_code(0);
>>       }
>>   
>> +    /*
>> +     * If it looks like a Linux binary, i.e. there is the "S390EP" magic from
>> +     * arch/s390/kernel/head.S here, then let's jump to the well-known Linux
>> +     * kernel start address (when jumping to the PSW-at-zero address instead,
>> +     * the kernel startup code fails when we booted from a network device).
>> +     */
>> +    if (!memcmp((char *)0x10008, "S390EP", 6)) {
>> +        jump_to_IPL_code(KERN_IMAGE_START);
>> +    }
> 
> That feels a little bit dangerous ... I assume the order has been that way
> for a reason, e.g. I think we had to jump to KERN_IMAGE_START for some older
> versions of the Linux kernel since the startup code that was referenced by
> the PSW at address zero was not working in KVM...

Makes sense.  It does seem like a precarious piece of code.

> 
> What do you think about this alternate idea instead: Clear the memory at
> location 0x10008 at the very beginning of the main() function (or maybe in
> boot_setup())? 

This seems to work too (I put it in boot_setup(), prior to call to 
store_iplb()).

Then we can be sure that there is no stale S390EP magic
> dangling around anymore once we've loaded the new kernel...
> 
>   Thomas
>