Re: [PATCH v9 25/30] pc-bios/s390-ccw: Handle true secure IPL mode

[Collin Walling <walling@linux.ibm.com>]

On 3/5/26 17:41, Zhuoying Cai wrote:
> When secure boot is enabled (-secure-boot on) and certificate(s) are
> provided, the boot operates in True Secure IPL mode.
> 
> Any verification error during True Secure IPL mode will cause the
> entire boot process to terminate.
> 
> Secure IPL in audit mode requires at least one certificate provided in
> the key store along with necessary facilities. If secure boot is enabled
> but no certificate is provided, the boot process will also terminate, as
> this is not a valid secure boot configuration.
> 
> Note: True Secure IPL 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 | 13 +++++++++++++
>  pc-bios/s390-ccw/bootmap.c       |  8 ++++++++
>  pc-bios/s390-ccw/main.c          |  3 +++
>  pc-bios/s390-ccw/s390-ccw.h      |  2 ++
>  pc-bios/s390-ccw/secure-ipl.c    |  4 ++++
>  pc-bios/s390-ccw/secure-ipl.h    |  3 +++
>  6 files changed, 33 insertions(+)
> 
> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
> index 2465f8b26d..e0af086c38 100644
> --- a/docs/system/s390x/secure-ipl.rst
> +++ b/docs/system/s390x/secure-ipl.rst
> @@ -65,3 +65,16 @@ Configuration:
>  .. code-block:: shell
>  
>      qemu-system-s390x -machine s390-ccw-virtio,boot-certs.0.path=/.../qemu/certs,boot-certs.1.path=/another/path/cert.pem ...
> +
> +Secure Mode
> +-----------
> +
> +When the ``secure-boot=on`` option is set and certificates are provided,
> +a secure boot is performed with error reporting enabled. The boot process aborts
> +if any error occurs.
> +
> +Configuration:
> +
> +.. code-block:: shell
> +
> +    qemu-system-s390x -machine s390-ccw-virtio,secure-boot=on,boot-certs.0.path=/.../qemu/certs,boot-certs.1.path=/another/path/cert.pem ...
> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
> index 43a661325f..9a61e989e0 100644
> --- a/pc-bios/s390-ccw/bootmap.c
> +++ b/pc-bios/s390-ccw/bootmap.c
> @@ -738,6 +738,7 @@ static int zipl_run(ScsiBlockPtr *pte)
>      entry = (ComponentEntry *)(&header[1]);
>  
>      switch (boot_mode) {
> +    case ZIPL_BOOT_MODE_SECURE:
>      case ZIPL_BOOT_MODE_SECURE_AUDIT:
>          rc = zipl_run_secure(&entry, tmp_sec);
>          break;
> @@ -1120,9 +1121,16 @@ ZiplBootMode get_boot_mode(uint8_t hdr_flags)
>  {
>      bool sipl_set = hdr_flags & DIAG308_IPIB_FLAGS_SIPL;
>      bool iplir_set = hdr_flags & DIAG308_IPIB_FLAGS_IPLIR;
> +    VCStorageSizeBlock *vcssb;
>  
>      if (!sipl_set && iplir_set) {
>          return ZIPL_BOOT_MODE_SECURE_AUDIT;
> +    } else if (sipl_set && iplir_set) {
> +        vcssb = zipl_secure_get_vcssb();
> +        if (vcssb == NULL || vcssb->length == VCSSB_NO_VC) {
> +            return ZIPL_BOOT_MODE_INVALID;

Is an INVALID mode necessary, especially if the error is going to be
reported immediately after when the function exits?  Might as well just
put the `panic()` here instead.

Otherwise, patch LGTM:

Reviewed-by: Collin Walling <walling@linux.ibm.com>

> +        }
> +        return ZIPL_BOOT_MODE_SECURE;
>      }
>  
>      return ZIPL_BOOT_MODE_NORMAL;
> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> index 106cdf9dec..1678ede8fb 100644
> --- a/pc-bios/s390-ccw/main.c
> +++ b/pc-bios/s390-ccw/main.c
> @@ -329,6 +329,9 @@ void main(void)
>      }
>  
>      boot_mode = get_boot_mode(iplb->hdr_flags);
> +    if (boot_mode == ZIPL_BOOT_MODE_INVALID) {
> +        panic("Need at least one certificate for secure boot!");
> +    }
>  
>      while (have_iplb) {
>          boot_setup();
> diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
> index 7d1a9d4acc..7092942280 100644
> --- a/pc-bios/s390-ccw/s390-ccw.h
> +++ b/pc-bios/s390-ccw/s390-ccw.h
> @@ -96,8 +96,10 @@ int virtio_read(unsigned long sector, void *load_addr);
>  void zipl_load(void);
>  
>  typedef enum ZiplBootMode {
> +    ZIPL_BOOT_MODE_INVALID = -1,
>      ZIPL_BOOT_MODE_NORMAL = 0,
>      ZIPL_BOOT_MODE_SECURE_AUDIT = 1,
> +    ZIPL_BOOT_MODE_SECURE = 2,
>  } ZiplBootMode;
>  
>  extern ZiplBootMode boot_mode;
> diff --git a/pc-bios/s390-ccw/secure-ipl.c b/pc-bios/s390-ccw/secure-ipl.c
> index 840b88a699..76b72fc8f4 100644
> --- a/pc-bios/s390-ccw/secure-ipl.c
> +++ b/pc-bios/s390-ccw/secure-ipl.c
> @@ -288,6 +288,10 @@ static bool check_sclab_presence(uint8_t *sclab_magic,
>      }
>  
>      /* a missing SCLAB will not be reported in audit mode */
> +    if (boot_mode == ZIPL_BOOT_MODE_SECURE) {
> +        zipl_secure_handle("Magic does not match. SCLAB does not exist");
> +    }
> +
>      return false;
>  }
>  
> diff --git a/pc-bios/s390-ccw/secure-ipl.h b/pc-bios/s390-ccw/secure-ipl.h
> index 4e9f4f08b9..1e736d53fe 100644
> --- a/pc-bios/s390-ccw/secure-ipl.h
> +++ b/pc-bios/s390-ccw/secure-ipl.h
> @@ -60,6 +60,9 @@ static inline void zipl_secure_handle(const char *message)
>      case ZIPL_BOOT_MODE_SECURE_AUDIT:
>          IPL_check(false, message);
>          break;
> +    case ZIPL_BOOT_MODE_SECURE:
> +        panic(message);
> +        break;
>      default:
>          break;
>      }
-- 
Regards,
  Collin