Introduce a new `boot-certs` machine type option for the s390-ccw-virtio
machine. This allows users to specify one or more certificate file paths
or directories to be used during secure boot.

Each entry is specified using the syntax:
	boot-certs.<index>.path=/path/to/cert.pem

Multiple paths can be specify using array properties:
	boot-certs.0.path=/path/to/cert.pem,
	boot-certs.1.path=/path/to/cert-dir,
	boot-certs.2.path=/path/to/another-dir...

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 docs/system/s390x/secure-ipl.rst   | 20 ++++++++++++++++++++
 hw/s390x/s390-virtio-ccw.c         | 30 ++++++++++++++++++++++++++++++
 include/hw/s390x/s390-virtio-ccw.h |  2 ++
 qapi/machine-s390x.json            | 24 ++++++++++++++++++++++++
 qemu-options.hx                    |  6 +++++-
 5 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 docs/system/s390x/secure-ipl.rst

diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
new file mode 100644
index 0000000000..9b3fd25cc4
--- /dev/null
+++ b/docs/system/s390x/secure-ipl.rst
@@ -0,0 +1,20 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Secure IPL Command Line Options
+===============================
+
+New parameters have been introduced to s390-ccw-virtio machine type option
+to support secure IPL. These parameters allow users to provide certificates
+and enable secure IPL directly via the command line.
+
+Providing Certificates
+----------------------
+
+The certificate store can be populated by supplying a list of certificate file
+paths or directories on the command-line:
+
+.. code-block:: shell
+
+    qemu-system-s390x -machine s390-ccw-virtio, \
+                               boot-certs.0.path=/.../qemu/certs, \
+                               boot-certs.1.path=/another/path/cert.pem ...
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index c294106a74..9ac425c695 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -45,6 +45,7 @@
 #include "target/s390x/kvm/pv.h"
 #include "migration/blocker.h"
 #include "qapi/visitor.h"
+#include "qapi/qapi-visit-machine-s390x.h"
 #include "hw/s390x/cpu-topology.h"
 #include "kvm/kvm_s390x.h"
 #include "hw/virtio/virtio-md-pci.h"
@@ -798,6 +799,30 @@ static void machine_set_loadparm(Object *obj, Visitor *v,
     g_free(val);
 }
 
+static void machine_get_boot_certs(Object *obj, Visitor *v,
+                                   const char *name, void *opaque,
+                                   Error **errp)
+{
+    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
+    BootCertPathList **certs = &ms->boot_certs;
+
+    visit_type_BootCertPathList(v, name, certs, errp);
+}
+
+static void machine_set_boot_certs(Object *obj, Visitor *v, const char *name,
+                                   void *opaque, Error **errp)
+{
+    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
+    BootCertPathList *cert_list = NULL;
+
+    visit_type_BootCertPathList(v, name, &cert_list, errp);
+    if (!cert_list) {
+        return;
+    }
+
+    ms->boot_certs = cert_list;
+}
+
 static void ccw_machine_class_init(ObjectClass *oc, const void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -851,6 +876,11 @@ static void ccw_machine_class_init(ObjectClass *oc, const void *data)
             "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted"
             " to upper case) to pass to machine loader, boot manager,"
             " and guest kernel");
+
+    object_class_property_add(oc, "boot-certs", "BootCertPath",
+                              machine_get_boot_certs, machine_set_boot_certs, NULL, NULL);
+    object_class_property_set_description(oc, "boot-certs",
+            "provide paths to a directory and/or a certificate file for secure boot");
 }
 
 static inline void s390_machine_initfn(Object *obj)
diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index 526078a4e2..b90949355c 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -14,6 +14,7 @@
 #include "hw/boards.h"
 #include "qom/object.h"
 #include "hw/s390x/sclp.h"
+#include "qapi/qapi-types-machine-s390x.h"
 
 #define TYPE_S390_CCW_MACHINE               "s390-ccw-machine"
 
@@ -31,6 +32,7 @@ struct S390CcwMachineState {
     uint8_t loadparm[8];
     uint64_t memory_limit;
     uint64_t max_pagesize;
+    BootCertPathList *boot_certs;
 
     SCLPDevice *sclp;
 };
diff --git a/qapi/machine-s390x.json b/qapi/machine-s390x.json
index 966dbd61d2..3e89ef8320 100644
--- a/qapi/machine-s390x.json
+++ b/qapi/machine-s390x.json
@@ -119,3 +119,27 @@
 { 'command': 'query-s390x-cpu-polarization', 'returns': 'CpuPolarizationInfo',
   'features': [ 'unstable' ]
 }
+
+##
+# @BootCertPath:
+#
+# Boot certificate path.
+#
+# @path: path of certificate(s)
+#
+# Since: 10.1
+##
+{ 'struct': 'BootCertPath',
+  'data': {'path': 'str'} }
+
+##
+# @BootCerts:
+#
+# List of boot certificate paths.
+#
+# @boot-certs: List of BootCertPath
+#
+# Since: 10.1
+##
+{ 'struct': 'BootCerts',
+  'data': {'boot-certs': ['BootCertPath'] } }
diff --git a/qemu-options.hx b/qemu-options.hx
index ab23f14d21..ac497eb3a0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -44,7 +44,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
 #endif
     "                memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n"
     "                cxl-fmw.0.targets.0=firsttarget,cxl-fmw.0.targets.1=secondtarget,cxl-fmw.0.size=size[,cxl-fmw.0.interleave-granularity=granularity]\n"
-    "                smp-cache.0.cache=cachename,smp-cache.0.topology=topologylevel\n",
+    "                smp-cache.0.cache=cachename,smp-cache.0.topology=topologylevel\n"
+    "                boot-certs.0.path=/path/directory,boot-certs.1.path=/path/file provides paths to a directory and/or a certificate file\n",
     QEMU_ARCH_ALL)
 SRST
 ``-machine [type=]name[,prop=value[,...]]``
@@ -205,6 +206,9 @@ SRST
         ::
 
             -machine smp-cache.0.cache=l1d,smp-cache.0.topology=core,smp-cache.1.cache=l1i,smp-cache.1.topology=core
