On 2020-07-03 10:41, Thomas Huth wrote:
On 02/07/2020 18.31, Pierre Morel wrote:
After a channel is enabled we start a SENSE_ID command using
the SSCH instruction to recognize the control unit and device.
This tests the success of SSCH, the I/O interruption and the TSCH
instructions.
The SENSE_ID command response is tested to report 0xff inside
its reserved field and to report the same control unit type
as the cu_type kernel argument.
Without the cu_type kernel argument, the test expects a device
with a default control unit type of 0x3832, a.k.a virtio-net-ccw.
Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
[...]
diff --git a/lib/s390x/css.h b/lib/s390x/css.h
index 0ddceb1..9c22644 100644
--- a/lib/s390x/css.h
+++ b/lib/s390x/css.h
@@ -11,6 +11,8 @@
#ifndef CSS_H
#define CSS_H
+#define lowcore_ptr ((struct lowcore *)0x0)
I'd prefer if you could either put this into the css_lib.c file or in
lib/s390x/asm/arch_def.h.
I have a patch ready for this :)
But I did not want to add too much new things in this series that could
start a new discussion.
I have 2 versions of the patch:
- The simple one with just the declaration in arch_def.h
- The complete one with update of all tests (but smp) using a pointer to
lowcore.
...snip...
static inline int ssch(unsigned long schid, struct orb *addr)
@@ -251,6 +271,16 @@ void dump_orb(struct orb *op);
int css_enumerate(void);
#define MAX_ENABLE_RETRIES 5
-int css_enable(int schid);
+int css_enable(int schid, int isc);
+
+
In case you respin: Remove one empty line?
yes
+/* Library functions */
+int start_ccw1_chain(unsigned int sid, struct ccw1 *ccw);
...snip...
+ lowcore_ptr->io_int_param = 0;
+
+ memset(&senseid, 0, sizeof(senseid));
+ ret = start_single_ccw(test_device_sid, CCW_CMD_SENSE_ID,
+ &senseid, sizeof(senseid), CCW_F_SLI);
+ if (ret) {
+ report(0, "ssch failed for SENSE ID on sch %08x with cc %d",
+ test_device_sid, ret);
+ goto unreg_cb;
+ }
I'd maybe rather do something like:
report(ret == 0, "SENSE ID on sch %08x has good CC (%d)", ...)
if (ret)
goto unreg_cb;
and avoid report(0, ...) statements. Also for the other tests below. But
maybe that's really just a matter of taste.
I prefer to use report(0,....) when an unexpected error occurs: This
keep the test silent when what is expected occurs.
And use report(ret == xxx, ....) as the last report to report overall
success or failure of the test.
Other opinions?
+ wait_for_interrupt(PSW_MASK_IO);
...snip...
Apart from the nits, I'm fine with the patch.
Acked-by: Thomas Huth <thuth@xxxxxxxxxx>
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
