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

[Eric Farman]

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