On 3/30/23 13:42, Janosch Frank wrote:
Test if the IRQ enablement is turned off on a reset or zeroize PQAP.
Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
---
lib/s390x/ap.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
lib/s390x/ap.h | 4 +++
s390x/ap.c | 52 ++++++++++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+)
diff --git a/lib/s390x/ap.c b/lib/s390x/ap.c
index aaf5b4b9..d969b2a5 100644
--- a/lib/s390x/ap.c
+++ b/lib/s390x/ap.c
@@ -113,6 +113,74 @@ int ap_pqap_qci(struct ap_config_info *info)
return cc;
}
+static int pqap_reset(uint8_t ap, uint8_t qn, struct ap_queue_status *r1,
+ bool zeroize)
NIT. Personal opinion, I find using this bool a little obfuscating and I
would have prefer 2 different functions.
I see you added a ap_pqap_reset() and ap_pqap_zeroize() next in the code.
Why this intermediate level?
+{
+ struct pqap_r0 r0 = {};
+ int cc;
+
+ /*
+ * Reset/zeroize AP Queue
+ *
+ * Resets/zeroizes a queue and disables IRQs
+ *
+ * Inputs: 0
+ * Outputs: 1
+ * Asynchronous
+ */
+ r0.ap = ap;
+ r0.qn = qn;
+ r0.fc = zeroize ? PQAP_ZEROIZE_APQ : PQAP_RESET_APQ;
+ asm volatile(
+ " lgr 0,%[r0]\n"
+ " lgr 1,%[r1]\n"
+ " .insn rre,0xb2af0000,0,0\n" /* PQAP */
+ " ipm %[cc]\n"
+ " srl %[cc],28\n"
+ : [r1] "+&d" (r1), [cc] "=&d" (cc)
+ : [r0] "d" (r0)
+ : "memory");
+
+ return cc;
+}
+
+static int pqap_reset_wait(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw,
+ bool zeroize)
+{
+ struct pqap_r2 r2 = {};
+ int cc;
+
+ cc = pqap_reset(ap, qn, apqsw, zeroize);
+ /* On a cc == 3 / error we don't need to wait */
+ if (cc)
+ return cc;
+
+ /*
+ * TAPQ returns AP_RC_RESET_IN_PROGRESS if a reset is being
+ * processed
+ */
+ do {
+ cc = ap_pqap_tapq(ap, qn, apqsw, &r2);
+ /* Give it some time to process before the retry */
+ mdelay(20);
+ } while (apqsw->rc == AP_RC_RESET_IN_PROGRESS);
+
+ if (apqsw->rc)
+ printf("Wait for reset failed on ap %d queue %d with tapq rc %d.",
+ ap, qn, apqsw->rc);
+ return cc;
+}
+
+int ap_pqap_reset(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw)
+{
+ return pqap_reset_wait(ap, qn, apqsw, false);
+}
+
+int ap_pqap_reset_zeroize(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw)
+{
+ return pqap_reset_wait(ap, qn, apqsw, true);
+}
+
static int ap_get_apqn(uint8_t *ap, uint8_t *qn)
{
unsigned long *ptr;
diff --git a/lib/s390x/ap.h b/lib/s390x/ap.h
index 3f9e2eb6..f9343b5f 100644
--- a/lib/s390x/ap.h
+++ b/lib/s390x/ap.h
@@ -12,6 +12,8 @@
#ifndef _S390X_AP_H_
#define _S390X_AP_H_
+#define AP_RC_RESET_IN_PROGRESS 0x02
+
enum PQAP_FC {
PQAP_TEST_APQ,
PQAP_RESET_APQ,
@@ -94,6 +96,8 @@ _Static_assert(sizeof(struct ap_qirq_ctrl) == sizeof(uint64_t),
int ap_setup(uint8_t *ap, uint8_t *qn);
int ap_pqap_tapq(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw,
struct pqap_r2 *r2);
+int ap_pqap_reset(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw);
+int ap_pqap_reset_zeroize(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw);
int ap_pqap_qci(struct ap_config_info *info);
int ap_pqap_aqic(uint8_t ap, uint8_t qn, struct ap_queue_status *apqsw,
struct ap_qirq_ctrl aqic, unsigned long addr);
diff --git a/s390x/ap.c b/s390x/ap.c
index 31dcfe29..47b4f832 100644
--- a/s390x/ap.c
+++ b/s390x/ap.c
@@ -341,6 +341,57 @@ static void test_pqap_aqic(void)
report_prefix_pop();
}
+static void test_pqap_resets(void)
+{
+ struct ap_queue_status apqsw = {};
+ static uint8_t not_ind_byte;
+ struct ap_qirq_ctrl aqic = {};
+ struct pqap_r2 r2 = {};
+
+ int cc;
+
+ report_prefix_push("pqap");
+ report_prefix_push("rapq");
+
+ /* Enable IRQs which the resets will disable */
+ aqic.ir = 1;
+ cc = ap_pqap_aqic(apn, qn, &apqsw, aqic, (uintptr_t)¬_ind_byte);
+ report(cc == 0 && apqsw.rc == 0, "enable");
Depending on history I think we could have apqsw == 07 here.
(interrupt already set as requested)
+
+ do {
+ cc = ap_pqap_tapq(apn, qn, &apqsw, &r2);
may be a little delay before retry as you do above for ap_reset_wait()?
+ } while (cc == 0 && apqsw.irq_enabled == 0);
+ report(apqsw.irq_enabled == 1, "IRQs enabled");
+
+ ap_pqap_reset(apn, qn, &apqsw);
+ cc = ap_pqap_tapq(apn, qn, &apqsw, &r2);
+ assert(!cc);
+ report(apqsw.irq_enabled == 0, "IRQs have been disabled");
shouldn't we check that the APQ is fine apqsw.rc == 0 ?
+
+ report_prefix_pop();
+
+ report_prefix_push("zapq");
+
+ /* Enable IRQs which the resets will disable */
+ aqic.ir = 1;
+ cc = ap_pqap_aqic(apn, qn, &apqsw, aqic, (uintptr_t)¬_ind_byte);
+ report(cc == 0 && apqsw.rc == 0, "enable");
+
+ do {
+ cc = ap_pqap_tapq(apn, qn, &apqsw, &r2);
+ } while (cc == 0 && apqsw.irq_enabled == 0);
+ report(apqsw.irq_enabled == 1, "IRQs enabled");
+
+ ap_pqap_reset_zeroize(apn, qn, &apqsw);
+ cc = ap_pqap_tapq(apn, qn, &apqsw, &r2);
+ assert(!cc);
+ report(apqsw.irq_enabled == 0, "IRQs have been disabled");
+
+ report_prefix_pop();
+
+ report_prefix_pop();
+}
+
int main(void)
{
int setup_rc = ap_setup(&apn, &qn);
@@ -362,6 +413,7 @@ int main(void)
goto done;
}
test_pqap_aqic();
+ test_pqap_resets();
done:
report_prefix_pop();