qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: Zhuoying Cai <zycai@linux.ibm.com>,
	berrange@redhat.com, richard.henderson@linaro.org,
	david@redhat.com, jrossi@linux.ibm.com, qemu-s390x@nongnu.org,
	qemu-devel@nongnu.org
Cc: walling@linux.ibm.com, jjherne@linux.ibm.com,
	pasic@linux.ibm.com, borntraeger@linux.ibm.com,
	farman@linux.ibm.com, mjrosato@linux.ibm.com, iii@linux.ibm.com,
	eblake@redhat.com, armbru@redhat.com, alifm@linux.ibm.com
Subject: Re: [PATCH v6 25/28] pc-bios/s390-ccw: Handle secure boot with multiple boot devices
Date: Mon, 29 Sep 2025 20:11:59 +0200	[thread overview]
Message-ID: <554a136e-bd74-4f06-b9f9-b198422487b9@redhat.com> (raw)
In-Reply-To: <20250917232131.495848-26-zycai@linux.ibm.com>

On 18/09/2025 01.21, 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).
> 
> 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 <zycai@linux.ibm.com>
> ---
>   pc-bios/s390-ccw/bootmap.c  | 31 ++++++++-------
>   pc-bios/s390-ccw/main.c     | 75 ++++++++++++++++++++++++++++++++++---
>   pc-bios/s390-ccw/s390-ccw.h |  1 +
>   3 files changed, 88 insertions(+), 19 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
> index 3ab89b91fb..8297f22c3c 100644
> --- a/pc-bios/s390-ccw/bootmap.c
> +++ b/pc-bios/s390-ccw/bootmap.c
> @@ -1136,25 +1136,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));
> +    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;
> @@ -1165,11 +1175,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 c5b425209a..228b52a37e 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) 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,40 @@ 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));
> +    qipl_copy = malloc(sizeof(QemuIplParameters));

While IplParameterBlock is huge (4k), a malloc is justified for that. But 
QemuIplParameters is just a bunch of bytes (28 if I counted correctly), so I 
think it would be nicer to use a stack variable for that instead.

  Thomas

> +    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(qipl_copy);
> +    free(iplb_copy);
> +}



  reply	other threads:[~2025-09-29 18:13 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-17 23:21 [PATCH v6 00/28] Secure IPL Support for SCSI Scheme of virtio-blk/virtio-scsi Devices Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 01/28] Add boot-certs to s390-ccw-virtio machine type option Zhuoying Cai
