[RFC,QEMU] pc-bios/s390-ccw: Add zipl-like "BOOT_IMAGE=x" to the kernel parameters
diff mbox series

Message ID 20191216112432.13412-1-thuth@redhat.com
State New
Headers show
Series
  • [RFC,QEMU] pc-bios/s390-ccw: Add zipl-like "BOOT_IMAGE=x" to the kernel parameters
Related show

Commit Message

Thomas Huth Dec. 16, 2019, 11:24 a.m. UTC
ZIPL adds a "BOOT_IMAGE=x" to the kernel parameters to indicate which
kernel entry has been chosen during the boot process. Apparently some
Linux tools like "dracut" use this setting, so we should provide this
kernel parameter with the s390-ccw bios, too.

However, it's a little bit tricky to get additional parameters from the
s390-ccw bios into the kernel command line: Since we are running the
ZIPL stage 3 boot loader first (which then finally jumps into the Linux
kernel), we have to adapt to the parameter conventions of ZIPL and put
the argument into ZIPLs "COMMAND_LINE_EXTRA" area. Unfortunately, the
location of this area changed in the course of time (it has been moved
between ZIPL v2.9 and v2.10), so we need to detect the right version of
ZIPL here, too. The only reasonable way that I could figure out was the
start address of the ZIPL stage 3 bootloader which has been changed in
almost the same timeframe - just a little bit earlier, between v2.8 and
v2.9, so if a user is using exactly ZIPL v2.9, they won't see the new
BOOT_IMAGE parameter (but at least the new code in s390-ccw should also
not hurt in this case - the area where we write the parameter to is just
the lowest part of the stack area of ZIPL, which should be unused).

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1782026
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 Note: I've marked the patch as RFC since I'm not quite sure whether
 this is really the right way to address this issue: It's unfortunate
 that we have to mess with different location in ZIPL which might also
 change again in the future. As suggested by Christian on IRC last week,
 maybe it would make more sense to change ZIPL to add this parameter
 already when zipl is installed (i.e. by the Linux userspace "zipl" pro-
 gram), instead of adding it during boot time? Also, the BOOT_IMAGE para-
 meter on s390x is quite different from the BOOT_IMAGE paramter that is
 used on x86 - while s390x only uses one single number here, the x86
 variant (added by grub2, I guess) uses the boot device + full filename
 of the kernel on the boot partition. Should we maybe make the s390x
 variant more conform to x86? If so, I think this really has to be fixed
 in zipl userspace tool, and not in the s390-ccw bios (and zipl stage3
 bootloader).

 pc-bios/s390-ccw/bootmap.c  | 56 +++++++++++++++++++++++++++++++++++--
 pc-bios/s390-ccw/jump2ipl.c |  2 +-
 pc-bios/s390-ccw/s390-ccw.h |  2 ++
 3 files changed, 57 insertions(+), 3 deletions(-)

Comments

Christian Borntraeger Dec. 16, 2019, 11:29 a.m. UTC | #1
On 16.12.19 12:24, Thomas Huth wrote:
>  Note: I've marked the patch as RFC since I'm not quite sure whether
>  this is really the right way to address this issue: It's unfortunate
>  that we have to mess with different location in ZIPL which might also
>  change again in the future. As suggested by Christian on IRC last week,
>  maybe it would make more sense to change ZIPL to add this parameter
>  already when zipl is installed (i.e. by the Linux userspace "zipl" pro-
>  gram), instead of adding it during boot time? Also, the BOOT_IMAGE para-
>  meter on s390x is quite different from the BOOT_IMAGE paramter that is
>  used on x86 - while s390x only uses one single number here, the x86
>  variant (added by grub2, I guess) uses the boot device + full filename
>  of the kernel on the boot partition. Should we maybe make the s390x
>  variant more conform to x86? If so, I think this really has to be fixed
>  in zipl userspace tool, and not in the s390-ccw bios (and zipl stage3
>  bootloader).

Yes, I actually think we should revisit the whole BOOT_IMAGE scheme on s390.
Maybe we should use the kernel name, or the name of the boot menu entry.
And maybe we should not use 0 (when the default is running) but instead
really use to what 0 points to.
Cornelia Huck Dec. 16, 2019, 12:09 p.m. UTC | #2
On Mon, 16 Dec 2019 12:29:24 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 16.12.19 12:24, Thomas Huth wrote:
> >  Note: I've marked the patch as RFC since I'm not quite sure whether
> >  this is really the right way to address this issue: It's unfortunate
> >  that we have to mess with different location in ZIPL which might also
> >  change again in the future. As suggested by Christian on IRC last week,
> >  maybe it would make more sense to change ZIPL to add this parameter
> >  already when zipl is installed (i.e. by the Linux userspace "zipl" pro-
> >  gram), instead of adding it during boot time? Also, the BOOT_IMAGE para-
> >  meter on s390x is quite different from the BOOT_IMAGE paramter that is
> >  used on x86 - while s390x only uses one single number here, the x86
> >  variant (added by grub2, I guess) uses the boot device + full filename
> >  of the kernel on the boot partition. Should we maybe make the s390x
> >  variant more conform to x86? If so, I think this really has to be fixed
> >  in zipl userspace tool, and not in the s390-ccw bios (and zipl stage3
> >  bootloader).  
> 
> Yes, I actually think we should revisit the whole BOOT_IMAGE scheme on s390.
> Maybe we should use the kernel name, or the name of the boot menu entry.
> And maybe we should not use 0 (when the default is running) but instead
> really use to what 0 points to.

