diff mbox series

[kvm-unit-tests,v4,1/7] lib: s390x: Add ap library

Message ID 20240202145913.34831-2-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: Add base AP support | expand

Commit Message

Janosch Frank Feb. 2, 2024, 2:59 p.m. UTC 
Add functions and definitions needed to test the Adjunct
Processor (AP) crypto interface.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/ap.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/s390x/ap.h | 88 +++++++++++++++++++++++++++++++++++++++++++++
 s390x/Makefile |  1 +
 3 files changed, 186 insertions(+)
 create mode 100644 lib/s390x/ap.c
 create mode 100644 lib/s390x/ap.h

Comments

Anthony Krowiak Feb. 5, 2024, 6:15 p.m. UTC | #1 
I made a few comments and suggestions. I am not very well-versed in the 
inline assembly code, so I'll leave that up to someone who is more 
knowledgeable. I copied @Harald since I believe it was him who wrote it.

On 2/2/24 9:59 AM, Janosch Frank wrote:
> Add functions and definitions needed to test the Adjunct
> Processor (AP) crypto interface.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   lib/s390x/ap.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   lib/s390x/ap.h | 88 +++++++++++++++++++++++++++++++++++++++++++++
>   s390x/Makefile |  1 +
>   3 files changed, 186 insertions(+)
>   create mode 100644 lib/s390x/ap.c
>   create mode 100644 lib/s390x/ap.h
>
> diff --git a/lib/s390x/ap.c b/lib/s390x/ap.c
> new file mode 100644
> index 00000000..9560ff64
> --- /dev/null
> +++ b/lib/s390x/ap.c
> @@ -0,0 +1,97 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * AP crypto functions
> + *
> + * Some parts taken from the Linux AP driver.
> + *
> + * Copyright IBM Corp. 2024
> + * Author: Janosch Frank <frankja@linux.ibm.com>
> + *	   Tony Krowiak <akrowia@linux.ibm.com>
> + *	   Martin Schwidefsky <schwidefsky@de.ibm.com>
> + *	   Harald Freudenberger <freude@de.ibm.com>
> + */
> +
> +#include <libcflat.h>
> +#include <interrupt.h>
> +#include <ap.h>
> +#include <asm/time.h>
> +#include <asm/facility.h>
> +
> +int ap_pqap_tapq(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw,
> +		 struct pqap_r2 *r2)
> +{
> +	struct pqap_r0 r0 = {
> +		.ap = ap,
> +		.qn = qn,
> +		.fc = PQAP_TEST_APQ
> +	};
> +	uint64_t bogus_cc = 2;
> +	int cc;
> +
> +	/*
> +	 * Test AP Queue
> +	 *
> +	 * Writes AP configuration information to the memory pointed
> +	 * at by GR2.
> +	 *
> +	 * Inputs: GR0
> +	 * Outputs: GR1 (APQSW), GR2 (tapq data)
> +	 * Synchronous
> +	 */
> +	asm volatile(
> +		"       tmll	%[bogus_cc],3\n"
> +		"	lgr	0,%[r0]\n"
> +		"	.insn	rre,0xb2af0000,0,0\n" /* PQAP */
> +		"	stg	1,%[apqsw]\n"
> +		"	stg	2,%[r2]\n"
> +		"	ipm	%[cc]\n"
> +		"	srl	%[cc],28\n"
> +		: [apqsw] "=&T" (*apqsw), [r2] "=&T" (*r2), [cc] "=&d" (cc)
> +		: [r0] "d" (r0), [bogus_cc] "d" (bogus_cc));
> +
> +	return cc;
> +}
> +
> +int ap_pqap_qci(struct ap_config_info *info)
> +{
> +	struct pqap_r0 r0 = { .fc = PQAP_QUERY_AP_CONF_INFO };
> +	uint64_t bogus_cc = 2;
> +	int cc;
> +
> +	/*
> +	 * Query AP Configuration Information
> +	 *
> +	 * Writes AP configuration information to the memory pointed
> +	 * at by GR2.
> +	 *
> +	 * Inputs: GR0, GR2 (QCI block address)
> +	 * Outputs: memory at GR2 address
> +	 * Synchronous
> +	 */
> +	asm volatile(
> +		"       tmll	%[bogus_cc],3\n"
> +		"	lgr	0,%[r0]\n"
> +		"	lgr	2,%[info]\n"
> +		"	.insn	rre,0xb2af0000,0,0\n" /* PQAP */
> +		"	ipm	%[cc]\n"
> +		"	srl	%[cc],28\n"
> +		: [cc] "=&d" (cc)
> +		: [r0] "d" (r0), [info] "d" (info), [bogus_cc] "d" (bogus_cc)
> +		: "cc", "memory", "0", "2");
> +
> +	return cc;
> +}
> +
> +/* Will later be extended to a proper setup function */
> +bool ap_setup(void)
> +{
> +	/*
> +	 * Base AP support has no STFLE or SCLP feature bit but the
> +	 * PQAP QCI support is indicated via stfle bit 12. As this
> +	 * library relies on QCI we bail out if it's not available.
> +	 */
> +	if (!test_facility(12))
> +		return false;


The STFLE.12 can be turned off when starting the guest, so this may not 
be a valid test.

We use the ap_instructions_available function (in ap.h) which executes 
the TAPQ command to verify whether the AP instructions are installed or 
not. Maybe you can do something similar here:


static inline bool ap_instructions_available(void)

{
     unsigned long reg0 = 0x0000;
     unsigned long reg1 = 0;

     asm volatile(
         "    lgr    0,%[reg0]\n"        /* qid into gr0 */
         "    lghi    1,0\n"            /* 0 into gr1 */
         "    lghi    2,0\n"            /* 0 into gr2 */
         "    .insn    rre,0xb2af0000,0,0\n"    /* PQAP(TAPQ) */
         "0:    la    %[reg1],1\n"        /* 1 into reg1 */
         "1:\n"
         EX_TABLE(0b, 1b)
         : [reg1] "+&d" (reg1)
         : [reg0] "d" (reg0)
         : "cc", "0", "1", "2");
     return reg1 != 0;
}
> +
> +	return true;
> +}
> diff --git a/lib/s390x/ap.h b/lib/s390x/ap.h
> new file mode 100644
> index 00000000..b806513f
> --- /dev/null
> +++ b/lib/s390x/ap.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * AP definitions
> + *
> + * Some parts taken from the Linux AP driver.
> + *
> + * Copyright IBM Corp. 2024
> + * Author: Janosch Frank <frankja@linux.ibm.com>
> + *	   Tony Krowiak <akrowia@linux.ibm.com>
> + *	   Martin Schwidefsky <schwidefsky@de.ibm.com>
> + *	   Harald Freudenberger <freude@de.ibm.com>
> + */
> +
> +#ifndef _S390X_AP_H_
> +#define _S390X_AP_H_
> +
> +enum PQAP_FC {
> +	PQAP_TEST_APQ,
> +	PQAP_RESET_APQ,
> +	PQAP_ZEROIZE_APQ,
> +	PQAP_QUEUE_INT_CONTRL,
> +	PQAP_QUERY_AP_CONF_INFO,
> +	PQAP_QUERY_AP_COMP_TYPE,
> +	PQAP_BEST_AP,


Maybe use abbreviations like your function names above?

	PQAP_TAPQ,
	PQAP_RAPQ,
	PQAP_ZAPQ,
	PQAP_AQIC,
	PQAP_QCI,
	PQAP_QACT,
	PQAP_QBAP


> +};
> +
> +struct ap_queue_status {
> +	uint32_t pad0;			/* Ignored padding for zArch  */


Bit 0 is the E (queue empty) bit.


> +	uint32_t empty		: 1;
> +	uint32_t replies_waiting: 1;
> +	uint32_t full		: 1;
> +	uint32_t pad1		: 4;
> +	uint32_t irq_enabled	: 1;
> +	uint32_t rc		: 8;
> +	uint32_t pad2		: 16;
> +} __attribute__((packed))  __attribute__((aligned(8)));
> +_Static_assert(sizeof(struct ap_queue_status) == sizeof(uint64_t), "APQSW size");
> +
> +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 */
> +	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;


