On 6/9/26 11:44 AM, Zhuoying Cai wrote:
Thanks for the feedback and suggestion!
On 6/1/26 4:24 PM, Jared Rossi wrote:
On 5/5/26 4:18 PM, Zhuoying Cai wrote:
Enable secure IPL in audit mode, which performs signature verification,
but any error does not terminate the boot process. Only warnings will be
logged to the console instead.
Add a comp_len variable to store the length of a segment in
zipl_load_segment. comp_len variable is necessary to store the
calculated segment length and is used during signature verification.
Return the length on success, or a negative return code on failure.
Secure IPL in audit mode requires at least one certificate provided in
the key store along with necessary facilities (Secure IPL Facility,
Certificate Store Facility and secure IPL extension support).
Note: Secure IPL in audit mode is implemented for the SCSI scheme of
virtio-blk/virtio-scsi devices.
Signed-off-by: Zhuoying Cai <[email protected]>
---
docs/system/s390x/secure-ipl.rst | 15 ++
pc-bios/s390-ccw/Makefile | 2 +-
pc-bios/s390-ccw/bootmap.c | 16 ++
pc-bios/s390-ccw/bootmap.h | 9 +
pc-bios/s390-ccw/main.c | 10 +-
pc-bios/s390-ccw/s390-ccw.h | 24 ++
pc-bios/s390-ccw/sclp.c | 27 +++
pc-bios/s390-ccw/sclp.h | 6 +
pc-bios/s390-ccw/secure-ipl.c | 364 +++++++++++++++++++++++++++++++
pc-bios/s390-ccw/secure-ipl.h | 113 ++++++++++
10 files changed, 584 insertions(+), 2 deletions(-)
create mode 100644 pc-bios/s390-ccw/secure-ipl.c
create mode 100644 pc-bios/s390-ccw/secure-ipl.h
[...]
diff --git a/pc-bios/s390-ccw/secure-ipl.c b/pc-bios/s390-ccw/secure-ipl.c
new file mode 100644
index 0000000000..6e943446a7
--- /dev/null
+++ b/pc-bios/s390-ccw/secure-ipl.c
@@ -0,0 +1,364 @@
+/*
+ * S/390 Secure IPL
+ *
+ * Functions to support IPL in secure boot mode (DIAG 320, DIAG 508,
+ * signature verification, and certificate handling).
+ *
+ * For secure IPL overview: docs/system/s390x/secure-ipl.rst
+ * For secure IPL technical: docs/specs/s390x-secure-ipl.rst
+ *
+ * Copyright 2025 IBM Corp.
+ * Author(s): Zhuoying Cai <[email protected]>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+#include "bootmap.h"
+#include "s390-ccw.h"
+#include "sclp.h"
+#include "secure-ipl.h"
+
+static VCStorageSizeBlock vcssb __attribute__((__aligned__(8)));
+
+VCStorageSizeBlock *zipl_secure_get_vcssb(void)
+{
+ /* avoid retrieving vcssb multiple times */
+ if (vcssb.length == VCSSB_MIN_LEN) {
+ return &vcssb;
+ }
I don't quite understand what is happening with the VCSSB_MIN_LENGTH
here. Why
is the minimum length significant, as opposed to simply non-zero?
VCSSB must be set to VCSSB_MIN_LENGTH (128), as this represents the size
of the VCSSB struct and indicates that it is present. Any other value
would be considered an invalid length for VCSSB.
To avoid confusion, we could consider renaming VCSSB_MIN_LENGTH to
VCSSB_LEN.
Maybe it could be VCSSB_LEN_VALID? This would make it more clear that it
is a specific size we expect that any valid VCSSB will conform to.
+
+ vcssb.length = VCSSB_MIN_LEN;
+ if (_diag320(&vcssb, DIAG_320_SUBC_QUERY_VCSI) != DIAG_320_RC_OK) {
+ vcssb.length = 0;
+ return NULL;
+ }
+
+ return &vcssb;
+}
+
+static uint32_t get_total_certs_length(void)
+{
+ if (zipl_secure_get_vcssb() == NULL) {
+ return 0;
+ }
Should we check if vcssb.len == 4 for the early return? I think it
designates
an empty cert store and nothing else gets initialized.
Yes, this check is currently in [PATCH v11 27/32] pc-bios/s390-ccw:
Handle true secure IPL mode. I’ll refactor it so it can be performed
earlier during the secure boot process.
+
+ return vcssb.total_vcb_len - sizeof(VCBlockHeader) -
+ vcssb.total_vc_ct * sizeof(VCEntryHeader);
+}
+
+static uint32_t request_certificate(uint8_t *cert_buf, uint8_t index)
+{
+ VCEntryHeader *vce_hdr;
+ struct vcb {
+ VCBlockHeader vcb_hdr;
+ struct vce {
+ VCEntryHeader vce_hdr;
+ uint8_t cert_buf[CERT_BUF_MAX_LEN];
+ } vce;
+ } __attribute__((__aligned__(4096))) vcb = { 0 };
Regarding Collin's comments about using CERT_BUF_MAX_LEN versus
vcssb.max_single_vcb_len,
because the VCB must be 4k aligned we didn't want to malloc a variable
length
buffer, which is why the constant max was used. AFAIK malloc cannot enforce
any alignment requirements and we do not have the libraries for memalign
in BIOS.
I thought I saw that the cert store does already reject certs exceeding
this size,
but if not, I agree with Collin that it should.
Maybe we could use vcssb.max_single_vcb_len - sizeof(VCBlockHeader) -
sizeof(VCEntryHeader) instead of CERT_BUF_MAX_LEN when sizing cert_buf.
The cert store already rejects anything larger than CERT_BUF_MAX_LEN, so
the requested size is guaranteed to stay within bounds.
Since vcssb.max_single_vcb_len is also used as the input length for the
VCB, basing cert_buf on it would make the buffer size more accurate.
Does this sound reasonable to you?
Isn't the objective of using static allocation to make the 4K alignment
easier? I'm not strictly opposed to using dynamic sizing, but it seems to
add a lot of complexity for only a little benefit. You are correct that
using the max size will have some excess space allocated, but this is just
a temporary buffer, right? The data the IIRB points and what the kernel
reads will be the correct size in the end.
Regards,
Jared Rossi
