| Message ID | 20240927005117.1679506-16-jrossi@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Add Full Boot Order Support | expand |
On 27/09/2024 02.51, jrossi@linux.ibm.com wrote: > From: Jared Rossi <jrossi@linux.ibm.com> > > Build an IPLB for any device with a bootindex (up to a maximum of 8 devices). > > The IPLB chain is placed immediately before the BIOS in memory. Because this > is not a fixed address, the location of the next IPLB and number of remaining > boot devices is stored in the QIPL global variable for possible later access by > the guest during IPL. > > Signed-off-by: Jared Rossi <jrossi@linux.ibm.com> > Note: There's an empty line below your SoB in most of your patch descriptions ... it's just cosmetics, but in case you touch the patch anyway, please remove it. > --- ... > @@ -484,8 +499,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl) > lp = S390_CCW_MACHINE(qdev_get_machine())->loadparm; > } > > - s390_ipl_convert_loadparm((char *)lp, ipl->iplb.loadparm); > - ipl->iplb.flags |= DIAG308_FLAGS_LP_VALID; > + s390_ipl_convert_loadparm((char *)lp, iplb->loadparm); > + iplb->flags |= DIAG308_FLAGS_LP_VALID; > > return true; > } > @@ -493,6 +508,58 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl) > return false; > } > > +static bool s390_init_all_iplbs(S390IPLState *ipl) > +{ > + int iplb_num = 0; > + IplParameterBlock iplb_chain[7]; > + DeviceState *dev_st = get_boot_device(0); > + > + /* > + * Parse the boot devices. Generate an IPLB for the first boot device, > + * which will later be set with DIAG308. Index any fallback boot devices. > + */ The comment looks like it rather belongs to the whole function, and not to the if-statement below? So maybe move it at the top? > + if (!dev_st) { > + ipl->qipl.chain_len = 0; > + return false; > + } > + > + iplb_num = 1; > + s390_build_iplb(dev_st, &ipl->iplb); > + ipl->iplb.flags |= DIAG308_FLAGS_LP_VALID; Isn't DIAG308_FLAGS_LP_VALID set within s390_build_iplb() already? > + while (get_boot_device(iplb_num)) { > + iplb_num++; > + } I'd maybe move the code block below to this spot here, so you've got to assign ipl->qipl.chain_len only once: + if (iplb_num > MAX_IPLB_CHAIN + 1) { + warn_report("Excess boot devices defined! %d boot devices found, " + "but only the first %d will be considered.", + iplb_num, MAX_IPLB_CHAIN + 1); + + iplb_num = MAX_IPLB_CHAIN + 1; + } > + ipl->qipl.chain_len = iplb_num - 1; > + > + /* > + * Build fallback IPLBs for any boot devices above index 0, up to a > + * maximum amount as defined in ipl.h > + */ > + if (iplb_num > 1) { > + if (iplb_num > MAX_IPLB_CHAIN + 1) { > + warn_report("Excess boot devices defined! %d boot devices found, " > + "but only the first %d will be considered.", > + iplb_num, MAX_IPLB_CHAIN + 1); > + > + ipl->qipl.chain_len = MAX_IPLB_CHAIN; > + iplb_num = MAX_IPLB_CHAIN + 1; > + } i.e. move this code block -^ and remove the "ipl->qipl.chain_len = MAX_IPLB_CHAIN;" in there. > + /* Start at 1 because the IPLB for boot index 0 is not chained */ > + for (int i = 1; i < iplb_num; i++) { > + dev_st = get_boot_device(i); > + s390_build_iplb(dev_st, &iplb_chain[i - 1]); > + iplb_chain[i - 1].flags |= DIAG308_FLAGS_LP_VALID; Again, setting DIAG308_FLAGS_LP_VALID should not be necessary since it is already done in s390_build_iplb() ? > + } > + > + ipl->qipl.next_iplb = s390_ipl_map_iplb_chain(iplb_chain); > + } > + > + return iplb_num; > +} > + > static bool is_virtio_ccw_device_of_type(IplParameterBlock *iplb, > int virtio_id) > { > @@ -620,7 +687,7 @@ void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type) > * this is the original boot device's SCSI > * so restore IPL parameter info from it > */ > - ipl->iplb_valid = s390_gen_initial_iplb(ipl); > + ipl->iplb_valid = s390_build_iplb(get_boot_device(0), &ipl->iplb); > } > } > if (reset_type == S390_RESET_MODIFIED_CLEAR || > @@ -714,7 +781,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu) > if (!ipl->kernel || ipl->iplb_valid) { > cpu->env.psw.addr = ipl->bios_start_addr; > if (!ipl->iplb_valid) { > - ipl->iplb_valid = s390_gen_initial_iplb(ipl); > + ipl->iplb_valid = s390_init_all_iplbs(ipl); > + } else { > + ipl->qipl.chain_len = 0; > } > } > s390_ipl_set_boot_menu(ipl); Thomas
On 9/30/24 7:59 AM, Thomas Huth wrote: > On 27/09/2024 02.51, jrossi@linux.ibm.com wrote: >> From: Jared Rossi <jrossi@linux.ibm.com> >> ... >> @@ -484,8 +499,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl) >> lp = S390_CCW_MACHINE(qdev_get_machine())->loadparm; >> } >> - s390_ipl_convert_loadparm((char *)lp, ipl->iplb.loadparm); >> - ipl->iplb.flags |= DIAG308_FLAGS_LP_VALID; >> + s390_ipl_convert_loadparm((char *)lp, iplb->loadparm); >> + iplb->flags |= DIAG308_FLAGS_LP_VALID; >> return true; >> } >> @@ -493,6 +508,58 @@ static bool s390_gen_initial_iplb(S390IPLState >> *ipl) >> return false; >> } >> +static bool s390_init_all_iplbs(S390IPLState *ipl) >> +{ >> + int iplb_num = 0; >> + IplParameterBlock iplb_chain[7]; >> + DeviceState *dev_st = get_boot_device(0); >> + >> + /* >> + * Parse the boot devices. Generate an IPLB for the first boot >> device, >> + * which will later be set with DIAG308. Index any fallback boot >> devices. >> + */ > > The comment looks like it rather belongs to the whole function, and > not to the if-statement below? So maybe move it at the top? Agreed. > >> + if (!dev_st) { >> + ipl->qipl.chain_len = 0; >> + return false; >> + } >> + >> + iplb_num = 1; >> + s390_build_iplb(dev_st, &ipl->iplb); >> + ipl->iplb.flags |= DIAG308_FLAGS_LP_VALID; > > Isn't DIAG308_FLAGS_LP_VALID set within s390_build_iplb() already? > >> + while (get_boot_device(iplb_num)) { >> + iplb_num++; >> + } > > I'd maybe move the code block below to this spot here, so you've got > to assign ipl->qipl.chain_len only once: > > + if (iplb_num > MAX_IPLB_CHAIN + 1) { > + warn_report("Excess boot devices defined! %d boot devices > found, " > + "but only the first %d will be considered.", > + iplb_num, MAX_IPLB_CHAIN + 1); > + > + iplb_num = MAX_IPLB_CHAIN + 1; > + } > >> + ipl->qipl.chain_len = iplb_num - 1; >> + >> + /* >> + * Build fallback IPLBs for any boot devices above index 0, up to a >> + * maximum amount as defined in ipl.h >> + */ >> + if (iplb_num > 1) { >> + if (iplb_num > MAX_IPLB_CHAIN + 1) { >> + warn_report("Excess boot devices defined! %d boot >> devices found, " >> + "but only the first %d will be considered.", >> + iplb_num, MAX_IPLB_CHAIN + 1); >> + >> + ipl->qipl.chain_len = MAX_IPLB_CHAIN; >> + iplb_num = MAX_IPLB_CHAIN + 1; >> + } > > i.e. move this code block -^ > and remove the "ipl->qipl.chain_len = MAX_IPLB_CHAIN;" in there. > >> + /* Start at 1 because the IPLB for boot index 0 is not >> chained */ >> + for (int i = 1; i < iplb_num; i++) { >> + dev_st = get_boot_device(i); >> + s390_build_iplb(dev_st, &iplb_chain[i - 1]); >> + iplb_chain[i - 1].flags |= DIAG308_FLAGS_LP_VALID; > > Again, setting DIAG308_FLAGS_LP_VALID should not be necessary since it > is already done in s390_build_iplb() ? > I’ll see if I can clean section this up and remove redundant flag assignments. Jared Rossi
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h index b670bad551..e4af8e3782 100644 --- a/hw/s390x/ipl.h +++ b/hw/s390x/ipl.h @@ -20,6 +20,7 @@ #include "qom/object.h" #define DIAG308_FLAGS_LP_VALID 0x80 +#define MAX_IPLB_CHAIN 7 /* Max number of fallback boot devices */ void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp); void s390_ipl_fmt_loadparm(uint8_t *loadparm, char *str, Error **errp); diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h index d21a8f91e3..15342ac5cd 100644 --- a/include/hw/s390x/ipl/qipl.h +++ b/include/hw/s390x/ipl/qipl.h @@ -31,7 +31,9 @@ struct QemuIplParameters { uint8_t reserved1[3]; uint64_t reserved2; uint32_t boot_menu_timeout; - uint8_t reserved3[12]; + uint8_t reserved3[2]; + uint16_t chain_len; + uint64_t next_iplb; } QEMU_PACKED; typedef struct QemuIplParameters QemuIplParameters; diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 8a62ff6913..ba66847b9c 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -56,6 +56,13 @@ static bool iplb_extended_needed(void *opaque) return ipl->iplbext_migration; } +/* Place the IPLB chain immediately before the BIOS in memory */ +static uint64_t find_iplb_chain_addr(uint64_t bios_addr, uint16_t count) +{ + return (bios_addr & TARGET_PAGE_MASK) + - (count * sizeof(IplParameterBlock)); +} + static const VMStateDescription vmstate_iplb_extended = { .name = "ipl/iplb_extended", .version_id = 0, @@ -398,6 +405,17 @@ static CcwDevice *s390_get_ccw_device(DeviceState *dev_st, int *devtype) return ccw_dev; } +static uint64_t s390_ipl_map_iplb_chain(IplParameterBlock *iplb_chain) +{ + S390IPLState *ipl = get_ipl_device(); + uint16_t count = ipl->qipl.chain_len; + uint64_t len = sizeof(IplParameterBlock) * count; + uint64_t chain_addr = find_iplb_chain_addr(ipl->bios_start_addr, count); + + cpu_physical_memory_write(chain_addr, iplb_chain, len); + return chain_addr; +} + void s390_ipl_fmt_loadparm(uint8_t *loadparm, char *str, Error **errp) { int i; @@ -428,54 +446,51 @@ void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp) } } -static bool s390_gen_initial_iplb(S390IPLState *ipl) +static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb) { - DeviceState *dev_st; + S390IPLState *ipl = get_ipl_device(); CcwDevice *ccw_dev = NULL; SCSIDevice *sd; int devtype; uint8_t *lp; - dev_st = get_boot_device(0); - if (dev_st) { - ccw_dev = s390_get_ccw_device(dev_st, &devtype); - } - /* * Currently allow IPL only from CCW devices. */ + ccw_dev = s390_get_ccw_device(dev_st, &devtype); if (ccw_dev) { lp = ccw_dev->loadparm; switch (devtype) { case CCW_DEVTYPE_SCSI: sd = SCSI_DEVICE(dev_st); - ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN); - ipl->iplb.blk0_len = + iplb->len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN); + iplb->blk0_len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN - S390_IPLB_HEADER_LEN); - ipl->iplb.pbt = S390_IPL_TYPE_QEMU_SCSI; - ipl->iplb.scsi.lun = cpu_to_be32(sd->lun); - ipl->iplb.scsi.target = cpu_to_be16(sd->id); - ipl->iplb.scsi.channel = cpu_to_be16(sd->channel); - ipl->iplb.scsi.devno = cpu_to_be16(ccw_dev->sch->devno); - ipl->iplb.scsi.ssid = ccw_dev->sch->ssid & 3; + iplb->pbt = S390_IPL_TYPE_QEMU_SCSI; + iplb->scsi.lun = cpu_to_be32(sd->lun); + iplb->scsi.target = cpu_to_be16(sd->id); + iplb->scsi.channel = cpu_to_be16(sd->channel); + iplb->scsi.devno = cpu_to_be16(ccw_dev->sch->devno); + iplb->scsi.ssid = ccw_dev->sch->ssid & 3; break; case CCW_DEVTYPE_VFIO: - ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN); - ipl->iplb.pbt = S390_IPL_TYPE_CCW; - ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno); - ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3; + iplb->len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN); + iplb->pbt = S390_IPL_TYPE_CCW; + iplb->ccw.devno = cpu_to_be16(ccw_dev->sch->devno); + iplb->ccw.ssid = ccw_dev->sch->ssid & 3; break; case CCW_DEVTYPE_VIRTIO_NET: + /* The S390IPLState netboot is true if ANY IPLB may use netboot */ ipl->netboot = true; /* Fall through to CCW_DEVTYPE_VIRTIO case */ case CCW_DEVTYPE_VIRTIO: - ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN); - ipl->iplb.blk0_len = + iplb->len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN); + iplb->blk0_len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN - S390_IPLB_HEADER_LEN); - ipl->iplb.pbt = S390_IPL_TYPE_CCW; - ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno); - ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3; + iplb->pbt = S390_IPL_TYPE_CCW; + iplb->ccw.devno = cpu_to_be16(ccw_dev->sch->devno); + iplb->ccw.ssid = ccw_dev->sch->ssid & 3; break; } @@ -484,8 +499,8 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl) lp = S390_CCW_MACHINE(qdev_get_machine())->loadparm; } - s390_ipl_convert_loadparm((char *)lp, ipl->iplb.loadparm); - ipl->iplb.flags |= DIAG308_FLAGS_LP_VALID; + s390_ipl_convert_loadparm((char *)lp, iplb->loadparm); + iplb->flags |= DIAG308_FLAGS_LP_VALID; return true; } @@ -493,6 +508,58 @@ static bool s390_gen_initial_iplb(S390IPLState *ipl) return false; } +static bool s390_init_all_iplbs(S390IPLState *ipl) +{ + int iplb_num = 0; + IplParameterBlock iplb_chain[7]; + DeviceState *dev_st = get_boot_device(0); + + /* + * Parse the boot devices. Generate an IPLB for the first boot device, + * which will later be set with DIAG308. Index any fallback boot devices. + */ + if (!dev_st) { + ipl->qipl.chain_len = 0; + return false; + } + + iplb_num = 1; + s390_build_iplb(dev_st, &ipl->iplb); + ipl->iplb.flags |= DIAG308_FLAGS_LP_VALID; + + while (get_boot_device(iplb_num)) { + iplb_num++; + } + + ipl->qipl.chain_len = iplb_num - 1; + + /* + * Build fallback IPLBs for any boot devices above index 0, up to a + * maximum amount as defined in ipl.h + */ + if (iplb_num > 1) { + if (iplb_num > MAX_IPLB_CHAIN + 1) { + warn_report("Excess boot devices defined! %d boot devices found, " + "but only the first %d will be considered.", + iplb_num, MAX_IPLB_CHAIN + 1); + + ipl->qipl.chain_len = MAX_IPLB_CHAIN; + iplb_num = MAX_IPLB_CHAIN + 1; + } + + /* Start at 1 because the IPLB for boot index 0 is not chained */ + for (int i = 1; i < iplb_num; i++) { + dev_st = get_boot_device(i); + s390_build_iplb(dev_st, &iplb_chain[i - 1]); + iplb_chain[i - 1].flags |= DIAG308_FLAGS_LP_VALID; + } + + ipl->qipl.next_iplb = s390_ipl_map_iplb_chain(iplb_chain); + } + + return iplb_num; +} + static bool is_virtio_ccw_device_of_type(IplParameterBlock *iplb, int virtio_id) { @@ -620,7 +687,7 @@ void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type) * this is the original boot device's SCSI * so restore IPL parameter info from it */ - ipl->iplb_valid = s390_gen_initial_iplb(ipl); + ipl->iplb_valid = s390_build_iplb(get_boot_device(0), &ipl->iplb); } } if (reset_type == S390_RESET_MODIFIED_CLEAR || @@ -714,7 +781,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu) if (!ipl->kernel || ipl->iplb_valid) { cpu->env.psw.addr = ipl->bios_start_addr; if (!ipl->iplb_valid) { - ipl->iplb_valid = s390_gen_initial_iplb(ipl); + ipl->iplb_valid = s390_init_all_iplbs(ipl); + } else { + ipl->qipl.chain_len = 0; } } s390_ipl_set_boot_menu(ipl);