On 8/18/25 17:42, Zhuoying Cai wrote:
> DIAGNOSE 320 is introduced to support Certificate Store (CS)
> Facility, which includes operations such as query certificate
> storage information and provide certificates in the certificate
> store.
>
> Currently, only subcode 0 is supported with this patch, which is
> used to query the Installed Subcodes Mask (ISM).
>
> This subcode is only supported when the CS facility is enabled.
Please add in the commit message that this feature is available starting
with the gen16 CPU model.
>
> Availability of CS facility is determined by byte 134 bit 5 of the
> SCLP Read Info block. Byte 134's facilities cannot be represented
> without the availability of the extended-length-SCCB, so add it as
> a check for consistency.
>
> Note: secure IPL is not available for Secure Execution (SE) guests,
> as their images are already integrity protected, and an additional
> protection of the kernel by secure IPL is not necessary.
>
> Signed-off-by: Zhuoying Cai <zy...@linux.ibm.com>
Other than the question below regarding the documentation, code LGTM!
Reviewed-by: Collin Walling <wall...@linux.ibm.com>
> ---
> docs/specs/s390x-secure-ipl.rst | 25 ++++++++++++++++
> include/hw/s390x/ipl/diag320.h | 20 +++++++++++++
> target/s390x/cpu_features.c | 1 +
> target/s390x/cpu_features_def.h.inc | 1 +
> target/s390x/cpu_models.c | 2 ++
> target/s390x/diag.c | 44 +++++++++++++++++++++++++++++
> target/s390x/gen-features.c | 3 ++
> target/s390x/kvm/kvm.c | 16 +++++++++++
> target/s390x/s390x-internal.h | 2 ++
> target/s390x/tcg/misc_helper.c | 7 +++++
> 10 files changed, 121 insertions(+)
> create mode 100644 docs/specs/s390x-secure-ipl.rst
> create mode 100644 include/hw/s390x/ipl/diag320.h
>
> diff --git a/docs/specs/s390x-secure-ipl.rst b/docs/specs/s390x-secure-ipl.rst
> new file mode 100644
> index 0000000000..70e9a66fe0
> --- /dev/null
> +++ b/docs/specs/s390x-secure-ipl.rst
> @@ -0,0 +1,25 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +s390 Certificate Store and Functions
> +====================================
> +
> +s390 Certificate Store
> +----------------------
> +
> +A certificate store is implemented for s390-ccw guests to retain within
> +memory all certificates provided by the user via the command-line, which
> +are expected to be stored somewhere on the host's file system. The store
> +will keep track of the number of certificates, their respective size,
> +and a summation of the sizes.
> +
Perhaps the above should go with patch #4, since that one introduces the
cert store?
> +DIAGNOSE function code 'X'320' - Certificate Store Facility
> +-----------------------------------------------------------
> +
> +DIAGNOSE 'X'320' is used to provide support for userspace to directly
> +query the s390 certificate store. Userspace may be the s390-ccw BIOS or
> +the guest kernel.
> +
> +Subcode 0 - query installed subcodes
> + Returns a 256-bit installed subcodes mask (ISM) stored in the installed
> + subcodes block (ISB). This mask indicates which sucodes are currently
> + installed and available for use.
> diff --git a/include/hw/s390x/ipl/diag320.h b/include/hw/s390x/ipl/diag320.h
> new file mode 100644
> index 0000000000..aa04b699c6
> --- /dev/null
> +++ b/include/hw/s390x/ipl/diag320.h
> @@ -0,0 +1,20 @@
> +/*
> + * S/390 DIAGNOSE 320 definitions and structures
> + *
> + * Copyright 2025 IBM Corp.
> + * Author(s): Zhuoying Cai <zy...@linux.ibm.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef S390X_DIAG320_H
> +#define S390X_DIAG320_H
> +
> +#define DIAG_320_SUBC_QUERY_ISM 0
> +
> +#define DIAG_320_RC_OK 0x0001
> +#define DIAG_320_RC_NOT_SUPPORTED 0x0102
> +
> +#define DIAG_320_ISM_QUERY_SUBCODES 0x80000000
> +
> +#endif
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 4b5be6798e..436471f4b4 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -147,6 +147,7 @@ void s390_fill_feat_block(const S390FeatBitmap features,
> S390FeatType type,
> break;
> case S390_FEAT_TYPE_SCLP_FAC134:
> clear_be_bit(s390_feat_def(S390_FEAT_DIAG_318)->bit, data);
> + clear_be_bit(s390_feat_def(S390_FEAT_CERT_STORE)->bit, data);
> break;
> default:
> return;
> diff --git a/target/s390x/cpu_features_def.h.inc
> b/target/s390x/cpu_features_def.h.inc
> index c017bffcdc..941a69e013 100644
> --- a/target/s390x/cpu_features_def.h.inc
> +++ b/target/s390x/cpu_features_def.h.inc
> @@ -138,6 +138,7 @@ DEF_FEAT(SIE_IBS, "ibs", SCLP_CONF_CHAR_EXT, 10, "SIE:
> Interlock-and-broadcast-s
>
> /* Features exposed via SCLP SCCB Facilities byte 134 (bit numbers relative
> to byte-134) */
> DEF_FEAT(DIAG_318, "diag318", SCLP_FAC134, 0, "Control program name and
> version codes")
> +DEF_FEAT(CERT_STORE, "cstore", SCLP_FAC134, 5, "Provide Certificate Store
> functions")
>
> /* Features exposed via SCLP CPU info. */
> DEF_FEAT(SIE_F2, "sief2", SCLP_CPU, 4, "SIE: interception format 2 (Virtual
> SIE)")
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 954a7a99a9..6b8471700e 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -248,6 +248,7 @@ bool s390_has_feat(S390Feat feat)
> if (s390_is_pv()) {
> switch (feat) {
> case S390_FEAT_DIAG_318:
> + case S390_FEAT_CERT_STORE:
> case S390_FEAT_HPMA2:
> case S390_FEAT_SIE_F2:
> case S390_FEAT_SIE_SKEY:
> @@ -505,6 +506,7 @@ static void check_consistency(const S390CPUModel *model)
> { S390_FEAT_PTFF_STOUE, S390_FEAT_MULTIPLE_EPOCH },
> { S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, S390_FEAT_AP },
> { S390_FEAT_DIAG_318, S390_FEAT_EXTENDED_LENGTH_SCCB },
> + { S390_FEAT_CERT_STORE, S390_FEAT_EXTENDED_LENGTH_SCCB },
> { S390_FEAT_NNPA, S390_FEAT_VECTOR },
> { S390_FEAT_RDP, S390_FEAT_LOCAL_TLB_CLEARING },
> { S390_FEAT_UV_FEAT_AP, S390_FEAT_AP },
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index cff9fbc4b0..a35d808fd7 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -18,6 +18,7 @@
> #include "hw/watchdog/wdt_diag288.h"
> #include "system/cpus.h"
> #include "hw/s390x/ipl.h"
> +#include "hw/s390x/ipl/diag320.h"
> #include "hw/s390x/s390-virtio-ccw.h"
> #include "system/kvm.h"
> #include "kvm/kvm_s390x.h"
> @@ -191,3 +192,46 @@ out:
> break;
> }
> }
> +
> +void handle_diag_320(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t
> ra)
> +{
> + S390CPU *cpu = env_archcpu(env);
> + uint64_t subcode = env->regs[r3];
> + uint64_t addr = env->regs[r1];
> +
> + if (env->psw.mask & PSW_MASK_PSTATE) {
> + s390_program_interrupt(env, PGM_PRIVILEGED, ra);
> + return;
> + }
> +
> + if (!s390_has_feat(S390_FEAT_CERT_STORE)) {
> + s390_program_interrupt(env, PGM_SPECIFICATION, ra);
> + return;
> + }
> +
> + if ((subcode & ~0x000ffULL) || (r1 & 1)) {
> + s390_program_interrupt(env, PGM_SPECIFICATION, ra);
> + return;
> + }
> +
> + switch (subcode) {
> + case DIAG_320_SUBC_QUERY_ISM:
> + /*
> + * The Installed Subcode Block (ISB) can be up 8 words in size,
> + * but the current set of subcodes can fit within a single word
> + * for now.
> + */
> + uint32_t ism_word0 = cpu_to_be32(DIAG_320_ISM_QUERY_SUBCODES);
> +
> + if (s390_cpu_virt_mem_write(cpu, addr, r1, &ism_word0,
> sizeof(ism_word0))) {
> + s390_cpu_virt_mem_handle_exc(cpu, ra);
> + return;
> + }
> +
> + env->regs[r1 + 1] = DIAG_320_RC_OK;
> + break;
> + default:
> + env->regs[r1 + 1] = DIAG_320_RC_NOT_SUPPORTED;
> + break;
> + }
> +}
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 8218e6470e..6c20c3a862 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -720,6 +720,7 @@ static uint16_t full_GEN16_GA1[] = {
> S390_FEAT_PAIE,
> S390_FEAT_UV_FEAT_AP,
> S390_FEAT_UV_FEAT_AP_INTR,
> + S390_FEAT_CERT_STORE,
> };
>
> static uint16_t full_GEN17_GA1[] = {
> @@ -919,6 +920,8 @@ static uint16_t qemu_MAX[] = {
> S390_FEAT_KIMD_SHA_512,
> S390_FEAT_KLMD_SHA_512,
> S390_FEAT_PRNO_TRNG,
> + S390_FEAT_EXTENDED_LENGTH_SCCB,
> + S390_FEAT_CERT_STORE,
> };
>
> /****** END FEATURE DEFS ******/
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index 8ee33924df..5510fc2fc5 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -98,6 +98,7 @@
> #define DIAG_TIMEREVENT 0x288
> #define DIAG_IPL 0x308
> #define DIAG_SET_CONTROL_PROGRAM_CODES 0x318
> +#define DIAG_CERT_STORE 0x320
> #define DIAG_KVM_HYPERCALL 0x500
> #define DIAG_KVM_BREAKPOINT 0x501
>
> @@ -1560,6 +1561,16 @@ static void handle_diag_318(S390CPU *cpu, struct
> kvm_run *run)
> }
> }
>
> +static void kvm_handle_diag_320(S390CPU *cpu, struct kvm_run *run)
> +{
> + uint64_t r1, r3;
> +
> + r1 = (run->s390_sieic.ipa & 0x00f0) >> 4;
> + r3 = run->s390_sieic.ipa & 0x000f;
> +
> + handle_diag_320(&cpu->env, r1, r3, RA_IGNORED);
> +}
> +
> #define DIAG_KVM_CODE_MASK 0x000000000000ffff
>
> static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
> @@ -1590,6 +1601,9 @@ static int handle_diag(S390CPU *cpu, struct kvm_run
> *run, uint32_t ipb)
> case DIAG_KVM_BREAKPOINT:
> r = handle_sw_breakpoint(cpu, run);
> break;
> + case DIAG_CERT_STORE:
> + kvm_handle_diag_320(cpu, run);
> + break;
> default:
> trace_kvm_insn_diag(func_code);
> kvm_s390_program_interrupt(cpu, PGM_SPECIFICATION);
> @@ -2490,6 +2504,8 @@ bool kvm_s390_get_host_cpu_model(S390CPUModel *model,
> Error **errp)
> set_bit(S390_FEAT_DIAG_318, model->features);
> }
>
> + set_bit(S390_FEAT_CERT_STORE, model->features);
> +
> /* Test for Ultravisor features that influence secure guest behavior */
> query_uv_feat_guest(model->features);
>
> diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
> index 56cce2e7f5..ecff2d07a1 100644
> --- a/target/s390x/s390x-internal.h
> +++ b/target/s390x/s390x-internal.h
> @@ -391,6 +391,8 @@ int mmu_translate_real(CPUS390XState *env, target_ulong
> raddr, int rw,
> int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3);
> void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3,
> uintptr_t ra);
> +void handle_diag_320(CPUS390XState *env, uint64_t r1, uint64_t r3,
> + uintptr_t ra);
>
>
> /* translate.c */
> diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
> index f7101be574..412c34ed93 100644
> --- a/target/s390x/tcg/misc_helper.c
> +++ b/target/s390x/tcg/misc_helper.c
> @@ -142,6 +142,13 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1,
> uint32_t r3, uint32_t num)
> /* time bomb (watchdog) */
> r = handle_diag_288(env, r1, r3);
> break;
> + case 0x320:
> + /* cert store */
> + bql_lock();
> + handle_diag_320(env, r1, r3, GETPC());
> + bql_unlock();
> + r = 0;
> + break;
> default:
> r = -1;
> break;
--
Regards,
Collin