This is the APID part of an APQN, so how about renaming to 'apid'


> +	uint8_t qn;


This is the APQI  part of an APQN, so how about renaming to 'apqi'


> +} __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];


Is there a reason why the 'Classification'  field is left out?


> +	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_setup(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
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 7fce9f9d..4f6c627d 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -110,6 +110,7 @@ cflatobjs += lib/s390x/malloc_io.o
>   cflatobjs += lib/s390x/uv.o
>   cflatobjs += lib/s390x/sie.o
>   cflatobjs += lib/s390x/fault.o
> +cflatobjs += lib/s390x/ap.o
>   
>   OBJDIRS += lib/s390x
>
Harald Freudenberger Feb. 6, 2024, 8:48 a.m. UTC | #2 
On 2024-02-05 19:15, Anthony Krowiak wrote:
> I made a few comments and suggestions. I am not very well-versed in
> the inline assembly code, so I'll leave that up to someone who is more
> knowledgeable. I copied @Harald since I believe it was him who wrote
> it.
> 
> On 2/2/24 9:59 AM, Janosch Frank wrote:
>> Add functions and definitions needed to test the Adjunct
>> Processor (AP) crypto interface.
>> 
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   lib/s390x/ap.c | 97 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/s390x/ap.h | 88 +++++++++++++++++++++++++++++++++++++++++++++
>>   s390x/Makefile |  1 +
>>   3 files changed, 186 insertions(+)
>>   create mode 100644 lib/s390x/ap.c
>>   create mode 100644 lib/s390x/ap.h
>> 
>> diff --git a/lib/s390x/ap.c b/lib/s390x/ap.c
>> new file mode 100644
>> index 00000000..9560ff64
>> --- /dev/null
>> +++ b/lib/s390x/ap.c
>> @@ -0,0 +1,97 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * AP crypto functions
>> + *
>> + * Some parts taken from the Linux AP driver.
>> + *
>> + * Copyright IBM Corp. 2024
>> + * Author: Janosch Frank <frankja@linux.ibm.com>
>> + *	   Tony Krowiak <akrowia@linux.ibm.com>
>> + *	   Martin Schwidefsky <schwidefsky@de.ibm.com>
>> + *	   Harald Freudenberger <freude@de.ibm.com>
>> + */
>> +
>> +#include <libcflat.h>
>> +#include <interrupt.h>
>> +#include <ap.h>
>> +#include <asm/time.h>
>> +#include <asm/facility.h>
>> +
>> +int ap_pqap_tapq(uint8_t ap, uint8_t qn, struct ap_queue_status 
>> *apqsw,
>> +		 struct pqap_r2 *r2)
>> +{
>> +	struct pqap_r0 r0 = {
>> +		.ap = ap,
>> +		.qn = qn,
>> +		.fc = PQAP_TEST_APQ
>> +	};
>> +	uint64_t bogus_cc = 2;
>> +	int cc;
>> +
>> +	/*
>> +	 * Test AP Queue
>> +	 *
>> +	 * Writes AP configuration information to the memory pointed
>> +	 * at by GR2.
>> +	 *
>> +	 * Inputs: GR0
>> +	 * Outputs: GR1 (APQSW), GR2 (tapq data)
>> +	 * Synchronous
>> +	 */
>> +	asm volatile(
>> +		"       tmll	%[bogus_cc],3\n"
>> +		"	lgr	0,%[r0]\n"
>> +		"	.insn	rre,0xb2af0000,0,0\n" /* PQAP */
>> +		"	stg	1,%[apqsw]\n"
>> +		"	stg	2,%[r2]\n"
>> +		"	ipm	%[cc]\n"
>> +		"	srl	%[cc],28\n"
>> +		: [apqsw] "=&T" (*apqsw), [r2] "=&T" (*r2), [cc] "=&d" (cc)
>> +		: [r0] "d" (r0), [bogus_cc] "d" (bogus_cc));
>> +
>> +	return cc;
>> +}
>> +
>> +int ap_pqap_qci(struct ap_config_info *info)
>> +{
>> +	struct pqap_r0 r0 = { .fc = PQAP_QUERY_AP_CONF_INFO };
>> +	uint64_t bogus_cc = 2;
>> +	int cc;
>> +
>> +	/*
>> +	 * Query AP Configuration Information
>> +	 *
>> +	 * Writes AP configuration information to the memory pointed
>> +	 * at by GR2.
>> +	 *
>> +	 * Inputs: GR0, GR2 (QCI block address)
>> +	 * Outputs: memory at GR2 address
>> +	 * Synchronous
>> +	 */
>> +	asm volatile(
>> +		"       tmll	%[bogus_cc],3\n"
>> +		"	lgr	0,%[r0]\n"
>> +		"	lgr	2,%[info]\n"
>> +		"	.insn	rre,0xb2af0000,0,0\n" /* PQAP */
>> +		"	ipm	%[cc]\n"
>> +		"	srl	%[cc],28\n"
>> +		: [cc] "=&d" (cc)
>> +		: [r0] "d" (r0), [info] "d" (info), [bogus_cc] "d" (bogus_cc)
>> +		: "cc", "memory", "0", "2");
>> +
>> +	return cc;
>> +}
>> +
>> +/* Will later be extended to a proper setup function */
>> +bool ap_setup(void)
>> +{
>> +	/*
>> +	 * Base AP support has no STFLE or SCLP feature bit but the
>> +	 * PQAP QCI support is indicated via stfle bit 12. As this
>> +	 * library relies on QCI we bail out if it's not available.
>> +	 */
>> +	if (!test_facility(12))
>> +		return false;
> 
> 
> The STFLE.12 can be turned off when starting the guest, so this may
> not be a valid test.
> 
> We use the ap_instructions_available function (in ap.h) which executes
> the TAPQ command to verify whether the AP instructions are installed
> or not. Maybe you can do something similar here:
> 
> 
> static inline bool ap_instructions_available(void)
> 
> {
>     unsigned long reg0 = 0x0000;
>     unsigned long reg1 = 0;
> 
>     asm volatile(
>         "    lgr    0,%[reg0]\n"        /* qid into gr0 */
>         "    lghi    1,0\n"            /* 0 into gr1 */
>         "    lghi    2,0\n"            /* 0 into gr2 */
>         "    .insn    rre,0xb2af0000,0,0\n"    /* PQAP(TAPQ) */
>         "0:    la    %[reg1],1\n"        /* 1 into reg1 */
>         "1:\n"
>         EX_TABLE(0b, 1b)
>         : [reg1] "+&d" (reg1)
>         : [reg0] "d" (reg0)
>         : "cc", "0", "1", "2");
>     return reg1 != 0;
> }

Well, as Janos wrote - this lib functions rely on having QCI available.
However, be carefully using the above function as it is setting up
an exception table entry which is to catch the illegal instruction
which will arise when no AP bus support is available.

>> +
>> +	return true;
>> +}
>> diff --git a/lib/s390x/ap.h b/lib/s390x/ap.h
>> new file mode 100644
>> index 00000000..b806513f
>> --- /dev/null
>> +++ b/lib/s390x/ap.h
>> @@ -0,0 +1,88 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * AP definitions
>> + *
>> + * Some parts taken from the Linux AP driver.
>> + *
>> + * Copyright IBM Corp. 2024
>> + * Author: Janosch Frank <frankja@linux.ibm.com>
>> + *	   Tony Krowiak <akrowia@linux.ibm.com>
>> + *	   Martin Schwidefsky <schwidefsky@de.ibm.com>
>> + *	   Harald Freudenberger <freude@de.ibm.com>
>> + */
>> +
>> +#ifndef _S390X_AP_H_
>> +#define _S390X_AP_H_
>> +
>> +enum PQAP_FC {
>> +	PQAP_TEST_APQ,
>> +	PQAP_RESET_APQ,
>> +	PQAP_ZEROIZE_APQ,
>> +	PQAP_QUEUE_INT_CONTRL,
>> +	PQAP_QUERY_AP_CONF_INFO,
>> +	PQAP_QUERY_AP_COMP_TYPE,

What is that       ^^^^^^^^^^^^^ ?
The QACT ? But that's only one PQAP subfuntion.

>> +	PQAP_BEST_AP,

And this ^^^^^^^^^^^^ ? Never heard about.

> 
> 
> Maybe use abbreviations like your function names above?
> 
> 	PQAP_TAPQ,
> 	PQAP_RAPQ,
> 	PQAP_ZAPQ,
> 	PQAP_AQIC,
> 	PQAP_QCI,
> 	PQAP_QACT,
> 	PQAP_QBAP
> 
> 
>> +};
>> +
>> +struct ap_queue_status {
>> +	uint32_t pad0;			/* Ignored padding for zArch  */
> 
> 
> Bit 0 is the E (queue empty) bit.
> 
> 
>> +	uint32_t empty		: 1;
>> +	uint32_t replies_waiting: 1;
>> +	uint32_t full		: 1;
>> +	uint32_t pad1		: 4;
>> +	uint32_t irq_enabled	: 1;
>> +	uint32_t rc		: 8;
>> +	uint32_t pad2		: 16;
>> +} __attribute__((packed))  __attribute__((aligned(8)));
>> +_Static_assert(sizeof(struct ap_queue_status) == sizeof(uint64_t), 
>> "APQSW size");
>> +
>> +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 */
>> +	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;
> 
> 
> This is the APID part of an APQN, so how about renaming to 'apid'
> 
> 
>> +	uint8_t qn;
> 
> 
> This is the APQI  part of an APQN, so how about renaming to 'apqi'
> 
> 
>> +} __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];
> 
> 
> Is there a reason why the 'Classification'  field is left out?
> 
> 
>> +	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");
>> +

