On 2/27/26 7:58 AM, Thomas Huth wrote:
> On 12/02/2026 21.43, Zhuoying Cai wrote:
>> DIAG 320 subcode 1 provides information needed to determine
>> the amount of storage to store one or more certificates from the
>> certificate store.
>>
>> Upon successful completion, this subcode returns information of the current
>> cert store, such as the number of certificates stored and allowed in the cert
>> store, amount of space may need to be allocate to store a certificate,
>> etc for verification-certificate blocks (VCBs).
>>
>> 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. A VCSSB
>> length of 4 indicates that no certificate are available in the cert
>> store.
>>
>> Signed-off-by: Zhuoying Cai <[email protected]>
>> Reviewed-by: Farhan Ali <[email protected]>
>> Reviewed-by: Collin Walling <[email protected]>
>> ---
> ...
>> diff --git a/include/hw/s390x/ipl/diag320.h b/include/hw/s390x/ipl/diag320.h
>> index aa04b699c6..6e4779c699 100644
>> --- a/include/hw/s390x/ipl/diag320.h
>> +++ b/include/hw/s390x/ipl/diag320.h
>> @@ -11,10 +11,32 @@
>> #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 DIAG_320_RC_NOT_SUPPORTED 0x0102
>> +#define DIAG_320_RC_INVAL_VCSSB_LEN 0x0202
>>
>> #define DIAG_320_ISM_QUERY_SUBCODES 0x80000000
>> +#define DIAG_320_ISM_QUERY_VCSI 0x40000000
>> +
>> +#define VCSSB_NO_VC 4
>> +#define VCSSB_MIN_LEN 128
>> +#define VCE_HEADER_LEN 128
>> +#define VCB_HEADER_LEN 64
>> +
>> +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[11];
>> + uint32_t max_single_vcb_len;
>> + uint32_t total_vcb_len;
>> + uint32_t reserved4[10];
>> +};
>> +typedef struct VCStorageSizeBlock VCStorageSizeBlock;
>
> Since this API between QEMU and the guest, maybe add a
>
> QEMU_BUILD_BUG_ON(sizeof(VCStorageSizeBlock) != ...);
>
> here to make sure that there is no accidential padding.
> (should not happen since field are naturally aligned, but better be safe
> than sorry?)
>
Please correct me if I’m wrong. QEMU_BUILD_BUG_ON() is a QEMU-specific
macro and is not accessible from the guest, so using it here would cause
a compile error. I’m not aware of similar checks being used on the guest
side. Would QEMU_BUILD_BUG_ON() still be appropriate in this case, or is
there a better way to ensure that no unintended padding is introduced?
>> #endif
>> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
>> index e867fc2156..3c7e64eb05 100644
>> --- a/target/s390x/diag.c
>> +++ b/target/s390x/diag.c
>> @@ -197,11 +197,54 @@ out:
>> }
>> }
>>
>> +static int handle_diag320_query_vcsi(S390CPU *cpu, uint64_t addr, uint64_t
>> r1,
>> + uintptr_t ra, S390IPLCertificateStore
>> *cs)
>> +{
>> + g_autofree VCStorageSizeBlock *vcssb = NULL;
>> +
>> + vcssb = g_new0(VCStorageSizeBlock, 1);
>> + if (s390_cpu_virt_mem_read(cpu, addr, r1, vcssb, sizeof(*vcssb))) {
>> + s390_cpu_virt_mem_handle_exc(cpu, ra);
>> + return -1;
>> + }
>> +
>> + if (be32_to_cpu(vcssb->length) > sizeof(*vcssb)) {
>> + return -1;
>> + }
>
> Thanks for adding the check, but I think this should rather be :
>
> return DIAG_320_RC_INVAL_VCSSB_LEN;
>
> since we did not inject an exception in this case?
>
>> + if (be32_to_cpu(vcssb->length) < VCSSB_MIN_LEN) {
>> + return DIAG_320_RC_INVAL_VCSSB_LEN;
>> + }
>
> ...
>
>> + case DIAG_320_SUBC_QUERY_VCSI:
>> + if (!diag_parm_addr_valid(addr, sizeof(VCStorageSizeBlock), true)) {
>> + s390_program_interrupt(env, PGM_ADDRESSING, ra);
>> + return;
>> + }
>> +
>> + if (addr & 0x7) {
>> + s390_program_interrupt(env, PGM_ADDRESSING, ra);
>> + return;
>> + }
>> +
>> + rc = handle_diag320_query_vcsi(cpu, addr, r1, ra, cs);
>> + if (rc == -1) {
>> + return;
>
> ... otherwise the error will be ignored silently here and the guest will
> think that the call succeeded.
>
> Maybe you could also create some kvm-unit-tests for this new diag call that
> exercises these error scenarios, then you'll easily see whether the diag
> behaves as expected.
>
> Thanks,
> Thomas
>
>
Thanks for the suggestion. I’ll fix the return code and look into adding
kvm‑unit‑tests for this new diag call.
>> + }
>> + env->regs[r1 + 1] = rc;
>> + break;
>> default:
>> env->regs[r1 + 1] = DIAG_320_RC_NOT_SUPPORTED;
>> break;
>
