Re: [kvm-unit-tests PATCH v4 9/9] s390x: css: ping pong

[Pierre Morel <pmorel@linux.ibm.com>]

On 2019-12-13 10:50, Cornelia Huck wrote:
> On Wed, 11 Dec 2019 16:46:10 +0100
> Pierre Morel <pmorel@linux.ibm.com> wrote:
> 
>> To test a write command with the SSCH instruction we need a QEMU device,
>> with control unit type 0xC0CA. The PONG device is such a device.
>>
>> This type of device responds to PONG_WRITE requests by incrementing an
>> integer, stored as a string at offset 0 of the CCW data.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   s390x/css.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/s390x/css.c b/s390x/css.c
>> index 7b9bdb1..a09cdff 100644
>> --- a/s390x/css.c
>> +++ b/s390x/css.c
>> @@ -26,6 +26,9 @@
>>   
>>   #define CSS_TEST_INT_PARAM	0xcafec0ca
>>   #define PONG_CU_TYPE		0xc0ca
>> +/* Channel Commands for PONG device */
>> +#define PONG_WRITE	0x21 /* Write */
>> +#define PONG_READ	0x22 /* Read buffer */
>>   
>>   struct lowcore *lowcore = (void *)0x0;
>>   
>> @@ -302,6 +305,48 @@ unreg_cb:
>>   	unregister_io_int_func(irq_io);
>>   }
>>   
>> +static void test_ping(void)
>> +{
>> +	int success, result;
>> +	int cnt = 0, max = 4;
>> +
>> +	if (senseid.cu_type != PONG_CU) {
>> +		report_skip("No PONG, no ping-pong");
>> +		return;
>> +	}
>> +
>> +	result = register_io_int_func(irq_io);
>> +	if (result) {
>> +		report(0, "Could not register IRQ handler");
>> +		return;
>> +	}
>> +
>> +	while (cnt++ < max) {
>> +		snprintf(buffer, BUF_SZ, "%08x\n", cnt);
>> +		success = start_subchannel(PONG_WRITE, buffer, 8);
> 
> Magic value? Maybe introduce a #define for the lengths of the
> reads/writes?

OK, and I also do not need a buffer so big.

> 
> [This also got me thinking about your start_subchannel function
> again... do you also want to allow flags like e.g. SLI? It's not
> unusual for commands to return different lengths of data depending on
> what features are available; it might be worthwhile to allow short data
> if you're not sure that e.g. a command returns either the short or the
> long version of a structure.]

I would prefer to keep simple it in this series if you agree.

AFAIU the current QEMU implementation use a fix length and if a short 
read occurs it is an error.
Since we test on PONG, there should be no error.

I agree that for a general test we should change this, but currently the 
goal is just to verify that the remote device is PONG.

If we accept variable length, we need to check the length of what we 
received, and this could need some infrastructure changes that I would 
like to do later.

When the series is accepted I will begin to do more complicated things like:
- checking the exceptions for wrong parameters
   This is the first I will add.
- checking the response difference on flags (SLI, SKP)
- using CC and CD flags for chaining
- TIC, NOP, suspend/resume and PCI

These last one will be fun, we can also trying to play with prefetch 
while at it. :)

Regards,
Pierre


-- 
Pierre Morel
IBM Lab Boeblingen