2025-09-18  6:56   ` Markus Armbruster
2025-09-18  8:38     ` Daniel P. Berrangé
2025-09-18  8:51       ` Markus Armbruster
2025-09-23  1:31         ` Zhuoying Cai
2025-09-22 23:48       ` Zhuoying Cai
2025-09-29 18:29         ` Collin Walling
2025-10-08 17:49           ` Zhuoying Cai
2025-09-30  9:34   ` Thomas Huth
2025-09-30  9:37     ` Daniel P. Berrangé
2025-09-30  9:43       ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 02/28] crypto/x509-utils: Refactor with GNUTLS fallback Zhuoying Cai
2025-09-18 18:14   ` Farhan Ali
2025-09-30  9:38   ` Thomas Huth
2025-10-02 13:23   ` Daniel P. Berrangé
2025-09-17 23:21 ` [PATCH v6 03/28] crypto/x509-utils: Add helper functions for certificate store Zhuoying Cai
2025-09-18 18:24   ` Farhan Ali
2025-09-30  9:43   ` Thomas Huth
2025-10-02 13:24   ` Daniel P. Berrangé
2025-09-17 23:21 ` [PATCH v6 04/28] hw/s390x/ipl: Create " Zhuoying Cai
2025-09-18 19:46   ` Farhan Ali
2025-09-30 10:26   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 05/28] s390x/diag: Introduce DIAG 320 for Certificate Store Facility Zhuoying Cai
2025-09-18 20:07   ` Farhan Ali
2025-09-30 13:08   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 06/28] s390x/diag: Refactor address validation check from diag308_parm_check Zhuoying Cai
2025-09-18 20:38   ` Farhan Ali
2025-09-30 13:13   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 07/28] s390x/diag: Implement DIAG 320 subcode 1 Zhuoying Cai
2025-09-19 17:20   ` Farhan Ali
2025-09-30 13:30   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 08/28] crypto/x509-utils: Add helper functions for DIAG 320 subcode 2 Zhuoying Cai
2025-09-19 18:02   ` Farhan Ali
2025-10-07  9:34   ` Thomas Huth
2025-10-07  9:38     ` Daniel P. Berrangé
2025-10-07  9:41       ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 09/28] s390x/diag: Implement " Zhuoying Cai
2025-09-24 21:53   ` Farhan Ali
2025-09-26 13:42     ` Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 10/28] s390x/diag: Introduce DIAG 508 for secure IPL operations Zhuoying Cai
2025-09-25 20:50   ` Farhan Ali
2025-10-07  9:47   ` Thomas Huth
2025-10-07 19:46     ` Collin Walling
2025-09-17 23:21 ` [PATCH v6 11/28] crypto/x509-utils: Add helper functions for DIAG 508 subcode 1 Zhuoying Cai
2025-10-07  9:58   ` Thomas Huth
2025-10-07 10:10     ` Daniel P. Berrangé
2025-09-17 23:21 ` [PATCH v6 12/28] s390x/diag: Implement DIAG 508 subcode 1 for signature verification Zhuoying Cai
2025-09-25 21:30   ` Farhan Ali
2025-10-07 10:27   ` Thomas Huth
2025-10-10 16:37     ` Zhuoying Cai
2025-10-10 18:08       ` Thomas Huth
2025-10-07 20:22   ` Collin Walling
2025-09-17 23:21 ` [PATCH v6 13/28] pc-bios/s390-ccw: Introduce IPL Information Report Block (IIRB) Zhuoying Cai
2025-09-25 22:02   ` Farhan Ali
2025-09-17 23:21 ` [PATCH v6 14/28] pc-bios/s390-ccw: Define memory for IPLB and convert IPLB to pointers Zhuoying Cai
2025-09-30  5:17   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 15/28] hw/s390x/ipl: Add IPIB flags to IPL Parameter Block Zhuoying Cai
2025-09-29 21:21   ` Farhan Ali
2025-09-17 23:21 ` [PATCH v6 16/28] s390x: Guest support for Secure-IPL Facility Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 17/28] pc-bios/s390-ccw: Refactor zipl_run() Zhuoying Cai
2025-09-26 12:51   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 18/28] pc-bios/s390-ccw: Rework zipl_load_segment function Zhuoying Cai
2025-09-26 13:02   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 19/28] pc-bios/s390-ccw: Add signature verification for secure IPL in audit mode Zhuoying Cai
2025-09-26 13:10   ` Thomas Huth
2025-09-30 18:42   ` Farhan Ali
2025-10-10 18:00     ` Zhuoying Cai
2025-10-10 19:37       ` Farhan Ali
2025-09-17 23:21 ` [PATCH v6 20/28] s390x: Guest support for Secure-IPL Code Loading Attributes Facility (SCLAF) Zhuoying Cai
2025-09-29 12:25   ` Thomas Huth
2025-09-30 13:06     ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 21/28] pc-bios/s390-ccw: Add additional security checks for secure boot Zhuoying Cai
2025-09-29 13:30   ` Thomas Huth
2025-09-29 20:43     ` Zhuoying Cai
2025-09-30  5:14       ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 22/28] Add secure-boot to s390-ccw-virtio machine type option Zhuoying Cai
2025-09-29 14:05   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 23/28] hw/s390x/ipl: Set IPIB flags for secure IPL Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 24/28] pc-bios/s390-ccw: Handle true secure IPL mode Zhuoying Cai
2025-09-29 15:24   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 25/28] pc-bios/s390-ccw: Handle secure boot with multiple boot devices Zhuoying Cai
2025-09-29 18:11   ` Thomas Huth [this message]
2025-09-17 23:21 ` [PATCH v6 26/28] hw/s390x/ipl: Handle secure boot without specifying a boot device Zhuoying Cai
2025-09-17 23:21 ` [PATCH v6 27/28] docs/specs: Add secure IPL documentation Zhuoying Cai
2025-10-07 11:40   ` Thomas Huth
2025-09-17 23:21 ` [PATCH v6 28/28] docs/system/s390x: " Zhuoying Cai
2025-09-29 18:23   ` Thomas Huth
2025-09-26 12:38 ` [PATCH v6 00/28] Secure IPL Support for SCSI Scheme of virtio-blk/virtio-scsi Devices Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=554a136e-bd74-4f06-b9f9-b198422487b9@redhat.com \
    --to=thuth@redhat.com \
    --cc=alifm@linux.ibm.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=david@redhat.com \
    --cc=eblake@redhat.com \
    --cc=farman@linux.ibm.com \
    --cc=iii@linux.ibm.com \
    --cc=jjherne@linux.ibm.com \
    --cc=jrossi@linux.ibm.com \
    --cc=mjrosato@linux.ibm.com \
    --cc=pasic@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=walling@linux.ibm.com \
    --cc=zycai@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).