On 27/09/2024 02.51, jro...@linux.ibm.com wrote:
From: Jared Rossi <jro...@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 <jro...@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
