diff mbox series

[v8,04/22] s390/zcrypt: Integrate ap_asm.h into include/asm/ap.h.

Message ID 1533739472-7172-5-git-send-email-akrowiak@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show
Series vfio-ap: guest dedicated crypto adapters | expand

Commit Message

Tony Krowiak Aug. 8, 2018, 2:44 p.m. UTC
From: Harald Freudenberger <freude@de.ibm.com>

Move all the inline functions from the ap bus header
file ap_asm.h into the in-kernel api header file
arch/s390/include/asm/ap.h so that KVM can make use
of all the low level AP functions.

Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/include/asm/ap.h     |  284 ++++++++++++++++++++++++++++++++++++----
 drivers/s390/crypto/ap_asm.h   |  261 ------------------------------------
 drivers/s390/crypto/ap_bus.c   |   21 +---
 drivers/s390/crypto/ap_bus.h   |    1 +
 drivers/s390/crypto/ap_card.c  |    1 -
 drivers/s390/crypto/ap_queue.c |    1 -
 6 files changed, 259 insertions(+), 310 deletions(-)
 delete mode 100644 drivers/s390/crypto/ap_asm.h

Comments

Cornelia Huck Aug. 9, 2018, 9:06 a.m. UTC | #1
On Wed,  8 Aug 2018 10:44:14 -0400
Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:

> From: Harald Freudenberger <freude@de.ibm.com>
> 
> Move all the inline functions from the ap bus header
> file ap_asm.h into the in-kernel api header file
> arch/s390/include/asm/ap.h so that KVM can make use
> of all the low level AP functions.
> 
> Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

You should add your own s-o-b if you are sending on patches written by
others (even if it does not matter in the end, when they are merged
through a different path anyway.)

> ---
>  arch/s390/include/asm/ap.h     |  284 ++++++++++++++++++++++++++++++++++++----
>  drivers/s390/crypto/ap_asm.h   |  261 ------------------------------------
>  drivers/s390/crypto/ap_bus.c   |   21 +---
>  drivers/s390/crypto/ap_bus.h   |    1 +
>  drivers/s390/crypto/ap_card.c  |    1 -
>  drivers/s390/crypto/ap_queue.c |    1 -
>  6 files changed, 259 insertions(+), 310 deletions(-)
>  delete mode 100644 drivers/s390/crypto/ap_asm.h
> 
> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
> index c1bedb4..046e044 100644
> --- a/arch/s390/include/asm/ap.h
> +++ b/arch/s390/include/asm/ap.h
> @@ -47,6 +47,50 @@ struct ap_queue_status {
>  };
>  
>  /**
> + * ap_intructions_available() - Test if AP instructions are available.
> + *
> + * Returns 0 if the AP instructions are installed.

Stumbled over this when I was looking at the usage in patch 7: if I see
a function called '_available' return 0, I'd assume that whatever the
function tests for is *not* available.

Rather call this function ap_instructions_check_availability() (and
keep the return code convention), or switch this to return 0 if not
available and !0 if available?

> + */
> +static inline int ap_instructions_available(void)
> +{
> +	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
> +	register unsigned long reg1 asm ("1") = -ENODEV;
> +	register unsigned long reg2 asm ("2");
> +
> +	asm volatile(
> +		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
> +		"0: la    %0,0\n"
> +		"1:\n"
> +		EX_TABLE(0b, 1b)
> +		: "+d" (reg1), "=d" (reg2)
> +		: "d" (reg0)
> +		: "cc");
> +	return reg1;
> +}
Harald Freudenberger Aug. 9, 2018, 9:17 a.m. UTC | #2
On 09.08.2018 11:06, Cornelia Huck wrote:
> On Wed,  8 Aug 2018 10:44:14 -0400
> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>
>> From: Harald Freudenberger <freude@de.ibm.com>
>>
>> Move all the inline functions from the ap bus header
>> file ap_asm.h into the in-kernel api header file
>> arch/s390/include/asm/ap.h so that KVM can make use
>> of all the low level AP functions.
>>
>> Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> You should add your own s-o-b if you are sending on patches written by
> others (even if it does not matter in the end, when they are merged
> through a different path anyway.)
>
>> ---
>>  arch/s390/include/asm/ap.h     |  284 ++++++++++++++++++++++++++++++++++++----
>>  drivers/s390/crypto/ap_asm.h   |  261 ------------------------------------
>>  drivers/s390/crypto/ap_bus.c   |   21 +---
>>  drivers/s390/crypto/ap_bus.h   |    1 +
>>  drivers/s390/crypto/ap_card.c  |    1 -
>>  drivers/s390/crypto/ap_queue.c |    1 -
>>  6 files changed, 259 insertions(+), 310 deletions(-)
>>  delete mode 100644 drivers/s390/crypto/ap_asm.h
>>
>> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
>> index c1bedb4..046e044 100644
>> --- a/arch/s390/include/asm/ap.h
>> +++ b/arch/s390/include/asm/ap.h
>> @@ -47,6 +47,50 @@ struct ap_queue_status {
>>  };
>>  
>>  /**
>> + * ap_intructions_available() - Test if AP instructions are available.
>> + *
>> + * Returns 0 if the AP instructions are installed.
> Stumbled over this when I was looking at the usage in patch 7: if I see
> a function called '_available' return 0, I'd assume that whatever the
> function tests for is *not* available.
>
> Rather call this function ap_instructions_check_availability() (and
> keep the return code convention), or switch this to return 0 if not
> available and !0 if available?
Good catch, Cony you are right. I'll fix this to return 1 if AP instructions
are available and 0 if not. However, this patch will come via Martin's pipe
to the Linus Torwald kernel sources.
>
>> + */
>> +static inline int ap_instructions_available(void)
>> +{
>> +	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
>> +	register unsigned long reg1 asm ("1") = -ENODEV;
>> +	register unsigned long reg2 asm ("2");
>> +
>> +	asm volatile(
>> +		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
>> +		"0: la    %0,0\n"
>> +		"1:\n"
>> +		EX_TABLE(0b, 1b)
>> +		: "+d" (reg1), "=d" (reg2)
>> +		: "d" (reg0)
>> +		: "cc");
>> +	return reg1;
>> +}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-s390" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Harald Freudenberger Aug. 9, 2018, 1:45 p.m. UTC | #3
Here is now a reworked version of the integrate ap_asm.h into include/asm/ap.h patch
which can be used to apply it within the AP virtualisation patch queue for testing:

From c81710e7cd073c4f9a904f3539ecf17fd89c9c2d Mon Sep 17 00:00:00 2001
From: Harald Freudenberger <freude@de.ibm.com>
Date: Tue, 12 Jun 2018 15:42:36 +0200
Subject: [PATCH] s390/zcrypt: Integrate ap_asm.h into include/asm/ap.h.

Move all the inline functions from the ap bus header
file ap_asm.h into the in-kernel api header file
arch/s390/include/asm/ap.h so that KVM can make use
of all the low level AP functions.

Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
---
 arch/s390/include/asm/ap.h     | 284 +++++++++++++++++++++++++++++----
 drivers/s390/crypto/ap_asm.h   | 261 ------------------------------
 drivers/s390/crypto/ap_bus.c   |  23 +--
 drivers/s390/crypto/ap_bus.h   |   1 +
 drivers/s390/crypto/ap_card.c  |   1 -
 drivers/s390/crypto/ap_queue.c |   1 -
 6 files changed, 260 insertions(+), 311 deletions(-)
 delete mode 100644 drivers/s390/crypto/ap_asm.h

diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
index c1bedb4c8de0..887494aa164f 100644
--- a/arch/s390/include/asm/ap.h
+++ b/arch/s390/include/asm/ap.h
@@ -46,6 +46,50 @@ struct ap_queue_status {
     unsigned int _pad2        : 16;
 };
 
+/**
+ * ap_intructions_available() - Test if AP instructions are available.
+ *
+ * Returns 1 if the AP instructions are installed, otherwise 0.
+ */
+static inline int ap_instructions_available(void)
+{
+    register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
+    register unsigned long reg1 asm ("1") = 0;
+    register unsigned long reg2 asm ("2") = 0;
+
+    asm volatile(
+        "   .long 0xb2af0000\n"        /* PQAP(TAPQ) */
+        "0: la    %0,1\n"
+        "1:\n"
+        EX_TABLE(0b, 1b)
+        : "+d" (reg1), "+d" (reg2)
+        : "d" (reg0)
+        : "cc");
+    return reg1;
+}
+
+/**
+ * ap_tapq(): Test adjunct processor queue.
+ * @qid: The AP queue number
+ * @info: Pointer to queue descriptor
+ *
+ * Returns AP queue status structure.
+ */
+static inline struct ap_queue_status ap_tapq(ap_qid_t qid, unsigned long *info)
+{
+    register unsigned long reg0 asm ("0") = qid;
+    register struct ap_queue_status reg1 asm ("1");
+    register unsigned long reg2 asm ("2");
+
+    asm volatile(".long 0xb2af0000"        /* PQAP(TAPQ) */
+             : "=d" (reg1), "=d" (reg2)
+             : "d" (reg0)
+             : "cc");
+    if (info)
+        *info = reg2;
+    return reg1;
+}
+
 /**
  * ap_test_queue(): Test adjunct processor queue.
  * @qid: The AP queue number
@@ -54,10 +98,57 @@ struct ap_queue_status {
  *
  * Returns AP queue status structure.
  */
-struct ap_queue_status ap_test_queue(ap_qid_t qid,
-                     int tbit,
-                     unsigned long *info);
+static inline struct ap_queue_status ap_test_queue(ap_qid_t qid,
+                           int tbit,
+                           unsigned long *info)
+{
+    if (tbit)
+        qid |= 1UL << 23; /* set T bit*/
+    return ap_tapq(qid, info);
+}
 
