Re: [PATCH v4 07/28] s390x/diag: Implement DIAG 320 subcode 1

[Eric Farman <farman@linux.ibm.com>]

On Fri, 2025-07-11 at 17:10 -0400, Zhuoying Cai wrote:
> DIAG 320 subcode 1 provides information needed to determine
> the amount of storage to store one or more certificates.
> 
> The subcode value is denoted by setting the left-most bit
> of an 8-byte field.
> 
> The verification-certificate-storage-size block (VCSSB) contains
> the output data when the operation completes successfully.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>  include/hw/s390x/ipl/diag320.h | 23 ++++++++++++++++++++++
>  target/s390x/diag.c            | 36 +++++++++++++++++++++++++++++++++-
>  2 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/s390x/ipl/diag320.h b/include/hw/s390x/ipl/diag320.h
> index 713570545d..3916a2915e 100644
> --- a/include/hw/s390x/ipl/diag320.h
> +++ b/include/hw/s390x/ipl/diag320.h
> @@ -11,7 +11,30 @@
>  #define S390X_DIAG320_H
>  
>  #define DIAG_320_SUBC_QUERY_ISM     0
> +#define DIAG_320_SUBC_QUERY_VCSI    1
>  
>  #define DIAG_320_RC_OK              0x0001
>  
> +#define VCSSB_MAX_LEN   128
> +#define VCE_HEADER_LEN  128
> +#define VCB_HEADER_LEN  64
> +
> +#define DIAG_320_ISM_QUERY_VCSI     0x4000000000000000
> +
> +struct VCStorageSizeBlock {
> +    uint32_t length;
> +    uint8_t reserved0[3];
> +    uint8_t version;
> +    uint32_t reserved1[6];
> +    uint16_t total_vc_ct;
> +    uint16_t max_vc_ct;
> +    uint32_t reserved3[7];
> +    uint32_t max_vce_len;
> +    uint32_t reserved4[3];
> +    uint32_t max_single_vcb_len;
> +    uint32_t total_vcb_len;
> +    uint32_t reserved5[10];
> +};
> +typedef struct VCStorageSizeBlock VCStorageSizeBlock;
> +
>  #endif
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index 7b9b47a171..1f7d0cb2f6 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -191,9 +191,13 @@ out:
>      }
>  }
>  
> +QEMU_BUILD_BUG_MSG(sizeof(VCStorageSizeBlock) != 128,
> +                   "size of VCStorageSizeBlock is wrong");
> +

The message doesn't provide any information that QEMU_BUILD_BUG_ON wouldn't give us, so maybe just
use that.

Also, maybe replace 128 with VCSSB_MAX_LEN?

>  void handle_diag_320(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
>  {
>      S390CPU *cpu = env_archcpu(env);
> +    S390IPLCertificateStore *qcs = s390_ipl_get_certificate_store();
>      uint64_t subcode = env->regs[r3];
>      uint64_t addr = env->regs[r1];
>      int rc;
> @@ -215,13 +219,43 @@ void handle_diag_320(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
>  
>      switch (subcode) {
>      case DIAG_320_SUBC_QUERY_ISM:
> -        uint64_t ism =  0;
> +        uint64_t ism = cpu_to_be64(DIAG_320_ISM_QUERY_VCSI);

Oh... Doesn't this suggest that there should have been a bit here stating that Query ISM itself is
supported? (I agree it's a catch-22, but...)

>  
>          if (s390_cpu_virt_mem_write(cpu, addr, r1, &ism, sizeof(ism))) {
>              s390_cpu_virt_mem_handle_exc(cpu, ra);
>              return;
>          }
>  
> +        rc = DIAG_320_RC_OK;
> +        break;
> +    case DIAG_320_SUBC_QUERY_VCSI:
> +        VCStorageSizeBlock vcssb;
> +

Verify that addr is doubleword aligned?

> +        if (!diag_parm_addr_valid(addr, sizeof(VCStorageSizeBlock),
> +                                  true)) {
> +            s390_program_interrupt(env, PGM_ADDRESSING, ra);
> +            return;
> +        }
> +
> +        if (!qcs || !qcs->count) {

For my own curiosity, can qcs actually be NULL? Looks like s390_ipl_get_certificate_store() returns
the address of the cert_store struct in the IPL device.

> +            vcssb.length = cpu_to_be32(4);
> +        } else {
> +            vcssb.length = cpu_to_be32(VCSSB_MAX_LEN);
> +            vcssb.version = 0;
> +            vcssb.total_vc_ct = cpu_to_be16(qcs->count);
> +            vcssb.max_vc_ct = cpu_to_be16(MAX_CERTIFICATES);
> +            vcssb.max_vce_len = cpu_to_be32(VCE_HEADER_LEN + qcs->max_cert_size);
> +            vcssb.max_single_vcb_len = cpu_to_be32(VCB_HEADER_LEN + VCE_HEADER_LEN +
> +                                                   qcs->max_cert_size);
> +            vcssb.total_vcb_len = cpu_to_be32(VCB_HEADER_LEN +
> +                                              qcs->count * VCE_HEADER_LEN +
> +                                              qcs->total_bytes);
> +        }
> +
> +        if (s390_cpu_virt_mem_write(cpu, addr, r1, &vcssb, sizeof(VCStorageSizeBlock))) {
> +            s390_cpu_virt_mem_handle_exc(cpu, ra);
> +            return;
> +        }
>          rc = DIAG_320_RC_OK;
>          break;
>      default: