Re: [kvm-unit-tests PATCH 1/5] lib: s390x: Add ap library

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Thu, 30 Mar 2023 11:42:40 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Add functions and definitions needed to test the Adjunct
> Processor (AP) crypto interface.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>

[...]

> +bool ap_check(void)
> +{
> +	struct ap_queue_status r1 = {};
> +	struct pqap_r2 r2 = {};
> +
> +	/* Base AP support has no STFLE or SCLP feature bit */

this is true, but you are also indiscriminately using a feature for
which there is a STFLE feature. since it seems you depend on that, you
might as well just check bit for STFLE.12 and assume the base support
is there if it's set

> +	expect_pgm_int();
> +	ap_pqap_tapq(0, 0, &r1, &r2);
> +
> +	if (clear_pgm_int() == PGM_INT_CODE_OPERATION)
> +		return false;
> +
> +	return true;
> +}

[...]

> +struct ap_config_info {
> +	uint8_t apsc	 : 1;	/* S bit */
> +	uint8_t apxa	 : 1;	/* N bit */
> +	uint8_t qact	 : 1;	/* C bit */
> +	uint8_t rc8a	 : 1;	/* R bit */
> +	uint8_t l	 : 1;	/* L bit */
> +	uint8_t lext	 : 3;	/* Lext bits */
> +	uint8_t reserved2[3];
> +	uint8_t Na;		/* max # of APs - 1 */
> +	uint8_t Nd;		/* max # of Domains - 1 */
> +	uint8_t reserved6[10];
> +	uint32_t apm[8];	/* AP ID mask */

is there a specific reason why these are uint32_t?
uint64_t would maybe make your life easier in subsequent patches (see my
comments there)

> +	uint32_t aqm[8];	/* AP (usage) queue mask */
> +	uint32_t adm[8];	/* AP (control) domain mask */
> +	uint8_t _reserved4[16];
> +} __attribute__((aligned(8))) __attribute__ ((__packed__));
> +_Static_assert(sizeof(struct ap_config_info) == 128, "PQAP QCI size");
> +
> +struct pqap_r0 {
> +	uint32_t pad0;
> +	uint8_t fc;
> +	uint8_t t : 1;		/* Test facilities (TAPQ)*/
> +	uint8_t pad1 : 7;
> +	uint8_t ap;
> +	uint8_t qn;
> +} __attribute__((packed))  __attribute__((aligned(8)));
> +
> +struct pqap_r2 {
> +	uint8_t s : 1;		/* Special Command facility */
> +	uint8_t m : 1;		/* AP4KM */
> +	uint8_t c : 1;		/* AP4KC */
> +	uint8_t cop : 1;	/* AP is in coprocessor mode */
> +	uint8_t acc : 1;	/* AP is in accelerator mode */
> +	uint8_t xcp : 1;	/* AP is in XCP-mode */
> +	uint8_t n : 1;		/* AP extended addressing facility */
> +	uint8_t pad_0 : 1;
> +	uint8_t pad_1[3];
> +	uint8_t at;
> +	uint8_t nd;
> +	uint8_t pad_6;
> +	uint8_t pad_7 : 4;
> +	uint8_t qd : 4;
> +} __attribute__((packed))  __attribute__((aligned(8)));
> +_Static_assert(sizeof(struct pqap_r2) == sizeof(uint64_t), "pqap_r2 size");
> +
> +bool ap_check(void);
> +int ap_pqap_tapq(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw,
> +		 struct pqap_r2 *r2);
> +int ap_pqap_qci(struct ap_config_info *info);
> +#endif