| Message ID | 1574945167-29677-7-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 Thu, 28 Nov 2019 13:46:04 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > First step by testing the channel subsystem is to enumerate the css and s/by/for/ > retrieve the css devices. > > This test the success of STSCH I/O instruction. s/test/tests/ > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > s390x/Makefile | 4 ++- > s390x/css.c | 86 +++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 4 +++ > 3 files changed, 93 insertions(+), 1 deletion(-) > create mode 100644 s390x/css.c > (...) > diff --git a/s390x/css.c b/s390x/css.c > new file mode 100644 > index 0000000..8186f55 > --- /dev/null > +++ b/s390x/css.c > @@ -0,0 +1,86 @@ > +/* > + * Channel Sub-System tests s/Sub-System/Subsystem/ > + * > + * Copyright (c) 2019 IBM Corp > + * > + * Authors: > + * Pierre Morel <pmorel@linux.ibm.com> > + * > + * This code is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2. > + */ > + > +#include <libcflat.h> > + > +#include <css.h> > + > +#define SID_ONE 0x00010000 > + > +static struct schib schib; > + > +static const char *Channel_type[3] = { > + "I/O", "CHSC", "MSG" No EADM? :) I don't think we plan to emulate anything beyond I/O in QEMU, though. > +}; > + > +static int test_device_sid; > + > +static void test_enumerate(void) > +{ > + struct pmcw *pmcw = &schib.pmcw; > + int sid; > + int ret, i; > + int found = 0; > + > + for (sid = 0; sid < 0xffff; sid++) { > + ret = stsch(sid|SID_ONE, &schib); This seems a bit odd. You are basically putting the subchannel number into sid, OR in the one, and then use the resulting value as the sid (subchannel identifier). > + if (!ret && (pmcw->flags & PMCW_DNV)) { > + report_info("SID %04x Type %s PIM %x", sid, That's not a sid, but the subchannel number (see above). > + Channel_type[pmcw->st], pmcw->pim); > + for (i = 0; i < 8; i++) { > + if ((pmcw->pim << i) & 0x80) { > + report_info("CHPID[%d]: %02x", i, > + pmcw->chpid[i]); > + break; > + } > + } > + found++; > + > + } Here, you iterate over the 0-0xffff range, even if you got a condition code 3 (indicating no more subchannels in that set). Is that intentional? > + if (found && !test_device_sid) > + test_device_sid = sid|SID_ONE; You set test_device_sid to the last valid subchannel? Why? > + } > + if (!found) { > + report("Found %d devices", 0, found); > + return; > + } > + ret = stsch(test_device_sid, &schib); Why do you do a stsch() again? > + if (ret) { > + report("Err %d on stsch on sid %08x", 0, ret, test_device_sid); > + return; > + } > + report("Tested", 1); > + return; I don't think you need this return statement. Your test only enumerates devices in the first subchannel set. Do you plan to enhance the test to enable the MSS facility and iterate over all subchannel sets? > +} > + > +static struct { > + const char *name; > + void (*func)(void); > +} tests[] = { > + { "enumerate (stsch)", test_enumerate }, > + { NULL, NULL } > +}; > + > +int main(int argc, char *argv[]) > +{ > + int i; > + > + report_prefix_push("Channel Sub-System"); s/Sub-System/Subsystem/ > + for (i = 0; tests[i].name; i++) { > + report_prefix_push(tests[i].name); > + tests[i].func(); > + report_prefix_pop(); > + } > + report_prefix_pop(); > + > + return report_summary(); > +} (...)
On 2019-12-02 15:22, Cornelia Huck wrote: > On Thu, 28 Nov 2019 13:46:04 +0100 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> First step by testing the channel subsystem is to enumerate the css and > > s/by/for/ ok > >> retrieve the css devices. >> >> This test the success of STSCH I/O instruction. > > s/test/tests/ yes > >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> s390x/Makefile | 4 ++- >> s390x/css.c | 86 +++++++++++++++++++++++++++++++++++++++++++++ >> s390x/unittests.cfg | 4 +++ >> 3 files changed, 93 insertions(+), 1 deletion(-) >> create mode 100644 s390x/css.c >> > > (...) > >> diff --git a/s390x/css.c b/s390x/css.c >> new file mode 100644 >> index 0000000..8186f55 >> --- /dev/null >> +++ b/s390x/css.c >> @@ -0,0 +1,86 @@ >> +/* >> + * Channel Sub-System tests > > s/Sub-System/Subsystem/ yes too > >> + * >> + * Copyright (c) 2019 IBM Corp >> + * >> + * Authors: >> + * Pierre Morel <pmorel@linux.ibm.com> >> + * >> + * This code is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License version 2. >> + */ >> + >> +#include <libcflat.h> >> + >> +#include <css.h> >> + >> +#define SID_ONE 0x00010000 >> + >> +static struct schib schib; >> + >> +static const char *Channel_type[3] = { >> + "I/O", "CHSC", "MSG" > > No EADM? :) I forgot EADM, I will add it! > > I don't think we plan to emulate anything beyond I/O in QEMU, though. Even, yes, no plan to use it for now. > >> +}; >> + >> +static int test_device_sid; >> + >> +static void test_enumerate(void) >> +{ >> + struct pmcw *pmcw = &schib.pmcw; >> + int sid; >> + int ret, i; >> + int found = 0; >> + >> + for (sid = 0; sid < 0xffff; sid++) { >> + ret = stsch(sid|SID_ONE, &schib); > > This seems a bit odd. You are basically putting the subchannel number > into sid, OR in the one, and then use the resulting value as the sid > (subchannel identifier). > >> + if (!ret && (pmcw->flags & PMCW_DNV)) { >> + report_info("SID %04x Type %s PIM %x", sid, > > That's not a sid, but the subchannel number (see above). > >> + Channel_type[pmcw->st], pmcw->pim); >> + for (i = 0; i < 8; i++) { >> + if ((pmcw->pim << i) & 0x80) { >> + report_info("CHPID[%d]: %02x", i, >> + pmcw->chpid[i]); >> + break; >> + } >> + } >> + found++; >> + >> + } > > Here, you iterate over the 0-0xffff range, even if you got a condition > code 3 (indicating no more subchannels in that set). Is that > intentional? I thought there could be more subchannels. I need then a break in the loop when this happens. I will reread the PoP to see how to find that no more subchannel are in that set. > >> + if (found && !test_device_sid) >> + test_device_sid = sid|SID_ONE; > > You set test_device_sid to the last valid subchannel? Why? The last ? I wanted the first one I wanted something easy but I should have explain. To avoid doing complicated things like doing a sense on each valid subchannel I just take the first one. Should be enough as we do not go to the device in this test. > >> + } >> + if (!found) { >> + report("Found %d devices", 0, found); >> + return; >> + } >> + ret = stsch(test_device_sid, &schib); > > Why do you do a stsch() again? right, no need. In an internal version I used to print some informations from the SCHIB. Since in between I overwrote the SHIB, I did it again. But in this version; no need. > >> + if (ret) { >> + report("Err %d on stsch on sid %08x", 0, ret, test_device_sid); >> + return; >> + } >> + report("Tested", 1); >> + return; > > I don't think you need this return statement. right I have enough work. :) > > Your test only enumerates devices in the first subchannel set. Do you > plan to enhance the test to enable the MSS facility and iterate over > all subchannel sets? Yes, it is something we can do in a following series > >> +} >> + >> +static struct { >> + const char *name; >> + void (*func)(void); >> +} tests[] = { >> + { "enumerate (stsch)", test_enumerate }, >> + { NULL, NULL } >> +}; >> + >> +int main(int argc, char *argv[]) >> +{ >> + int i; >> + >> + report_prefix_push("Channel Sub-System"); > > s/Sub-System/Subsystem/ yes, again. Thanks for the review. Regards, Pierre
On Mon, 2 Dec 2019 18:53:16 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > On 2019-12-02 15:22, Cornelia Huck wrote: > > On Thu, 28 Nov 2019 13:46:04 +0100 > > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> +static int test_device_sid; > >> + > >> +static void test_enumerate(void) > >> +{ > >> + struct pmcw *pmcw = &schib.pmcw; > >> + int sid; > >> + int ret, i; > >> + int found = 0; > >> + > >> + for (sid = 0; sid < 0xffff; sid++) { > >> + ret = stsch(sid|SID_ONE, &schib); > > > > This seems a bit odd. You are basically putting the subchannel number > > into sid, OR in the one, and then use the resulting value as the sid > > (subchannel identifier). > > > >> + if (!ret && (pmcw->flags & PMCW_DNV)) { > >> + report_info("SID %04x Type %s PIM %x", sid, > > > > That's not a sid, but the subchannel number (see above). > > > >> + Channel_type[pmcw->st], pmcw->pim); > >> + for (i = 0; i < 8; i++) { > >> + if ((pmcw->pim << i) & 0x80) { > >> + report_info("CHPID[%d]: %02x", i, > >> + pmcw->chpid[i]); > >> + break; > >> + } > >> + } > >> + found++; > >> + > >> + } > > > > Here, you iterate over the 0-0xffff range, even if you got a condition > > code 3 (indicating no more subchannels in that set). Is that > > intentional? > > I thought there could be more subchannels. > I need then a break in the loop when this happens. > I will reread the PoP to see how to find that no more subchannel are in > that set. The fact that cc 3 for stsch == no more subchannels is unfortunately a bit scattered across the PoP :/ Dug it out some time ago, maybe it's still in the archives somewhere... > > > > >> + if (found && !test_device_sid) > >> + test_device_sid = sid|SID_ONE; > > > > You set test_device_sid to the last valid subchannel? Why? > > The last ? I wanted the first one It is indeed the first one, -ENOCOFFEE. > > I wanted something easy but I should have explain. > > To avoid doing complicated things like doing a sense on each valid > subchannel I just take the first one. > Should be enough as we do not go to the device in this test. Yes; but you plan to reuse that code, don't you? > > > > >> + } > >> + if (!found) { > >> + report("Found %d devices", 0, found); Now that I look at this again: If you got here, you always found 0 devices, so that message is not super helpful :) > >> + return; > >> + } > >> + ret = stsch(test_device_sid, &schib); > > > > Why do you do a stsch() again? > > right, no need. > In an internal version I used to print some informations from the SCHIB. > Since in between I overwrote the SHIB, I did it again. > But in this version; no need. You could copy the schib of the subchannel to be tested to a different place, but I'm not sure it's worth it. > > > > >> + if (ret) { > >> + report("Err %d on stsch on sid %08x", 0, ret, test_device_sid); > >> + return; > >> + } > >> + report("Tested", 1); > >> + return; > > > > I don't think you need this return statement. > > right I have enough work. :) > > > > > Your test only enumerates devices in the first subchannel set. Do you > > plan to enhance the test to enable the MSS facility and iterate over > > all subchannel sets? > > Yes, it is something we can do in a following series Sure, just asked out of interest :)
On 2019-12-02 19:15, Cornelia Huck wrote: > On Mon, 2 Dec 2019 18:53:16 +0100 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> On 2019-12-02 15:22, Cornelia Huck wrote: >>> On Thu, 28 Nov 2019 13:46:04 +0100 >>> Pierre Morel <pmorel@linux.ibm.com> wrote: > >>>> +static int test_device_sid; >>>> + >>>> +static void test_enumerate(void) >>>> +{ >>>> + struct pmcw *pmcw = &schib.pmcw; >>>> + int sid; >>>> + int ret, i; >>>> + int found = 0; >>>> + >>>> + for (sid = 0; sid < 0xffff; sid++) { >>>> + ret = stsch(sid|SID_ONE, &schib); >>> >>> This seems a bit odd. You are basically putting the subchannel number >>> into sid, OR in the one, and then use the resulting value as the sid >>> (subchannel identifier). >>> >>>> + if (!ret && (pmcw->flags & PMCW_DNV)) { >>>> + report_info("SID %04x Type %s PIM %x", sid, >>> >>> That's not a sid, but the subchannel number (see above). >>> >>>> + Channel_type[pmcw->st], pmcw->pim); >>>> + for (i = 0; i < 8; i++) { >>>> + if ((pmcw->pim << i) & 0x80) { >>>> + report_info("CHPID[%d]: %02x", i, >>>> + pmcw->chpid[i]); >>>> + break; >>>> + } >>>> + } >>>> + found++; >>>> + >>>> + } >>> >>> Here, you iterate over the 0-0xffff range, even if you got a condition >>> code 3 (indicating no more subchannels in that set). Is that >>> intentional? >> >> I thought there could be more subchannels. >> I need then a break in the loop when this happens. >> I will reread the PoP to see how to find that no more subchannel are in >> that set. > > The fact that cc 3 for stsch == no more subchannels is unfortunately a > bit scattered across the PoP :/ Dug it out some time ago, maybe it's > still in the archives somewhere... So the the subchannel are always one after the other? > >> >>> >>>> + if (found && !test_device_sid) >>>> + test_device_sid = sid|SID_ONE; >>> >>> You set test_device_sid to the last valid subchannel? Why? >> >> The last ? I wanted the first one > > It is indeed the first one, -ENOCOFFEE. Would never happend to me. > >> >> I wanted something easy but I should have explain. >> >> To avoid doing complicated things like doing a sense on each valid >> subchannel I just take the first one. >> Should be enough as we do not go to the device in this test. > > Yes; but you plan to reuse that code, don't you? yes, so I must think about this > >> >>> >>>> + } >>>> + if (!found) { >>>> + report("Found %d devices", 0, found); > > Now that I look at this again: If you got here, you always found 0 > devices, so that message is not super helpful :) yes, found is too much. A cut and past from the time I was happy to find even one! :) > >>>> + return; >>>> + } >>>> + ret = stsch(test_device_sid, &schib); >>> >>> Why do you do a stsch() again? >> >> right, no need. >> In an internal version I used to print some informations from the SCHIB. >> Since in between I overwrote the SHIB, I did it again. >> But in this version; no need. > > You could copy the schib of the subchannel to be tested to a different > place, but I'm not sure it's worth it. > >> >>> >>>> + if (ret) { >>>> + report("Err %d on stsch on sid %08x", 0, ret, test_device_sid); >>>> + return; >>>> + } >>>> + report("Tested", 1); >>>> + return; >>> >>> I don't think you need this return statement. >> >> right I have enough work. :) >> >>> >>> Your test only enumerates devices in the first subchannel set. Do you >>> plan to enhance the test to enable the MSS facility and iterate over >>> all subchannel sets? >> >> Yes, it is something we can do in a following series > > Sure, just asked out of interest :) > Thanks, Regards, Pierre
On Mon, 2 Dec 2019 19:33:59 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > On 2019-12-02 19:15, Cornelia Huck wrote: > > On Mon, 2 Dec 2019 18:53:16 +0100 > > Pierre Morel <pmorel@linux.ibm.com> wrote: > > > >> On 2019-12-02 15:22, Cornelia Huck wrote: > >>> On Thu, 28 Nov 2019 13:46:04 +0100 > >>> Pierre Morel <pmorel@linux.ibm.com> wrote: > > > >>>> +static int test_device_sid; > >>>> + > >>>> +static void test_enumerate(void) > >>>> +{ > >>>> + struct pmcw *pmcw = &schib.pmcw; > >>>> + int sid; > >>>> + int ret, i; > >>>> + int found = 0; > >>>> + > >>>> + for (sid = 0; sid < 0xffff; sid++) { > >>>> + ret = stsch(sid|SID_ONE, &schib); > >>> > >>> This seems a bit odd. You are basically putting the subchannel number > >>> into sid, OR in the one, and then use the resulting value as the sid > >>> (subchannel identifier). > >>> > >>>> + if (!ret && (pmcw->flags & PMCW_DNV)) { > >>>> + report_info("SID %04x Type %s PIM %x", sid, > >>> > >>> That's not a sid, but the subchannel number (see above). > >>> > >>>> + Channel_type[pmcw->st], pmcw->pim); > >>>> + for (i = 0; i < 8; i++) { > >>>> + if ((pmcw->pim << i) & 0x80) { > >>>> + report_info("CHPID[%d]: %02x", i, > >>>> + pmcw->chpid[i]); > >>>> + break; > >>>> + } > >>>> + } > >>>> + found++; > >>>> + > >>>> + } > >>> > >>> Here, you iterate over the 0-0xffff range, even if you got a condition > >>> code 3 (indicating no more subchannels in that set). Is that > >>> intentional? > >> > >> I thought there could be more subchannels. > >> I need then a break in the loop when this happens. > >> I will reread the PoP to see how to find that no more subchannel are in > >> that set. > > > > The fact that cc 3 for stsch == no more subchannels is unfortunately a > > bit scattered across the PoP :/ Dug it out some time ago, maybe it's > > still in the archives somewhere... > > So the the subchannel are always one after the other? While QEMU (and z/VM) usually do that, they can really be scattered around. For the in-between I/O subchannels that don't lead to a device, you'll still get cc 0, it's just the dnv bit that is 0. The cc 3 basically just tells you that you can stop looking.
On 2019-12-02 20:49, Cornelia Huck wrote: > On Mon, 2 Dec 2019 19:33:59 +0100 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> On 2019-12-02 19:15, Cornelia Huck wrote: >>> On Mon, 2 Dec 2019 18:53:16 +0100 >>> Pierre Morel <pmorel@linux.ibm.com> wrote: >>> >>>> On 2019-12-02 15:22, Cornelia Huck wrote: >>>>> On Thu, 28 Nov 2019 13:46:04 +0100 >>>>> Pierre Morel <pmorel@linux.ibm.com> wrote: >>> >>>>>> +static int test_device_sid; >>>>>> + >>>>>> +static void test_enumerate(void) >>>>>> +{ >>>>>> + struct pmcw *pmcw = &schib.pmcw; >>>>>> + int sid; >>>>>> + int ret, i; >>>>>> + int found = 0; >>>>>> + >>>>>> + for (sid = 0; sid < 0xffff; sid++) { >>>>>> + ret = stsch(sid|SID_ONE, &schib); >>>>> >>>>> This seems a bit odd. You are basically putting the subchannel number >>>>> into sid, OR in the one, and then use the resulting value as the sid >>>>> (subchannel identifier). >>>>> >>>>>> + if (!ret && (pmcw->flags & PMCW_DNV)) { >>>>>> + report_info("SID %04x Type %s PIM %x", sid, >>>>> >>>>> That's not a sid, but the subchannel number (see above). >>>>> >>>>>> + Channel_type[pmcw->st], pmcw->pim); >>>>>> + for (i = 0; i < 8; i++) { >>>>>> + if ((pmcw->pim << i) & 0x80) { >>>>>> + report_info("CHPID[%d]: %02x", i, >>>>>> + pmcw->chpid[i]); >>>>>> + break; >>>>>> + } >>>>>> + } >>>>>> + found++; >>>>>> + >>>>>> + } >>>>> >>>>> Here, you iterate over the 0-0xffff range, even if you got a condition >>>>> code 3 (indicating no more subchannels in that set). Is that >>>>> intentional? >>>> >>>> I thought there could be more subchannels. >>>> I need then a break in the loop when this happens. >>>> I will reread the PoP to see how to find that no more subchannel are in >>>> that set. >>> >>> The fact that cc 3 for stsch == no more subchannels is unfortunately a >>> bit scattered across the PoP :/ Dug it out some time ago, maybe it's >>> still in the archives somewhere... >> >> So the the subchannel are always one after the other? > > While QEMU (and z/VM) usually do that, they can really be scattered > around. For the in-between I/O subchannels that don't lead to a device, > you'll still get cc 0, it's just the dnv bit that is 0. The cc 3 > basically just tells you that you can stop looking. > Thanks for the explanation, I will change the code accordingly. Regards, Pierre
diff --git a/s390x/Makefile b/s390x/Makefile index e9da618..167ba05 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -1,4 +1,4 @@ -tests = $(TEST_DIR)/selftest.elf +#tests = $(TEST_DIR)/selftest.elf tests += $(TEST_DIR)/intercept.elf tests += $(TEST_DIR)/emulator.elf tests += $(TEST_DIR)/sieve.elf @@ -19,6 +19,7 @@ tests += $(TEST_DIR)/smp.elf tests += $(TEST_DIR)/uv-guest.elf tests += $(TEST_DIR)/uv-host.elf tests += $(TEST_DIR)/uv-limit.elf +tests += $(TEST_DIR)/css.elf tests_binary = $(patsubst %.elf,%.bin,$(tests)) tests_img = $(patsubst %.elf,%.img,$(tests)) @@ -56,6 +57,7 @@ cflatobjs += lib/s390x/sclp-console.o cflatobjs += lib/s390x/interrupt.o cflatobjs += lib/s390x/mmu.o cflatobjs += lib/s390x/smp.o +cflatobjs += lib/s390x/css_dump.o OBJDIRS += lib/s390x diff --git a/s390x/css.c b/s390x/css.c new file mode 100644 index 0000000..8186f55 --- /dev/null +++ b/s390x/css.c @@ -0,0 +1,86 @@ +/* + * Channel Sub-System tests + * + * Copyright (c) 2019 IBM Corp + * + * Authors: + * Pierre Morel <pmorel@linux.ibm.com> + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2. + */ + +#include <libcflat.h> + +#include <css.h> + +#define SID_ONE 0x00010000 + +static struct schib schib; + +static const char *Channel_type[3] = { + "I/O", "CHSC", "MSG" +}; + +static int test_device_sid; + +static void test_enumerate(void) +{ + struct pmcw *pmcw = &schib.pmcw; + int sid; + int ret, i; + int found = 0; + + for (sid = 0; sid < 0xffff; sid++) { + ret = stsch(sid|SID_ONE, &schib); + if (!ret && (pmcw->flags & PMCW_DNV)) { + report_info("SID %04x Type %s PIM %x", sid, + Channel_type[pmcw->st], pmcw->pim); + for (i = 0; i < 8; i++) { + if ((pmcw->pim << i) & 0x80) { + report_info("CHPID[%d]: %02x", i, + pmcw->chpid[i]); + break; + } + } + found++; + + } + if (found && !test_device_sid) + test_device_sid = sid|SID_ONE; + } + if (!found) { + report("Found %d devices", 0, found); + return; + } + ret = stsch(test_device_sid, &schib); + if (ret) { + report("Err %d on stsch on sid %08x", 0, ret, test_device_sid); + return; + } + report("Tested", 1); + return; +} + +static struct { + const char *name; + void (*func)(void); +} tests[] = { + { "enumerate (stsch)", test_enumerate }, + { NULL, NULL } +}; + +int main(int argc, char *argv[]) +{ + int i; + + report_prefix_push("Channel Sub-System"); + for (i = 0; tests[i].name; i++) { + report_prefix_push(tests[i].name); + tests[i].func(); + report_prefix_pop(); + } + report_prefix_pop(); + + return report_summary(); +} diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index ff4c088..efdf954 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -78,3 +78,7 @@ extra_params =-smp 2 [uv-guest] file = uv-guest.elf + +[css] +file = css.elf +extra_params =-device ccw-pong
First step by testing the channel subsystem is to enumerate the css and retrieve the css devices. This test the success of STSCH I/O instruction. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- s390x/Makefile | 4 ++- s390x/css.c | 86 +++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 4 +++ 3 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 s390x/css.c