On 4/4/23 13:40, Janosch Frank wrote:
On 4/3/23 16:57, Pierre Morel wrote:
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.
Yes, because the names of the functions include the zeroize parts
which makes it easier for developers to understand how they work
instead of having a bool argument where they need to look up at which
argument position it is.
Why this intermediate level?
So I don't need to repeat the function below for a different r0.fc, no?
question of taste anyway.
[...]
+ } 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 ?
Isn't that covered by the assert above?
May be.
This is the kind of thing where I find the implementation and
documentation not very logical.
- CC = 0 means that the instruction was processed correctly.
- APQSW reports the status of the AP queue
For any operation but TAPQ I understand that CC=3 if APQSW is != 0
but for TAPQ, if it is processed correctly it should give back the
APQSW. Isn't it exactly what we ask the TAPQ to do?
I am probably not the only one to think that CC for TAPQ is at least not
useful, the Linux implementation ignores it.
You are probably right but in doubt I would do as in Linux
implementation and ignore CC,
