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

[Thomas Huth <thuth@redhat.com>]

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.

[...]
> +
> +	report((senseid.cu_type == cu_type),

Please drop the innermost parentheses here.

> +	       "cu_type: expect 0x%04x got 0x%04x",
> +	       (uint16_t) cu_type, senseid.cu_type);
> +
> +unreg_cb:
> +	unregister_io_int_func(irq_io);
> +}

 Thomas