Re: [kvm-unit-tests PATCH v6 09/10] s390x: css: ssch/tsch with sense and interrupt

Cornelia Huck
2020-05-14T12:24:23Z

On Fri, 24 Apr 2020 12:45:51 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:

> We add a new css_lib file to contain the I/O function we may

s/function/functions/

> share with different tests.
> First function is the subchannel_enable() function.
>
> When a channel is enabled we can 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 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/asm/arch_def.h | 1 +
> lib/s390x/css.h | 20 ++++++
> lib/s390x/css_lib.c | 55 +++++++++++++++
> s390x/Makefile | 1 +
> s390x/css.c | 149 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 226 insertions(+)
> create mode 100644 lib/s390x/css_lib.c
>

(...)

> +static void irq_io(void)
> +{
> + int ret = 0;
> + char *flags;
> + int sid;
> +
> + report_prefix_push("Interrupt");
> + /* Lowlevel set the SID as interrupt parameter. */
> + if (lowcore->io_int_param != test_device_sid) {
> + report(0,
> + "Bad io_int_param: %x expected %x",
> + lowcore->io_int_param, test_device_sid);
> + goto pop;
> + }
> + report_prefix_pop();
> +
> + report_prefix_push("tsch");
> + sid = lowcore->subsys_id_word;
> + ret = tsch(sid, &irb);
> + switch (ret) {
> + case 1:
> + dump_irb(&irb);
> + flags = dump_scsw_flags(irb.scsw.ctrl);
> + report(0,
> + "I/O interrupt, but sch not status pending: %s", flags);

"...but tsch reporting sch as not status pending" ?

A buggy implementation might give the wrong cc for tsch, but still
indicate status pending in the control block.

> + break;
> + case 2:
> + report(0, "TSCH returns unexpected CC 2");
> + break;
> + case 3:
> + report(0, "Subchannel %08x not operational", sid);

"tsch reporting subchannel %08x as not operational" ?

> + break;
> + case 0:
> + /* Stay humble on success */
> + break;
> + }
> +pop:
> + report_prefix_pop();
> + lowcore->io_old_psw.mask &= ~PSW_MASK_WAIT;
> +}

(...)

> +/*
> + * 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;
> +
> + if (!test_device_sid) {
> + report_skip("No device");
> + return;
> + }
> +
> + ret = enable_subchannel(test_device_sid);
> + if (ret < 0) {
> + report(0,
> + "Could not enable the subchannel: %08x",
> + test_device_sid);
> + return;
> + }
> +
> + ret = register_io_int_func(irq_io);
> + if (ret) {
> + report(0, "Could not register IRQ handler");
> + goto unreg_cb;
> + }
> +
> + lowcore->io_int_param = 0;
> +
> + ret = start_subchannel(CCW_CMD_SENSE_ID, &senseid, sizeof(senseid));

You're always send the full (extended) sense id block. What if the the
machine you're running on doesn't support extended sense id? Would the
SLI ccw flag help?

> + if (!ret) {
> + report(0, "start_senseid failed");

"ssch failed for SENSE ID on sch <sch>" ?

> + goto unreg_cb;
> + }
> +
> + wfi(PSW_MASK_IO);
> +
> + if (lowcore->io_int_param != test_device_sid) {
> + report(0,
> + "No interrupts. io_int_param: expect 0x%08x, got 0x%08x",
> + test_device_sid, lowcore->io_int_param);

Doesn't irq_io() already moan here?

> + goto unreg_cb;
> + }
> +
> + report_info("reserved %02x cu_type %04x cu_model %02x dev_type %04x dev_model %02x",
> + senseid.reserved, senseid.cu_type, senseid.cu_model,
> + senseid.dev_type, senseid.dev_model);
> +
> + if (senseid.cu_type == PONG_CU)
> + report(1, "cu_type: expect 0x%04x got 0x%04x",
> + PONG_CU_TYPE, senseid.cu_type);
> + else
> + report(0, "cu_type: expect 0x%04x got 0x%04x",
> + PONG_CU_TYPE, senseid.cu_type);
> +
> +unreg_cb:
> + unregister_io_int_func(irq_io);
> +}
> +