Re: [kvm-unit-tests PATCH v2 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-02 20:54, Cornelia Huck wrote:
On Mon, 2 Dec 2019 19:18:20 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:

On 2019-12-02 15:55, Cornelia Huck wrote:
On Thu, 28 Nov 2019 13:46:06 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:

+	ccw[0].code = code ;

Extra ' ' before ';'

yes, thanks

+	ccw[0].flags = CCW_F_PCI;

Huh, what's that PCI for?

Program Control Interruption

Yes; but why do you need it? Doesn't the QEMU device provide you with
an interrupt for the final status? I don't think PCI makes sense unless
you want a notification for the progress through a chain.

Right, but this is for a later test.
So I do not need it. Will remove it.



I will add a comment :)

Good idea; the PCI is bound to confuse people :)


+	ccw[0].count = count;
+	ccw[0].data = (int)(unsigned long)data;

Can you be sure that data is always below 2G?

Currently yes, the program is loaded at 0x10000 and is quite small
also doing a test does not hurt for the case the function is used in
another test someday.

Nod.

Will do.



+	orb_p->intparm = 0xcafec0ca;
+	orb_p->ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
+	orb_p->cpa = (unsigned int) (unsigned long)&ccw[0];
+
+	report_prefix_push("Start Subchannel");
+	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;
+}
+
+static void test_sense(void)
+{
+	int success;
+
+	enable_io_irq();
+
+	success = start_subchannel(CCW_CMD_SENSE_ID, buffer, sizeof(senseid));
+	if (!success) {
+		report("start_subchannel failed", 0);
+		return;
+	}
+
+	senseid.cu_type = buffer[2] | (buffer[1] << 8);
+	delay(1000);
+
+	/* Sense ID is non packed cut_type is at offset +1 byte */
+	if (senseid.cu_type == PONG_CU)
+		report("cu_type: expect c0ca, got %04x", 1, senseid.cu_type);
+	else
+		report("cu_type: expect c0ca, got %04x", 0, senseid.cu_type);
+}

I'm not really convinced by that logic here. This will fall apart if
you don't have your pong device exactly in the right place, and it does
not make it easy to extend this for more devices in the future.

Wanted to keep things simple. PONG must be the first valid channel.
also, should be documented at least.

Yes, please :)


Thanks.

Regards,
Pierre

--
Pierre Morel
IBM Lab Boeblingen




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux