Re: [PATCH v6 24/28] pc-bios/s390-ccw: Handle true secure IPL mode

[Thomas Huth <thuth@redhat.com>]

On 18/09/2025 01.21, 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 | 16 ++++++++++++++++
>   pc-bios/s390-ccw/bootmap.c       | 19 ++++++++++++++++---
>   pc-bios/s390-ccw/main.c          |  7 ++++++-
>   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, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
> index 205de8bc02..579b7b4993 100644
> --- a/docs/system/s390x/secure-ipl.rst
> +++ b/docs/system/s390x/secure-ipl.rst
> @@ -67,3 +67,19 @@ Configuration:
>       qemu-system-s390x -machine s390-ccw-virtio, \
>                                  boot-certs.0.path=/.../qemu/certs, \
>                                  boot-certs.1.path=/another/path/cert.pem ...
> +
> +Secure Mode
> +-----------
> +
> +With *both* the presence of certificates in the store and the ``secure-boot=on``
> +option, it is understood that secure boot should be performed with errors

"it is understood" sounds weird to me here ... maybe rather:

If both, certificates are provided and the ``secure-boot=on`` option has 
been set, a secure boot is performed with error reporting enabled, and the 
boot process will abort on any error.

?

> +reported and boot will abort.
> +
> +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/secure-ipl.c b/pc-bios/s390-ccw/secure-ipl.c
> index cd798c1198..92e3e1e021 100644
> --- a/pc-bios/s390-ccw/secure-ipl.c
> +++ b/pc-bios/s390-ccw/secure-ipl.c
> @@ -287,6 +287,10 @@ static bool check_sclab_presence(uint8_t *sclab_magic,
>           comps->device_entries[comp_index].cei |= S390_IPL_COMPONENT_CEI_INVALID_SCLAB;
>   
>           /* a missing SCLAB will not be reported in audit mode */
> +        if (boot_mode == ZIPL_BOOT_MODE_SECURE) {
> +            zipl_secure_handle("Magic is not matched. SCLAB does not exist");

I'm not a native speaker, but maybe rather "Magic does not match" ?

> +         }

Indentation of the } is off by 1 here.

>           return false;
>       }
>   
> diff --git a/pc-bios/s390-ccw/secure-ipl.h b/pc-bios/s390-ccw/secure-ipl.h
> index 87aa6e1465..d7786158c4 100644
> --- a/pc-bios/s390-ccw/secure-ipl.h
> +++ b/pc-bios/s390-ccw/secure-ipl.h
> @@ -58,6 +58,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:
> +        IPL_assert(false, message);

Using IPL_assert() with false looks weird. Why not simply panic(message) 
instead?

> +        break;
>       default:
>           break;
>       }

  Thomas