Re: [kvm-unit-tests PATCH v3 8/9] s390x: css: ssch/tsch with sense and interrupt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2019-12-09 18:22, Cornelia Huck wrote:
On Fri,  6 Dec 2019 17:26:27 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
---
  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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux