From: Pierre Morel <pmorel@linux.ibm.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: kvm@vger.kernel.org, linux-s390@vger.kernel.org,
frankja@linux.ibm.com, david@redhat.com, thuth@redhat.com
Subject: Re: [kvm-unit-tests PATCH v3 8/9] s390x: css: ssch/tsch with sense and interrupt
Date: Tue, 10 Dec 2019 10:12:29 +0100 [thread overview]
Message-ID: <502977b7-ad73-3468-92c9-7798cff6f754@linux.ibm.com> (raw)
In-Reply-To: <20191209182213.69e14263.cohuck@redhat.com>
On 2019-12-09 18:22, Cornelia Huck wrote:
> On Fri, 6 Dec 2019 17:26:27 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> When a channel is enabled we can start a SENSE 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 test expects a device with a control unit type of 0xC0CA as the
>> first subchannel of the CSS.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> lib/s390x/css.h | 13 ++++
>> s390x/css.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 177 insertions(+)
>>
>
>> +static void irq_io(void)
>> +{
>> + int ret = 0;
>> + char *flags;
>> +
>> + report_prefix_push("Interrupt");
>> + if (lowcore->io_int_param != 0xcafec0ca) {
>> + report("Bad io_int_param: %x", 0, lowcore->io_int_param);
>
> Use a #define for the intparm and print got vs. expected on mismatch?
OK
>
>> + report_prefix_pop();
>> + return;
>> + }
>> + report("io_int_param: %x", 1, lowcore->io_int_param);
>
> Well, at that moment you already know what the intparm is, don't you? :)
Yes, I can remove this, was for debug purpose.
>
>> + report_prefix_pop();
>> +
>> + ret = tsch(lowcore->subsys_id_word, &irb);
>> + dump_irb(&irb);
>> + flags = dump_scsw_flags(irb.scsw.ctrl);
>> +
>> + if (ret)
>> + report("IRB scsw flags: %s", 0, flags);
>
> I think you should also distinguish between cc 1 (not status pending)
> and cc 3 (not operational) here (or at least also print that info).
Yes, right, thanks.
>
>> + else
>> + report("IRB scsw flags: %s", 1, flags);
>> + report_prefix_pop();
>> +}
>> +
>> +static int start_subchannel(int code, char *data, int count)
>> +{
>> + int ret;
>> + struct pmcw *p = &schib.pmcw;
>> + struct orb *orb_p = &orb[0];
>> +
>> + /* Verify that a test subchannel has been set */
>> + if (!test_device_sid) {
>> + report_skip("No device");
>> + return 0;
>> + }
>> +
>> + /* Verify that the subchannel has been enabled */
>> + ret = stsch(test_device_sid, &schib);
>> + if (ret) {
>> + report("Err %d on stsch on sid %08x", 0, ret, test_device_sid);
>> + return 0;
>> + }
>> + if (!(p->flags & PMCW_ENABLE)) {
>> + report_skip("Device (sid %08x) not enabled", test_device_sid);
>> + return 0;
>> + }
>> +
>> + report_prefix_push("Start Subchannel");
>> + /* Build the CCW chain with a single CCW */
>> + ccw[0].code = code;
>> + ccw[0].flags = 0; /* No flags need to be set */
>> + ccw[0].count = count;
>> + ccw[0].data_address = (int)(unsigned long)data;
>> + orb_p->intparm = 0xcafec0ca;
>> + orb_p->ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
>> + if ((unsigned long)&ccw[0] >= 0x80000000UL) {
>> + report("Data above 2G! %016lx", 0, (unsigned long)&ccw[0]);
>
> Check for data under 2G before you set up data_address as well?
yes. Even more important since it is a parameter.
>
>> + report_prefix_pop();
>> + return 0;
>> + }
>> + orb_p->cpa = (unsigned int) (unsigned long)&ccw[0];
>> +
>> + ret = ssch(test_device_sid, orb_p);
>> + if (ret) {
>> + report("ssch cc=%d", 0, ret);
>> + report_prefix_pop();
>> + return 0;
>> + }
>> + report_prefix_pop();
>> + return 1;
>> +}
>> +
>> +/*
>> + * test_sense
>> + * Pre-requisits:
>> + * We need the QEMU PONG device as the first recognized
>> + * device by the enumeration.
>> + * ./s390x-run s390x/css.elf -device ccw-pong,cu_type=0xc0ca
>> + */
>> +static void test_sense(void)
>> +{
>> + int ret;
>> +
>> + ret = register_io_int_func(irq_io);
>> + if (ret) {
>> + report("Could not register IRQ handler", 0);
>> + goto unreg_cb;
>> + }
>> +
>> + enable_io_irq();
>> +
>> + ret = start_subchannel(CCW_CMD_SENSE_ID, buffer, sizeof(senseid));
>> + if (!ret) {
>> + report("start_subchannel failed", 0);
>> + goto unreg_cb;
>> + }
>> +
>> + senseid.cu_type = buffer[2] | (buffer[1] << 8);
>> + delay(100);
>
> Hm... registering an interrupt handler and then doing a random delay
> seems a bit odd. I'd rather expect something like
>
> (a) check for an indication that an interrupt has arrived (global
> variable)
> (b) wait for a bit
> (c) if timeout has not yet been hit: goto (a)
>
> Or do a tpi loop, if this can't be done fully asynchronous?
Currently the test is done on the io_int_parameter.
And you are right there is a problem, if no interrupt happen the test is
silently skipped
>
> Also, I don't understand what you are doing with the buffer and
> senseid: Can't you make senseid a pointer to buffer, so that it can
> simply access the fields after they have been filled by sense id?
>
> Lastly, it might make sense if the reserved field of senseid has been
> filled with 0xff; that way you can easily distinguish 'device is not a
> pong device' from 'senseid has not been filled out correctly'.
yes, thanks.
Thanks for comemnts,
Regards
Pierre
--
Pierre Morel
IBM Lab Boeblingen
next prev parent reply other threads:[~2019-12-10 9:12 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-06 16:26 [kvm-unit-tests PATCH v3 0/9] s390x: Testing the Channel Subsystem I/O Pierre Morel
2019-12-06 16:26 ` [kvm-unit-tests PATCH v3 1/9] s390x: saving regs for interrupts Pierre Morel
2019-12-06 16:26 ` [kvm-unit-tests PATCH v3 2/9] s390x: Define the PSW bits Pierre Morel
2019-12-06 16:29 ` Thomas Huth
2019-12-09 8:53 ` Pierre Morel
2019-12-06 16:26 ` [kvm-unit-tests PATCH v3 3/9] s390: interrupt registration Pierre Morel
2019-12-09 11:40 ` Thomas Huth
2019-12-09 16:44 ` Pierre Morel
2019-12-09 11:48 ` David Hildenbrand
2019-12-09 16:44 ` Pierre Morel
2019-12-06 16:26 ` [kvm-unit-tests PATCH v3 4/9] s390x: export the clock get_clock_ms() utility Pierre Morel
2019-12-09 11:42 ` Thomas Huth
2019-12-09 11:49 ` David Hildenbrand
2019-12-09 16:43 ` Pierre Morel
2019-12-09 16:44 ` Pierre Morel
2019-12-06 16:26 ` [kvm-unit-tests PATCH v3 5/9] s390x: Library resources for CSS tests Pierre Morel
2019-12-09 11:49 ` Thomas Huth
2019-12-09 16:43 ` Pierre Morel
2019-12-10 10:07 ` Pierre Morel
2019-12-10 10:28 ` Thomas Huth
2019-12-10 11:22 ` Pierre Morel
2019-12-10 11:27 ` Thomas Huth
2019-12-10 11:28 ` Pierre Morel
2019-12-06 16:26 ` [kvm-unit-tests PATCH v3 6/9] s390x: css: stsch, enumeration test Pierre Morel
2019-12-09 11:52 ` Thomas Huth
2019-12-09 16:42 ` Pierre Morel
2019-12-09 16:49 ` Cornelia Huck
2019-12-10 8:56 ` Pierre Morel
2019-12-10 11:37 ` Pierre Morel
2019-12-06 16:26 ` [kvm-unit-tests PATCH v3 7/9] s390x: css: msch, enable test Pierre Morel
2019-12-09 16:54 ` Cornelia Huck
2019-12-10 9:01 ` Pierre Morel
2019-12-10 9:09 ` Cornelia Huck
2019-12-10 17:35 ` Pierre Morel
2019-12-06 16:26 ` [kvm-unit-tests PATCH v3 8/9] s390x: css: ssch/tsch with sense and interrupt Pierre Morel
2019-12-09 17:22 ` Cornelia Huck
2019-12-10 9:12 ` Pierre Morel [this message]
2019-12-06 16:26 ` [kvm-unit-tests PATCH v3 9/9] s390x: css: ping pong Pierre Morel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=502977b7-ad73-3468-92c9-7798cff6f754@linux.ibm.com \
--to=pmorel@linux.ibm.com \
--cc=cohuck@redhat.com \
--cc=david@redhat.com \
--cc=frankja@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).