+
+    ``boot-certs.0.path=/path/directory,boot-certs.1.path=/path/file``
+        Provide paths to a directory and/or a certificate file on the host [s390x only].
 ERST
 
 DEF("M", HAS_ARG, QEMU_OPTION_M,
-- 
2.50.1

Zhuoying Cai <zycai@linux.ibm.com> writes:

> Introduce a new `boot-certs` machine type option for the s390-ccw-virtio
> machine. This allows users to specify one or more certificate file paths
> or directories to be used during secure boot.
>
> Each entry is specified using the syntax:
> 	boot-certs.<index>.path=/path/to/cert.pem
>
> Multiple paths can be specify using array properties:
> 	boot-certs.0.path=/path/to/cert.pem,
> 	boot-certs.1.path=/path/to/cert-dir,
> 	boot-certs.2.path=/path/to/another-dir...
>
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>  docs/system/s390x/secure-ipl.rst   | 20 ++++++++++++++++++++
>  hw/s390x/s390-virtio-ccw.c         | 30 ++++++++++++++++++++++++++++++
>  include/hw/s390x/s390-virtio-ccw.h |  2 ++
>  qapi/machine-s390x.json            | 24 ++++++++++++++++++++++++
>  qemu-options.hx                    |  6 +++++-
>  5 files changed, 81 insertions(+), 1 deletion(-)
>  create mode 100644 docs/system/s390x/secure-ipl.rst
>
> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
> new file mode 100644
> index 0000000000..9b3fd25cc4
> --- /dev/null
> +++ b/docs/system/s390x/secure-ipl.rst
> @@ -0,0 +1,20 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +Secure IPL Command Line Options
> +===============================
> +
> +New parameters have been introduced to s390-ccw-virtio machine type option
> +to support secure IPL. These parameters allow users to provide certificates
> +and enable secure IPL directly via the command line.

All too soon these parameters will no longer be new.  Consider something
like "The s390-ccw-virtio machine type supports secure TPL.  To enable
it, you need to provide certificates."

> +
> +Providing Certificates
> +----------------------
> +
> +The certificate store can be populated by supplying a list of certificate file
> +paths or directories on the command-line:

File is clear enough (use the certificate found in the file).  What does
directory do?

> +
> +.. code-block:: shell
> +
> +    qemu-system-s390x -machine s390-ccw-virtio, \
> +                               boot-certs.0.path=/.../qemu/certs, \
> +                               boot-certs.1.path=/another/path/cert.pem ...
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index c294106a74..9ac425c695 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -45,6 +45,7 @@
>  #include "target/s390x/kvm/pv.h"
>  #include "migration/blocker.h"
>  #include "qapi/visitor.h"
> +#include "qapi/qapi-visit-machine-s390x.h"
>  #include "hw/s390x/cpu-topology.h"
>  #include "kvm/kvm_s390x.h"
>  #include "hw/virtio/virtio-md-pci.h"
> @@ -798,6 +799,30 @@ static void machine_set_loadparm(Object *obj, Visitor *v,
>      g_free(val);
>  }
>  
> +static void machine_get_boot_certs(Object *obj, Visitor *v,
> +                                   const char *name, void *opaque,
> +                                   Error **errp)
> +{
> +    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> +    BootCertPathList **certs = &ms->boot_certs;
> +
> +    visit_type_BootCertPathList(v, name, certs, errp);
> +}
> +
> +static void machine_set_boot_certs(Object *obj, Visitor *v, const char *name,
> +                                   void *opaque, Error **errp)
> +{
> +    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> +    BootCertPathList *cert_list = NULL;
> +
> +    visit_type_BootCertPathList(v, name, &cert_list, errp);
> +    if (!cert_list) {
> +        return;
> +    }
> +
> +    ms->boot_certs = cert_list;
> +}
> +
>  static void ccw_machine_class_init(ObjectClass *oc, const void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -851,6 +876,11 @@ static void ccw_machine_class_init(ObjectClass *oc, const void *data)
>              "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted"
>              " to upper case) to pass to machine loader, boot manager,"
>              " and guest kernel");
> +
> +    object_class_property_add(oc, "boot-certs", "BootCertPath",

Isn't this a BootCertPathList, not a BootCertPath?

> +                              machine_get_boot_certs, machine_set_boot_certs, NULL, NULL);
> +    object_class_property_set_description(oc, "boot-certs",
> +            "provide paths to a directory and/or a certificate file for secure boot");
>  }
>  
>  static inline void s390_machine_initfn(Object *obj)
> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> index 526078a4e2..b90949355c 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -14,6 +14,7 @@
>  #include "hw/boards.h"
>  #include "qom/object.h"
>  #include "hw/s390x/sclp.h"
> +#include "qapi/qapi-types-machine-s390x.h"
>  
>  #define TYPE_S390_CCW_MACHINE               "s390-ccw-machine"
>  
> @@ -31,6 +32,7 @@ struct S390CcwMachineState {
>      uint8_t loadparm[8];
>      uint64_t memory_limit;
>      uint64_t max_pagesize;
> +    BootCertPathList *boot_certs;
>  
>      SCLPDevice *sclp;
>  };
> diff --git a/qapi/machine-s390x.json b/qapi/machine-s390x.json
> index 966dbd61d2..3e89ef8320 100644
> --- a/qapi/machine-s390x.json
> +++ b/qapi/machine-s390x.json
> @@ -119,3 +119,27 @@
>  { 'command': 'query-s390x-cpu-polarization', 'returns': 'CpuPolarizationInfo',
>    'features': [ 'unstable' ]
>  }
> +
> +##
> +# @BootCertPath:
> +#
> +# Boot certificate path.
> +#
> +# @path: path of certificate(s)
> +#
> +# Since: 10.1
> +##

No mention of file vs. directory.

Why do you wrap the file name in a struct?  One possible reason is
providing for future extensions.  Can you think of any?

If you extend it, the name BootCertPath could become suboptimal.
BootCertificate?

> +{ 'struct': 'BootCertPath',
> +  'data': {'path': 'str'} }
> +
> +##
> +# @BootCerts:
> +#
> +# List of boot certificate paths.
> +#
> +# @boot-certs: List of BootCertPath

Anti-pattern: the description text restates the type.

> +#
> +# Since: 10.1
> +##
> +{ 'struct': 'BootCerts',
> +  'data': {'boot-certs': ['BootCertPath'] } }

Where is this type used?

