Re: [kvm-unit-tests PATCH v8 12/12] s390x: css: ssch/tsch with sense and interrupt

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

On 2020-06-09 10:15, Thomas Huth wrote:
> On 08/06/2020 10.13, Pierre Morel wrote:
>> After a channel is enabled we 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 SENSE_ID command response is tested to report 0xff inside
>> its reserved field and to report the same control unit type
>> as the cu_type kernel argument.
>>
>> Without the cu_type kernel argument, the test expects a device
>> with a default control unit type of 0x3832, a.k.a virtio-net-ccw.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   lib/s390x/css.h     |  20 ++++++
>>   lib/s390x/css_lib.c |  46 ++++++++++++++
>>   s390x/css.c         | 149 +++++++++++++++++++++++++++++++++++++++++++-
>>   3 files changed, 214 insertions(+), 1 deletion(-)
> [...]
>> +}
>> diff --git a/s390x/css.c b/s390x/css.c
>> index 6f58d4a..79c997d 100644
>> --- a/s390x/css.c
>> +++ b/s390x/css.c
>> @@ -16,10 +16,26 @@
>>   #include <string.h>
>>   #include <interrupt.h>
>>   #include <asm/arch_def.h>
>> +#include <kernel-args.h>
>>   
>>   #include <css.h>
>>   
>> +#define DEFAULT_CU_TYPE		0x3832
>> +static unsigned long cu_type = DEFAULT_CU_TYPE;
>> +
>> +struct lowcore *lowcore = (void *)0x0;
>> +
>>   static int test_device_sid;
>> +static struct irb irb;
>> +static struct senseid senseid;
>> +
>> +static void set_io_irq_subclass_mask(uint64_t const new_mask)
>> +{
>> +	asm volatile (
>> +		"lctlg %%c6, %%c6, %[source]\n"
>> +		: /* No outputs */
>> +		: [source] "R" (new_mask));
> 
> I think the "R" constraint is wrong here - this instruction does not use
> an index register. "Q" is likely the better choice. But it might be
> easier to use the lctlg() wrapper from lib/s390x/asm/arch_def.h instead.

OK, yes I do this.

> 
> [...]
>> +
>> +	report((senseid.cu_type == cu_type),
> 
> Please drop the innermost parentheses here.
> 

OK,
Thanks,

Pierre


-- 
Pierre Morel
IBM Lab Boeblingen