On 2019-12-12 13:01, Cornelia Huck wrote:
On Wed, 11 Dec 2019 16:46:08 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
A second step when testing the channel subsystem is to prepare a channel
for use.
This includes:
- Get the current SubCHannel Information Block (SCHIB) using STSCH
- Update it in memory to set the ENABLE bit
- Tell the CSS that the SCHIB has been modified using MSCH
- Get the SCHIB from the CSS again to verify that the subchannel is
enabled.
This tests the success of the MSCH instruction by enabling a channel.
Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
s390x/css.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/s390x/css.c b/s390x/css.c
index dfab35f..b8824ad 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -19,12 +19,24 @@
#include <asm/time.h>
#include <css.h>
+#include <asm/time.h>
#define SID_ONE 0x00010000
static struct schib schib;
static int test_device_sid;
+static inline void delay(unsigned long ms)
+{
+ unsigned long startclk;
+
+ startclk = get_clock_ms();
+ for (;;) {
+ if (get_clock_ms() - startclk > ms)
+ break;
+ }
+} >
Would this function be useful for other callers as well? I.e., should
it go into a common header?
Yes, I wanted to put it in the new time.h with the get_clock_ms() but
did not since I already got the RB.
I also did not want to add a patch to the series, but since you ask, I
can put it in a separate patch to keep the RB and to add it in the time.h
+
static void test_enumerate(void)
{
struct pmcw *pmcw = &schib.pmcw;
@@ -64,11 +76,64 @@ out:
report(1, "Devices, tested: %d, I/O type: %d", scn, scn_found);
}
+static void test_enable(void)
+{
+ struct pmcw *pmcw = &schib.pmcw;
+ int count = 0;
Odd indentation.
indeed!
+ int cc;
+
+ if (!test_device_sid) {
+ report_skip("No device");
+ return;
+ }
+ /* Read the SCHIB for this subchannel */
+ cc = stsch(test_device_sid, &schib);
+ if (cc) {
+ report(0, "stsch cc=%d", cc);
+ return;
+ }
+
+ /* Update the SCHIB to enable the channel */
+ pmcw->flags |= PMCW_ENABLE;
+
+ /* Tell the CSS we want to modify the subchannel */
+ cc = msch(test_device_sid, &schib);
+ if (cc) {
+ /*
+ * If the subchannel is status pending or
+ * if a function is in progress,
+ * we consider both cases as errors.
+ */
+ report(0, "msch cc=%d", cc);
+ return;
+ }
+
+ /*
+ * Read the SCHIB again to verify the enablement
+ * insert a little delay and try 5 times.
+ */
+ do {
+ cc = stsch(test_device_sid, &schib);
+ if (cc) {
+ report(0, "stsch cc=%d", cc);
+ return;
+ }
+ delay(10);
That's just a short delay to avoid a busy loop, right? msch should be
immediate,
Thought you told to me that it may not be immediate in zVM did I
misunderstand?
and you probably should not delay on success?
yes, it is not optimized, I can test PMCW_ENABLE in the loop this way we
can see if, in the zVM case we need to do retries or not.
+ } while (!(pmcw->flags & PMCW_ENABLE) && count++ < 5);
How is this supposed to work? Doesn't the stsch overwrite the control
block again, so you need to re-set the enable bit before you retry?
I do not think so, there is no msch() in the loop.
Do I miss something?
Thanks for the review,
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
