On 08/12/2025 22.32, Zhuoying Cai wrote:
The current approach to enable secure boot relies on providing
secure-boot and boot-certs parameters of s390-ccw-virtio machine
type option, which apply to all boot devices.
With the possibility of multiple boot devices, secure boot expects all
provided devices to be supported and eligible (e.g.,
virtio-blk/virtio-scsi using the SCSI scheme).
Just double-checking: Wouldn't it be easier to do this on the QEMU side
already? Or is there something in a spec somewhere saying that this has to
be done by the guest?
If multiple boot devices are provided and include an unsupported (e.g.,
ECKD, VFIO) or a non-eligible (e.g., Net) device, the boot process will
terminate with an error logged to the console.
Signed-off-by: Zhuoying Cai <[email protected]>
---
pc-bios/s390-ccw/bootmap.c | 31 +++++++++-------
pc-bios/s390-ccw/main.c | 73 ++++++++++++++++++++++++++++++++++---
pc-bios/s390-ccw/s390-ccw.h | 1 +
3 files changed, 86 insertions(+), 19 deletions(-)
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index cc9a8cec6a..e3c12697e0 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -1139,25 +1139,35 @@ ZiplBootMode zipl_mode(uint8_t hdr_flags)
return ZIPL_BOOT_MODE_NORMAL;
}
+int zipl_check_scsi_mbr_magic(void)
+{
+ ScsiMbr *mbr = (void *)sec;
+
+ /* Grab the MBR */
+ memset(sec, FREE_SPACE_FILLER, sizeof(sec));
Mixing different pointer types that point to the same buffer always make me
nervous - with regards to GCC's strict aliasing optimizations.
(see e.g.
https://stackoverflow.com/questions/98650/what-is-the-strict-aliasing-rule
if you don't know that yet).
Since you're only interested in the MBR magic here, I suggest to replace the
above memset with:
memset(mbr->magic, FREE_SPACE_FILLER, sizeof(mbr->magic));
That's certainly less risky and also faster.
+ if (virtio_read(0, mbr)) {
+ puts("Cannot read block 0");
+ return -EIO;
+ }
+
+ if (!magic_match(mbr->magic, ZIPL_MAGIC)) {
+ return -1;
+ }
+
+ return 0;
+}
+
void zipl_load(void)
{
VDev *vdev = virtio_get_device();
if (vdev->is_cdrom) {
- if (boot_mode == ZIPL_BOOT_MODE_SECURE_AUDIT ||
- boot_mode == ZIPL_BOOT_MODE_SECURE) {
- panic("Secure boot from ISO image is not supported!");
- }
ipl_iso_el_torito();
puts("Failed to IPL this ISO image!");
return;
}
if (virtio_get_device_type() == VIRTIO_ID_NET) {
- if (boot_mode == ZIPL_BOOT_MODE_SECURE_AUDIT ||
- boot_mode == ZIPL_BOOT_MODE_SECURE) {
- panic("Virtio net boot device does not support secure boot!");
- }
netmain();
puts("Failed to IPL from this network!");
return;
@@ -1168,11 +1178,6 @@ void zipl_load(void)
return;
}
- if (boot_mode == ZIPL_BOOT_MODE_SECURE_AUDIT ||
- boot_mode == ZIPL_BOOT_MODE_SECURE) {
- panic("ECKD boot device does not support secure boot!");
- }
-
switch (virtio_get_device_type()) {
case VIRTIO_ID_BLOCK:
zipl_load_vblk();
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 5cea9d3ac7..7ce4761d34 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -271,8 +271,43 @@ static int virtio_setup(void)
return ret;
}
-static void ipl_boot_device(void)
+static void validate_secure_boot_device(void)
+{
+ switch (cutype) {
+ case CU_TYPE_DASD_3990:
+ case CU_TYPE_DASD_2107:
+ panic("Passthrough (vfio) CCW device does not support secure boot!");
+ break;
+ case CU_TYPE_VIRTIO:
+ if (virtio_setup() == 0) {
+ VDev *vdev = virtio_get_device();
+
+ if (vdev->is_cdrom) {
+ panic("Secure boot from ISO image is not supported!");
+ }
+
+ if (virtio_get_device_type() == VIRTIO_ID_NET) {
+ panic("Virtio net boot device does not support secure boot!");
+ }
+
+ if (zipl_check_scsi_mbr_magic()) {
+ panic("ECKD boot device does not support secure boot!");
+ }
+ }
+ break;
+ default:
+ panic("Secure boot from unexpected device type is not supported!");
+ }
+
+ printf("SCSI boot device supports secure boot.\n");
+}
+
+static void check_secure_boot_support(void)
{
+ bool have_iplb_copy;
+ IplParameterBlock *iplb_copy;
+ QemuIplParameters qipl_copy;
+
if (boot_mode == ZIPL_BOOT_MODE_UNSPECIFIED) {
boot_mode = zipl_mode(iplb->hdr_flags);
}
@@ -281,14 +316,38 @@ static void ipl_boot_device(void)
panic("Need at least one certificate for secure boot!");
}
+ if (boot_mode == ZIPL_BOOT_MODE_NORMAL) {
+ return;
+ }
+
+ /*
+ * Store copies of have_iplb, iplb and qipl.
+ * They will be updated in load_next_iplb().
+ */
+ have_iplb_copy = have_iplb;
+ iplb_copy = malloc(sizeof(IplParameterBlock));
Please add:
IPL_assert(iplb_copy != NULL, "out of memory");
or so.
+ memcpy(&qipl_copy, &qipl, sizeof(QemuIplParameters));
+ memcpy(iplb_copy, iplb, sizeof(IplParameterBlock));
+
+ while (have_iplb_copy) {
+ if (have_iplb_copy && find_boot_device()) {
+ validate_secure_boot_device();
+ }
+ have_iplb_copy = load_next_iplb();
+ }
+
+ memcpy(&qipl, &qipl_copy, sizeof(QemuIplParameters));
+ memcpy(iplb, iplb_copy, sizeof(IplParameterBlock));
+
+ free(iplb_copy);
+}
Do we really have to check all devices in advance here? I don't quite get
the logic here yet, if we check the MBR of each and every disk in advance,
that means that each of the devices has to be bootable in secure mode. But
what sense does it make to specify multiple devices if all of them are
bootable, i.e. we will always boot from the first given device?
Should we maybe rather simply disallow multiple boot devices from the QEMU
side already and only allow one bootable device when running in secure mode?
Thomas
+static void ipl_boot_device(void)
+{
switch (cutype) {
case CU_TYPE_DASD_3990:
case CU_TYPE_DASD_2107:
- if (boot_mode == ZIPL_BOOT_MODE_SECURE_AUDIT ||
- boot_mode == ZIPL_BOOT_MODE_SECURE) {
- panic("Passthrough (vfio) CCW device does not support secure
boot!");
- }
-
dasd_ipl(blk_schid, cutype);
break;
case CU_TYPE_VIRTIO:
@@ -338,6 +397,8 @@ void main(void)
probe_boot_device();
}
+ check_secure_boot_support();
+
while (have_iplb) {
boot_setup();
if (have_iplb && find_boot_device()) {
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 389cc8ea7c..3009104686 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -93,6 +93,7 @@ typedef enum ZiplBootMode {
extern ZiplBootMode boot_mode;
ZiplBootMode zipl_mode(uint8_t hdr_flags);
+int zipl_check_scsi_mbr_magic(void);
/* jump2ipl.c */
void write_reset_psw(uint64_t psw);
