Re: [PATCH v13 23/33] pc-bios/s390-ccw: Add signature verification for secure IPL in audit mode

[Eric Farman <farman@linux.ibm.com>]

On Fri, 2026-07-03 at 09:56 -0400, Zhuoying Cai wrote:
> On 7/3/26 7:40 AM, Eric Farman wrote:
> > On Thu, 2026-07-02 at 22:29 -0400, 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.
> > > 
> > > 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 <zycai@linux.ibm.com>
> > > ---
> > >  docs/system/s390x/secure-ipl.rst |  15 ++
> > >  hw/s390x/ipl.c                   |   9 +
> > >  pc-bios/s390-ccw/Makefile        |   2 +-
> > >  pc-bios/s390-ccw/bootmap.c       |  27 +++
> > >  pc-bios/s390-ccw/bootmap.h       |   9 +
> > >  pc-bios/s390-ccw/jump2ipl.c      |   7 +
> > >  pc-bios/s390-ccw/main.c          |  18 +-
> > >  pc-bios/s390-ccw/s390-ccw.h      |  20 ++
> > >  pc-bios/s390-ccw/sclp.c          |  27 +++
> > >  pc-bios/s390-ccw/sclp.h          |   6 +
> > >  pc-bios/s390-ccw/secure-ipl.c    | 363 +++++++++++++++++++++++++++++++
> > >  pc-bios/s390-ccw/secure-ipl.h    | 115 ++++++++++
> > >  12 files changed, 616 insertions(+), 2 deletions(-)
> > >  create mode 100644 pc-bios/s390-ccw/secure-ipl.c
> > >  create mode 100644 pc-bios/s390-ccw/secure-ipl.h
> > > 
> > 
> > ...snip...
> > 
> > > diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> > > index cd3d0776b0..7484429c9a 100644
> > > --- a/pc-bios/s390-ccw/main.c
> > > +++ b/pc-bios/s390-ccw/main.c
> > > @@ -20,6 +20,7 @@
> > >  #include "dasd-ipl.h"
> > >  #include "clp.h"
> > >  #include "virtio-pci.h"
> > > +#include "secure-ipl.h"
> > >  
> > >  static SubChannelId blk_schid = { .one = 1 };
> > >  static char loadparm_str[LOADPARM_LEN + 1];
> > > @@ -383,6 +384,8 @@ static void probe_boot_device(void)
> > >  
> > >  void main(void)
> > >  {
> > > +    int vcssb_len;
> > > +
> > >      iplb = &ipl_blocks.iplb;
> > >  
> > >      copy_qipl();
> > > @@ -394,7 +397,20 @@ void main(void)
> > >          probe_boot_device();
> > >      }
> > >  
> > > -    boot_mode = ZIPL_BOOT_MODE_NORMAL;
> > > +    boot_mode = get_boot_mode(iplb->hdr_flags);
> > > +    switch (boot_mode) {
> > > +    case ZIPL_BOOT_MODE_SECURE_AUDIT:
> > > +        if (!secure_ipl_supported()) {
> > > +            panic("Unable to boot in audit mode");
> > > +        }
> > > +
> > > +        vcssb_len = zipl_secure_get_vcssb();
> > > +        if (vcssb_len == 0) {
> > > +            panic("Failed to query certificate storage information!");
> > > +        }
> > 
> > break? fallthrough?
> > 
> 
> Thanks for catching this!
> 
> > > +    default:
> > > +        break;
> > > +    }
> > >  
> > >      while (have_iplb) {
> > >          boot_setup();
> > > diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
> > > index 5420443ad2..ca2737054d 100644
> > > --- a/pc-bios/s390-ccw/s390-ccw.h
> > > +++ b/pc-bios/s390-ccw/s390-ccw.h
> > > @@ -40,6 +40,22 @@ typedef unsigned long long u64;
> > >                              ((b) == 0 ? (a) : (MIN(a, b))))
> > >  #endif
> > >  
> > > +/*
> > > + * Round number down to multiple. Requires that d be a power of 2.
> > > + * Works even if d is a smaller type than n.
> > > + */
> > > +#ifndef ROUND_DOWN
> > > +#define ROUND_DOWN(n, d) ((n) & -(0 ? (n) : (d)))
> > > +#endif
> > > +
> > > +/*
> > > + * Round number up to multiple. Requires that d be a power of 2.
> > > + * Works even if d is a smaller type than n.
> > > + */
> > > +#ifndef ROUND_UP
> > > +#define ROUND_UP(n, d) ROUND_DOWN((n) + (d) - 1, (d))
> > > +#endif
> > > +
> > >  #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
> > >  
> > >  #include "cio.h"
> > > @@ -64,6 +80,8 @@ void sclp_print(const char *string);
> > >  void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask);
> > >  void sclp_setup(void);
> > >  void sclp_get_loadparm_ascii(char *loadparm);
> > > +bool sclp_is_diag320_on(void);
> > > +bool sclp_is_fac_ipl_flag_on(uint16_t fac_ipl_flag);
> > >  int sclp_read(char *str, size_t count);
> > >  
> > >  /* bootmap.c */
> > > @@ -71,9 +89,11 @@ void zipl_load(void);
> > >  
> > >  typedef enum ZiplBootMode {
> > >      ZIPL_BOOT_MODE_NORMAL = 0,
> > > +    ZIPL_BOOT_MODE_SECURE_AUDIT = 1,
> > >  } ZiplBootMode;
> > >  
> > >  extern ZiplBootMode boot_mode;
> > > +ZiplBootMode get_boot_mode(uint8_t hdr_flags);
> > >  
> > >  /* jump2ipl.c */
> > >  void write_reset_psw(uint64_t psw);
> > > diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c
> > > index 4a07de018d..48bdfedf1f 100644
> > > --- a/pc-bios/s390-ccw/sclp.c
> > > +++ b/pc-bios/s390-ccw/sclp.c
> > > @@ -113,6 +113,33 @@ void sclp_get_loadparm_ascii(char *loadparm)
> > >      }
> > >  }
> > >  
> > > +bool sclp_is_diag320_on(void)
> > > +{
> > > +    ReadInfo *sccb = (void *)_sccb;
> > > +
> > > +    memset((char *)_sccb, 0, sizeof(ReadInfo));
> > > +    sccb->h.length = SCCB_SIZE;
> > > +    if (!sclp_service_call(SCLP_CMDW_READ_SCP_INFO, sccb)) {
> > > +        return sccb->fac134 & SCCB_FAC134_DIAG320_BIT;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +/* check if specified IPL facility flag is enabled */
> > > +bool sclp_is_fac_ipl_flag_on(uint16_t fac_ipl_flag)
> > > +{
> > > +    ReadInfo *sccb = (void *)_sccb;
> > > +
> > > +    memset((char *)_sccb, 0, sizeof(ReadInfo));
> > > +    sccb->h.length = SCCB_SIZE;
> > > +    if (!sclp_service_call(SCLP_CMDW_READ_SCP_INFO, sccb)) {
> > > +        return sccb->fac_ipl & fac_ipl_flag;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > >  int sclp_read(char *str, size_t count)
> > >  {
> > >      ReadEventData *sccb = (void *)_sccb;
> > > diff --git a/pc-bios/s390-ccw/sclp.h b/pc-bios/s390-ccw/sclp.h
> > > index 64b53cad29..a8a41cd004 100644
> > > --- a/pc-bios/s390-ccw/sclp.h
> > > +++ b/pc-bios/s390-ccw/sclp.h
> > > @@ -50,6 +50,8 @@ typedef struct SCCBHeader {
> > >  } __attribute__((packed)) SCCBHeader;
> > >  
> > >  #define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader))
> > > +#define SCCB_FAC134_DIAG320_BIT 0x4
> > > +#define SCCB_FAC_IPL_SIPL_BIT   0x4000
> > >  
> > >  typedef struct ReadInfo {
> > >      SCCBHeader h;
> > > @@ -57,6 +59,10 @@ typedef struct ReadInfo {
> > >      uint8_t rnsize;
> > >      uint8_t reserved[13];
> > >      uint8_t loadparm[LOADPARM_LEN];
> > > +    uint8_t reserved1[102];
> > > +    uint8_t fac134;
> > > +    uint8_t reserved2;
> > > +    uint16_t fac_ipl;
> > >  } __attribute__((packed)) ReadInfo;
> > >  
> > >  typedef struct SCCB {
> > > diff --git a/pc-bios/s390-ccw/secure-ipl.c b/pc-bios/s390-ccw/secure-ipl.c
> > > new file mode 100644
> > > index 0000000000..2cfa16fb68
> > > --- /dev/null
> > > +++ b/pc-bios/s390-ccw/secure-ipl.c
> > > @@ -0,0 +1,363 @@
> > > +/*
> > > + * 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 <zycai@linux.ibm.com>
> > > + *
> > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > + */
> > > +
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > +#include <stdio.h>
> > > +#include "s390-ccw.h"
> > > +#include "sclp.h"
> > > +#include "secure-ipl.h"
> > > +
> > > +static VCStorageSizeBlock vcssb __attribute__((__aligned__(8)));
> > > +
> > > +#define for_each_rb_entry(entry, list) \
> > > +    for (entry = (void *)(list) + sizeof((list)->ipl_info_header); \
> > > +         (void *)(entry) + sizeof(*(entry)) <= \
> > > +         (void *)(list) + (list)->ipl_info_header.len; \
> > > +         entry++)
> > > +
> > > +int zipl_secure_get_vcssb(void)
> > > +{
> > > +    /* avoid retrieving vcssb multiple times */
> > > +    if (vcssb.length == VCSSB_LEN_VALID) {
> > > +        goto out;
> > > +    }
> > > +
> > > +    vcssb.length = VCSSB_LEN_VALID;
> > > +    if (_diag320(&vcssb, DIAG_320_SUBC_QUERY_VCSI) != DIAG_320_RC_OK) {
> > > +        vcssb.length = 0;
> > > +    }
> > > +
> > > +out:
> > > +    return vcssb.length;
> > > +}
> > > +
> > > +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__(PAGE_SIZE))) vcb = { 0 };
> > > +
> > > +    /*
> > > +     * Request single entry
> > > +     * Fill input fields of single-entry VCB
> > > +     *
> > > +     * First and last index must be equal because only one
> > > +     * VCE per VCB is currently supported
> > > +     */
> > > +    vcb.vcb_hdr.in_len = ROUND_UP(vcssb.max_single_vcb_len, PAGE_SIZE);
> > 
> > Don't you need to convert max_single_vcb_len from be to cpu?
> > 
> 
> I don't think we need to convert max_single_vcb_len here, but please
> correct me if I'm missing something. If I understand correctly, the
> fields are written and read in big-endian format, so no additional
> conversion should be necessary.

No, you're right... Got my wires crossed about qemu vs bios. Sorry about that.

> 
> > > +    vcb.vcb_hdr.first_vc_index = index;
> > > +    vcb.vcb_hdr.last_vc_index = index;
> > > +
> > > +    if (_diag320(&vcb, DIAG_320_SUBC_STORE_VC) != DIAG_320_RC_OK) {
> > > +        puts("Could not get certificate");
> > > +        return 0;
> > > +    }
> > > +
> > > +    if (vcb.vcb_hdr.out_len == sizeof(VCBlockHeader)) {
> > > +        puts("No certificate entry");
> > > +        return 0;
> > > +    }
> > > +
> > > +    if (vcb.vcb_hdr.remain_ct != 0) {
> > > +        panic("Not enough memory to store requested certificate");
> > > +    }
> > > +
> > > +    vce_hdr = &vcb.vce.vce_hdr;
> > > +    if (!(vce_hdr->flags & DIAG_320_VCE_FLAGS_VALID)) {
> > > +        puts("Invalid certificate");
> > > +        return 0;
> > > +    }
> > > +
> > > +    memcpy(cert_buf, (uint8_t *)&vcb.vce + vce_hdr->cert_offset, vce_hdr->cert_len);
> > > +
> > > +    return vce_hdr->cert_len;
> > > +}
> > > +
> [...]