> diff --git a/qemu-options.hx b/qemu-options.hx
> index ab23f14d21..ac497eb3a0 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -44,7 +44,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>  #endif
>      "                memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n"
>      "                cxl-fmw.0.targets.0=firsttarget,cxl-fmw.0.targets.1=secondtarget,cxl-fmw.0.size=size[,cxl-fmw.0.interleave-granularity=granularity]\n"
> -    "                smp-cache.0.cache=cachename,smp-cache.0.topology=topologylevel\n",
> +    "                smp-cache.0.cache=cachename,smp-cache.0.topology=topologylevel\n"
> +    "                boot-certs.0.path=/path/directory,boot-certs.1.path=/path/file provides paths to a directory and/or a certificate file\n",
>      QEMU_ARCH_ALL)
>  SRST
>  ``-machine [type=]name[,prop=value[,...]]``
> @@ -205,6 +206,9 @@ SRST
>          ::
>  
>              -machine smp-cache.0.cache=l1d,smp-cache.0.topology=core,smp-cache.1.cache=l1i,smp-cache.1.topology=core
> +
> +    ``boot-certs.0.path=/path/directory,boot-certs.1.path=/path/file``
> +        Provide paths to a directory and/or a certificate file on the host [s390x only].
>  ERST
>  
>  DEF("M", HAS_ARG, QEMU_OPTION_M,

Thanks for the feedback.

On 9/11/25 3:24 AM, Markus Armbruster wrote:
> Zhuoying Cai <zycai@linux.ibm.com> writes:
> 
>> Introduce a new `boot-certs` machine type option for the s390-ccw-virtio
>> machine. This allows users to specify one or more certificate file paths
>> or directories to be used during secure boot.
>>
>> Each entry is specified using the syntax:
>> 	boot-certs.<index>.path=/path/to/cert.pem
>>
>> Multiple paths can be specify using array properties:
>> 	boot-certs.0.path=/path/to/cert.pem,
>> 	boot-certs.1.path=/path/to/cert-dir,
>> 	boot-certs.2.path=/path/to/another-dir...
>>
>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
>> ---
>>  docs/system/s390x/secure-ipl.rst   | 20 ++++++++++++++++++++
>>  hw/s390x/s390-virtio-ccw.c         | 30 ++++++++++++++++++++++++++++++
>>  include/hw/s390x/s390-virtio-ccw.h |  2 ++
>>  qapi/machine-s390x.json            | 24 ++++++++++++++++++++++++
>>  qemu-options.hx                    |  6 +++++-
>>  5 files changed, 81 insertions(+), 1 deletion(-)
>>  create mode 100644 docs/system/s390x/secure-ipl.rst
>>
>> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
>> new file mode 100644
>> index 0000000000..9b3fd25cc4
>> --- /dev/null
>> +++ b/docs/system/s390x/secure-ipl.rst
>> @@ -0,0 +1,20 @@
>> +.. SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +Secure IPL Command Line Options
>> +===============================
>> +
>> +New parameters have been introduced to s390-ccw-virtio machine type option
>> +to support secure IPL. These parameters allow users to provide certificates
>> +and enable secure IPL directly via the command line.
> 
> All too soon these parameters will no longer be new.  Consider something
> like "The s390-ccw-virtio machine type supports secure TPL.  To enable
> it, you need to provide certificates."
> 
>> +
>> +Providing Certificates
>> +----------------------
>> +
>> +The certificate store can be populated by supplying a list of certificate file
>> +paths or directories on the command-line:
> 
> File is clear enough (use the certificate found in the file).  What does
> directory do?
> 

A directory contains a list of certificate files, and allowing both
files and directories could make the CLI more flexible.

>> +
>> +.. code-block:: shell
>> +
>> +    qemu-system-s390x -machine s390-ccw-virtio, \
>> +                               boot-certs.0.path=/.../qemu/certs, \
>> +                               boot-certs.1.path=/another/path/cert.pem ...
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index c294106a74..9ac425c695 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -45,6 +45,7 @@
>>  #include "target/s390x/kvm/pv.h"
>>  #include "migration/blocker.h"
>>  #include "qapi/visitor.h"
>> +#include "qapi/qapi-visit-machine-s390x.h"
>>  #include "hw/s390x/cpu-topology.h"
>>  #include "kvm/kvm_s390x.h"
>>  #include "hw/virtio/virtio-md-pci.h"
>> @@ -798,6 +799,30 @@ static void machine_set_loadparm(Object *obj, Visitor *v,
>>      g_free(val);
>>  }
>>  
>> +static void machine_get_boot_certs(Object *obj, Visitor *v,
>> +                                   const char *name, void *opaque,
>> +                                   Error **errp)
>> +{
>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>> +    BootCertPathList **certs = &ms->boot_certs;
>> +
>> +    visit_type_BootCertPathList(v, name, certs, errp);
>> +}
>> +
>> +static void machine_set_boot_certs(Object *obj, Visitor *v, const char *name,
>> +                                   void *opaque, Error **errp)
>> +{
>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>> +    BootCertPathList *cert_list = NULL;
>> +
>> +    visit_type_BootCertPathList(v, name, &cert_list, errp);
>> +    if (!cert_list) {
>> +        return;
>> +    }
>> +
>> +    ms->boot_certs = cert_list;
>> +}
>> +
>>  static void ccw_machine_class_init(ObjectClass *oc, const void *data)
>>  {
>>      MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -851,6 +876,11 @@ static void ccw_machine_class_init(ObjectClass *oc, const void *data)
>>              "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted"
>>              " to upper case) to pass to machine loader, boot manager,"
>>              " and guest kernel");
>> +
>> +    object_class_property_add(oc, "boot-certs", "BootCertPath",
> 
> Isn't this a BootCertPathList, not a BootCertPath?
> 

If I understand correctly, would BootCerts also be the correct option to
use here?

>> +                              machine_get_boot_certs, machine_set_boot_certs, NULL, NULL);
>> +    object_class_property_set_description(oc, "boot-certs",
>> +            "provide paths to a directory and/or a certificate file for secure boot");
>>  }
>>  
>>  static inline void s390_machine_initfn(Object *obj)
>> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
>> index 526078a4e2..b90949355c 100644
>> --- a/include/hw/s390x/s390-virtio-ccw.h
>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>> @@ -14,6 +14,7 @@
>>  #include "hw/boards.h"
>>  #include "qom/object.h"
>>  #include "hw/s390x/sclp.h"
>> +#include "qapi/qapi-types-machine-s390x.h"
>>  
>>  #define TYPE_S390_CCW_MACHINE               "s390-ccw-machine"
>>  
>> @@ -31,6 +32,7 @@ struct S390CcwMachineState {
>>      uint8_t loadparm[8];
>>      uint64_t memory_limit;
>>      uint64_t max_pagesize;
>> +    BootCertPathList *boot_certs;
>>  
>>      SCLPDevice *sclp;
>>  };
>> diff --git a/qapi/machine-s390x.json b/qapi/machine-s390x.json
>> index 966dbd61d2..3e89ef8320 100644
>> --- a/qapi/machine-s390x.json
>> +++ b/qapi/machine-s390x.json
>> @@ -119,3 +119,27 @@
>>  { 'command': 'query-s390x-cpu-polarization', 'returns': 'CpuPolarizationInfo',
>>    'features': [ 'unstable' ]
>>  }
>> +
>> +##
>> +# @BootCertPath:
>> +#
>> +# Boot certificate path.
>> +#
>> +# @path: path of certificate(s)
>> +#
>> +# Since: 10.1
>> +##
> 
> No mention of file vs. directory.
> 
> Why do you wrap the file name in a struct?  One possible reason is
> providing for future extensions.  Can you think of any?
> 
> If you extend it, the name BootCertPath could become suboptimal.
> BootCertificate?
> 

I wrapped the path in a struct to follow the array-style structure used
by cxl-fmw and smp-cache options (as Daniel suggested).

```
  It would be something like this on the CLI:


boot-certs.0.path=/path/to/dir,boot-certs.1.path=/to/other/dir,boot-certs.2.path=/some/...
```

This could potentially leave room for future extensions, such as
including details about the certificate (e.g., issuer, hashing
algorithm, etc).

>> +{ 'struct': 'BootCertPath',
>> +  'data': {'path': 'str'} }
>> +
>> +##
>> +# @BootCerts:
>> +#
>> +# List of boot certificate paths.
>> +#
>> +# @boot-certs: List of BootCertPath
> 
> Anti-pattern: the description text restates the type.
> 
>> +#
>> +# Since: 10.1
>> +##
>> +{ 'struct': 'BootCerts',
>> +  'data': {'boot-certs': ['BootCertPath'] } }
> 
> Where is this type used?
> 

Please correct me if I'm wrong, but from what I've seen, it is not used
directly in the implementation. It provides a list property for
BootCertPath, which makes the BootCertPathList definition valid and able
to accept multiple paths. If BootCerts is removed, then BootCertPathList
becomes underfined and results in compilation errors.

