Re: [PATCH v8 04/22] s390/zcrypt: Integrate ap_asm.h into include/asm/ap.h.
From: Tony Krowiak
Date: Fri Aug 10 2018 - 11:54:10 EST
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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx> wrote:
From: Harald Freudenberger <freude@xxxxxxxxxx>
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@xxxxxxxxxx>
Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
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;
+}