Isn't this all going into the kernel? So consider using the
arch/s390/include/asm/ap.h header file instead of re-defining the 
structs.
Feel free to improve this file with reworked structs and field names if
that's required.

>> +bool ap_setup(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
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index 7fce9f9d..4f6c627d 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -110,6 +110,7 @@ cflatobjs += lib/s390x/malloc_io.o
>>   cflatobjs += lib/s390x/uv.o
>>   cflatobjs += lib/s390x/sie.o
>>   cflatobjs += lib/s390x/fault.o
>> +cflatobjs += lib/s390x/ap.o
>>     OBJDIRS += lib/s390x
>>
Janosch Frank Feb. 6, 2024, 1:42 p.m. UTC | #3 
On 2/5/24 19:15, Anthony Krowiak wrote:
> I made a few comments and suggestions. I am not very well-versed in the
> inline assembly code, so I'll leave that up to someone who is more
> knowledgeable. I copied @Harald since I believe it was him who wrote it.
> 
> On 2/2/24 9:59 AM, Janosch Frank wrote:
>> Add functions and definitions needed to test the Adjunct
>> Processor (AP) crypto interface.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>

[...]

>> +/* Will later be extended to a proper setup function */
>> +bool ap_setup(void)
>> +{
>> +	/*
>> +	 * Base AP support has no STFLE or SCLP feature bit but the
>> +	 * PQAP QCI support is indicated via stfle bit 12. As this
>> +	 * library relies on QCI we bail out if it's not available.
>> +	 */
>> +	if (!test_facility(12))
>> +		return false;
> 
> 
> The STFLE.12 can be turned off when starting the guest, so this may not
> be a valid test.
> 
> We use the ap_instructions_available function (in ap.h) which executes
> the TAPQ command to verify whether the AP instructions are installed or
> not. Maybe you can do something similar here:

This library relies on QCI, hence we only check for stfle.
I see no sense in manually probing the whole APQN space.


If stfle 12 is indicated I'd expect AP instructions to not generate 
exceptions or do they in a sane CPU model?


>> +
>> +	return true;
>> +}
>> diff --git a/lib/s390x/ap.h b/lib/s390x/ap.h
>> new file mode 100644
>> index 00000000..b806513f
>> --- /dev/null
>> +++ b/lib/s390x/ap.h
>> @@ -0,0 +1,88 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * AP definitions
>> + *
>> + * Some parts taken from the Linux AP driver.
>> + *
>> + * Copyright IBM Corp. 2024
>> + * Author: Janosch Frank <frankja@linux.ibm.com>
>> + *	   Tony Krowiak <akrowia@linux.ibm.com>
>> + *	   Martin Schwidefsky <schwidefsky@de.ibm.com>
>> + *	   Harald Freudenberger <freude@de.ibm.com>
>> + */
>> +
>> +#ifndef _S390X_AP_H_
>> +#define _S390X_AP_H_
>> +
>> +enum PQAP_FC {
>> +	PQAP_TEST_APQ,
>> +	PQAP_RESET_APQ,
>> +	PQAP_ZEROIZE_APQ,
>> +	PQAP_QUEUE_INT_CONTRL,
>> +	PQAP_QUERY_AP_CONF_INFO,
>> +	PQAP_QUERY_AP_COMP_TYPE,
>> +	PQAP_BEST_AP,
> 
> 
> Maybe use abbreviations like your function names above?
> 
> 	PQAP_TAPQ,
> 	PQAP_RAPQ,
> 	PQAP_ZAPQ,
> 	PQAP_AQIC,
> 	PQAP_QCI,
> 	PQAP_QACT,
> 	PQAP_QBAP
> 

Hmmmmmmm(TM)
My guess is that I tried making these constants readable without 
consulting architecture documents. But another option is using the 
constants that you suggested and adding comments with a long version.

Will do

[...]

>> +struct pqap_r0 {
>> +	uint32_t pad0;
>> +	uint8_t fc;
>> +	uint8_t t : 1;		/* Test facilities (TAPQ)*/
>> +	uint8_t pad1 : 7;
>> +	uint8_t ap;
> 
> 
> This is the APID part of an APQN, so how about renaming to 'apid'
> 
> 
>> +	uint8_t qn;
> 
> 
> This is the APQI  part of an APQN, so how about renaming to 'apqi'

Hmm Linux uses qid
I'll change it to the Linux naming convention, might take me a while though

> 
> 
>> +} __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];
> 
> 
> Is there a reason why the 'Classification'  field is left out?
> 

It's not used in this library and therefore I chose to not name it to 
make structs a bit more readable.

> 
>> +	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_setup(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
>> diff --git a/s390x/Makefile b/s390x/Makefile
>> index 7fce9f9d..4f6c627d 100644
>> --- a/s390x/Makefile
>> +++ b/s390x/Makefile
>> @@ -110,6 +110,7 @@ cflatobjs += lib/s390x/malloc_io.o
>>    cflatobjs += lib/s390x/uv.o
>>    cflatobjs += lib/s390x/sie.o
>>    cflatobjs += lib/s390x/fault.o
>> +cflatobjs += lib/s390x/ap.o
>>    
>>    OBJDIRS += lib/s390x
>>
Anthony Krowiak Feb. 6, 2024, 3:45 p.m. UTC | #4 
On 2/6/24 3:48 AM, Harald Freudenberger wrote:
> On 2024-02-05 19:15, Anthony Krowiak wrote:
>> I made a few comments and suggestions. I am not very well-versed in
>> the inline assembly code, so I'll leave that up to someone who is more
>> knowledgeable. I copied @Harald since I believe it was him who wrote
>> it.
>>
>> On 2/2/24 9:59 AM, Janosch Frank wrote:
>>> Add functions and definitions needed to test the Adjunct
>>> Processor (AP) crypto interface.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>   lib/s390x/ap.c | 97 
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   lib/s390x/ap.h | 88 +++++++++++++++++++++++++++++++++++++++++++++
>>>   s390x/Makefile |  1 +
>>>   3 files changed, 186 insertions(+)
>>>   create mode 100644 lib/s390x/ap.c
>>>   create mode 100644 lib/s390x/ap.h
>>>
>>> diff --git a/lib/s390x/ap.c b/lib/s390x/ap.c
>>> new file mode 100644
>>> index 00000000..9560ff64
>>> --- /dev/null
>>> +++ b/lib/s390x/ap.c
>>> @@ -0,0 +1,97 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * AP crypto functions
>>> + *
>>> + * Some parts taken from the Linux AP driver.
>>> + *
>>> + * Copyright IBM Corp. 2024
>>> + * Author: Janosch Frank <frankja@linux.ibm.com>
>>> + *       Tony Krowiak <akrowia@linux.ibm.com>
>>> + *       Martin Schwidefsky <schwidefsky@de.ibm.com>
>>> + *       Harald Freudenberger <freude@de.ibm.com>
>>> + */
>>> +
>>> +#include <libcflat.h>
>>> +#include <interrupt.h>
>>> +#include <ap.h>
>>> +#include <asm/time.h>
>>> +#include <asm/facility.h>
>>> +
>>> +int ap_pqap_tapq(uint8_t ap, uint8_t qn, struct ap_queue_status 
>>> *apqsw,
>>> +         struct pqap_r2 *r2)
>>> +{
>>> +    struct pqap_r0 r0 = {
>>> +        .ap = ap,
>>> +        .qn = qn,
>>> +        .fc = PQAP_TEST_APQ
>>> +    };
>>> +    uint64_t bogus_cc = 2;
>>> +    int cc;
>>> +
>>> +    /*
>>> +     * Test AP Queue
>>> +     *
>>> +     * Writes AP configuration information to the memory pointed
>>> +     * at by GR2.
>>> +     *
>>> +     * Inputs: GR0
>>> +     * Outputs: GR1 (APQSW), GR2 (tapq data)
>>> +     * Synchronous
>>> +     */
>>> +    asm volatile(
>>> +        "       tmll    %[bogus_cc],3\n"
>>> +        "    lgr    0,%[r0]\n"
>>> +        "    .insn    rre,0xb2af0000,0,0\n" /* PQAP */
>>> +        "    stg    1,%[apqsw]\n"
>>> +        "    stg    2,%[r2]\n"
>>> +        "    ipm    %[cc]\n"
>>> +        "    srl    %[cc],28\n"
>>> +        : [apqsw] "=&T" (*apqsw), [r2] "=&T" (*r2), [cc] "=&d" (cc)
>>> +        : [r0] "d" (r0), [bogus_cc] "d" (bogus_cc));
>>> +
>>> +    return cc;
>>> +}
>>> +
>>> +int ap_pqap_qci(struct ap_config_info *info)
>>> +{
>>> +    struct pqap_r0 r0 = { .fc = PQAP_QUERY_AP_CONF_INFO };
>>> +    uint64_t bogus_cc = 2;
>>> +    int cc;
>>> +
>>> +    /*
>>> +     * Query AP Configuration Information
>>> +     *
>>> +     * Writes AP configuration information to the memory pointed
>>> +     * at by GR2.
>>> +     *
>>> +     * Inputs: GR0, GR2 (QCI block address)
>>> +     * Outputs: memory at GR2 address
>>> +     * Synchronous
>>> +     */
>>> +    asm volatile(
>>> +        "       tmll    %[bogus_cc],3\n"
>>> +        "    lgr    0,%[r0]\n"
>>> +        "    lgr    2,%[info]\n"
>>> +        "    .insn    rre,0xb2af0000,0,0\n" /* PQAP */
>>> +        "    ipm    %[cc]\n"
>>> +        "    srl    %[cc],28\n"
>>> +        : [cc] "=&d" (cc)
>>> +        : [r0] "d" (r0), [info] "d" (info), [bogus_cc] "d" (bogus_cc)
>>> +        : "cc", "memory", "0", "2");
>>> +
>>> +    return cc;
>>> +}
>>> +
>>> +/* Will later be extended to a proper setup function */
>>> +bool ap_setup(void)
>>> +{
>>> +    /*
>>> +     * Base AP support has no STFLE or SCLP feature bit but the
>>> +     * PQAP QCI support is indicated via stfle bit 12. As this
>>> +     * library relies on QCI we bail out if it's not available.
>>> +     */
>>> +    if (!test_facility(12))
>>> +        return false;
>>
>>
>> The STFLE.12 can be turned off when starting the guest, so this may
>> not be a valid test.
>>
>> We use the ap_instructions_available function (in ap.h) which executes
>> the TAPQ command to verify whether the AP instructions are installed
>> or not. Maybe you can do something similar here:
>>
>>
>> static inline bool ap_instructions_available(void)
>>
>> {
>>     unsigned long reg0 = 0x0000;
>>     unsigned long reg1 = 0;
>>
>>     asm volatile(
>>         "    lgr    0,%[reg0]\n"        /* qid into gr0 */
>>         "    lghi    1,0\n"            /* 0 into gr1 */
>>         "    lghi    2,0\n"            /* 0 into gr2 */
>>         "    .insn    rre,0xb2af0000,0,0\n"    /* PQAP(TAPQ) */
>>         "0:    la    %[reg1],1\n"        /* 1 into reg1 */
>>         "1:\n"
>>         EX_TABLE(0b, 1b)
>>         : [reg1] "+&d" (reg1)
>>         : [reg0] "d" (reg0)
>>         : "cc", "0", "1", "2");
>>     return reg1 != 0;
>> }
>
> Well, as Janos wrote - this lib functions rely on having QCI available.
> However, be carefully using the above function as it is setting up
> an exception table entry which is to catch the illegal instruction
> which will arise when no AP bus support is available.
>
>>> +
>>> +    return true;
>>> +}
>>> diff --git a/lib/s390x/ap.h b/lib/s390x/ap.h
>>> new file mode 100644
>>> index 00000000..b806513f
>>> --- /dev/null
>>> +++ b/lib/s390x/ap.h
>>> @@ -0,0 +1,88 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * AP definitions
>>> + *
>>> + * Some parts taken from the Linux AP driver.
>>> + *
>>> + * Copyright IBM Corp. 2024
>>> + * Author: Janosch Frank <frankja@linux.ibm.com>
>>> + *       Tony Krowiak <akrowia@linux.ibm.com>
>>> + *       Martin Schwidefsky <schwidefsky@de.ibm.com>
>>> + *       Harald Freudenberger <freude@de.ibm.com>
>>> + */
>>> +
>>> +#ifndef _S390X_AP_H_
>>> +#define _S390X_AP_H_
>>> +
>>> +enum PQAP_FC {
>>> +    PQAP_TEST_APQ,
>>> +    PQAP_RESET_APQ,
>>> +    PQAP_ZEROIZE_APQ,
>>> +    PQAP_QUEUE_INT_CONTRL,
>>> +    PQAP_QUERY_AP_CONF_INFO,
>>> +    PQAP_QUERY_AP_COMP_TYPE,
>
> What is that       ^^^^^^^^^^^^^ ?
> The QACT ? But that's only one PQAP subfuntion.


PQAP(QBAP)?


>
>>> +    PQAP_BEST_AP,
>
> And this ^^^^^^^^^^^^ ? Never heard about.
>
>>
>>
>> Maybe use abbreviations like your function names above?
>>
>>     PQAP_TAPQ,
>>     PQAP_RAPQ,
>>     PQAP_ZAPQ,
>>     PQAP_AQIC,
>>     PQAP_QCI,
>>     PQAP_QACT,
>>     PQAP_QBAP
>>
>>
>>> +};
>>> +
>>> +struct ap_queue_status {
>>> +    uint32_t pad0;            /* Ignored padding for zArch */
>>
>>
>> Bit 0 is the E (queue empty) bit.
>>
>>
>>> +    uint32_t empty        : 1;
>>> +    uint32_t replies_waiting: 1;
>>> +    uint32_t full        : 1;
>>> +    uint32_t pad1        : 4;
>>> +    uint32_t irq_enabled    : 1;
>>> +    uint32_t rc        : 8;
>>> +    uint32_t pad2        : 16;
>>> +} __attribute__((packed))  __attribute__((aligned(8)));
>>> +_Static_assert(sizeof(struct ap_queue_status) == sizeof(uint64_t), 
>>> "APQSW size");
>>> +
>>> +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 */
>>> +    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;
>>
>>
>> This is the APID part of an APQN, so how about renaming to 'apid'
>>
>>
>>> +    uint8_t qn;
>>
>>
>> This is the APQI  part of an APQN, so how about renaming to 'apqi'
>>
>>
>>> +} __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];
>>
>>
>> Is there a reason why the 'Classification'  field is left out?
>>
>>
>>> +    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");
>>> +
>
> Isn't this all going into the kernel? So consider using the
> arch/s390/include/asm/ap.h header file instead of re-defining the 
> structs.
> Feel free to improve this file with reworked structs and field names if
> that's required.
>
>>> +bool ap_setup(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
>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>> index 7fce9f9d..4f6c627d 100644
>>> --- a/s390x/Makefile
>>> +++ b/s390x/Makefile
>>> @@ -110,6 +110,7 @@ cflatobjs += lib/s390x/malloc_io.o
>>>   cflatobjs += lib/s390x/uv.o
>>>   cflatobjs += lib/s390x/sie.o
>>>   cflatobjs += lib/s390x/fault.o
>>> +cflatobjs += lib/s390x/ap.o
>>>     OBJDIRS += lib/s390x
>>>
>
Anthony Krowiak Feb. 6, 2024, 3:55 p.m. UTC | #5 
On 2/6/24 8:42 AM, Janosch Frank wrote:
> On 2/5/24 19:15, Anthony Krowiak wrote:
>> I made a few comments and suggestions. I am not very well-versed in the
>> inline assembly code, so I'll leave that up to someone who is more
>> knowledgeable. I copied @Harald since I believe it was him who wrote it.
>>
>> On 2/2/24 9:59 AM, Janosch Frank wrote:
>>> Add functions and definitions needed to test the Adjunct
>>> Processor (AP) crypto interface.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>
> [...]
>
>>> +/* Will later be extended to a proper setup function */
>>> +bool ap_setup(void)
>>> +{
>>> +    /*
>>> +     * Base AP support has no STFLE or SCLP feature bit but the
>>> +     * PQAP QCI support is indicated via stfle bit 12. As this
>>> +     * library relies on QCI we bail out if it's not available.
>>> +     */
>>> +    if (!test_facility(12))
>>> +        return false;
>>
>>
>> The STFLE.12 can be turned off when starting the guest, so this may not
>> be a valid test.
>>
>> We use the ap_instructions_available function (in ap.h) which executes
>> the TAPQ command to verify whether the AP instructions are installed or
>> not. Maybe you can do something similar here:
>
> This library relies on QCI, hence we only check for stfle.
> I see no sense in manually probing the whole APQN space.


Makes sense. I was thrown off by the PQAP_FC enumeration which includes 
all of the AP function codes.


>
>
> If stfle 12 is indicated I'd expect AP instructions to not generate 
> exceptions or do they in a sane CPU model?


No, I would not expect PQAP(QCI) to generate an exception if STFLE 12 is 
indicated.


>
>
>>> +
>>> +    return true;
>>> +}
>>> diff --git a/lib/s390x/ap.h b/lib/s390x/ap.h
>>> new file mode 100644
>>> index 00000000..b806513f
>>> --- /dev/null
>>> +++ b/lib/s390x/ap.h
>>> @@ -0,0 +1,88 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * AP definitions
>>> + *
>>> + * Some parts taken from the Linux AP driver.
>>> + *
>>> + * Copyright IBM Corp. 2024
>>> + * Author: Janosch Frank <frankja@linux.ibm.com>
>>> + *       Tony Krowiak <akrowia@linux.ibm.com>
>>> + *       Martin Schwidefsky <schwidefsky@de.ibm.com>
>>> + *       Harald Freudenberger <freude@de.ibm.com>
>>> + */
>>> +
>>> +#ifndef _S390X_AP_H_
>>> +#define _S390X_AP_H_
>>> +
>>> +enum PQAP_FC {
>>> +    PQAP_TEST_APQ,
>>> +    PQAP_RESET_APQ,
>>> +    PQAP_ZEROIZE_APQ,
>>> +    PQAP_QUEUE_INT_CONTRL,
>>> +    PQAP_QUERY_AP_CONF_INFO,
>>> +    PQAP_QUERY_AP_COMP_TYPE,
>>> +    PQAP_BEST_AP,
>>
>>
>> Maybe use abbreviations like your function names above?
>>
>>     PQAP_TAPQ,
>>     PQAP_RAPQ,
>>     PQAP_ZAPQ,
>>     PQAP_AQIC,
>>     PQAP_QCI,
>>     PQAP_QACT,
>>     PQAP_QBAP
>>
>
> Hmmmmmmm(TM)
> My guess is that I tried making these constants readable without 
> consulting architecture documents. But another option is using the 
> constants that you suggested and adding comments with a long version.


I think that works out better; you won't have to abbreviate the longer 
version which will make it easier to understand.


>
> Will do
>
> [...]
>
>>> +struct pqap_r0 {
>>> +    uint32_t pad0;
>>> +    uint8_t fc;
>>> +    uint8_t t : 1;        /* Test facilities (TAPQ)*/
>>> +    uint8_t pad1 : 7;
>>> +    uint8_t ap;
>>
>>
>> This is the APID part of an APQN, so how about renaming to 'apid'
>>
>>
>>> +    uint8_t qn;
>>
>>
>> This is the APQI  part of an APQN, so how about renaming to 'apqi'
>
> Hmm Linux uses qid
> I'll change it to the Linux naming convention, might take me a while 
> though


Well, the AP bus uses qid, but the vfio_ap module and the architecture 
doc uses APQN. In any case, it's a nit and I'm not terribly concerned 
about it.


>
>>
>>
>>> +} __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];
>>
>>
>> Is there a reason why the 'Classification'  field is left out?
>>
>
> It's not used in this library and therefore I chose to not name it to 
> make structs a bit more readable.


Okay, not a problem.


>
>>
>>> +    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_setup(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
>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>> index 7fce9f9d..4f6c627d 100644
>>> --- a/s390x/Makefile
>>> +++ b/s390x/Makefile
>>> @@ -110,6 +110,7 @@ cflatobjs += lib/s390x/malloc_io.o
>>>    cflatobjs += lib/s390x/uv.o
>>>    cflatobjs += lib/s390x/sie.o
>>>    cflatobjs += lib/s390x/fault.o
>>> +cflatobjs += lib/s390x/ap.o
>>>       OBJDIRS += lib/s390x
>
Harald Freudenberger Feb. 7, 2024, 8:06 a.m. UTC | #6 
On 2024-02-06 16:55, Anthony Krowiak wrote:
> On 2/6/24 8:42 AM, Janosch Frank wrote:
>> On 2/5/24 19:15, Anthony Krowiak wrote:
>>> I made a few comments and suggestions. I am not very well-versed in 
>>> the
>>> inline assembly code, so I'll leave that up to someone who is more
>>> knowledgeable. I copied @Harald since I believe it was him who wrote 
>>> it.
>>> 
>>> On 2/2/24 9:59 AM, Janosch Frank wrote:
>>>> Add functions and definitions needed to test the Adjunct
>>>> Processor (AP) crypto interface.
>>>> 
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> 
>> [...]
>> 
>>>> +/* Will later be extended to a proper setup function */
>>>> +bool ap_setup(void)
>>>> +{
>>>> +    /*
>>>> +     * Base AP support has no STFLE or SCLP feature bit but the
>>>> +     * PQAP QCI support is indicated via stfle bit 12. As this
>>>> +     * library relies on QCI we bail out if it's not available.
>>>> +     */
>>>> +    if (!test_facility(12))
>>>> +        return false;
>>> 
>>> 
>>> The STFLE.12 can be turned off when starting the guest, so this may 
>>> not
>>> be a valid test.
>>> 
>>> We use the ap_instructions_available function (in ap.h) which 
>>> executes
>>> the TAPQ command to verify whether the AP instructions are installed 
>>> or
>>> not. Maybe you can do something similar here:
>> 
>> This library relies on QCI, hence we only check for stfle.
>> I see no sense in manually probing the whole APQN space.
> 
> 
> Makes sense. I was thrown off by the PQAP_FC enumeration which
> includes all of the AP function codes.
> 
> 
>> 
>> 
>> If stfle 12 is indicated I'd expect AP instructions to not generate 
>> exceptions or do they in a sane CPU model?
> 
> 
> No, I would not expect PQAP(QCI) to generate an exception if STFLE 12
> is indicated.
> 

Hm, I am not sure if you can rely just on checking stfle bit 12 and if 
that's available assume
you have AP instructions. I never tried this. But as far as I know the 
KVM guys there is a chance
that you see a stfle bit 12 but get an illegal instruction exception the 
moment you call
an AP instruction... Maybe check this before relying on such a thing.

>> 
>> 
>>>> +
>>>> +    return true;
>>>> +}
>>>> diff --git a/lib/s390x/ap.h b/lib/s390x/ap.h
>>>> new file mode 100644
>>>> index 00000000..b806513f
>>>> --- /dev/null
>>>> +++ b/lib/s390x/ap.h
>>>> @@ -0,0 +1,88 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>> +/*
>>>> + * AP definitions
>>>> + *
>>>> + * Some parts taken from the Linux AP driver.
>>>> + *
>>>> + * Copyright IBM Corp. 2024
>>>> + * Author: Janosch Frank <frankja@linux.ibm.com>
>>>> + *       Tony Krowiak <akrowia@linux.ibm.com>
>>>> + *       Martin Schwidefsky <schwidefsky@de.ibm.com>
>>>> + *       Harald Freudenberger <freude@de.ibm.com>
>>>> + */
>>>> +
>>>> +#ifndef _S390X_AP_H_
>>>> +#define _S390X_AP_H_
>>>> +
>>>> +enum PQAP_FC {
>>>> +    PQAP_TEST_APQ,
>>>> +    PQAP_RESET_APQ,
>>>> +    PQAP_ZEROIZE_APQ,
>>>> +    PQAP_QUEUE_INT_CONTRL,
>>>> +    PQAP_QUERY_AP_CONF_INFO,
>>>> +    PQAP_QUERY_AP_COMP_TYPE,
>>>> +    PQAP_BEST_AP,
>>> 
>>> 
>>> Maybe use abbreviations like your function names above?
>>> 
>>>     PQAP_TAPQ,
>>>     PQAP_RAPQ,
>>>     PQAP_ZAPQ,
>>>     PQAP_AQIC,
>>>     PQAP_QCI,
>>>     PQAP_QACT,
>>>     PQAP_QBAP
>>> 
>> 
>> Hmmmmmmm(TM)
>> My guess is that I tried making these constants readable without 
>> consulting architecture documents. But another option is using the 
>> constants that you suggested and adding comments with a long version.
> 
> 
> I think that works out better; you won't have to abbreviate the longer
> version which will make it easier to understand.
> 
> 
>> 
>> Will do
>> 
>> [...]
>> 
>>>> +struct pqap_r0 {
>>>> +    uint32_t pad0;
>>>> +    uint8_t fc;
>>>> +    uint8_t t : 1;        /* Test facilities (TAPQ)*/
>>>> +    uint8_t pad1 : 7;
>>>> +    uint8_t ap;
>>> 
>>> 
>>> This is the APID part of an APQN, so how about renaming to 'apid'
>>> 
>>> 
>>>> +    uint8_t qn;
>>> 
>>> 
>>> This is the APQI  part of an APQN, so how about renaming to 'apqi'
>> 
>> Hmm Linux uses qid
>> I'll change it to the Linux naming convention, might take me a while 
>> though
> 
> 
> Well, the AP bus uses qid, but the vfio_ap module and the architecture
> doc uses APQN. In any case, it's a nit and I'm not terribly concerned
> about it.
> 
> 
>> 
>>> 
>>> 
>>>> +} __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];
>>> 
>>> 
>>> Is there a reason why the 'Classification'  field is left out?
>>> 
>> 
>> It's not used in this library and therefore I chose to not name it to 
>> make structs a bit more readable.
> 
> 
> Okay, not a problem.
> 
> 
>> 
>>> 
>>>> +    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_setup(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
>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>> index 7fce9f9d..4f6c627d 100644
>>>> --- a/s390x/Makefile
>>>> +++ b/s390x/Makefile
>>>> @@ -110,6 +110,7 @@ cflatobjs += lib/s390x/malloc_io.o
>>>>    cflatobjs += lib/s390x/uv.o
>>>>    cflatobjs += lib/s390x/sie.o
>>>>    cflatobjs += lib/s390x/fault.o
>>>> +cflatobjs += lib/s390x/ap.o
>>>>       OBJDIRS += lib/s390x
>>
Anthony Krowiak Feb. 7, 2024, 2:30 p.m. UTC | #7 
On 2/7/24 3:06 AM, Harald Freudenberger wrote:
> On 2024-02-06 16:55, Anthony Krowiak wrote:
>> On 2/6/24 8:42 AM, Janosch Frank wrote:
>>> On 2/5/24 19:15, Anthony Krowiak wrote:
>>>> I made a few comments and suggestions. I am not very well-versed in 
>>>> the
>>>> inline assembly code, so I'll leave that up to someone who is more
>>>> knowledgeable. I copied @Harald since I believe it was him who 
>>>> wrote it.
>>>>
>>>> On 2/2/24 9:59 AM, Janosch Frank wrote:
>>>>> Add functions and definitions needed to test the Adjunct
>>>>> Processor (AP) crypto interface.
>>>>>
>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>
>>> [...]
>>>
>>>>> +/* Will later be extended to a proper setup function */
>>>>> +bool ap_setup(void)
>>>>> +{
>>>>> +    /*
>>>>> +     * Base AP support has no STFLE or SCLP feature bit but the
>>>>> +     * PQAP QCI support is indicated via stfle bit 12. As this
>>>>> +     * library relies on QCI we bail out if it's not available.
>>>>> +     */
>>>>> +    if (!test_facility(12))
>>>>> +        return false;
>>>>
>>>>
>>>> The STFLE.12 can be turned off when starting the guest, so this may 
>>>> not
>>>> be a valid test.
>>>>
>>>> We use the ap_instructions_available function (in ap.h) which executes
>>>> the TAPQ command to verify whether the AP instructions are 
>>>> installed or
>>>> not. Maybe you can do something similar here:
>>>
>>> This library relies on QCI, hence we only check for stfle.
>>> I see no sense in manually probing the whole APQN space.
>>
>>
>> Makes sense. I was thrown off by the PQAP_FC enumeration which
>> includes all of the AP function codes.
>>
>>
>>>
>>>
>>> If stfle 12 is indicated I'd expect AP instructions to not generate 
>>> exceptions or do they in a sane CPU model?
>>
>>
>> No, I would not expect PQAP(QCI) to generate an exception if STFLE 12
>> is indicated.
>>
>
> Hm, I am not sure if you can rely just on checking stfle bit 12 and if 
> that's available assume
> you have AP instructions. I never tried this. But as far as I know the 
> KVM guys there is a chance
> that you see a stfle bit 12 but get an illegal instruction exception 
> the moment you call
> an AP instruction... Maybe check this before relying on such a thing.


In order to set stfle bit 12 in the CPU model for a guest (apqci=on), 
you first have to set ap=on in the CPU model indicating that you want AP 
installed on the guest; so I don't think you would ever see stfle 12 
without having AP instructions. You can, however, have AP instructions 
without stfle 12 (apqci=off) in the CPU model.


>
>>>
>>>
>>>>> +
>>>>> +    return true;
>>>>> +}
>>>>> diff --git a/lib/s390x/ap.h b/lib/s390x/ap.h
>>>>> new file mode 100644
>>>>> index 00000000..b806513f
>>>>> --- /dev/null
>>>>> +++ b/lib/s390x/ap.h
>>>>> @@ -0,0 +1,88 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>>> +/*
>>>>> + * AP definitions
>>>>> + *
>>>>> + * Some parts taken from the Linux AP driver.
>>>>> + *
>>>>> + * Copyright IBM Corp. 2024
>>>>> + * Author: Janosch Frank <frankja@linux.ibm.com>
>>>>> + *       Tony Krowiak <akrowia@linux.ibm.com>
>>>>> + *       Martin Schwidefsky <schwidefsky@de.ibm.com>
>>>>> + *       Harald Freudenberger <freude@de.ibm.com>
>>>>> + */
>>>>> +
>>>>> +#ifndef _S390X_AP_H_
>>>>> +#define _S390X_AP_H_
>>>>> +
>>>>> +enum PQAP_FC {
>>>>> +    PQAP_TEST_APQ,
>>>>> +    PQAP_RESET_APQ,
>>>>> +    PQAP_ZEROIZE_APQ,
>>>>> +    PQAP_QUEUE_INT_CONTRL,
>>>>> +    PQAP_QUERY_AP_CONF_INFO,
>>>>> +    PQAP_QUERY_AP_COMP_TYPE,
>>>>> +    PQAP_BEST_AP,
>>>>
>>>>
>>>> Maybe use abbreviations like your function names above?
>>>>
>>>>     PQAP_TAPQ,
>>>>     PQAP_RAPQ,
>>>>     PQAP_ZAPQ,
>>>>     PQAP_AQIC,
>>>>     PQAP_QCI,
>>>>     PQAP_QACT,
>>>>     PQAP_QBAP
>>>>
>>>
>>> Hmmmmmmm(TM)
>>> My guess is that I tried making these constants readable without 
>>> consulting architecture documents. But another option is using the 
>>> constants that you suggested and adding comments with a long version.
>>
>>
>> I think that works out better; you won't have to abbreviate the longer
>> version which will make it easier to understand.
>>
>>
>>>
>>> Will do
>>>
>>> [...]
>>>
>>>>> +struct pqap_r0 {
>>>>> +    uint32_t pad0;
>>>>> +    uint8_t fc;
>>>>> +    uint8_t t : 1;        /* Test facilities (TAPQ)*/
>>>>> +    uint8_t pad1 : 7;
>>>>> +    uint8_t ap;
>>>>
>>>>
>>>> This is the APID part of an APQN, so how about renaming to 'apid'
>>>>
>>>>
>>>>> +    uint8_t qn;
>>>>
>>>>
>>>> This is the APQI  part of an APQN, so how about renaming to 'apqi'
>>>
>>> Hmm Linux uses qid
>>> I'll change it to the Linux naming convention, might take me a while 
>>> though
>>
>>
>> Well, the AP bus uses qid, but the vfio_ap module and the architecture
>> doc uses APQN. In any case, it's a nit and I'm not terribly concerned
>> about it.
>>
>>
>>>
>>>>
>>>>
>>>>> +} __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];
>>>>
>>>>
>>>> Is there a reason why the 'Classification'  field is left out?
>>>>
>>>
>>> It's not used in this library and therefore I chose to not name it 
>>> to make structs a bit more readable.
>>
>>
>> Okay, not a problem.
>>
>>
>>>
>>>>
>>>>> +    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_setup(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
>>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>>> index 7fce9f9d..4f6c627d 100644
>>>>> --- a/s390x/Makefile
>>>>> +++ b/s390x/Makefile
>>>>> @@ -110,6 +110,7 @@ cflatobjs += lib/s390x/malloc_io.o
>>>>>    cflatobjs += lib/s390x/uv.o
>>>>>    cflatobjs += lib/s390x/sie.o
>>>>>    cflatobjs += lib/s390x/fault.o
>>>>> +cflatobjs += lib/s390x/ap.o
>>>>>       OBJDIRS += lib/s390x
>>>
>
diff mbox series

Patch

diff --git a/lib/s390x/ap.c b/lib/s390x/ap.c
new file mode 100644
index 00000000..9560ff64
--- /dev/null
+++ b/lib/s390x/ap.c
@@ -0,0 +1,97 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AP crypto functions
+ *
+ * Some parts taken from the Linux AP driver.
+ *
+ * Copyright IBM Corp. 2024
+ * Author: Janosch Frank <frankja@linux.ibm.com>
+ *	   Tony Krowiak <akrowia@linux.ibm.com>
+ *	   Martin Schwidefsky <schwidefsky@de.ibm.com>
+ *	   Harald Freudenberger <freude@de.ibm.com>
+ */
+
+#include <libcflat.h>
+#include <interrupt.h>
+#include <ap.h>
+#include <asm/time.h>
+#include <asm/facility.h>
+
+int ap_pqap_tapq(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw,
+		 struct pqap_r2 *r2)
+{
+	struct pqap_r0 r0 = {
+		.ap = ap,
+		.qn = qn,
+		.fc = PQAP_TEST_APQ
+	};
+	uint64_t bogus_cc = 2;
+	int cc;
+
+	/*
+	 * Test AP Queue
+	 *
+	 * Writes AP configuration information to the memory pointed
+	 * at by GR2.
+	 *
+	 * Inputs: GR0
+	 * Outputs: GR1 (APQSW), GR2 (tapq data)
+	 * Synchronous
+	 */
+	asm volatile(
+		"       tmll	%[bogus_cc],3\n"
+		"	lgr	0,%[r0]\n"
+		"	.insn	rre,0xb2af0000,0,0\n" /* PQAP */
+		"	stg	1,%[apqsw]\n"
+		"	stg	2,%[r2]\n"
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28\n"
+		: [apqsw] "=&T" (*apqsw), [r2] "=&T" (*r2), [cc] "=&d" (cc)
+		: [r0] "d" (r0), [bogus_cc] "d" (bogus_cc));
+
+	return cc;
+}
+
+int ap_pqap_qci(struct ap_config_info *info)
+{
+	struct pqap_r0 r0 = { .fc = PQAP_QUERY_AP_CONF_INFO };
+	uint64_t bogus_cc = 2;
+	int cc;
+
+	/*
+	 * Query AP Configuration Information
+	 *
+	 * Writes AP configuration information to the memory pointed
+	 * at by GR2.
+	 *
+	 * Inputs: GR0, GR2 (QCI block address)
+	 * Outputs: memory at GR2 address
+	 * Synchronous
+	 */
+	asm volatile(
+		"       tmll	%[bogus_cc],3\n"
+		"	lgr	0,%[r0]\n"
+		"	lgr	2,%[info]\n"
+		"	.insn	rre,0xb2af0000,0,0\n" /* PQAP */
+		"	ipm	%[cc]\n"
+		"	srl	%[cc],28\n"
+		: [cc] "=&d" (cc)
+		: [r0] "d" (r0), [info] "d" (info), [bogus_cc] "d" (bogus_cc)
+		: "cc", "memory", "0", "2");
+
+	return cc;
+}
+
+/* Will later be extended to a proper setup function */
+bool ap_setup(void)
+{
+	/*
+	 * Base AP support has no STFLE or SCLP feature bit but the
+	 * PQAP QCI support is indicated via stfle bit 12. As this
+	 * library relies on QCI we bail out if it's not available.
+	 */
+	if (!test_facility(12))
+		return false;
+
+	return true;
+}
diff --git a/lib/s390x/ap.h b/lib/s390x/ap.h
new file mode 100644
index 00000000..b806513f
--- /dev/null
+++ b/lib/s390x/ap.h
@@ -0,0 +1,88 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AP definitions
+ *
+ * Some parts taken from the Linux AP driver.
+ *
+ * Copyright IBM Corp. 2024
+ * Author: Janosch Frank <frankja@linux.ibm.com>
+ *	   Tony Krowiak <akrowia@linux.ibm.com>
+ *	   Martin Schwidefsky <schwidefsky@de.ibm.com>
+ *	   Harald Freudenberger <freude@de.ibm.com>
+ */
+
+#ifndef _S390X_AP_H_
+#define _S390X_AP_H_
+
+enum PQAP_FC {
+	PQAP_TEST_APQ,
+	PQAP_RESET_APQ,
+	PQAP_ZEROIZE_APQ,
+	PQAP_QUEUE_INT_CONTRL,
+	PQAP_QUERY_AP_CONF_INFO,
+	PQAP_QUERY_AP_COMP_TYPE,
+	PQAP_BEST_AP,
+};
+
+struct ap_queue_status {
+	uint32_t pad0;			/* Ignored padding for zArch  */
+	uint32_t empty		: 1;
+	uint32_t replies_waiting: 1;
+	uint32_t full		: 1;
+	uint32_t pad1		: 4;
+	uint32_t irq_enabled	: 1;
+	uint32_t rc		: 8;
+	uint32_t pad2		: 16;
+} __attribute__((packed))  __attribute__((aligned(8)));
+_Static_assert(sizeof(struct ap_queue_status) == sizeof(uint64_t), "APQSW size");
+
+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 */
+	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_setup(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
diff --git a/s390x/Makefile b/s390x/Makefile
index 7fce9f9d..4f6c627d 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -110,6 +110,7 @@  cflatobjs += lib/s390x/malloc_io.o
 cflatobjs += lib/s390x/uv.o
 cflatobjs += lib/s390x/sie.o
 cflatobjs += lib/s390x/fault.o
+cflatobjs += lib/s390x/ap.o
 
 OBJDIRS += lib/s390x