>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ab23f14d21..ac497eb3a0 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -44,7 +44,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>>  #endif
>>      "                memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n"
>>      "                cxl-fmw.0.targets.0=firsttarget,cxl-fmw.0.targets.1=secondtarget,cxl-fmw.0.size=size[,cxl-fmw.0.interleave-granularity=granularity]\n"
>> -    "                smp-cache.0.cache=cachename,smp-cache.0.topology=topologylevel\n",
>> +    "                smp-cache.0.cache=cachename,smp-cache.0.topology=topologylevel\n"
>> +    "                boot-certs.0.path=/path/directory,boot-certs.1.path=/path/file provides paths to a directory and/or a certificate file\n",
>>      QEMU_ARCH_ALL)
>>  SRST
>>  ``-machine [type=]name[,prop=value[,...]]``
>> @@ -205,6 +206,9 @@ SRST
>>          ::
>>  
>>              -machine smp-cache.0.cache=l1d,smp-cache.0.topology=core,smp-cache.1.cache=l1i,smp-cache.1.topology=core
>> +
>> +    ``boot-certs.0.path=/path/directory,boot-certs.1.path=/path/file``
>> +        Provide paths to a directory and/or a certificate file on the host [s390x only].
>>  ERST
>>  
>>  DEF("M", HAS_ARG, QEMU_OPTION_M,
>

Zhuoying Cai <zycai@linux.ibm.com> writes:

> Thanks for the feedback.
>
> On 9/11/25 3:24 AM, Markus Armbruster wrote:
>> Zhuoying Cai <zycai@linux.ibm.com> writes:
>> 
>>> Introduce a new `boot-certs` machine type option for the s390-ccw-virtio
>>> machine. This allows users to specify one or more certificate file paths
>>> or directories to be used during secure boot.
>>>
>>> Each entry is specified using the syntax:
>>> 	boot-certs.<index>.path=/path/to/cert.pem
>>>
>>> Multiple paths can be specify using array properties:
>>> 	boot-certs.0.path=/path/to/cert.pem,
>>> 	boot-certs.1.path=/path/to/cert-dir,
>>> 	boot-certs.2.path=/path/to/another-dir...
>>>
>>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
>>> ---
>>>  docs/system/s390x/secure-ipl.rst   | 20 ++++++++++++++++++++
>>>  hw/s390x/s390-virtio-ccw.c         | 30 ++++++++++++++++++++++++++++++
>>>  include/hw/s390x/s390-virtio-ccw.h |  2 ++
>>>  qapi/machine-s390x.json            | 24 ++++++++++++++++++++++++
>>>  qemu-options.hx                    |  6 +++++-
>>>  5 files changed, 81 insertions(+), 1 deletion(-)
>>>  create mode 100644 docs/system/s390x/secure-ipl.rst
>>>
>>> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
>>> new file mode 100644
>>> index 0000000000..9b3fd25cc4
>>> --- /dev/null
>>> +++ b/docs/system/s390x/secure-ipl.rst
>>> @@ -0,0 +1,20 @@
>>> +.. SPDX-License-Identifier: GPL-2.0-or-later
>>> +
>>> +Secure IPL Command Line Options
>>> +===============================
>>> +
>>> +New parameters have been introduced to s390-ccw-virtio machine type option
>>> +to support secure IPL. These parameters allow users to provide certificates
>>> +and enable secure IPL directly via the command line.
>> 
>> All too soon these parameters will no longer be new.  Consider something
>> like "The s390-ccw-virtio machine type supports secure TPL.  To enable
>> it, you need to provide certificates."
>> 
>>> +
>>> +Providing Certificates
>>> +----------------------
>>> +
>>> +The certificate store can be populated by supplying a list of certificate file
>>> +paths or directories on the command-line:
>> 
>> File is clear enough (use the certificate found in the file).  What does
>> directory do?
>
> A directory contains a list of certificate files, and allowing both
> files and directories could make the CLI more flexible.

I figure when @path names a file, it's an error when the file doesn't
contain a valid cetificate.

What is @path names a directory, and one of the directory's files
doesn't contain a valid certificate?

Can a single file contain multiple certificates?

>>> +
>>> +.. code-block:: shell
>>> +
>>> +    qemu-system-s390x -machine s390-ccw-virtio, \
>>> +                               boot-certs.0.path=/.../qemu/certs, \
>>> +                               boot-certs.1.path=/another/path/cert.pem ...
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index c294106a74..9ac425c695 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -45,6 +45,7 @@
>>>  #include "target/s390x/kvm/pv.h"
>>>  #include "migration/blocker.h"
>>>  #include "qapi/visitor.h"
>>> +#include "qapi/qapi-visit-machine-s390x.h"
>>>  #include "hw/s390x/cpu-topology.h"
>>>  #include "kvm/kvm_s390x.h"
>>>  #include "hw/virtio/virtio-md-pci.h"
>>> @@ -798,6 +799,30 @@ static void machine_set_loadparm(Object *obj, Visitor *v,
>>>      g_free(val);
>>>  }
>>>  
>>> +static void machine_get_boot_certs(Object *obj, Visitor *v,
>>> +                                   const char *name, void *opaque,
>>> +                                   Error **errp)
>>> +{
>>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>>> +    BootCertPathList **certs = &ms->boot_certs;
>>> +
>>> +    visit_type_BootCertPathList(v, name, certs, errp);
>>> +}
>>> +
>>> +static void machine_set_boot_certs(Object *obj, Visitor *v, const char *name,
>>> +                                   void *opaque, Error **errp)
>>> +{
>>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>>> +    BootCertPathList *cert_list = NULL;
>>> +
>>> +    visit_type_BootCertPathList(v, name, &cert_list, errp);
>>> +    if (!cert_list) {
>>> +        return;
>>> +    }
>>> +
>>> +    ms->boot_certs = cert_list;
>>> +}
>>> +
>>>  static void ccw_machine_class_init(ObjectClass *oc, const void *data)
>>>  {
>>>      MachineClass *mc = MACHINE_CLASS(oc);
>>> @@ -851,6 +876,11 @@ static void ccw_machine_class_init(ObjectClass *oc, const void *data)
>>>              "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted"
>>>              " to upper case) to pass to machine loader, boot manager,"
>>>              " and guest kernel");
>>> +
>>> +    object_class_property_add(oc, "boot-certs", "BootCertPath",
>> 
>> Isn't this a BootCertPathList, not a BootCertPath?
>
> If I understand correctly, would BootCerts also be the correct option to
> use here?

This is object_class_property_add()'s @type argument.  It's an arbitrary
string, not checked in any way.  When the property's actual type is a
QAPI type, like it is here, then the @type argument should be the name
of the QAPI type.

Questions?

>>> +                              machine_get_boot_certs, machine_set_boot_certs, NULL, NULL);
>>> +    object_class_property_set_description(oc, "boot-certs",
>>> +            "provide paths to a directory and/or a certificate file for secure boot");
>>>  }
>>>  
>>>  static inline void s390_machine_initfn(Object *obj)
>>> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
>>> index 526078a4e2..b90949355c 100644
>>> --- a/include/hw/s390x/s390-virtio-ccw.h
>>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>>> @@ -14,6 +14,7 @@
>>>  #include "hw/boards.h"
>>>  #include "qom/object.h"
>>>  #include "hw/s390x/sclp.h"
>>> +#include "qapi/qapi-types-machine-s390x.h"
>>>  
>>>  #define TYPE_S390_CCW_MACHINE               "s390-ccw-machine"
>>>  
>>> @@ -31,6 +32,7 @@ struct S390CcwMachineState {
>>>      uint8_t loadparm[8];
>>>      uint64_t memory_limit;
>>>      uint64_t max_pagesize;
>>> +    BootCertPathList *boot_certs;
>>>  
>>>      SCLPDevice *sclp;
>>>  };
>>> diff --git a/qapi/machine-s390x.json b/qapi/machine-s390x.json
>>> index 966dbd61d2..3e89ef8320 100644
>>> --- a/qapi/machine-s390x.json
>>> +++ b/qapi/machine-s390x.json
>>> @@ -119,3 +119,27 @@
>>>  { 'command': 'query-s390x-cpu-polarization', 'returns': 'CpuPolarizationInfo',
>>>    'features': [ 'unstable' ]
>>>  }
>>> +
>>> +##
>>> +# @BootCertPath:
>>> +#
>>> +# Boot certificate path.
>>> +#
>>> +# @path: path of certificate(s)
>>> +#
>>> +# Since: 10.1
>>> +##
>> 
>> No mention of file vs. directory.
>> 
>> Why do you wrap the file name in a struct?  One possible reason is
>> providing for future extensions.  Can you think of any?
>> 
>> If you extend it, the name BootCertPath could become suboptimal.
>> BootCertificate?
>> 
>
> I wrapped the path in a struct to follow the array-style structure used
> by cxl-fmw and smp-cache options (as Daniel suggested).
>
> ```
>   It would be something like this on the CLI:
>
>
> boot-certs.0.path=/path/to/dir,boot-certs.1.path=/to/other/dir,boot-certs.2.path=/some/...
> ```
>
> This could potentially leave room for future extensions, such as
> including details about the certificate (e.g., issuer, hashing
> algorithm, etc).