+/**
+ * ap_pqap_rapq(): Reset adjunct processor queue.
+ * @qid: The AP queue number
+ *
+ * Returns AP queue status structure.
+ */
+static inline struct ap_queue_status ap_rapq(ap_qid_t qid)
+{
+    register unsigned long reg0 asm ("0") = qid | (1UL << 24);
+    register struct ap_queue_status reg1 asm ("1");
+
+    asm volatile(
+        ".long 0xb2af0000"        /* PQAP(RAPQ) */
+        : "=d" (reg1)
+        : "d" (reg0)
+        : "cc");
+    return reg1;
+}
+
+/**
+ * ap_pqap_zapq(): Reset and zeroize adjunct processor queue.
+ * @qid: The AP queue number
+ *
+ * Returns AP queue status structure.
+ */
+static inline struct ap_queue_status ap_zapq(ap_qid_t qid)
+{
+    register unsigned long reg0 asm ("0") = qid | (2UL << 24);
+    register struct ap_queue_status reg1 asm ("1");
+
+    asm volatile(
+        ".long 0xb2af0000"        /* PQAP(ZAPQ) */
+        : "=d" (reg1)
+        : "d" (reg0)
+        : "cc");
+    return reg1;
+}
+
+/**
+ * struct ap_config_info - convenience struct for AP crypto
+ * config info as returned by the ap_qci() function.
+ */
 struct ap_config_info {
     unsigned int apsc     : 1;    /* S bit */
     unsigned int apxa     : 1;    /* N bit */
@@ -74,50 +165,189 @@ struct ap_config_info {
     unsigned char _reserved4[16];
 } __aligned(8);
 
-/*
- * ap_query_configuration(): Fetch cryptographic config info
+/**
+ * ap_qci(): Get AP configuration data
  *
- * Returns the ap configuration info fetched via PQAP(QCI).
- * On success 0 is returned, on failure a negative errno
- * is returned, e.g. if the PQAP(QCI) instruction is not
- * available, the return value will be -EOPNOTSUPP.
+ * Returns 0 on success, or -EOPNOTSUPP.
  */
-int ap_query_configuration(struct ap_config_info *info);
+static inline int ap_qci(struct ap_config_info *config)
+{
+    register unsigned long reg0 asm ("0") = 4UL << 24;
+    register unsigned long reg1 asm ("1") = -EOPNOTSUPP;
+    register struct ap_config_info *reg2 asm ("2") = config;
+
+    asm volatile(
+        ".long 0xb2af0000\n"        /* PQAP(QCI) */
+        "0: la    %0,0\n"
+        "1:\n"
+        EX_TABLE(0b, 1b)
+        : "+d" (reg1)
+        : "d" (reg0), "d" (reg2)
+        : "cc", "memory");
+
+    return reg1;
+}
 
 /*
  * struct ap_qirq_ctrl - convenient struct for easy invocation
- * of the ap_queue_irq_ctrl() function. This struct is passed
- * as GR1 parameter to the PQAP(AQIC) instruction. For details
- * please see the AR documentation.
+ * of the ap_aqic() function. This struct is passed as GR1
+ * parameter to the PQAP(AQIC) instruction. For details please
+ * see the AR documentation.
  */
 struct ap_qirq_ctrl {
     unsigned int _res1 : 8;
-    unsigned int zone  : 8;  /* zone info */
-    unsigned int ir    : 1;  /* ir flag: enable (1) or disable (0) irq */
+    unsigned int zone  : 8;    /* zone info */
+    unsigned int ir    : 1;    /* ir flag: enable (1) or disable (0) irq */
     unsigned int _res2 : 4;
-    unsigned int gisc  : 3;  /* guest isc field */
+    unsigned int gisc  : 3;    /* guest isc field */
     unsigned int _res3 : 6;
-    unsigned int gf    : 2;  /* gisa format */
+    unsigned int gf    : 2;    /* gisa format */
     unsigned int _res4 : 1;
-    unsigned int gisa  : 27; /* gisa origin */
+    unsigned int gisa  : 27;    /* gisa origin */
     unsigned int _res5 : 1;
-    unsigned int isc   : 3;  /* irq sub class */
+    unsigned int isc   : 3;    /* irq sub class */
 };
 
 /**
- * ap_queue_irq_ctrl(): Control interruption on a AP queue.
+ * ap_aqic(): Control interruption for a specific AP.
  * @qid: The AP queue number
- * @qirqctrl: struct ap_qirq_ctrl, see above
+ * @qirqctrl: struct ap_qirq_ctrl (64 bit value)
  * @ind: The notification indicator byte
  *
  * Returns AP queue status.
+ */
+static inline struct ap_queue_status ap_aqic(ap_qid_t qid,
+                         struct ap_qirq_ctrl qirqctrl,
+                         void *ind)
+{
+    register unsigned long reg0 asm ("0") = qid | (3UL << 24);
+    register struct ap_qirq_ctrl reg1_in asm ("1") = qirqctrl;
+    register struct ap_queue_status reg1_out asm ("1");
+    register void *reg2 asm ("2") = ind;
+
+    asm volatile(
+        ".long 0xb2af0000"        /* PQAP(AQIC) */
+        : "=d" (reg1_out)
+        : "d" (reg0), "d" (reg1_in), "d" (reg2)
+        : "cc");
+    return reg1_out;
+}
+
+/*
+ * union ap_qact_ap_info - used together with the
+ * ap_aqic() function to provide a convenient way
+ * to handle the ap info needed by the qact function.
+ */
+union ap_qact_ap_info {
+    unsigned long val;
+    struct {
+        unsigned int      : 3;
+        unsigned int mode : 3;
+        unsigned int      : 26;
+        unsigned int cat  : 8;
+        unsigned int      : 8;
+        unsigned char ver[2];
+    };
+};
+
+/**
+ * ap_qact(): Query AP combatibility type.
+ * @qid: The AP queue number
+ * @apinfo: On input the info about the AP queue. On output the
+ *        alternate AP queue info provided by the qact function
+ *        in GR2 is stored in.
  *
- * Control interruption on the given AP queue.
- * Just a simple wrapper function for the low level PQAP(AQIC)
- * instruction available for other kernel modules.
+ * Returns AP queue status. Check response_code field for failures.
  */
-struct ap_queue_status ap_queue_irq_ctrl(ap_qid_t qid,
-                     struct ap_qirq_ctrl qirqctrl,
-                     void *ind);
+static inline struct ap_queue_status ap_qact(ap_qid_t qid, int ifbit,
+                         union ap_qact_ap_info *apinfo)
+{
+    register unsigned long reg0 asm ("0") = qid | (5UL << 24)
+        | ((ifbit & 0x01) << 22);
+    register unsigned long reg1_in asm ("1") = apinfo->val;
+    register struct ap_queue_status reg1_out asm ("1");
+    register unsigned long reg2 asm ("2");
+
+    asm volatile(
+        ".long 0xb2af0000"        /* PQAP(QACT) */
+        : "+d" (reg1_in), "=d" (reg1_out), "=d" (reg2)
+        : "d" (reg0)
+        : "cc");
+    apinfo->val = reg2;
+    return reg1_out;
+}
+
+/**
+ * ap_nqap(): Send message to adjunct processor queue.
+ * @qid: The AP queue number
+ * @psmid: The program supplied message identifier
+ * @msg: The message text
+ * @length: The message length
+ *
+ * Returns AP queue status structure.
+ * Condition code 1 on NQAP can't happen because the L bit is 1.
+ * Condition code 2 on NQAP also means the send is incomplete,
+ * because a segment boundary was reached. The NQAP is repeated.
+ */
+static inline struct ap_queue_status ap_nqap(ap_qid_t qid,
+                         unsigned long long psmid,
+                         void *msg, size_t length)
+{
+    register unsigned long reg0 asm ("0") = qid | 0x40000000UL;
+    register struct ap_queue_status reg1 asm ("1");
+    register unsigned long reg2 asm ("2") = (unsigned long) msg;
+    register unsigned long reg3 asm ("3") = (unsigned long) length;
+    register unsigned long reg4 asm ("4") = (unsigned int) (psmid >> 32);
+    register unsigned long reg5 asm ("5") = psmid & 0xffffffff;
+
+    asm volatile (
+        "0: .long 0xb2ad0042\n"        /* NQAP */
+        "   brc   2,0b"
+        : "+d" (reg0), "=d" (reg1), "+d" (reg2), "+d" (reg3)
+        : "d" (reg4), "d" (reg5)
+        : "cc", "memory");
+    return reg1;
+}
+
+/**
+ * ap_dqap(): Receive message from adjunct processor queue.
+ * @qid: The AP queue number
+ * @psmid: Pointer to program supplied message identifier
+ * @msg: The message text
+ * @length: The message length
+ *
+ * Returns AP queue status structure.
+ * Condition code 1 on DQAP means the receive has taken place
+ * but only partially.    The response is incomplete, hence the
+ * DQAP is repeated.
+ * Condition code 2 on DQAP also means the receive is incomplete,
+ * this time because a segment boundary was reached. Again, the
+ * DQAP is repeated.
+ * Note that gpr2 is used by the DQAP instruction to keep track of
+ * any 'residual' length, in case the instruction gets interrupted.
+ * Hence it gets zeroed before the instruction.
+ */
+static inline struct ap_queue_status ap_dqap(ap_qid_t qid,
+                         unsigned long long *psmid,
+                         void *msg, size_t length)
+{
+    register unsigned long reg0 asm("0") = qid | 0x80000000UL;
+    register struct ap_queue_status reg1 asm ("1");
+    register unsigned long reg2 asm("2") = 0UL;
+    register unsigned long reg4 asm("4") = (unsigned long) msg;
+    register unsigned long reg5 asm("5") = (unsigned long) length;
+    register unsigned long reg6 asm("6") = 0UL;
+    register unsigned long reg7 asm("7") = 0UL;
+
+
+    asm volatile(
+        "0: .long 0xb2ae0064\n"        /* DQAP */
+        "   brc   6,0b\n"
+        : "+d" (reg0), "=d" (reg1), "+d" (reg2),
+          "+d" (reg4), "+d" (reg5), "+d" (reg6), "+d" (reg7)
+        : : "cc", "memory");
+    *psmid = (((unsigned long long) reg6) << 32) + reg7;
+    return reg1;
+}
 
 #endif /* _ASM_S390_AP_H_ */
diff --git a/drivers/s390/crypto/ap_asm.h b/drivers/s390/crypto/ap_asm.h
deleted file mode 100644
index e22ee126c9df..000000000000
--- a/drivers/s390/crypto/ap_asm.h
+++ /dev/null
@@ -1,261 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright IBM Corp. 2016
- * Author(s): Martin Schwidefsky <schwidefsky@de.ibm.com>
- *
- * Adjunct processor bus inline assemblies.
- */
-
-#ifndef _AP_ASM_H_
-#define _AP_ASM_H_
-
-#include <asm/isc.h>
-
-/**
- * ap_intructions_available() - Test if AP instructions are available.
- *
- * Returns 0 if the AP instructions are installed.
- */
-static inline int ap_instructions_available(void)
-{
-    register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
-    register unsigned long reg1 asm ("1") = -ENODEV;
-    register unsigned long reg2 asm ("2");
-
-    asm volatile(
-        "   .long 0xb2af0000\n"        /* PQAP(TAPQ) */
-        "0: la    %0,0\n"
-        "1:\n"
-        EX_TABLE(0b, 1b)
-        : "+d" (reg1), "=d" (reg2)
-        : "d" (reg0)
-        : "cc");
-    return reg1;
-}
-
-/**
- * ap_tapq(): Test adjunct processor queue.
- * @qid: The AP queue number
- * @info: Pointer to queue descriptor
- *
- * Returns AP queue status structure.
- */
-static inline struct ap_queue_status ap_tapq(ap_qid_t qid, unsigned long *info)
-{
-    register unsigned long reg0 asm ("0") = qid;
-    register struct ap_queue_status reg1 asm ("1");
-    register unsigned long reg2 asm ("2");
-
-    asm volatile(".long 0xb2af0000"        /* PQAP(TAPQ) */
-             : "=d" (reg1), "=d" (reg2)
-             : "d" (reg0)
-             : "cc");
-    if (info)
-        *info = reg2;
-    return reg1;
-}
-
-/**
- * ap_pqap_rapq(): Reset adjunct processor queue.
- * @qid: The AP queue number
- *
- * Returns AP queue status structure.
- */
-static inline struct ap_queue_status ap_rapq(ap_qid_t qid)
-{
-    register unsigned long reg0 asm ("0") = qid | (1UL << 24);
-    register struct ap_queue_status reg1 asm ("1");
-
-    asm volatile(
-        ".long 0xb2af0000"        /* PQAP(RAPQ) */
-        : "=d" (reg1)
-        : "d" (reg0)
-        : "cc");
-    return reg1;
-}
-
-/**
- * ap_pqap_zapq(): Reset and zeroize adjunct processor queue.
- * @qid: The AP queue number
- *
- * Returns AP queue status structure.
- */
-static inline struct ap_queue_status ap_zapq(ap_qid_t qid)
-{
-    register unsigned long reg0 asm ("0") = qid | (2UL << 24);
-    register struct ap_queue_status reg1 asm ("1");
-
-    asm volatile(
-        ".long 0xb2af0000"        /* PQAP(ZAPQ) */
-        : "=d" (reg1)
-        : "d" (reg0)
-        : "cc");
-    return reg1;
-}
-
-/**
- * ap_aqic(): Control interruption for a specific AP.
- * @qid: The AP queue number
- * @qirqctrl: struct ap_qirq_ctrl (64 bit value)
- * @ind: The notification indicator byte
- *
- * Returns AP queue status.
- */
-static inline struct ap_queue_status ap_aqic(ap_qid_t qid,
-                         struct ap_qirq_ctrl qirqctrl,
-                         void *ind)
-{
-    register unsigned long reg0 asm ("0") = qid | (3UL << 24);
-    register struct ap_qirq_ctrl reg1_in asm ("1") = qirqctrl;
-    register struct ap_queue_status reg1_out asm ("1");
-    register void *reg2 asm ("2") = ind;
-
-    asm volatile(
-        ".long 0xb2af0000"        /* PQAP(AQIC) */
-        : "=d" (reg1_out)
-        : "d" (reg0), "d" (reg1_in), "d" (reg2)
-        : "cc");
-    return reg1_out;
-}
-
-/**
- * ap_qci(): Get AP configuration data
- *
- * Returns 0 on success, or -EOPNOTSUPP.
- */
-static inline int ap_qci(void *config)
-{
-    register unsigned long reg0 asm ("0") = 4UL << 24;
-    register unsigned long reg1 asm ("1") = -EINVAL;
-    register void *reg2 asm ("2") = (void *) config;
-
-    asm volatile(
-        ".long 0xb2af0000\n"        /* PQAP(QCI) */
-        "0: la    %0,0\n"
-        "1:\n"
-        EX_TABLE(0b, 1b)
-        : "+d" (reg1)
-        : "d" (reg0), "d" (reg2)
-        : "cc", "memory");
-
-    return reg1;
-}
-
-/*
- * union ap_qact_ap_info - used together with the
- * ap_aqic() function to provide a convenient way
- * to handle the ap info needed by the qact function.
- */
-union ap_qact_ap_info {
-    unsigned long val;
-    struct {
-        unsigned int      : 3;
-        unsigned int mode : 3;
-        unsigned int      : 26;
-        unsigned int cat  : 8;
-        unsigned int      : 8;
-        unsigned char ver[2];
-    };
-};
-
-/**
- * ap_qact(): Query AP combatibility type.
- * @qid: The AP queue number
- * @apinfo: On input the info about the AP queue. On output the
- *        alternate AP queue info provided by the qact function
- *        in GR2 is stored in.
- *
- * Returns AP queue status. Check response_code field for failures.
- */
-static inline struct ap_queue_status ap_qact(ap_qid_t qid, int ifbit,
-                         union ap_qact_ap_info *apinfo)
-{
-    register unsigned long reg0 asm ("0") = qid | (5UL << 24)
-        | ((ifbit & 0x01) << 22);
-    register unsigned long reg1_in asm ("1") = apinfo->val;
-    register struct ap_queue_status reg1_out asm ("1");
-    register unsigned long reg2 asm ("2");
-
-    asm volatile(
-        ".long 0xb2af0000"        /* PQAP(QACT) */
-        : "+d" (reg1_in), "=d" (reg1_out), "=d" (reg2)
-        : "d" (reg0)
-        : "cc");
-    apinfo->val = reg2;
-    return reg1_out;
-}
-
-/**
- * ap_nqap(): Send message to adjunct processor queue.
- * @qid: The AP queue number
- * @psmid: The program supplied message identifier
- * @msg: The message text
- * @length: The message length
- *
- * Returns AP queue status structure.
- * Condition code 1 on NQAP can't happen because the L bit is 1.
- * Condition code 2 on NQAP also means the send is incomplete,
- * because a segment boundary was reached. The NQAP is repeated.
- */
-static inline struct ap_queue_status ap_nqap(ap_qid_t qid,
-                         unsigned long long psmid,
-                         void *msg, size_t length)
-{
-    register unsigned long reg0 asm ("0") = qid | 0x40000000UL;
-    register struct ap_queue_status reg1 asm ("1");
-    register unsigned long reg2 asm ("2") = (unsigned long) msg;
-    register unsigned long reg3 asm ("3") = (unsigned long) length;
-    register unsigned long reg4 asm ("4") = (unsigned int) (psmid >> 32);
-    register unsigned long reg5 asm ("5") = psmid & 0xffffffff;
-
-    asm volatile (
-        "0: .long 0xb2ad0042\n"        /* NQAP */
-        "   brc   2,0b"
-        : "+d" (reg0), "=d" (reg1), "+d" (reg2), "+d" (reg3)
-        : "d" (reg4), "d" (reg5)
-        : "cc", "memory");
-    return reg1;
-}
-
-/**
- * ap_dqap(): Receive message from adjunct processor queue.
- * @qid: The AP queue number
- * @psmid: Pointer to program supplied message identifier
- * @msg: The message text
- * @length: The message length
- *
- * Returns AP queue status structure.
- * Condition code 1 on DQAP means the receive has taken place
- * but only partially.    The response is incomplete, hence the
- * DQAP is repeated.
- * Condition code 2 on DQAP also means the receive is incomplete,
- * this time because a segment boundary was reached. Again, the
- * DQAP is repeated.
- * Note that gpr2 is used by the DQAP instruction to keep track of
- * any 'residual' length, in case the instruction gets interrupted.
- * Hence it gets zeroed before the instruction.
- */
-static inline struct ap_queue_status ap_dqap(ap_qid_t qid,
-                         unsigned long long *psmid,
-                         void *msg, size_t length)
-{
-    register unsigned long reg0 asm("0") = qid | 0x80000000UL;
-    register struct ap_queue_status reg1 asm ("1");
-    register unsigned long reg2 asm("2") = 0UL;
-    register unsigned long reg4 asm("4") = (unsigned long) msg;
-    register unsigned long reg5 asm("5") = (unsigned long) length;
-    register unsigned long reg6 asm("6") = 0UL;
-    register unsigned long reg7 asm("7") = 0UL;
-
-
-    asm volatile(
-        "0: .long 0xb2ae0064\n"        /* DQAP */
-        "   brc   6,0b\n"
-        : "+d" (reg0), "=d" (reg1), "+d" (reg2),
-          "+d" (reg4), "+d" (reg5), "+d" (reg6), "+d" (reg7)
-        : : "cc", "memory");
-    *psmid = (((unsigned long long) reg6) << 32) + reg7;
-    return reg1;
-}
-
-#endif /* _AP_ASM_H_ */
diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
index 35a0c2b52f82..aa1bbe6d7842 100644
--- a/drivers/s390/crypto/ap_bus.c
+++ b/drivers/s390/crypto/ap_bus.c
@@ -36,7 +36,6 @@
 #include <linux/debugfs.h>
 
 #include "ap_bus.h"
-#include "ap_asm.h"
 #include "ap_debug.h"
 
 /*
@@ -174,24 +173,6 @@ static inline int ap_qact_available(void)
     return 0;
 }
 
-/**
- * ap_test_queue(): Test adjunct processor queue.
- * @qid: The AP queue number
- * @tbit: Test facilities bit
- * @info: Pointer to queue descriptor
- *
- * Returns AP queue status structure.
- */
-struct ap_queue_status ap_test_queue(ap_qid_t qid,
-                     int tbit,
-                     unsigned long *info)
-{
-    if (tbit)
-        qid |= 1UL << 23; /* set T bit*/
-    return ap_tapq(qid, info);
-}
-EXPORT_SYMBOL(ap_test_queue);
-
 /*
  * ap_query_configuration(): Fetch cryptographic config info
  *
@@ -200,7 +181,7 @@ EXPORT_SYMBOL(ap_test_queue);
  * is returned, e.g. if the PQAP(QCI) instruction is not
  * available, the return value will be -EOPNOTSUPP.
  */
-int ap_query_configuration(struct ap_config_info *info)
+static inline int ap_query_configuration(struct ap_config_info *info)
 {
     if (!ap_configuration_available())
         return -EOPNOTSUPP;
@@ -1220,7 +1201,7 @@ static int __init ap_module_init(void)
     if (rc)
         return rc;
 
-    if (ap_instructions_available() != 0) {
+    if (!ap_instructions_available()) {
         pr_warn("The hardware system does not support AP instructions\n");
         return -ENODEV;
     }
diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h
index 6a273c5ebca5..936541937e15 100644
--- a/drivers/s390/crypto/ap_bus.h
+++ b/drivers/s390/crypto/ap_bus.h
@@ -15,6 +15,7 @@
 
 #include <linux/device.h>
 #include <linux/types.h>
+#include <asm/isc.h>
 #include <asm/ap.h>
 
 #define AP_DEVICES 256        /* Number of AP devices. */
diff --git a/drivers/s390/crypto/ap_card.c b/drivers/s390/crypto/ap_card.c
index 2c726df210f6..c13e43292cb7 100644
--- a/drivers/s390/crypto/ap_card.c
+++ b/drivers/s390/crypto/ap_card.c
@@ -14,7 +14,6 @@
 #include <asm/facility.h>
 
 #include "ap_bus.h"
-#include "ap_asm.h"
 
 /*
  * AP card related attributes.
diff --git a/drivers/s390/crypto/ap_queue.c b/drivers/s390/crypto/ap_queue.c
index ba3a2e13b0eb..d83c1fa291ae 100644
--- a/drivers/s390/crypto/ap_queue.c
+++ b/drivers/s390/crypto/ap_queue.c
@@ -14,7 +14,6 @@
 #include <asm/facility.h>
 
 #include "ap_bus.h"
-#include "ap_asm.h"
 
 /**
  * ap_queue_irq_ctrl(): Control interruption on a AP queue.
Anthony Krowiak Aug. 9, 2018, 3:18 p.m. UTC | #4
On 08/09/2018 05:06 AM, Cornelia Huck wrote:
> On Wed,  8 Aug 2018 10:44:14 -0400
> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>
>> From: Harald Freudenberger <freude@de.ibm.com>
>>
>> Move all the inline functions from the ap bus header
>> file ap_asm.h into the in-kernel api header file
>> arch/s390/include/asm/ap.h so that KVM can make use
>> of all the low level AP functions.
>>
>> Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> You should add your own s-o-b if you are sending on patches written by
> others (even if it does not matter in the end, when they are merged
> through a different path anyway.)

That's rather ironic given I was told in an internal review that I should
not sign off on patches I did not write.

>
>> ---
>>   arch/s390/include/asm/ap.h     |  284 ++++++++++++++++++++++++++++++++++++----
>>   drivers/s390/crypto/ap_asm.h   |  261 ------------------------------------
>>   drivers/s390/crypto/ap_bus.c   |   21 +---
>>   drivers/s390/crypto/ap_bus.h   |    1 +
>>   drivers/s390/crypto/ap_card.c  |    1 -
>>   drivers/s390/crypto/ap_queue.c |    1 -
>>   6 files changed, 259 insertions(+), 310 deletions(-)
>>   delete mode 100644 drivers/s390/crypto/ap_asm.h
>>
>> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
>> index c1bedb4..046e044 100644
>> --- a/arch/s390/include/asm/ap.h
>> +++ b/arch/s390/include/asm/ap.h
>> @@ -47,6 +47,50 @@ struct ap_queue_status {
>>   };
>>   
>>   /**
>> + * ap_intructions_available() - Test if AP instructions are available.
>> + *
>> + * Returns 0 if the AP instructions are installed.
> Stumbled over this when I was looking at the usage in patch 7: if I see
> a function called '_available' return 0, I'd assume that whatever the
> function tests for is *not* available.
>
> Rather call this function ap_instructions_check_availability() (and
> keep the return code convention), or switch this to return 0 if not
> available and !0 if available?
>
>> + */
>> +static inline int ap_instructions_available(void)
>> +{
>> +	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
>> +	register unsigned long reg1 asm ("1") = -ENODEV;
>> +	register unsigned long reg2 asm ("2");
>> +
>> +	asm volatile(
>> +		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
>> +		"0: la    %0,0\n"
>> +		"1:\n"
>> +		EX_TABLE(0b, 1b)
>> +		: "+d" (reg1), "=d" (reg2)
>> +		: "d" (reg0)
>> +		: "cc");
>> +	return reg1;
>> +}
Heiko Carstens Aug. 9, 2018, 3:43 p.m. UTC | #5
On Thu, Aug 09, 2018 at 11:18:05AM -0400, Tony Krowiak wrote:
> On 08/09/2018 05:06 AM, Cornelia Huck wrote:
> >On Wed,  8 Aug 2018 10:44:14 -0400
> >Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
> >
> >>From: Harald Freudenberger <freude@de.ibm.com>
> >>
> >>Move all the inline functions from the ap bus header
> >>file ap_asm.h into the in-kernel api header file
> >>arch/s390/include/asm/ap.h so that KVM can make use
> >>of all the low level AP functions.
> >>
> >>Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
> >>Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >You should add your own s-o-b if you are sending on patches written by
> >others (even if it does not matter in the end, when they are merged
> >through a different path anyway.)
> 
> That's rather ironic given I was told in an internal review that I should
> not sign off on patches I did not write.

Please point the one who said that to
Documentation/process/submitting-patches.rst / Developer's Certificate of Origin

You really should sign off patches you forward, even if you did not write
them on your own.
Anthony Krowiak Aug. 9, 2018, 4:06 p.m. UTC | #6
On 08/09/2018 05:17 AM, Harald Freudenberger wrote:
> On 09.08.2018 11:06, Cornelia Huck wrote:
>> On Wed,  8 Aug 2018 10:44:14 -0400
>> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>>
>>> From: Harald Freudenberger <freude@de.ibm.com>
>>>
>>> Move all the inline functions from the ap bus header
>>> file ap_asm.h into the in-kernel api header file
>>> arch/s390/include/asm/ap.h so that KVM can make use
>>> of all the low level AP functions.
>>>
>>> Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> You should add your own s-o-b if you are sending on patches written by
>> others (even if it does not matter in the end, when they are merged
>> through a different path anyway.)
>>
>>> ---
>>>   arch/s390/include/asm/ap.h     |  284 ++++++++++++++++++++++++++++++++++++----
>>>   drivers/s390/crypto/ap_asm.h   |  261 ------------------------------------
>>>   drivers/s390/crypto/ap_bus.c   |   21 +---
>>>   drivers/s390/crypto/ap_bus.h   |    1 +
>>>   drivers/s390/crypto/ap_card.c  |    1 -
>>>   drivers/s390/crypto/ap_queue.c |    1 -
>>>   6 files changed, 259 insertions(+), 310 deletions(-)
>>>   delete mode 100644 drivers/s390/crypto/ap_asm.h
>>>
>>> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
>>> index c1bedb4..046e044 100644
>>> --- a/arch/s390/include/asm/ap.h
>>> +++ b/arch/s390/include/asm/ap.h
>>> @@ -47,6 +47,50 @@ struct ap_queue_status {
>>>   };
>>>   
>>>   /**
>>> + * ap_intructions_available() - Test if AP instructions are available.
>>> + *
>>> + * Returns 0 if the AP instructions are installed.
>> Stumbled over this when I was looking at the usage in patch 7: if I see
>> a function called '_available' return 0, I'd assume that whatever the
>> function tests for is *not* available.
>>
>> Rather call this function ap_instructions_check_availability() (and
>> keep the return code convention), or switch this to return 0 if not
>> available and !0 if available?
> Good catch, Cony you are right. I'll fix this to return 1 if AP instructions
> are available and 0 if not. However, this patch will come via Martin's pipe
> to the Linus Torwald kernel sources.

Is your intent to simply indicate whether the AP instructions are 
available or
not; or is the intention to indicate whether the AP instructions are 
available
and if not, they why? In the former, then I agree that a boolean should be
returned; however, if the case is the latter, then what you have is fine but
maybe the function name should be changed as Connie suggests.

>>> + */
>>> +static inline int ap_instructions_available(void)
>>> +{
>>> +	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
>>> +	register unsigned long reg1 asm ("1") = -ENODEV;
>>> +	register unsigned long reg2 asm ("2");
>>> +
>>> +	asm volatile(
>>> +		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
>>> +		"0: la    %0,0\n"
>>> +		"1:\n"
>>> +		EX_TABLE(0b, 1b)
>>> +		: "+d" (reg1), "=d" (reg2)
>>> +		: "d" (reg0)
>>> +		: "cc");
>>> +	return reg1;
>>> +}
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-s390" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
Anthony Krowiak Aug. 9, 2018, 4:55 p.m. UTC | #7
On 08/09/2018 11:43 AM, Heiko Carstens wrote:
> On Thu, Aug 09, 2018 at 11:18:05AM -0400, Tony Krowiak wrote:
>> On 08/09/2018 05:06 AM, Cornelia Huck wrote:
>>> On Wed,  8 Aug 2018 10:44:14 -0400
>>> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>>>
>>>> From: Harald Freudenberger <freude@de.ibm.com>
>>>>
>>>> Move all the inline functions from the ap bus header
>>>> file ap_asm.h into the in-kernel api header file
>>>> arch/s390/include/asm/ap.h so that KVM can make use
>>>> of all the low level AP functions.
>>>>
>>>> Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> You should add your own s-o-b if you are sending on patches written by
>>> others (even if it does not matter in the end, when they are merged
>>> through a different path anyway.)
>> That's rather ironic given I was told in an internal review that I should
>> not sign off on patches I did not write.
> Please point the one who said that to
> Documentation/process/submitting-patches.rst / Developer's Certificate of Origin
>
> You really should sign off patches you forward, even if you did not write
> them on your own.

Duly noted and I will do so on future posts. Thanks for the reference.
Cornelia Huck Aug. 10, 2018, 8:49 a.m. UTC | #8
On Thu, 9 Aug 2018 12:06:56 -0400
Tony Krowiak <akrowiak@linux.ibm.com> wrote:

> On 08/09/2018 05:17 AM, Harald Freudenberger wrote:
> > On 09.08.2018 11:06, Cornelia Huck wrote:  
> >> On Wed,  8 Aug 2018 10:44:14 -0400
> >> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
> >>  
> >>> From: Harald Freudenberger <freude@de.ibm.com>
> >>>
> >>> Move all the inline functions from the ap bus header
> >>> file ap_asm.h into the in-kernel api header file
> >>> arch/s390/include/asm/ap.h so that KVM can make use
> >>> of all the low level AP functions.
> >>>
> >>> Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
> >>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>  
> >> You should add your own s-o-b if you are sending on patches written by
> >> others (even if it does not matter in the end, when they are merged
> >> through a different path anyway.)
> >>  
> >>> ---
> >>>   arch/s390/include/asm/ap.h     |  284 ++++++++++++++++++++++++++++++++++++----
> >>>   drivers/s390/crypto/ap_asm.h   |  261 ------------------------------------
> >>>   drivers/s390/crypto/ap_bus.c   |   21 +---
> >>>   drivers/s390/crypto/ap_bus.h   |    1 +
> >>>   drivers/s390/crypto/ap_card.c  |    1 -
> >>>   drivers/s390/crypto/ap_queue.c |    1 -
> >>>   6 files changed, 259 insertions(+), 310 deletions(-)
> >>>   delete mode 100644 drivers/s390/crypto/ap_asm.h
> >>>
> >>> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
> >>> index c1bedb4..046e044 100644
> >>> --- a/arch/s390/include/asm/ap.h
> >>> +++ b/arch/s390/include/asm/ap.h
> >>> @@ -47,6 +47,50 @@ struct ap_queue_status {
> >>>   };
> >>>   
> >>>   /**
> >>> + * ap_intructions_available() - Test if AP instructions are available.
> >>> + *
> >>> + * Returns 0 if the AP instructions are installed.  
> >> Stumbled over this when I was looking at the usage in patch 7: if I see
> >> a function called '_available' return 0, I'd assume that whatever the
> >> function tests for is *not* available.
> >>
> >> Rather call this function ap_instructions_check_availability() (and
> >> keep the return code convention), or switch this to return 0 if not
> >> available and !0 if available?  
> > Good catch, Cony you are right. I'll fix this to return 1 if AP instructions
> > are available and 0 if not. However, this patch will come via Martin's pipe
> > to the Linus Torwald kernel sources.  
> 
> Is your intent to simply indicate whether the AP instructions are 
> available or
> not; or is the intention to indicate whether the AP instructions are 
> available
> and if not, they why? In the former, then I agree that a boolean should be
> returned; however, if the case is the latter, then what you have is fine but
> maybe the function name should be changed as Connie suggests.

So, can this actually fail for any reason other than "instructions not
installed"? Even if it did, the end result is that the instructions are
not usable -- I don't think the "why" would be interesting at that
point.

> 
> >>> + */
> >>> +static inline int ap_instructions_available(void)
> >>> +{
> >>> +	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
> >>> +	register unsigned long reg1 asm ("1") = -ENODEV;
> >>> +	register unsigned long reg2 asm ("2");
> >>> +
> >>> +	asm volatile(
> >>> +		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
> >>> +		"0: la    %0,0\n"
> >>> +		"1:\n"
> >>> +		EX_TABLE(0b, 1b)
> >>> +		: "+d" (reg1), "=d" (reg2)
> >>> +		: "d" (reg0)
> >>> +		: "cc");
> >>> +	return reg1;
> >>> +}
Harald Freudenberger Aug. 10, 2018, 9:37 a.m. UTC | #9
On 10.08.2018 10:49, Cornelia Huck wrote:
> On Thu, 9 Aug 2018 12:06:56 -0400
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>
>> On 08/09/2018 05:17 AM, Harald Freudenberger wrote:
>>> On 09.08.2018 11:06, Cornelia Huck wrote:  
>>>> On Wed,  8 Aug 2018 10:44:14 -0400
>>>> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>>>>  
>>>>> From: Harald Freudenberger <freude@de.ibm.com>
>>>>>
>>>>> Move all the inline functions from the ap bus header
>>>>> file ap_asm.h into the in-kernel api header file
>>>>> arch/s390/include/asm/ap.h so that KVM can make use
>>>>> of all the low level AP functions.
>>>>>
>>>>> Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
>>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>  
>>>> You should add your own s-o-b if you are sending on patches written by
>>>> others (even if it does not matter in the end, when they are merged
>>>> through a different path anyway.)
>>>>  
>>>>> ---
>>>>>   arch/s390/include/asm/ap.h     |  284 ++++++++++++++++++++++++++++++++++++----
>>>>>   drivers/s390/crypto/ap_asm.h   |  261 ------------------------------------
>>>>>   drivers/s390/crypto/ap_bus.c   |   21 +---
>>>>>   drivers/s390/crypto/ap_bus.h   |    1 +
>>>>>   drivers/s390/crypto/ap_card.c  |    1 -
>>>>>   drivers/s390/crypto/ap_queue.c |    1 -
>>>>>   6 files changed, 259 insertions(+), 310 deletions(-)
>>>>>   delete mode 100644 drivers/s390/crypto/ap_asm.h
>>>>>
>>>>> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
>>>>> index c1bedb4..046e044 100644
>>>>> --- a/arch/s390/include/asm/ap.h
>>>>> +++ b/arch/s390/include/asm/ap.h
>>>>> @@ -47,6 +47,50 @@ struct ap_queue_status {
>>>>>   };
>>>>>   
>>>>>   /**
>>>>> + * ap_intructions_available() - Test if AP instructions are available.
>>>>> + *
>>>>> + * Returns 0 if the AP instructions are installed.  
>>>> Stumbled over this when I was looking at the usage in patch 7: if I see
>>>> a function called '_available' return 0, I'd assume that whatever the
>>>> function tests for is *not* available.
>>>>
>>>> Rather call this function ap_instructions_check_availability() (and
>>>> keep the return code convention), or switch this to return 0 if not
>>>> available and !0 if available?  
>>> Good catch, Cony you are right. I'll fix this to return 1 if AP instructions
>>> are available and 0 if not. However, this patch will come via Martin's pipe
>>> to the Linus Torwald kernel sources.  
>> Is your intent to simply indicate whether the AP instructions are 
>> available or
>> not; or is the intention to indicate whether the AP instructions are 
>> available
>> and if not, they why? In the former, then I agree that a boolean should be
>> returned; however, if the case is the latter, then what you have is fine but
>> maybe the function name should be changed as Connie suggests.
> So, can this actually fail for any reason other than "instructions not
> installed"? Even if it did, the end result is that the instructions are
> not usable -- I don't think the "why" would be interesting at that
> point.
I can not think of any other reason why the PQAP(TAPQ) would fail
other than the AP instructions are not available at all. However,
the old implementation returned -ENODEV on failure and 0 on
success. The new implementation now returns 1 for success
and 0 for failure. This is just one of a couple of functions related
to ap xxx available. I'll rework them all to return a boolean value
soon.
>
>>>>> + */
>>>>> +static inline int ap_instructions_available(void)
>>>>> +{
>>>>> +	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
>>>>> +	register unsigned long reg1 asm ("1") = -ENODEV;
>>>>> +	register unsigned long reg2 asm ("2");
>>>>> +
>>>>> +	asm volatile(
>>>>> +		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
>>>>> +		"0: la    %0,0\n"
>>>>> +		"1:\n"
>>>>> +		EX_TABLE(0b, 1b)
>>>>> +		: "+d" (reg1), "=d" (reg2)
>>>>> +		: "d" (reg0)
>>>>> +		: "cc");
>>>>> +	return reg1;
>>>>> +}
Anthony Krowiak Aug. 10, 2018, 3:50 p.m. UTC | #10
On 08/10/2018 04:49 AM, Cornelia Huck wrote:
> On Thu, 9 Aug 2018 12:06:56 -0400
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>
>> On 08/09/2018 05:17 AM, Harald Freudenberger wrote:
>>> On 09.08.2018 11:06, Cornelia Huck wrote:
>>>> On Wed,  8 Aug 2018 10:44:14 -0400
>>>> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>>>>   
>>>>> From: Harald Freudenberger <freude@de.ibm.com>
>>>>>
>>>>> Move all the inline functions from the ap bus header
>>>>> file ap_asm.h into the in-kernel api header file
>>>>> arch/s390/include/asm/ap.h so that KVM can make use
>>>>> of all the low level AP functions.
>>>>>
>>>>> Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
>>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>> You should add your own s-o-b if you are sending on patches written by
>>>> others (even if it does not matter in the end, when they are merged
>>>> through a different path anyway.)
>>>>   
>>>>> ---
>>>>>    arch/s390/include/asm/ap.h     |  284 ++++++++++++++++++++++++++++++++++++----
>>>>>    drivers/s390/crypto/ap_asm.h   |  261 ------------------------------------
>>>>>    drivers/s390/crypto/ap_bus.c   |   21 +---
>>>>>    drivers/s390/crypto/ap_bus.h   |    1 +
>>>>>    drivers/s390/crypto/ap_card.c  |    1 -
>>>>>    drivers/s390/crypto/ap_queue.c |    1 -
>>>>>    6 files changed, 259 insertions(+), 310 deletions(-)
>>>>>    delete mode 100644 drivers/s390/crypto/ap_asm.h
>>>>>
>>>>> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
>>>>> index c1bedb4..046e044 100644
>>>>> --- a/arch/s390/include/asm/ap.h
>>>>> +++ b/arch/s390/include/asm/ap.h
>>>>> @@ -47,6 +47,50 @@ struct ap_queue_status {
>>>>>    };
>>>>>    
>>>>>    /**
>>>>> + * ap_intructions_available() - Test if AP instructions are available.
>>>>> + *
>>>>> + * Returns 0 if the AP instructions are installed.
>>>> Stumbled over this when I was looking at the usage in patch 7: if I see
>>>> a function called '_available' return 0, I'd assume that whatever the
>>>> function tests for is *not* available.
>>>>
>>>> Rather call this function ap_instructions_check_availability() (and
>>>> keep the return code convention), or switch this to return 0 if not
>>>> available and !0 if available?
>>> Good catch, Cony you are right. I'll fix this to return 1 if AP instructions
>>> are available and 0 if not. However, this patch will come via Martin's pipe
>>> to the Linus Torwald kernel sources.
>> Is your intent to simply indicate whether the AP instructions are
>> available or
>> not; or is the intention to indicate whether the AP instructions are
>> available
>> and if not, they why? In the former, then I agree that a boolean should be
>> returned; however, if the case is the latter, then what you have is fine but
>> maybe the function name should be changed as Connie suggests.
> So, can this actually fail for any reason other than "instructions not
> installed"? Even if it did, the end result is that the instructions are
> not usable -- I don't think the "why" would be interesting at that
> point.

The only case I can think of is if something is hosed and it causes an
exception. In that case, should we proceed?

>
>>>>> + */
>>>>> +static inline int ap_instructions_available(void)
>>>>> +{
>>>>> +	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
>>>>> +	register unsigned long reg1 asm ("1") = -ENODEV;
>>>>> +	register unsigned long reg2 asm ("2");
>>>>> +
>>>>> +	asm volatile(
>>>>> +		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
>>>>> +		"0: la    %0,0\n"
>>>>> +		"1:\n"
>>>>> +		EX_TABLE(0b, 1b)
>>>>> +		: "+d" (reg1), "=d" (reg2)
>>>>> +		: "d" (reg0)
>>>>> +		: "cc");
>>>>> +	return reg1;
>>>>> +}
Anthony Krowiak Aug. 10, 2018, 3:53 p.m. UTC | #11
On 08/10/2018 05:37 AM, Harald Freudenberger wrote:
> On 10.08.2018 10:49, Cornelia Huck wrote:
>> On Thu, 9 Aug 2018 12:06:56 -0400
>> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>>
>>> On 08/09/2018 05:17 AM, Harald Freudenberger wrote:
>>>> On 09.08.2018 11:06, Cornelia Huck wrote:
>>>>> On Wed,  8 Aug 2018 10:44:14 -0400
>>>>> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>>>>>   
>>>>>> From: Harald Freudenberger <freude@de.ibm.com>
>>>>>>
>>>>>> Move all the inline functions from the ap bus header
>>>>>> file ap_asm.h into the in-kernel api header file
>>>>>> arch/s390/include/asm/ap.h so that KVM can make use
>>>>>> of all the low level AP functions.
>>>>>>
>>>>>> Signed-off-by: Harald Freudenberger <freude@de.ibm.com>
>>>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>> You should add your own s-o-b if you are sending on patches written by
>>>>> others (even if it does not matter in the end, when they are merged
>>>>> through a different path anyway.)
>>>>>   
>>>>>> ---
>>>>>>    arch/s390/include/asm/ap.h     |  284 ++++++++++++++++++++++++++++++++++++----
>>>>>>    drivers/s390/crypto/ap_asm.h   |  261 ------------------------------------
>>>>>>    drivers/s390/crypto/ap_bus.c   |   21 +---
>>>>>>    drivers/s390/crypto/ap_bus.h   |    1 +
>>>>>>    drivers/s390/crypto/ap_card.c  |    1 -
>>>>>>    drivers/s390/crypto/ap_queue.c |    1 -
>>>>>>    6 files changed, 259 insertions(+), 310 deletions(-)
>>>>>>    delete mode 100644 drivers/s390/crypto/ap_asm.h
>>>>>>
>>>>>> diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
>>>>>> index c1bedb4..046e044 100644
>>>>>> --- a/arch/s390/include/asm/ap.h
>>>>>> +++ b/arch/s390/include/asm/ap.h
>>>>>> @@ -47,6 +47,50 @@ struct ap_queue_status {
>>>>>>    };
>>>>>>    
>>>>>>    /**
>>>>>> + * ap_intructions_available() - Test if AP instructions are available.
>>>>>> + *
>>>>>> + * Returns 0 if the AP instructions are installed.
>>>>> Stumbled over this when I was looking at the usage in patch 7: if I see
>>>>> a function called '_available' return 0, I'd assume that whatever the
>>>>> function tests for is *not* available.
>>>>>
>>>>> Rather call this function ap_instructions_check_availability() (and
>>>>> keep the return code convention), or switch this to return 0 if not
>>>>> available and !0 if available?
>>>> Good catch, Cony you are right. I'll fix this to return 1 if AP instructions
>>>> are available and 0 if not. However, this patch will come via Martin's pipe
>>>> to the Linus Torwald kernel sources.
>>> Is your intent to simply indicate whether the AP instructions are
>>> available or
>>> not; or is the intention to indicate whether the AP instructions are
>>> available
>>> and if not, they why? In the former, then I agree that a boolean should be
>>> returned; however, if the case is the latter, then what you have is fine but
>>> maybe the function name should be changed as Connie suggests.
>> So, can this actually fail for any reason other than "instructions not
>> installed"? Even if it did, the end result is that the instructions are
>> not usable -- I don't think the "why" would be interesting at that
>> point.
> I can not think of any other reason why the PQAP(TAPQ) would fail
> other than the AP instructions are not available at all. However,
> the old implementation returned -ENODEV on failure and 0 on
> success. The new implementation now returns 1 for success
> and 0 for failure. This is just one of a couple of functions related
> to ap xxx available. I'll rework them all to return a boolean value
> soon.

How would you recommend I proceed given I have functions that call this
interface that check the rc and I've had to include your patches in
this series because of that dependence?

>>>>>> + */
>>>>>> +static inline int ap_instructions_available(void)
>>>>>> +{
>>>>>> +	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
>>>>>> +	register unsigned long reg1 asm ("1") = -ENODEV;
>>>>>> +	register unsigned long reg2 asm ("2");
>>>>>> +
>>>>>> +	asm volatile(
>>>>>> +		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
>>>>>> +		"0: la    %0,0\n"
>>>>>> +		"1:\n"
>>>>>> +		EX_TABLE(0b, 1b)
>>>>>> +		: "+d" (reg1), "=d" (reg2)
>>>>>> +		: "d" (reg0)
>>>>>> +		: "cc");
>>>>>> +	return reg1;
>>>>>> +}
Harald Freudenberger Aug. 13, 2018, 9:24 a.m. UTC | #12
Here is now the add-on patch which changes the returncode for the
ap_instructions_available() function.

I talked with Heiko Carstens and this patch will go into the next pull
request for the 4.19 kernel.

-----------------------------------------------------------------------------------------
From 9e050f843f3281da1f65292422e30f2dd1fd6d98 Mon Sep 17 00:00:00 2001
From: Harald Freudenberger <freude@linux.ibm.com>
Date: Thu, 9 Aug 2018 11:59:34 +0200
Subject: [PATCH] s390/zcrypt: fix ap_instructions_available() returncodes

During review of KVM patches it was complained that the
ap_instructions_available() function returns 0 if AP
instructions are available and -ENODEV if not. The function
acts like a boolean function to check for AP instructions
available and thus should return 0 on failure and != 0 on
success. Changed to the suggested behaviour and adapted
the one and only caller of this function which is the ap
bus core code.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 arch/s390/include/asm/ap.h   | 10 +++++-----
 drivers/s390/crypto/ap_bus.c |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
index 046e044a48d0..887494aa164f 100644
--- a/arch/s390/include/asm/ap.h
+++ b/arch/s390/include/asm/ap.h
@@ -49,20 +49,20 @@ struct ap_queue_status {
 /**
  * ap_intructions_available() - Test if AP instructions are available.
  *
- * Returns 0 if the AP instructions are installed.
+ * Returns 1 if the AP instructions are installed, otherwise 0.
  */
 static inline int ap_instructions_available(void)
 {
     register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
-    register unsigned long reg1 asm ("1") = -ENODEV;
-    register unsigned long reg2 asm ("2");
+    register unsigned long reg1 asm ("1") = 0;
+    register unsigned long reg2 asm ("2") = 0;
 
     asm volatile(
         "   .long 0xb2af0000\n"        /* PQAP(TAPQ) */
-        "0: la    %0,0\n"
+        "0: la    %0,1\n"
         "1:\n"
         EX_TABLE(0b, 1b)
-        : "+d" (reg1), "=d" (reg2)
+        : "+d" (reg1), "+d" (reg2)
         : "d" (reg0)
         : "cc");
     return reg1;
diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
index b023e35786ec..b1b73945d746 100644
--- a/drivers/s390/crypto/ap_bus.c
+++ b/drivers/s390/crypto/ap_bus.c
@@ -1475,7 +1475,7 @@ static int __init ap_module_init(void)
     if (rc)
         return rc;
 
-    if (ap_instructions_available() != 0) {
+    if (!ap_instructions_available()) {
         pr_warn("The hardware system does not support AP instructions\n");
         return -ENODEV;
     }
Cornelia Huck Aug. 13, 2018, 9:34 a.m. UTC | #13
On Mon, 13 Aug 2018 11:24:48 +0200
Harald Freudenberger <freude@linux.ibm.com> wrote:

> Here is now the add-on patch which changes the returncode for the
> ap_instructions_available() function.
> 
> I talked with Heiko Carstens and this patch will go into the next pull
> request for the 4.19 kernel.

I was just about to ask :)

> 
> -----------------------------------------------------------------------------------------
> From 9e050f843f3281da1f65292422e30f2dd1fd6d98 Mon Sep 17 00:00:00 2001
> From: Harald Freudenberger <freude@linux.ibm.com>
> Date: Thu, 9 Aug 2018 11:59:34 +0200
> Subject: [PATCH] s390/zcrypt: fix ap_instructions_available() returncodes
> 
> During review of KVM patches it was complained that the
> ap_instructions_available() function returns 0 if AP
> instructions are available and -ENODEV if not. The function
> acts like a boolean function to check for AP instructions
> available and thus should return 0 on failure and != 0 on
> success. Changed to the suggested behaviour and adapted
> the one and only caller of this function which is the ap
> bus core code.
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
> ---
>  arch/s390/include/asm/ap.h   | 10 +++++-----
>  drivers/s390/crypto/ap_bus.c |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)

Looks sane to me.

Acked-by: Cornelia Huck <cohuck@redhat.com>
diff mbox series

Patch

diff --git a/arch/s390/include/asm/ap.h b/arch/s390/include/asm/ap.h
index c1bedb4..046e044 100644
--- a/arch/s390/include/asm/ap.h
+++ b/arch/s390/include/asm/ap.h
@@ -47,6 +47,50 @@  struct ap_queue_status {
 };
 
 /**
+ * ap_intructions_available() - Test if AP instructions are available.
+ *
+ * Returns 0 if the AP instructions are installed.
+ */
+static inline int ap_instructions_available(void)
+{
+	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
+	register unsigned long reg1 asm ("1") = -ENODEV;
+	register unsigned long reg2 asm ("2");
+
+	asm volatile(
+		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
+		"0: la    %0,0\n"
+		"1:\n"
+		EX_TABLE(0b, 1b)
+		: "+d" (reg1), "=d" (reg2)
+		: "d" (reg0)
+		: "cc");
+	return reg1;
+}
+
+/**
+ * ap_tapq(): Test adjunct processor queue.
+ * @qid: The AP queue number
+ * @info: Pointer to queue descriptor
+ *
+ * Returns AP queue status structure.
+ */
+static inline struct ap_queue_status ap_tapq(ap_qid_t qid, unsigned long *info)
+{
+	register unsigned long reg0 asm ("0") = qid;
+	register struct ap_queue_status reg1 asm ("1");
+	register unsigned long reg2 asm ("2");
+
+	asm volatile(".long 0xb2af0000"		/* PQAP(TAPQ) */
+		     : "=d" (reg1), "=d" (reg2)
+		     : "d" (reg0)
+		     : "cc");
+	if (info)
+		*info = reg2;
+	return reg1;
+}
+
+/**
  * ap_test_queue(): Test adjunct processor queue.
  * @qid: The AP queue number
  * @tbit: Test facilities bit
@@ -54,10 +98,57 @@  struct ap_queue_status {
  *
  * Returns AP queue status structure.
  */
-struct ap_queue_status ap_test_queue(ap_qid_t qid,
-				     int tbit,
-				     unsigned long *info);
+static inline struct ap_queue_status ap_test_queue(ap_qid_t qid,
+						   int tbit,
+						   unsigned long *info)
+{
+	if (tbit)
+		qid |= 1UL << 23; /* set T bit*/
+	return ap_tapq(qid, info);
+}
 
+/**
+ * ap_pqap_rapq(): Reset adjunct processor queue.
+ * @qid: The AP queue number
+ *
+ * Returns AP queue status structure.
+ */
+static inline struct ap_queue_status ap_rapq(ap_qid_t qid)
+{
+	register unsigned long reg0 asm ("0") = qid | (1UL << 24);
+	register struct ap_queue_status reg1 asm ("1");
+
+	asm volatile(
+		".long 0xb2af0000"		/* PQAP(RAPQ) */
+		: "=d" (reg1)
+		: "d" (reg0)
+		: "cc");
+	return reg1;
+}
+
+/**
+ * ap_pqap_zapq(): Reset and zeroize adjunct processor queue.
+ * @qid: The AP queue number
+ *
+ * Returns AP queue status structure.
+ */
+static inline struct ap_queue_status ap_zapq(ap_qid_t qid)
+{
+	register unsigned long reg0 asm ("0") = qid | (2UL << 24);
+	register struct ap_queue_status reg1 asm ("1");
+
+	asm volatile(
+		".long 0xb2af0000"		/* PQAP(ZAPQ) */
+		: "=d" (reg1)
+		: "d" (reg0)
+		: "cc");
+	return reg1;
+}
+
+/**
+ * struct ap_config_info - convenience struct for AP crypto
+ * config info as returned by the ap_qci() function.
+ */
 struct ap_config_info {
 	unsigned int apsc	 : 1;	/* S bit */
 	unsigned int apxa	 : 1;	/* N bit */
@@ -74,50 +165,189 @@  struct ap_config_info {
 	unsigned char _reserved4[16];
 } __aligned(8);
 
-/*
- * ap_query_configuration(): Fetch cryptographic config info
+/**
+ * ap_qci(): Get AP configuration data
  *
- * Returns the ap configuration info fetched via PQAP(QCI).
- * On success 0 is returned, on failure a negative errno
- * is returned, e.g. if the PQAP(QCI) instruction is not
- * available, the return value will be -EOPNOTSUPP.
+ * Returns 0 on success, or -EOPNOTSUPP.
  */
-int ap_query_configuration(struct ap_config_info *info);
+static inline int ap_qci(struct ap_config_info *config)
+{
+	register unsigned long reg0 asm ("0") = 4UL << 24;
+	register unsigned long reg1 asm ("1") = -EOPNOTSUPP;
+	register struct ap_config_info *reg2 asm ("2") = config;
+
+	asm volatile(
+		".long 0xb2af0000\n"		/* PQAP(QCI) */
+		"0: la    %0,0\n"
+		"1:\n"
+		EX_TABLE(0b, 1b)
+		: "+d" (reg1)
+		: "d" (reg0), "d" (reg2)
+		: "cc", "memory");
+
+	return reg1;
+}
 
 /*
  * struct ap_qirq_ctrl - convenient struct for easy invocation
- * of the ap_queue_irq_ctrl() function. This struct is passed
- * as GR1 parameter to the PQAP(AQIC) instruction. For details
- * please see the AR documentation.
+ * of the ap_aqic() function. This struct is passed as GR1
+ * parameter to the PQAP(AQIC) instruction. For details please
+ * see the AR documentation.
  */
 struct ap_qirq_ctrl {
 	unsigned int _res1 : 8;
-	unsigned int zone  : 8;  /* zone info */
-	unsigned int ir    : 1;  /* ir flag: enable (1) or disable (0) irq */
+	unsigned int zone  : 8;	/* zone info */
+	unsigned int ir    : 1;	/* ir flag: enable (1) or disable (0) irq */
 	unsigned int _res2 : 4;
-	unsigned int gisc  : 3;  /* guest isc field */
+	unsigned int gisc  : 3;	/* guest isc field */
 	unsigned int _res3 : 6;
-	unsigned int gf    : 2;  /* gisa format */
+	unsigned int gf    : 2;	/* gisa format */
 	unsigned int _res4 : 1;
-	unsigned int gisa  : 27; /* gisa origin */
+	unsigned int gisa  : 27;	/* gisa origin */
 	unsigned int _res5 : 1;
-	unsigned int isc   : 3;  /* irq sub class */
+	unsigned int isc   : 3;	/* irq sub class */
 };
 
 /**
- * ap_queue_irq_ctrl(): Control interruption on a AP queue.
+ * ap_aqic(): Control interruption for a specific AP.
  * @qid: The AP queue number
- * @qirqctrl: struct ap_qirq_ctrl, see above
+ * @qirqctrl: struct ap_qirq_ctrl (64 bit value)
  * @ind: The notification indicator byte
  *
  * Returns AP queue status.
+ */
+static inline struct ap_queue_status ap_aqic(ap_qid_t qid,
+					     struct ap_qirq_ctrl qirqctrl,
+					     void *ind)
+{
+	register unsigned long reg0 asm ("0") = qid | (3UL << 24);
+	register struct ap_qirq_ctrl reg1_in asm ("1") = qirqctrl;
+	register struct ap_queue_status reg1_out asm ("1");
+	register void *reg2 asm ("2") = ind;
+
+	asm volatile(
+		".long 0xb2af0000"		/* PQAP(AQIC) */
+		: "=d" (reg1_out)
+		: "d" (reg0), "d" (reg1_in), "d" (reg2)
+		: "cc");
+	return reg1_out;
+}
+
+/*
+ * union ap_qact_ap_info - used together with the
+ * ap_aqic() function to provide a convenient way
+ * to handle the ap info needed by the qact function.
+ */
+union ap_qact_ap_info {
+	unsigned long val;
+	struct {
+		unsigned int	  : 3;
+		unsigned int mode : 3;
+		unsigned int	  : 26;
+		unsigned int cat  : 8;
+		unsigned int	  : 8;
+		unsigned char ver[2];
+	};
+};
+
+/**
+ * ap_qact(): Query AP combatibility type.
+ * @qid: The AP queue number
+ * @apinfo: On input the info about the AP queue. On output the
+ *	    alternate AP queue info provided by the qact function
+ *	    in GR2 is stored in.
  *
- * Control interruption on the given AP queue.
- * Just a simple wrapper function for the low level PQAP(AQIC)
- * instruction available for other kernel modules.
+ * Returns AP queue status. Check response_code field for failures.
  */
-struct ap_queue_status ap_queue_irq_ctrl(ap_qid_t qid,
-					 struct ap_qirq_ctrl qirqctrl,
-					 void *ind);
+static inline struct ap_queue_status ap_qact(ap_qid_t qid, int ifbit,
+					     union ap_qact_ap_info *apinfo)
+{
+	register unsigned long reg0 asm ("0") = qid | (5UL << 24)
+		| ((ifbit & 0x01) << 22);
+	register unsigned long reg1_in asm ("1") = apinfo->val;
+	register struct ap_queue_status reg1_out asm ("1");
+	register unsigned long reg2 asm ("2");
+
+	asm volatile(
+		".long 0xb2af0000"		/* PQAP(QACT) */
+		: "+d" (reg1_in), "=d" (reg1_out), "=d" (reg2)
+		: "d" (reg0)
+		: "cc");
+	apinfo->val = reg2;
+	return reg1_out;
+}
+
+/**
+ * ap_nqap(): Send message to adjunct processor queue.
+ * @qid: The AP queue number
+ * @psmid: The program supplied message identifier
+ * @msg: The message text
+ * @length: The message length
+ *
+ * Returns AP queue status structure.
+ * Condition code 1 on NQAP can't happen because the L bit is 1.
+ * Condition code 2 on NQAP also means the send is incomplete,
+ * because a segment boundary was reached. The NQAP is repeated.
+ */
+static inline struct ap_queue_status ap_nqap(ap_qid_t qid,
+					     unsigned long long psmid,
+					     void *msg, size_t length)
+{
+	register unsigned long reg0 asm ("0") = qid | 0x40000000UL;
+	register struct ap_queue_status reg1 asm ("1");
+	register unsigned long reg2 asm ("2") = (unsigned long) msg;
+	register unsigned long reg3 asm ("3") = (unsigned long) length;
+	register unsigned long reg4 asm ("4") = (unsigned int) (psmid >> 32);
+	register unsigned long reg5 asm ("5") = psmid & 0xffffffff;
+
+	asm volatile (
+		"0: .long 0xb2ad0042\n"		/* NQAP */
+		"   brc   2,0b"
+		: "+d" (reg0), "=d" (reg1), "+d" (reg2), "+d" (reg3)
+		: "d" (reg4), "d" (reg5)
+		: "cc", "memory");
+	return reg1;
+}
+
+/**
+ * ap_dqap(): Receive message from adjunct processor queue.
+ * @qid: The AP queue number
+ * @psmid: Pointer to program supplied message identifier
+ * @msg: The message text
+ * @length: The message length
+ *
+ * Returns AP queue status structure.
+ * Condition code 1 on DQAP means the receive has taken place
+ * but only partially.	The response is incomplete, hence the
+ * DQAP is repeated.
+ * Condition code 2 on DQAP also means the receive is incomplete,
+ * this time because a segment boundary was reached. Again, the
+ * DQAP is repeated.
+ * Note that gpr2 is used by the DQAP instruction to keep track of
+ * any 'residual' length, in case the instruction gets interrupted.
+ * Hence it gets zeroed before the instruction.
+ */
+static inline struct ap_queue_status ap_dqap(ap_qid_t qid,
+					     unsigned long long *psmid,
+					     void *msg, size_t length)
+{
+	register unsigned long reg0 asm("0") = qid | 0x80000000UL;
+	register struct ap_queue_status reg1 asm ("1");
+	register unsigned long reg2 asm("2") = 0UL;
+	register unsigned long reg4 asm("4") = (unsigned long) msg;
+	register unsigned long reg5 asm("5") = (unsigned long) length;
+	register unsigned long reg6 asm("6") = 0UL;
+	register unsigned long reg7 asm("7") = 0UL;
+
+
+	asm volatile(
+		"0: .long 0xb2ae0064\n"		/* DQAP */
+		"   brc   6,0b\n"
+		: "+d" (reg0), "=d" (reg1), "+d" (reg2),
+		  "+d" (reg4), "+d" (reg5), "+d" (reg6), "+d" (reg7)
+		: : "cc", "memory");
+	*psmid = (((unsigned long long) reg6) << 32) + reg7;
+	return reg1;
+}
 
 #endif /* _ASM_S390_AP_H_ */
diff --git a/drivers/s390/crypto/ap_asm.h b/drivers/s390/crypto/ap_asm.h
deleted file mode 100644
index e22ee12..0000000
--- a/drivers/s390/crypto/ap_asm.h
+++ /dev/null
@@ -1,261 +0,0 @@ 
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright IBM Corp. 2016
- * Author(s): Martin Schwidefsky <schwidefsky@de.ibm.com>
- *
- * Adjunct processor bus inline assemblies.
- */
-
-#ifndef _AP_ASM_H_
-#define _AP_ASM_H_
-
-#include <asm/isc.h>
-
-/**
- * ap_intructions_available() - Test if AP instructions are available.
- *
- * Returns 0 if the AP instructions are installed.
- */
-static inline int ap_instructions_available(void)
-{
-	register unsigned long reg0 asm ("0") = AP_MKQID(0, 0);
-	register unsigned long reg1 asm ("1") = -ENODEV;
-	register unsigned long reg2 asm ("2");
-
-	asm volatile(
-		"   .long 0xb2af0000\n"		/* PQAP(TAPQ) */
-		"0: la    %0,0\n"
-		"1:\n"
-		EX_TABLE(0b, 1b)
-		: "+d" (reg1), "=d" (reg2)
-		: "d" (reg0)
-		: "cc");
-	return reg1;
-}
-
-/**
- * ap_tapq(): Test adjunct processor queue.
- * @qid: The AP queue number
- * @info: Pointer to queue descriptor
- *
- * Returns AP queue status structure.
- */
-static inline struct ap_queue_status ap_tapq(ap_qid_t qid, unsigned long *info)
-{
-	register unsigned long reg0 asm ("0") = qid;
-	register struct ap_queue_status reg1 asm ("1");
-	register unsigned long reg2 asm ("2");
-
-	asm volatile(".long 0xb2af0000"		/* PQAP(TAPQ) */
-		     : "=d" (reg1), "=d" (reg2)
-		     : "d" (reg0)
-		     : "cc");
-	if (info)
-		*info = reg2;
-	return reg1;
-}
-
-/**
- * ap_pqap_rapq(): Reset adjunct processor queue.
- * @qid: The AP queue number
- *
- * Returns AP queue status structure.
- */
-static inline struct ap_queue_status ap_rapq(ap_qid_t qid)
-{
-	register unsigned long reg0 asm ("0") = qid | (1UL << 24);
-	register struct ap_queue_status reg1 asm ("1");
-
-	asm volatile(
-		".long 0xb2af0000"		/* PQAP(RAPQ) */
-		: "=d" (reg1)
-		: "d" (reg0)
-		: "cc");
-	return reg1;
-}
-
-/**
- * ap_pqap_zapq(): Reset and zeroize adjunct processor queue.
- * @qid: The AP queue number
- *
- * Returns AP queue status structure.
- */
-static inline struct ap_queue_status ap_zapq(ap_qid_t qid)
-{
-	register unsigned long reg0 asm ("0") = qid | (2UL << 24);
-	register struct ap_queue_status reg1 asm ("1");
-
-	asm volatile(
-		".long 0xb2af0000"		/* PQAP(ZAPQ) */
-		: "=d" (reg1)
-		: "d" (reg0)
-		: "cc");
-	return reg1;
-}
-
-/**
- * ap_aqic(): Control interruption for a specific AP.
- * @qid: The AP queue number
- * @qirqctrl: struct ap_qirq_ctrl (64 bit value)
- * @ind: The notification indicator byte
- *
- * Returns AP queue status.
- */
-static inline struct ap_queue_status ap_aqic(ap_qid_t qid,
-					     struct ap_qirq_ctrl qirqctrl,
-					     void *ind)
-{
-	register unsigned long reg0 asm ("0") = qid | (3UL << 24);
-	register struct ap_qirq_ctrl reg1_in asm ("1") = qirqctrl;
-	register struct ap_queue_status reg1_out asm ("1");
-	register void *reg2 asm ("2") = ind;
-
-	asm volatile(
-		".long 0xb2af0000"		/* PQAP(AQIC) */
-		: "=d" (reg1_out)
-		: "d" (reg0), "d" (reg1_in), "d" (reg2)
-		: "cc");
-	return reg1_out;
-}
-
-/**
- * ap_qci(): Get AP configuration data
- *
- * Returns 0 on success, or -EOPNOTSUPP.
- */
-static inline int ap_qci(void *config)
-{
-	register unsigned long reg0 asm ("0") = 4UL << 24;
-	register unsigned long reg1 asm ("1") = -EINVAL;
-	register void *reg2 asm ("2") = (void *) config;
-
-	asm volatile(
-		".long 0xb2af0000\n"		/* PQAP(QCI) */
-		"0: la    %0,0\n"
-		"1:\n"
-		EX_TABLE(0b, 1b)
-		: "+d" (reg1)
-		: "d" (reg0), "d" (reg2)
-		: "cc", "memory");
-
-	return reg1;
-}
-
-/*
- * union ap_qact_ap_info - used together with the
- * ap_aqic() function to provide a convenient way
- * to handle the ap info needed by the qact function.
- */
-union ap_qact_ap_info {
-	unsigned long val;
-	struct {
-		unsigned int	  : 3;
-		unsigned int mode : 3;
-		unsigned int	  : 26;
-		unsigned int cat  : 8;
-		unsigned int	  : 8;
-		unsigned char ver[2];
-	};
-};
-
-/**
- * ap_qact(): Query AP combatibility type.
- * @qid: The AP queue number
- * @apinfo: On input the info about the AP queue. On output the
- *	    alternate AP queue info provided by the qact function
- *	    in GR2 is stored in.
- *
- * Returns AP queue status. Check response_code field for failures.
- */
-static inline struct ap_queue_status ap_qact(ap_qid_t qid, int ifbit,
-					     union ap_qact_ap_info *apinfo)
-{
-	register unsigned long reg0 asm ("0") = qid | (5UL << 24)
-		| ((ifbit & 0x01) << 22);
-	register unsigned long reg1_in asm ("1") = apinfo->val;
-	register struct ap_queue_status reg1_out asm ("1");
-	register unsigned long reg2 asm ("2");
-
-	asm volatile(
-		".long 0xb2af0000"		/* PQAP(QACT) */
-		: "+d" (reg1_in), "=d" (reg1_out), "=d" (reg2)
-		: "d" (reg0)
-		: "cc");
-	apinfo->val = reg2;
-	return reg1_out;
-}
-
-/**
- * ap_nqap(): Send message to adjunct processor queue.
- * @qid: The AP queue number
- * @psmid: The program supplied message identifier
- * @msg: The message text
- * @length: The message length
- *
- * Returns AP queue status structure.
- * Condition code 1 on NQAP can't happen because the L bit is 1.
- * Condition code 2 on NQAP also means the send is incomplete,
- * because a segment boundary was reached. The NQAP is repeated.
- */
-static inline struct ap_queue_status ap_nqap(ap_qid_t qid,
-					     unsigned long long psmid,
-					     void *msg, size_t length)
-{
-	register unsigned long reg0 asm ("0") = qid | 0x40000000UL;
-	register struct ap_queue_status reg1 asm ("1");
-	register unsigned long reg2 asm ("2") = (unsigned long) msg;
-	register unsigned long reg3 asm ("3") = (unsigned long) length;
-	register unsigned long reg4 asm ("4") = (unsigned int) (psmid >> 32);
-	register unsigned long reg5 asm ("5") = psmid & 0xffffffff;
-
-	asm volatile (
-		"0: .long 0xb2ad0042\n"		/* NQAP */
-		"   brc   2,0b"
-		: "+d" (reg0), "=d" (reg1), "+d" (reg2), "+d" (reg3)
-		: "d" (reg4), "d" (reg5)
-		: "cc", "memory");
-	return reg1;
-}
-
-/**
- * ap_dqap(): Receive message from adjunct processor queue.
- * @qid: The AP queue number
- * @psmid: Pointer to program supplied message identifier
- * @msg: The message text
- * @length: The message length
- *
- * Returns AP queue status structure.
- * Condition code 1 on DQAP means the receive has taken place
- * but only partially.	The response is incomplete, hence the
- * DQAP is repeated.
- * Condition code 2 on DQAP also means the receive is incomplete,
- * this time because a segment boundary was reached. Again, the
- * DQAP is repeated.
- * Note that gpr2 is used by the DQAP instruction to keep track of
- * any 'residual' length, in case the instruction gets interrupted.
- * Hence it gets zeroed before the instruction.
- */
-static inline struct ap_queue_status ap_dqap(ap_qid_t qid,
-					     unsigned long long *psmid,
-					     void *msg, size_t length)
-{
-	register unsigned long reg0 asm("0") = qid | 0x80000000UL;
-	register struct ap_queue_status reg1 asm ("1");
-	register unsigned long reg2 asm("2") = 0UL;
-	register unsigned long reg4 asm("4") = (unsigned long) msg;
-	register unsigned long reg5 asm("5") = (unsigned long) length;
-	register unsigned long reg6 asm("6") = 0UL;
-	register unsigned long reg7 asm("7") = 0UL;
-
-
-	asm volatile(
-		"0: .long 0xb2ae0064\n"		/* DQAP */
-		"   brc   6,0b\n"
-		: "+d" (reg0), "=d" (reg1), "+d" (reg2),
-		  "+d" (reg4), "+d" (reg5), "+d" (reg6), "+d" (reg7)
-		: : "cc", "memory");
-	*psmid = (((unsigned long long) reg6) << 32) + reg7;
-	return reg1;
-}
-
-#endif /* _AP_ASM_H_ */
diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
index 35a0c2b..c0a6723 100644
--- a/drivers/s390/crypto/ap_bus.c
+++ b/drivers/s390/crypto/ap_bus.c
@@ -36,7 +36,6 @@ 
 #include <linux/debugfs.h>
 
 #include "ap_bus.h"
-#include "ap_asm.h"
 #include "ap_debug.h"
 
 /*
@@ -174,24 +173,6 @@  static inline int ap_qact_available(void)
 	return 0;
 }
 
-/**
- * ap_test_queue(): Test adjunct processor queue.
- * @qid: The AP queue number
- * @tbit: Test facilities bit
- * @info: Pointer to queue descriptor
- *
- * Returns AP queue status structure.
- */
-struct ap_queue_status ap_test_queue(ap_qid_t qid,
-				     int tbit,
-				     unsigned long *info)
-{
-	if (tbit)
-		qid |= 1UL << 23; /* set T bit*/
-	return ap_tapq(qid, info);
-}
-EXPORT_SYMBOL(ap_test_queue);
-
 /*
  * ap_query_configuration(): Fetch cryptographic config info
  *
@@ -200,7 +181,7 @@  struct ap_queue_status ap_test_queue(ap_qid_t qid,
  * is returned, e.g. if the PQAP(QCI) instruction is not
  * available, the return value will be -EOPNOTSUPP.
  */
-int ap_query_configuration(struct ap_config_info *info)
+static inline int ap_query_configuration(struct ap_config_info *info)
 {
 	if (!ap_configuration_available())
 		return -EOPNOTSUPP;
diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h
index 6a273c5..9365419 100644
--- a/drivers/s390/crypto/ap_bus.h
+++ b/drivers/s390/crypto/ap_bus.h
@@ -15,6 +15,7 @@ 
 
 #include <linux/device.h>
 #include <linux/types.h>
+#include <asm/isc.h>
 #include <asm/ap.h>
 
 #define AP_DEVICES 256		/* Number of AP devices. */
diff --git a/drivers/s390/crypto/ap_card.c b/drivers/s390/crypto/ap_card.c
index 2c726df..c13e432 100644
--- a/drivers/s390/crypto/ap_card.c
+++ b/drivers/s390/crypto/ap_card.c
@@ -14,7 +14,6 @@ 
 #include <asm/facility.h>
 
 #include "ap_bus.h"
-#include "ap_asm.h"
 
 /*
  * AP card related attributes.
diff --git a/drivers/s390/crypto/ap_queue.c b/drivers/s390/crypto/ap_queue.c
index ba3a2e1..d83c1fa 100644
--- a/drivers/s390/crypto/ap_queue.c
+++ b/drivers/s390/crypto/ap_queue.c
@@ -14,7 +14,6 @@ 
 #include <asm/facility.h>
 
 #include "ap_bus.h"
-#include "ap_asm.h"
 
 /**
  * ap_queue_irq_ctrl(): Control interruption on a AP queue.