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

[Eric Farman]

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