No objections to the wrapping.  I'd prefer naming the struct
BootCertificates.

>>> +{ 'struct': 'BootCertPath',
>>> +  'data': {'path': 'str'} }
>>> +
>>> +##
>>> +# @BootCerts:
>>> +#
>>> +# List of boot certificate paths.
>>> +#
>>> +# @boot-certs: List of BootCertPath
>> 
>> Anti-pattern: the description text restates the type.
>> 
>>> +#
>>> +# Since: 10.1
>>> +##
>>> +{ 'struct': 'BootCerts',
>>> +  'data': {'boot-certs': ['BootCertPath'] } }
>> 
>> Where is this type used?
>
> Please correct me if I'm wrong, but from what I've seen, it is not used
> directly in the implementation. It provides a list property for
> BootCertPath, which makes the BootCertPathList definition valid and able
> to accept multiple paths. If BootCerts is removed, then BootCertPathList
> becomes underfined and results in compilation errors.

Aha!

Your QOM property setter and getter need visit_type_BootCertPathList().

The QAPI generator generates code for BootCertPathList, including
visit_type_BootCertPathList(), only when it knows it's actually used.
It can only see uses within the QAPI schema.  So, when ['BootCertPath']
doesn't occur there, visit_type_BootCertPathList() won't exist, and your
QOM code won't compile.

Since you don't actually need BootCertPathList in the schema, you need
to add an artifical use just to get it generated.  The common way to do
that is using it in a dummy type like this:

    ##
    # @DummyForceS390Arrays:
    #
    # Not used by QMP; hack to let us use BootCertPathList internally
    #
    # Since: 10.2
    ##
    { 'struct': 'DummyForceArrays',
      'data': { 'unused': ['BootCertPath'] } }

You also need to add it to pragme documentation-exceptions is
pragma.json.

Yes, this is clunky.  Sorry :)

[...]

On 9/12/25 2:42 AM, Markus Armbruster wrote:
> Zhuoying Cai <zycai@linux.ibm.com> writes:
> 
>> Thanks for the feedback.
>>
>> On 9/11/25 3:24 AM, Markus Armbruster wrote:
>>> Zhuoying Cai <zycai@linux.ibm.com> writes:
>>>
>>>> Introduce a new `boot-certs` machine type option for the s390-ccw-virtio
>>>> machine. This allows users to specify one or more certificate file paths
>>>> or directories to be used during secure boot.
>>>>
>>>> Each entry is specified using the syntax:
>>>> 	boot-certs.<index>.path=/path/to/cert.pem
>>>>
>>>> Multiple paths can be specify using array properties:
>>>> 	boot-certs.0.path=/path/to/cert.pem,
>>>> 	boot-certs.1.path=/path/to/cert-dir,
>>>> 	boot-certs.2.path=/path/to/another-dir...
>>>>
>>>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
>>>> ---
>>>>  docs/system/s390x/secure-ipl.rst   | 20 ++++++++++++++++++++
>>>>  hw/s390x/s390-virtio-ccw.c         | 30 ++++++++++++++++++++++++++++++
>>>>  include/hw/s390x/s390-virtio-ccw.h |  2 ++
>>>>  qapi/machine-s390x.json            | 24 ++++++++++++++++++++++++
>>>>  qemu-options.hx                    |  6 +++++-
>>>>  5 files changed, 81 insertions(+), 1 deletion(-)
>>>>  create mode 100644 docs/system/s390x/secure-ipl.rst
>>>>
>>>> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
>>>> new file mode 100644
>>>> index 0000000000..9b3fd25cc4
>>>> --- /dev/null
>>>> +++ b/docs/system/s390x/secure-ipl.rst
>>>> @@ -0,0 +1,20 @@
>>>> +.. SPDX-License-Identifier: GPL-2.0-or-later
>>>> +
>>>> +Secure IPL Command Line Options
>>>> +===============================
>>>> +
>>>> +New parameters have been introduced to s390-ccw-virtio machine type option
>>>> +to support secure IPL. These parameters allow users to provide certificates
>>>> +and enable secure IPL directly via the command line.
>>>
>>> All too soon these parameters will no longer be new.  Consider something
>>> like "The s390-ccw-virtio machine type supports secure TPL.  To enable
>>> it, you need to provide certificates."
>>>
>>>> +
>>>> +Providing Certificates
>>>> +----------------------
>>>> +
>>>> +The certificate store can be populated by supplying a list of certificate file
>>>> +paths or directories on the command-line:
>>>
>>> File is clear enough (use the certificate found in the file).  What does
>>> directory do?
>>
>> A directory contains a list of certificate files, and allowing both
>> files and directories could make the CLI more flexible.
> 
> I figure when @path names a file, it's an error when the file doesn't
> contain a valid cetificate.
> 
> What is @path names a directory, and one of the directory's files
> doesn't contain a valid certificate?
> 
> Can a single file contain multiple certificates?
> 

A certificate file path is expected to contain exactly one certificate.

Certificates provided through the CLI, whether as individual files or
within a directory, are validated before use. If a certificate is
invalid (e.g., unsupported format), it will be skipped and not added to
the S390 certificate store.

When iterating through the provided paths, the program will terminate on
fatal configuration errors, such as when a specified path is neither a
file nor a directory.