Probably dumb question: Is booting via the s390-ccw bios the only time
we boot without going through zipl? What about e.g. booting from the
reader under z/VM? There's probably no BOOT_IMAGE= statement there,
either?
Christian Borntraeger Dec. 16, 2019, 12:15 p.m. UTC | #3
On 16.12.19 13:09, Cornelia Huck wrote:
> On Mon, 16 Dec 2019 12:29:24 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 16.12.19 12:24, Thomas Huth wrote:
>>>  Note: I've marked the patch as RFC since I'm not quite sure whether
>>>  this is really the right way to address this issue: It's unfortunate
>>>  that we have to mess with different location in ZIPL which might also
>>>  change again in the future. As suggested by Christian on IRC last week,
>>>  maybe it would make more sense to change ZIPL to add this parameter
>>>  already when zipl is installed (i.e. by the Linux userspace "zipl" pro-
>>>  gram), instead of adding it during boot time? Also, the BOOT_IMAGE para-
>>>  meter on s390x is quite different from the BOOT_IMAGE paramter that is
>>>  used on x86 - while s390x only uses one single number here, the x86
>>>  variant (added by grub2, I guess) uses the boot device + full filename
>>>  of the kernel on the boot partition. Should we maybe make the s390x
>>>  variant more conform to x86? If so, I think this really has to be fixed
>>>  in zipl userspace tool, and not in the s390-ccw bios (and zipl stage3
>>>  bootloader).  
>>
>> Yes, I actually think we should revisit the whole BOOT_IMAGE scheme on s390.
>> Maybe we should use the kernel name, or the name of the boot menu entry.
>> And maybe we should not use 0 (when the default is running) but instead
>> really use to what 0 points to.
> 
> Probably dumb question: Is booting via the s390-ccw bios the only time
> we boot without going through zipl? What about e.g. booting from the
> reader under z/VM? There's probably no BOOT_IMAGE= statement there,
> either?

I just learned from Peter that booting SCSI also has no BOOT_IMAGE (as
we have no menu). So Thomas, can you find out the use case for the initial
bug report.  That might give an indication on how to proceed for all cases.
Thomas Huth Dec. 16, 2019, 12:18 p.m. UTC | #4
On 16/12/2019 13.15, Christian Borntraeger wrote:
[...]
> I just learned from Peter that booting SCSI also has no BOOT_IMAGE (as
> we have no menu). So Thomas, can you find out the use case for the initial
> bug report.  That might give an indication on how to proceed for all cases.

Apparently this parameter is used by Dracut, see:

https://bugzilla.redhat.com/show_bug.cgi?id=1782026#c4

 Thomas
Peter Oberparleiter Dec. 16, 2019, 1:43 p.m. UTC | #5
On 16.12.2019 12:29, Christian Borntraeger wrote:
> 
> 
> On 16.12.19 12:24, Thomas Huth wrote:
>>  Note: I've marked the patch as RFC since I'm not quite sure whether
>>  this is really the right way to address this issue: It's unfortunate
>>  that we have to mess with different location in ZIPL which might also
>>  change again in the future.

Having QEMU or any other tooling rely on undocumented on-disk format
specifics of zipl is definitely wrong and prone to break with the next
change. This is _not_ an ABI.

>>  As suggested by Christian on IRC last week,
>>  maybe it would make more sense to change ZIPL to add this parameter
>>  already when zipl is installed (i.e. by the Linux userspace "zipl" pro-
>>  gram), instead of adding it during boot time? Also, the BOOT_IMAGE para-
>>  meter on s390x is quite different from the BOOT_IMAGE paramter that is
>>  used on x86 - while s390x only uses one single number here, the x86
>>  variant (added by grub2, I guess) uses the boot device + full filename
>>  of the kernel on the boot partition. Should we maybe make the s390x
>>  variant more conform to x86? If so, I think this really has to be fixed
>>  in zipl userspace tool, and not in the s390-ccw bios (and zipl stage3
>>  bootloader).
> 
> Yes, I actually think we should revisit the whole BOOT_IMAGE scheme on s390.
> Maybe we should use the kernel name, or the name of the boot menu entry.
> And maybe we should not use 0 (when the default is running) but instead
> really use to what 0 points to.

BOOT_IMAGE on s390 currently only exists for DASD, so any tooling that
relies on it today would be broken for SCSI boot. The equivalent
information for SCSI would be the boot program selector at
/sys/firmware/ipl/bootprog. There is currently no other way to get this
information when booting from DASD.

