| Message ID | 1576079170-7244-10-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Testing the Channel Subsystem I/O | expand |
On Wed, 11 Dec 2019 16:46:10 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > To test a write command with the SSCH instruction we need a QEMU device, > with control unit type 0xC0CA. The PONG device is such a device. > > This type of device responds to PONG_WRITE requests by incrementing an > integer, stored as a string at offset 0 of the CCW data. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > s390x/css.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/s390x/css.c b/s390x/css.c > index 7b9bdb1..a09cdff 100644 > --- a/s390x/css.c > +++ b/s390x/css.c > @@ -26,6 +26,9 @@ > > #define CSS_TEST_INT_PARAM 0xcafec0ca > #define PONG_CU_TYPE 0xc0ca > +/* Channel Commands for PONG device */ > +#define PONG_WRITE 0x21 /* Write */ > +#define PONG_READ 0x22 /* Read buffer */ > > struct lowcore *lowcore = (void *)0x0; > > @@ -302,6 +305,48 @@ unreg_cb: > unregister_io_int_func(irq_io); > } > > +static void test_ping(void) > +{ > + int success, result; > + int cnt = 0, max = 4; > + > + if (senseid.cu_type != PONG_CU) { > + report_skip("No PONG, no ping-pong"); > + return; > + } > + > + result = register_io_int_func(irq_io); > + if (result) { > + report(0, "Could not register IRQ handler"); > + return; > + } > + > + while (cnt++ < max) { > + snprintf(buffer, BUF_SZ, "%08x\n", cnt); > + success = start_subchannel(PONG_WRITE, buffer, 8); Magic value? Maybe introduce a #define for the lengths of the reads/writes? [This also got me thinking about your start_subchannel function again... do you also want to allow flags like e.g. SLI? It's not unusual for commands to return different lengths of data depending on what features are available; it might be worthwhile to allow short data if you're not sure that e.g. a command returns either the short or the long version of a structure.] > + if (!success) { > + report(0, "start_subchannel failed"); > + goto unreg_cb; > + } > + delay(100); > + success = start_subchannel(PONG_READ, buffer, 8); > + if (!success) { > + report(0, "start_subchannel failed"); > + goto unreg_cb; > + } > + result = atol(buffer); > + if (result != (cnt + 1)) { > + report(0, "Bad answer from pong: %08x - %08x", > + cnt, result); > + goto unreg_cb; > + } > + } > + report(1, "ping-pong count 0x%08x", cnt); > + > +unreg_cb: > + unregister_io_int_func(irq_io); > +} > + > static struct { > const char *name; > void (*func)(void); > @@ -309,6 +354,7 @@ static struct { > { "enumerate (stsch)", test_enumerate }, > { "enable (msch)", test_enable }, > { "sense (ssch/tsch)", test_sense }, > + { "ping-pong (ssch/tsch)", test_ping }, > { NULL, NULL } > }; >
On 2019-12-13 10:50, Cornelia Huck wrote: > On Wed, 11 Dec 2019 16:46:10 +0100 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> To test a write command with the SSCH instruction we need a QEMU device, >> with control unit type 0xC0CA. The PONG device is such a device. >> >> This type of device responds to PONG_WRITE requests by incrementing an >> integer, stored as a string at offset 0 of the CCW data. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> s390x/css.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> >> diff --git a/s390x/css.c b/s390x/css.c >> index 7b9bdb1..a09cdff 100644 >> --- a/s390x/css.c >> +++ b/s390x/css.c >> @@ -26,6 +26,9 @@ >> >> #define CSS_TEST_INT_PARAM 0xcafec0ca >> #define PONG_CU_TYPE 0xc0ca >> +/* Channel Commands for PONG device */ >> +#define PONG_WRITE 0x21 /* Write */ >> +#define PONG_READ 0x22 /* Read buffer */ >> >> struct lowcore *lowcore = (void *)0x0; >> >> @@ -302,6 +305,48 @@ unreg_cb: >> unregister_io_int_func(irq_io); >> } >> >> +static void test_ping(void) >> +{ >> + int success, result; >> + int cnt = 0, max = 4; >> + >> + if (senseid.cu_type != PONG_CU) { >> + report_skip("No PONG, no ping-pong"); >> + return; >> + } >> + >> + result = register_io_int_func(irq_io); >> + if (result) { >> + report(0, "Could not register IRQ handler"); >> + return; >> + } >> + >> + while (cnt++ < max) { >> + snprintf(buffer, BUF_SZ, "%08x\n", cnt); >> + success = start_subchannel(PONG_WRITE, buffer, 8); > > Magic value? Maybe introduce a #define for the lengths of the > reads/writes? OK, and I also do not need a buffer so big. > > [This also got me thinking about your start_subchannel function > again... do you also want to allow flags like e.g. SLI? It's not > unusual for commands to return different lengths of data depending on > what features are available; it might be worthwhile to allow short data > if you're not sure that e.g. a command returns either the short or the > long version of a structure.] I would prefer to keep simple it in this series if you agree. AFAIU the current QEMU implementation use a fix length and if a short read occurs it is an error. Since we test on PONG, there should be no error. I agree that for a general test we should change this, but currently the goal is just to verify that the remote device is PONG. If we accept variable length, we need to check the length of what we received, and this could need some infrastructure changes that I would like to do later. When the series is accepted I will begin to do more complicated things like: - checking the exceptions for wrong parameters This is the first I will add. - checking the response difference on flags (SLI, SKP) - using CC and CD flags for chaining - TIC, NOP, suspend/resume and PCI These last one will be fun, we can also trying to play with prefetch while at it. :) Regards, Pierre
On Fri, 13 Dec 2019 17:50:02 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > On 2019-12-13 10:50, Cornelia Huck wrote: > > [This also got me thinking about your start_subchannel function > > again... do you also want to allow flags like e.g. SLI? It's not > > unusual for commands to return different lengths of data depending on > > what features are available; it might be worthwhile to allow short data > > if you're not sure that e.g. a command returns either the short or the > > long version of a structure.] > > I would prefer to keep simple it in this series if you agree. Sure, that's fine. > > AFAIU the current QEMU implementation use a fix length and if a short > read occurs it is an error. > Since we test on PONG, there should be no error. It all depends on how the QEMU device is supposed to work. For a command reading/writing some data, I'd usually expect the following: - If SLI is not set, require the length to be the exact value expected by the device; otherwise, generate an error. - If SLI is set, require the length to be between the minimum length that makes sense and the full length of the buffer; otherwise, generate an error. Of course, if minimum length == full length, SLI has no real effect :) > > I agree that for a general test we should change this, but currently the > goal is just to verify that the remote device is PONG. > > If we accept variable length, we need to check the length of what we > received, and this could need some infrastructure changes that I would > like to do later. You mean at the device level? At the driver level (== here), you should simply get an error or not, I guess. > > When the series is accepted I will begin to do more complicated things like: > - checking the exceptions for wrong parameters > This is the first I will add. Agreed, that's probably the most useful one. > - checking the response difference on flags (SLI, SKP) > - using CC and CD flags for chaining > - TIC, NOP, suspend/resume and PCI > > These last one will be fun, we can also trying to play with prefetch > while at it. :) I think any kind of ccw chain will already be fun :) It's probably not so well tested anyway, as virtio is basically single-command.
diff --git a/s390x/css.c b/s390x/css.c index 7b9bdb1..a09cdff 100644 --- a/s390x/css.c +++ b/s390x/css.c @@ -26,6 +26,9 @@ #define CSS_TEST_INT_PARAM 0xcafec0ca #define PONG_CU_TYPE 0xc0ca +/* Channel Commands for PONG device */ +#define PONG_WRITE 0x21 /* Write */ +#define PONG_READ 0x22 /* Read buffer */ struct lowcore *lowcore = (void *)0x0; @@ -302,6 +305,48 @@ unreg_cb: unregister_io_int_func(irq_io); } +static void test_ping(void) +{ + int success, result; + int cnt = 0, max = 4; + + if (senseid.cu_type != PONG_CU) { + report_skip("No PONG, no ping-pong"); + return; + } + + result = register_io_int_func(irq_io); + if (result) { + report(0, "Could not register IRQ handler"); + return; + } + + while (cnt++ < max) { + snprintf(buffer, BUF_SZ, "%08x\n", cnt); + success = start_subchannel(PONG_WRITE, buffer, 8); + if (!success) { + report(0, "start_subchannel failed"); + goto unreg_cb; + } + delay(100); + success = start_subchannel(PONG_READ, buffer, 8); + if (!success) { + report(0, "start_subchannel failed"); + goto unreg_cb; + } + result = atol(buffer); + if (result != (cnt + 1)) { + report(0, "Bad answer from pong: %08x - %08x", + cnt, result); + goto unreg_cb; + } + } + report(1, "ping-pong count 0x%08x", cnt); + +unreg_cb: + unregister_io_int_func(irq_io); +} + static struct { const char *name; void (*func)(void); @@ -309,6 +354,7 @@ static struct { { "enumerate (stsch)", test_enumerate }, { "enable (msch)", test_enable }, { "sense (ssch/tsch)", test_sense }, + { "ping-pong (ssch/tsch)", test_ping }, { NULL, NULL } };
To test a write command with the SSCH instruction we need a QEMU device, with control unit type 0xC0CA. The PONG device is such a device. This type of device responds to PONG_WRITE requests by incrementing an integer, stored as a string at offset 0 of the CCW data. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- s390x/css.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)