>>>> +
>>>> +.. code-block:: shell
>>>> +
>>>> +    qemu-system-s390x -machine s390-ccw-virtio, \
>>>> +                               boot-certs.0.path=/.../qemu/certs, \
>>>> +                               boot-certs.1.path=/another/path/cert.pem ...
>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>> index c294106a74..9ac425c695 100644
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -45,6 +45,7 @@
>>>>  #include "target/s390x/kvm/pv.h"
>>>>  #include "migration/blocker.h"
>>>>  #include "qapi/visitor.h"
>>>> +#include "qapi/qapi-visit-machine-s390x.h"
>>>>  #include "hw/s390x/cpu-topology.h"
>>>>  #include "kvm/kvm_s390x.h"
>>>>  #include "hw/virtio/virtio-md-pci.h"
>>>> @@ -798,6 +799,30 @@ static void machine_set_loadparm(Object *obj, Visitor *v,
>>>>      g_free(val);
>>>>  }
>>>>  
>>>> +static void machine_get_boot_certs(Object *obj, Visitor *v,
>>>> +                                   const char *name, void *opaque,
>>>> +                                   Error **errp)
>>>> +{
>>>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>>>> +    BootCertPathList **certs = &ms->boot_certs;
>>>> +
>>>> +    visit_type_BootCertPathList(v, name, certs, errp);
>>>> +}
>>>> +
>>>> +static void machine_set_boot_certs(Object *obj, Visitor *v, const char *name,
>>>> +                                   void *opaque, Error **errp)
>>>> +{
>>>> +    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>>>> +    BootCertPathList *cert_list = NULL;
>>>> +
>>>> +    visit_type_BootCertPathList(v, name, &cert_list, errp);
>>>> +    if (!cert_list) {
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    ms->boot_certs = cert_list;
>>>> +}
>>>> +
>>>>  static void ccw_machine_class_init(ObjectClass *oc, const void *data)
>>>>  {
>>>>      MachineClass *mc = MACHINE_CLASS(oc);
>>>> @@ -851,6 +876,11 @@ static void ccw_machine_class_init(ObjectClass *oc, const void *data)
>>>>              "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted"
>>>>              " to upper case) to pass to machine loader, boot manager,"
>>>>              " and guest kernel");
>>>> +
>>>> +    object_class_property_add(oc, "boot-certs", "BootCertPath",
>>>
>>> Isn't this a BootCertPathList, not a BootCertPath?
>>
>> If I understand correctly, would BootCerts also be the correct option to
>> use here?
> 
> This is object_class_property_add()'s @type argument.  It's an arbitrary
> string, not checked in any way.  When the property's actual type is a
> QAPI type, like it is here, then the @type argument should be the name
> of the QAPI type.
> 
> Questions?
> 
>>>> +                              machine_get_boot_certs, machine_set_boot_certs, NULL, NULL);
>>>> +    object_class_property_set_description(oc, "boot-certs",
>>>> +            "provide paths to a directory and/or a certificate file for secure boot");
>>>>  }
>>>>  
>>>>  static inline void s390_machine_initfn(Object *obj)
>>>> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
>>>> index 526078a4e2..b90949355c 100644
>>>> --- a/include/hw/s390x/s390-virtio-ccw.h
>>>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>>>> @@ -14,6 +14,7 @@
>>>>  #include "hw/boards.h"
>>>>  #include "qom/object.h"
>>>>  #include "hw/s390x/sclp.h"
>>>> +#include "qapi/qapi-types-machine-s390x.h"
>>>>  
>>>>  #define TYPE_S390_CCW_MACHINE               "s390-ccw-machine"
>>>>  
>>>> @@ -31,6 +32,7 @@ struct S390CcwMachineState {
>>>>      uint8_t loadparm[8];
>>>>      uint64_t memory_limit;
>>>>      uint64_t max_pagesize;
>>>> +    BootCertPathList *boot_certs;
>>>>  
>>>>      SCLPDevice *sclp;
>>>>  };
>>>> diff --git a/qapi/machine-s390x.json b/qapi/machine-s390x.json
>>>> index 966dbd61d2..3e89ef8320 100644
>>>> --- a/qapi/machine-s390x.json
>>>> +++ b/qapi/machine-s390x.json
>>>> @@ -119,3 +119,27 @@
>>>>  { 'command': 'query-s390x-cpu-polarization', 'returns': 'CpuPolarizationInfo',
>>>>    'features': [ 'unstable' ]
>>>>  }
>>>> +
>>>> +##
>>>> +# @BootCertPath:
>>>> +#
>>>> +# Boot certificate path.
>>>> +#
>>>> +# @path: path of certificate(s)
>>>> +#
>>>> +# Since: 10.1
>>>> +##
>>>
>>> No mention of file vs. directory.
>>>
>>> Why do you wrap the file name in a struct?  One possible reason is
>>> providing for future extensions.  Can you think of any?
>>>
>>> If you extend it, the name BootCertPath could become suboptimal.
>>> BootCertificate?
>>>
>>
>> I wrapped the path in a struct to follow the array-style structure used
>> by cxl-fmw and smp-cache options (as Daniel suggested).
>>
>> ```
>>   It would be something like this on the CLI:
>>
>>
>> boot-certs.0.path=/path/to/dir,boot-certs.1.path=/to/other/dir,boot-certs.2.path=/some/...
>> ```
>>
>> This could potentially leave room for future extensions, such as
>> including details about the certificate (e.g., issuer, hashing
>> algorithm, etc).
> 
> No objections to the wrapping.  I'd prefer naming the struct
> BootCertificates.
> 

I'll update this in the next version.

>>>> +{ 'struct': 'BootCertPath',
>>>> +  'data': {'path': 'str'} }
>>>> +
>>>> +##
>>>> +# @BootCerts:
>>>> +#
>>>> +# List of boot certificate paths.
>>>> +#
>>>> +# @boot-certs: List of BootCertPath
>>>
>>> Anti-pattern: the description text restates the type.
>>>
>>>> +#
>>>> +# Since: 10.1
>>>> +##
>>>> +{ 'struct': 'BootCerts',
>>>> +  'data': {'boot-certs': ['BootCertPath'] } }
>>>
>>> Where is this type used?
>>
>> Please correct me if I'm wrong, but from what I've seen, it is not used
>> directly in the implementation. It provides a list property for
>> BootCertPath, which makes the BootCertPathList definition valid and able
>> to accept multiple paths. If BootCerts is removed, then BootCertPathList
>> becomes underfined and results in compilation errors.
> 
> Aha!
> 
> Your QOM property setter and getter need visit_type_BootCertPathList().
> 
> The QAPI generator generates code for BootCertPathList, including
> visit_type_BootCertPathList(), only when it knows it's actually used.
> It can only see uses within the QAPI schema.  So, when ['BootCertPath']
> doesn't occur there, visit_type_BootCertPathList() won't exist, and your
> QOM code won't compile.
> 
> Since you don't actually need BootCertPathList in the schema, you need
> to add an artifical use just to get it generated.  The common way to do
> that is using it in a dummy type like this:
> 
>     ##
>     # @DummyForceS390Arrays:
>     #
>     # Not used by QMP; hack to let us use BootCertPathList internally
>     #
>     # Since: 10.2
>     ##
>     { 'struct': 'DummyForceArrays',
>       'data': { 'unused': ['BootCertPath'] } }
> 
> You also need to add it to pragme documentation-exceptions is
> pragma.json.
> 
> Yes, this is clunky.  Sorry :)
> 

Thanks for all the explanations! I'll make the changes in the next version.

> [...]
>

Zhuoying Cai <zycai@linux.ibm.com> writes:

> On 9/12/25 2:42 AM, Markus Armbruster wrote:
>> Zhuoying Cai <zycai@linux.ibm.com> writes:
>> 
>>> Thanks for the feedback.
>>>
>>> On 9/11/25 3:24 AM, Markus Armbruster wrote:
>>>> Zhuoying Cai <zycai@linux.ibm.com> writes:
>>>>
>>>>> Introduce a new `boot-certs` machine type option for the s390-ccw-virtio
>>>>> machine. This allows users to specify one or more certificate file paths
>>>>> or directories to be used during secure boot.
>>>>>
>>>>> Each entry is specified using the syntax:
>>>>> 	boot-certs.<index>.path=/path/to/cert.pem
>>>>>
>>>>> Multiple paths can be specify using array properties:
>>>>> 	boot-certs.0.path=/path/to/cert.pem,
>>>>> 	boot-certs.1.path=/path/to/cert-dir,
>>>>> 	boot-certs.2.path=/path/to/another-dir...
>>>>>
>>>>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
>>>>> ---
>>>>>  docs/system/s390x/secure-ipl.rst   | 20 ++++++++++++++++++++
>>>>>  hw/s390x/s390-virtio-ccw.c         | 30 ++++++++++++++++++++++++++++++
>>>>>  include/hw/s390x/s390-virtio-ccw.h |  2 ++
>>>>>  qapi/machine-s390x.json            | 24 ++++++++++++++++++++++++
>>>>>  qemu-options.hx                    |  6 +++++-
>>>>>  5 files changed, 81 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 docs/system/s390x/secure-ipl.rst
>>>>>
>>>>> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
>>>>> new file mode 100644
>>>>> index 0000000000..9b3fd25cc4
>>>>> --- /dev/null
>>>>> +++ b/docs/system/s390x/secure-ipl.rst
>>>>> @@ -0,0 +1,20 @@
>>>>> +.. SPDX-License-Identifier: GPL-2.0-or-later
>>>>> +
>>>>> +Secure IPL Command Line Options
>>>>> +===============================
>>>>> +
>>>>> +New parameters have been introduced to s390-ccw-virtio machine type option
>>>>> +to support secure IPL. These parameters allow users to provide certificates
>>>>> +and enable secure IPL directly via the command line.
>>>>
>>>> All too soon these parameters will no longer be new.  Consider something
>>>> like "The s390-ccw-virtio machine type supports secure TPL.  To enable
>>>> it, you need to provide certificates."
>>>>
>>>>> +
>>>>> +Providing Certificates
>>>>> +----------------------
>>>>> +
>>>>> +The certificate store can be populated by supplying a list of certificate file
>>>>> +paths or directories on the command-line:
>>>>
>>>> File is clear enough (use the certificate found in the file).  What does
>>>> directory do?
>>>
>>> A directory contains a list of certificate files, and allowing both
>>> files and directories could make the CLI more flexible.
>> 
>> I figure when @path names a file, it's an error when the file doesn't
>> contain a valid cetificate.
>> 
>> What is @path names a directory, and one of the directory's files
>> doesn't contain a valid certificate?
>> 
>> Can a single file contain multiple certificates?
>
> A certificate file path is expected to contain exactly one certificate.
>
> Certificates provided through the CLI, whether as individual files or
> within a directory, are validated before use. If a certificate is
> invalid (e.g., unsupported format), it will be skipped and not added to
> the S390 certificate store.

Hmm.  What exactly happens when I configure a certificate file like

    -machine s390-ccw-virtio,boot-certs.0.path=/dev/null

or some other file that doesn't contain a valud certificate?  Is it
silently ignored, or is it an error?

> When iterating through the provided paths, the program will terminate on
> fatal configuration errors, such as when a specified path is neither a
> file nor a directory.

[...]

On 9/15/25 2:44 AM, Markus Armbruster wrote:
> Zhuoying Cai <zycai@linux.ibm.com> writes:
> 
>> On 9/12/25 2:42 AM, Markus Armbruster wrote:
>>> Zhuoying Cai <zycai@linux.ibm.com> writes:
>>>
>>>> Thanks for the feedback.
>>>>
>>>> On 9/11/25 3:24 AM, Markus Armbruster wrote:
>>>>> Zhuoying Cai <zycai@linux.ibm.com> writes:
>>>>>
>>>>>> Introduce a new `boot-certs` machine type option for the s390-ccw-virtio
>>>>>> machine. This allows users to specify one or more certificate file paths
>>>>>> or directories to be used during secure boot.
>>>>>>
>>>>>> Each entry is specified using the syntax:
>>>>>> 	boot-certs.<index>.path=/path/to/cert.pem
>>>>>>
>>>>>> Multiple paths can be specify using array properties:
>>>>>> 	boot-certs.0.path=/path/to/cert.pem,
>>>>>> 	boot-certs.1.path=/path/to/cert-dir,
>>>>>> 	boot-certs.2.path=/path/to/another-dir...
>>>>>>
>>>>>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
>>>>>> ---
>>>>>>  docs/system/s390x/secure-ipl.rst   | 20 ++++++++++++++++++++
>>>>>>  hw/s390x/s390-virtio-ccw.c         | 30 ++++++++++++++++++++++++++++++
>>>>>>  include/hw/s390x/s390-virtio-ccw.h |  2 ++
>>>>>>  qapi/machine-s390x.json            | 24 ++++++++++++++++++++++++
>>>>>>  qemu-options.hx                    |  6 +++++-
>>>>>>  5 files changed, 81 insertions(+), 1 deletion(-)
>>>>>>  create mode 100644 docs/system/s390x/secure-ipl.rst
>>>>>>
>>>>>> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
>>>>>> new file mode 100644
>>>>>> index 0000000000..9b3fd25cc4
>>>>>> --- /dev/null
>>>>>> +++ b/docs/system/s390x/secure-ipl.rst
>>>>>> @@ -0,0 +1,20 @@
>>>>>> +.. SPDX-License-Identifier: GPL-2.0-or-later
>>>>>> +
>>>>>> +Secure IPL Command Line Options
>>>>>> +===============================
>>>>>> +
>>>>>> +New parameters have been introduced to s390-ccw-virtio machine type option
>>>>>> +to support secure IPL. These parameters allow users to provide certificates
>>>>>> +and enable secure IPL directly via the command line.
>>>>>
>>>>> All too soon these parameters will no longer be new.  Consider something
>>>>> like "The s390-ccw-virtio machine type supports secure TPL.  To enable
>>>>> it, you need to provide certificates."
>>>>>
>>>>>> +
>>>>>> +Providing Certificates
>>>>>> +----------------------
>>>>>> +
>>>>>> +The certificate store can be populated by supplying a list of certificate file
>>>>>> +paths or directories on the command-line:
>>>>>
>>>>> File is clear enough (use the certificate found in the file).  What does
>>>>> directory do?
>>>>
>>>> A directory contains a list of certificate files, and allowing both
>>>> files and directories could make the CLI more flexible.
>>>
>>> I figure when @path names a file, it's an error when the file doesn't
>>> contain a valid cetificate.
>>>
>>> What is @path names a directory, and one of the directory's files
>>> doesn't contain a valid certificate?
>>>
>>> Can a single file contain multiple certificates?
>>
>> A certificate file path is expected to contain exactly one certificate.
>>
>> Certificates provided through the CLI, whether as individual files or
>> within a directory, are validated before use. If a certificate is
>> invalid (e.g., unsupported format), it will be skipped and not added to
>> the S390 certificate store.
> 
> Hmm.  What exactly happens when I configure a certificate file like
> 
>     -machine s390-ccw-virtio,boot-certs.0.path=/dev/null
> 
> or some other file that doesn't contain a valud certificate?  Is it
> silently ignored, or is it an error?
> 
>> When iterating through the provided paths, the program will terminate on
>> fatal configuration errors, such as when a specified path is neither a
>> file nor a directory.
> 

If /dev/null is provided, the program will report an error and terminates.

   qemu-system-s390x: Path '/dev/null' is neither a file nor a directory

If a file exists but doesn't contain a valid certificate, the program
will report the error but continues validating other remaining
certificates. For example, providing a certificate in DER format
(cert.pem) is not supported; only PEM format certificates are accepted:

   -machine s390-ccw-virtio,boot-certs.0.path=/root/cert.pem, \
                            boot-certs.1.path=/root/pem-test/rsa.pem, \
                            boot-certs.2.path=/root/pem-test/rsa-oaep.pem

   qemu-system-s390x: Failed to initialize certificate: /root/cert.pem:
Failed to import certificate: Base64 unexpected header error.
Using virtio-blk.
SCSI boot device supports secure boot.
LOADPARM=[        ]

Using virtio-blk.
Using SCSI scheme.
....Verified component
...Verified component
...

In summary, QEMU first collects a list of .pem files from the CLI, and
then validate each certificate in that list.