Also note that the format of BOOT_IMAGE is dependent on the boot loader
that created it. The use of the menu number (and 0 for default) has the
advantage that this number can be used, e.g. to select the same number
for the next boot using the LOADPARM. Changing BOOT_IMAGE to show the
kernel name would take away that use case.

At this time I would suggest to start by identifying any current users
of BOOT_IMAGE and to understand what their actual requirement is. Once
that information is available, we can think about how this requirement
could best be implemented. Looking at the dracut link it seems like
their requirement cannot be met at all with the information currently
provided on s390 via the BOOT_IMAGE parameter.

Patch
diff mbox series

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index d13b7cbd15..bc7fa597b4 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -49,6 +49,56 @@  static inline bool is_iso_vd_valid(IsoVolDesc *vd)
            vd->type <= VOL_DESC_TYPE_PARTITION;
 }
 
+/**
+ * The ZIPL boot loader adds a BOOT_IMAGE=x to the kernel parameters
+ * (where x is the number of the selected boot entry). Since some
+ * programs might rely on this parameter, we mimic this behavior here.
+ */
+static void add_boot_image_param(uint64_t start_addr, int index)
+{
+    /* "BOOT_IMAGE=00" in EBCDIC */
+    char bootimg_str[15] = {
+        0xc2, 0xd6, 0xd6, 0xe3, 0x6d, 0xc9, 0xd4, 0xc1, 0xc7, 0xc5, 0x7e,
+        0xf0, 0xf0, 0
+    };
+
+    /* Only do it for Linux images */
+    if (memcmp((char *)LINUX_MAGIC_ADDR, "S390EP", 6) != 0) {
+        return;
+    }
+
+    if (index < 10) {
+        bootimg_str[11] = 0xf0 + index;  /* 0xf0 is '0' in EBCDIC */
+        bootimg_str[12] = 0;
+    } else if (index < 100) {
+        bootimg_str[11] = 0xf0 + index / 10;
+        bootimg_str[12] = 0xf0 + index % 10;
+    } else {
+        /* This should never happen since index should be < MAX_BOOT_ENTRIES */
+        panic("BOOT_IMAGE index too big");
+    }
+
+    /*
+     * Now write the parameter to the COMMAND_LINE_EXTRA area of the zipl
+     * stage3 boot loader that we are going to run. Unfortunately, the
+     * location of this area changed in the course of time, but we can
+     * use the stage3 start address to determine which area we have to
+     * use (unless it is zipl v2.9 - the start address already has changed
+     * there but the area has not been moved yet ... so for this version
+     * we are writing the parameters into the unused stack area instead
+     * and thus the BOOT_PARAM won't show up there)
+     */
+    if ((start_addr & 0x7fffffff) == 0xa050) {
+        *(uint64_t *)0xa020 = true;
+        memcpy((char *)0xa000 - 0x400, bootimg_str, sizeof(bootimg_str));
+    } else if ((start_addr & 0x7fffffff) == 0xa000) {
+        *(uint64_t *)0x9020 = true;
+        memcpy((char *)0xe000, bootimg_str, sizeof(bootimg_str));
+    } else {
+        sclp_print("\nWarning: Unsupported ZIPL stage 3 start address.\n");
+    }
+}
+
 /***********************************************************************
  * IPL an ECKD DASD (CDL or LDL/CMS format)
  */
@@ -480,7 +530,7 @@  static void zipl_load_segment(ComponentEntry *entry)
 }
 
 /* Run a zipl program */
-static void zipl_run(ScsiBlockPtr *pte)
+static void zipl_run(ScsiBlockPtr *pte, int loadparm)
 {
     ComponentHeader *header;
     ComponentEntry *entry;
@@ -515,6 +565,8 @@  static void zipl_run(ScsiBlockPtr *pte)
 
     IPL_assert(entry->component_type == ZIPL_COMP_ENTRY_EXEC, "No EXEC entry");
 
+    add_boot_image_param(entry->load_address, loadparm);
+
     /* should not return */
     jump_to_IPL_code(entry->load_address);
 }
@@ -565,7 +617,7 @@  static void ipl_scsi(void)
     IPL_assert(loadparm < MAX_BOOT_ENTRIES, "loadparm value greater than"
                " maximum number of boot entries allowed");
 
-    zipl_run(&prog_table->entry[loadparm].scsi); /* no return */
+    zipl_run(&prog_table->entry[loadparm].scsi, loadparm); /* no return */
 }
 
 /***********************************************************************
diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
index 266f1502b9..36090631f9 100644
--- a/pc-bios/s390-ccw/jump2ipl.c
+++ b/pc-bios/s390-ccw/jump2ipl.c
@@ -77,7 +77,7 @@  void jump_to_low_kernel(void)
      * 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)) {
+    if (!memcmp((char *)LINUX_MAGIC_ADDR, "S390EP", 6)) {
         jump_to_IPL_code(KERN_IMAGE_START);
     }
 
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 11bce7d73c..3e23c3c400 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -46,6 +46,8 @@  typedef unsigned long long __u64;
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
 
+#define LINUX_MAGIC_ADDR  0x010008UL
+
 #include "cio.h"
 #include "iplb.h"