| Message ID | 20201119165729.63351-3-farman@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pc-bios/s390 fixes for reboot-to-vfio-ccw | expand |
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... 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())? Then we can be sure that there is no stale S390EP magic dangling around anymore once we've loaded the new kernel... Thomas
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 >
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) Thomas
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 >
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); + } + /* No other option left, so use the Linux kernel start address */ jump_to_IPL_code(KERN_IMAGE_START); }
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(-)