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

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

On 11/20/20 1:02 AM, Thomas Huth wrote:
> On 19/11/2020 22.11, Eric Farman wrote:
>>
>>
>> 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()).
> 
> Great, can you send the patch before your holidays next week? (If you don't
> have enough time, that's ok, too, it's trivial enough, so I think I could
> write such a patch, too)

Will do.  I have it written up, just didn't want to send it last night 
and have surprises when I came in this morning. :)

> 
>   Thomas
>