More specifically, QEMU first checks and normalizes the certificate
paths - each path must be a non-empty string, must exist, and must be
either a regular file or a directory. An invalid path will be reported
and cause the program to terminate. Regular files with a .pem suffix, as
well as .pem files inside directories, are added to the list.

Once the list is built, QEMU attempts to read and parse each
certificate. Errors at this stage (such as an invalid certificate
format) are reported, but the program continues validating the rest of
certificate in the list.

(Further implementation details are provided in [PATCH v5 04/29]
hw/s390x/ipl: Create certificate store.)

> [...]
>

On Mon, Sep 15, 2025 at 12:14:50PM -0400, Zhuoying Cai wrote:
> On 9/15/25 2:44 AM, Markus Armbruster wrote:
> > Zhuoying Cai <zycai@linux.ibm.com> writes:
> > 
> >> On 9/12/25 2:42 AM, Markus Armbruster wrote:
> >>> Zhuoying Cai <zycai@linux.ibm.com> writes:
> >>>
> >>>> Thanks for the feedback.
> >>>>
> >>>> On 9/11/25 3:24 AM, Markus Armbruster wrote:
> >>>>> Zhuoying Cai <zycai@linux.ibm.com> writes:
> >>>>>
> >>>>>> Introduce a new `boot-certs` machine type option for the s390-ccw-virtio
> >>>>>> machine. This allows users to specify one or more certificate file paths
> >>>>>> or directories to be used during secure boot.
> >>>>>>
> >>>>>> Each entry is specified using the syntax:
> >>>>>> 	boot-certs.<index>.path=/path/to/cert.pem
> >>>>>>
> >>>>>> Multiple paths can be specify using array properties:
> >>>>>> 	boot-certs.0.path=/path/to/cert.pem,
> >>>>>> 	boot-certs.1.path=/path/to/cert-dir,
> >>>>>> 	boot-certs.2.path=/path/to/another-dir...
> >>>>>>
> >>>>>> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> >>>>>> ---
> >>>>>>  docs/system/s390x/secure-ipl.rst   | 20 ++++++++++++++++++++
> >>>>>>  hw/s390x/s390-virtio-ccw.c         | 30 ++++++++++++++++++++++++++++++
> >>>>>>  include/hw/s390x/s390-virtio-ccw.h |  2 ++
> >>>>>>  qapi/machine-s390x.json            | 24 ++++++++++++++++++++++++
> >>>>>>  qemu-options.hx                    |  6 +++++-
> >>>>>>  5 files changed, 81 insertions(+), 1 deletion(-)
> >>>>>>  create mode 100644 docs/system/s390x/secure-ipl.rst
> >>>>>>
> >>>>>> diff --git a/docs/system/s390x/secure-ipl.rst b/docs/system/s390x/secure-ipl.rst
> >>>>>> new file mode 100644
> >>>>>> index 0000000000..9b3fd25cc4
> >>>>>> --- /dev/null
> >>>>>> +++ b/docs/system/s390x/secure-ipl.rst
> >>>>>> @@ -0,0 +1,20 @@
> >>>>>> +.. SPDX-License-Identifier: GPL-2.0-or-later
> >>>>>> +
> >>>>>> +Secure IPL Command Line Options
> >>>>>> +===============================
> >>>>>> +
> >>>>>> +New parameters have been introduced to s390-ccw-virtio machine type option
> >>>>>> +to support secure IPL. These parameters allow users to provide certificates
> >>>>>> +and enable secure IPL directly via the command line.
> >>>>>
> >>>>> All too soon these parameters will no longer be new.  Consider something
> >>>>> like "The s390-ccw-virtio machine type supports secure TPL.  To enable
> >>>>> it, you need to provide certificates."
> >>>>>
> >>>>>> +
> >>>>>> +Providing Certificates
> >>>>>> +----------------------
> >>>>>> +
> >>>>>> +The certificate store can be populated by supplying a list of certificate file
> >>>>>> +paths or directories on the command-line:
> >>>>>
> >>>>> File is clear enough (use the certificate found in the file).  What does
> >>>>> directory do?
> >>>>
> >>>> A directory contains a list of certificate files, and allowing both
> >>>> files and directories could make the CLI more flexible.
> >>>
> >>> I figure when @path names a file, it's an error when the file doesn't
> >>> contain a valid cetificate.
> >>>
> >>> What is @path names a directory, and one of the directory's files
> >>> doesn't contain a valid certificate?
> >>>
> >>> Can a single file contain multiple certificates?
> >>
> >> A certificate file path is expected to contain exactly one certificate.
> >>
> >> Certificates provided through the CLI, whether as individual files or
> >> within a directory, are validated before use. If a certificate is
> >> invalid (e.g., unsupported format), it will be skipped and not added to
> >> the S390 certificate store.
> > 
> > Hmm.  What exactly happens when I configure a certificate file like
> > 
> >     -machine s390-ccw-virtio,boot-certs.0.path=/dev/null
> > 
> > or some other file that doesn't contain a valud certificate?  Is it
> > silently ignored, or is it an error?
> > 
> >> When iterating through the provided paths, the program will terminate on
> >> fatal configuration errors, such as when a specified path is neither a
> >> file nor a directory.
> > 
> 
> If /dev/null is provided, the program will report an error and terminates.
> 
>    qemu-system-s390x: Path '/dev/null' is neither a file nor a directory
> 
> If a file exists but doesn't contain a valid certificate, the program
> will report the error but continues validating other remaining
> certificates. For example, providing a certificate in DER format
> (cert.pem) is not supported; only PEM format certificates are accepted:
> 
>    -machine s390-ccw-virtio,boot-certs.0.path=/root/cert.pem, \
>                             boot-certs.1.path=/root/pem-test/rsa.pem, \
>                             boot-certs.2.path=/root/pem-test/rsa-oaep.pem
> 
>    qemu-system-s390x: Failed to initialize certificate: /root/cert.pem:
> Failed to import certificate: Base64 unexpected header error.
> Using virtio-blk.
> SCSI boot device supports secure boot.
> LOADPARM=[        ]
> 
> Using virtio-blk.
> Using SCSI scheme.
> ....Verified component
> ...Verified component
> ...
> 
> In summary, QEMU first collects a list of .pem files from the CLI, and
> then validate each certificate in that list.
> 
> More specifically, QEMU first checks and normalizes the certificate
> paths - each path must be a non-empty string, must exist, and must be
> either a regular file or a directory. An invalid path will be reported
> and cause the program to terminate. Regular files with a .pem suffix, as
> well as .pem files inside directories, are added to the list.

IMHO we should be a bit more strict than this

 * If pointing to a file - loading must succeed; errors must be fatal
 * If pointing to a dir - the dir must exist, any file NOT ending in
   .pem should be ignored, any file ending in .pem must succeed, errors
   must be fatal
 * If pointing to neither a file, nor dir - must be fatal

ie we should be assuming that the configuration is expected to work, and
thus errors when loading are liable to be admin config mistakes that should
be fatal.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Daniel P. Berrangé <berrange@redhat.com> writes:

[...]

> IMHO we should be a bit more strict than this
>
>  * If pointing to a file - loading must succeed; errors must be fatal
>  * If pointing to a dir - the dir must exist, any file NOT ending in
>    .pem should be ignored, any file ending in .pem must succeed, errors
>    must be fatal
>  * If pointing to neither a file, nor dir - must be fatal
>
> ie we should be assuming that the configuration is expected to work, and
> thus errors when loading are liable to be admin config mistakes that should
> be fatal.

Seconded.  Thanks!