| Message ID | 20191216112432.13412-1-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,QEMU] pc-bios/s390-ccw: Add zipl-like "BOOT_IMAGE=x" to the kernel parameters | expand |
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.
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?
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.
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
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.
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"
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(-)