Re: [kvm-unit-tests PATCH 5/5] s390x: ap: Add reset tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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,






[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux