Re: [PATCH v5 09/29] s390x/diag: Implement DIAG 320 subcode 2

[Jared Rossi <jrossi@linux.ibm.com>]

On 8/18/25 5:43 PM, Zhuoying Cai wrote:
> DIAG 320 subcode 2 provides verification-certificates (VCs) that are in the
> certificate store. Only X509 certificates in DER format and SHA-256 hash
> type are recognized.
>
> The subcode value is denoted by setting the second-left-most bit
> of an 8-byte field.
>
> The Verification Certificate Block (VCB) contains the output data
> when the operation completes successfully. It includes a common
> header followed by zero or more Verification Certificate Entries (VCEs),
> depending on the VCB input length and the VC range (from the first VC
> index to the last VC index) in the certificate store.
>
> Each VCE contains information about a certificate retrieved from
> the S390IPLCertificateStore, such as the certificate name, key type,
> key ID length, hash length, and the raw certificate data.
> The key ID and hash are extracted from the raw certificate by the crypto API.
>
> Note: SHA2-256 VC hash type is required for retrieving the hash
> (fingerprint) of the certificate.
>
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   docs/specs/s390x-secure-ipl.rst |  13 ++
>   include/hw/s390x/ipl/diag320.h  |  49 ++++++
>   target/s390x/diag.c             | 286 +++++++++++++++++++++++++++++++-
>   3 files changed, 347 insertions(+), 1 deletion(-)
>
[snip...]
> +static VCEntry *diag_320_build_vce(S390IPLCertificate qcert, uint32_t vce_len, int idx)
> +{
> +    g_autofree VCEntry *vce = NULL;
> +    int rc;
> +
> +    /*
> +     * Construct VCE
> +     * Allocate enough memory for all certificate data (key id, hash and certificate).
> +     * Unused area following the VCE field contains zeros.
> +     */
> +    vce = g_malloc0(vce_len);
> +    rc = build_vce_header(vce, qcert, idx);
> +    if (rc) {
> +        vce->len = cpu_to_be32(VCE_INVALID_LEN);
> +        goto out;
> +    }
> +    vce->len = cpu_to_be32(VCE_HEADER_LEN);
> +
> +    rc = build_vce_data(vce, qcert);
> +    if (rc) {
> +        vce->len = cpu_to_be32(VCE_INVALID_LEN);
> +    }
> +
> +out:
> +    return g_steal_pointer(&vce);
> +}
> +
> +static int handle_diag320_store_vc(S390CPU *cpu, uint64_t addr, uint64_t r1, uintptr_t ra,
> +                                   S390IPLCertificateStore *qcs)
> +{
> +    g_autofree VCBlock *vcb = NULL;
> +    size_t vce_offset;
> +    size_t remaining_space;
> +    uint32_t vce_len;
> +    uint16_t first_vc_index;
> +    uint16_t last_vc_index;
> +    uint32_t in_len;
> +
> +    vcb = g_new0(VCBlock, 1);
> +    if (s390_cpu_virt_mem_read(cpu, addr, r1, vcb, sizeof(*vcb))) {
> +        s390_cpu_virt_mem_handle_exc(cpu, ra);
> +        return -1;
> +    }
> +
> +    in_len = be32_to_cpu(vcb->in_len);
> +    first_vc_index = be16_to_cpu(vcb->first_vc_index);
> +    last_vc_index = be16_to_cpu(vcb->last_vc_index);
> +
> +    if (in_len % TARGET_PAGE_SIZE != 0) {
> +        return DIAG_320_RC_INVAL_VCB_LEN;
> +    }
> +
> +    if (first_vc_index > last_vc_index) {
> +        return DIAG_320_RC_BAD_RANGE;
> +    }
> +
> +    if (first_vc_index == 0) {
> +        /*
> +         * Zero is a valid index for the first and last VC index.
> +         * Zero index results in the VCB header and zero certificates returned.
> +         */
> +        if (last_vc_index == 0) {
> +            goto out;
> +        }
> +
> +        /* DIAG320 certificate store remains a one origin for cert entries */
> +        vcb->first_vc_index = 1;
> +        first_vc_index = 1;
> +    }
> +
> +    vce_offset = VCB_HEADER_LEN;
> +    vcb->out_len = VCB_HEADER_LEN;
> +    remaining_space = in_len - VCB_HEADER_LEN;
> +
> +    for (int i = first_vc_index - 1; i < last_vc_index && i < qcs->count; i++) {
> +        VCEntry *vce;
> +        S390IPLCertificate qcert = qcs->certs[i];
> +        /*
> +         * Each VCE is word aligned.
> +         * Each variable length field within the VCE is also word aligned.
> +         */
> +        vce_len = VCE_HEADER_LEN +
> +                  ROUND_UP(qcert.key_id_size, 4) +
> +                  ROUND_UP(qcert.hash_size, 4) +
> +                  ROUND_UP(qcert.der_size, 4);
> +
> +        /*
> +         * If there is no more space to store the cert,
> +         * set the remaining verification cert count and
> +         * break early.
> +         */
> +        if (remaining_space < vce_len) {
> +            vcb->remain_ct = cpu_to_be16(last_vc_index - i);
> +            break;
> +        }
What is the significance of remain_ct != 0?

Should there be an error or warning that there was not enough space?

> +
> +        vce = diag_320_build_vce(qcert, vce_len, i);
> +
> +        /* Write VCE */
> +        if (s390_cpu_virt_mem_write(cpu, addr + vce_offset, r1,
> +                                    vce, be32_to_cpu(vce->len))) {
> +            s390_cpu_virt_mem_handle_exc(cpu, ra);
> +            return -1;
Missing vce free in this early return?

> +        }
> +
> +        vce_offset += be32_to_cpu(vce->len);
> +        vcb->out_len += be32_to_cpu(vce->len);
> +        remaining_space -= be32_to_cpu(vce->len);
> +        vcb->stored_ct++;
> +
> +        g_free(vce);
> +    }
> +
> +    vcb->out_len = cpu_to_be32(vcb->out_len);
> +    vcb->stored_ct = cpu_to_be16(vcb->stored_ct);
> +
> +out:
> +    /*
> +     * Write VCB header
> +     * All VCEs have been populated with the latest information
> +     * and write VCB header last.
> +     */
> +    if (s390_cpu_virt_mem_write(cpu, addr, r1, vcb, VCB_HEADER_LEN)) {
> +        s390_cpu_virt_mem_handle_exc(cpu, ra);
> +        return -1;
> +    }
> +
> +    return DIAG_320_RC_OK;
> +}
> +
>   void handle_diag_320(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
>   {
>       S390CPU *cpu = env_archcpu(env);
> @@ -256,7 +532,8 @@ void handle_diag_320(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
>            * for now.
>            */
>           uint32_t ism_word0 = cpu_to_be32(DIAG_320_ISM_QUERY_SUBCODES |
> -                                         DIAG_320_ISM_QUERY_VCSI);
> +                                         DIAG_320_ISM_QUERY_VCSI |
> +                                         DIAG_320_ISM_STORE_VC);
>   
>           if (s390_cpu_virt_mem_write(cpu, addr, r1, &ism_word0, sizeof(ism_word0))) {
>               s390_cpu_virt_mem_handle_exc(cpu, ra);
> @@ -282,6 +559,13 @@ void handle_diag_320(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
>           }
>           env->regs[r1 + 1] = rc;
>           break;
> +    case DIAG_320_SUBC_STORE_VC:
> +        rc = handle_diag320_store_vc(cpu, addr, r1, ra, qcs);
> +        if (rc == -1) {
> +            return;
> +        }
> +        env->regs[r1 + 1] = rc;
> +        break;
>       default:
>           env->regs[r1 + 1] = DIAG_320_RC_NOT_SUPPORTED;
>           break;
Regards,
Jared Rossi