Re: [kvm-unit-tests PATCH v2 5/8] s390x: lib: css: add SCSW ctrl expectations to check I/O completion

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Thu, 25 Mar 2021 10:39:04 +0100
Pierre Morel <pmorel@linux.ibm.com> wrote:

> When checking for an I/O completion may need to check the cause of
> the interrupt depending on the test case.
> 
> Let's provide the tests the possibility to check if the last
> valid IRQ received is for the function expected after executing
> an instruction or sequence of instructions and if all ctrl flags
> of the SCSW are set as expected.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  lib/s390x/css.h     |  4 ++--
>  lib/s390x/css_lib.c | 21 ++++++++++++++++-----
>  s390x/css.c         |  4 ++--
>  3 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
> index 5d1e1f0..1603781 100644
> --- a/lib/s390x/css.h
> +++ b/lib/s390x/css.h
> @@ -316,8 +316,8 @@ void css_irq_io(void);
>  int css_residual_count(unsigned int schid);
>  
>  void enable_io_isc(uint8_t isc);
> -int wait_and_check_io_completion(int schid);
> -int check_io_completion(int schid);
> +int wait_and_check_io_completion(int schid, uint32_t ctrl);
> +int check_io_completion(int schid, uint32_t ctrl);
>  
>  /*
>   * CHSC definitions
> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
> index 1e5c409..55e70e6 100644
> --- a/lib/s390x/css_lib.c
> +++ b/lib/s390x/css_lib.c
> @@ -488,21 +488,25 @@ struct ccw1 *ccw_alloc(int code, void *data,
> int count, unsigned char flags) 
>  /* wait_and_check_io_completion:
>   * @schid: the subchannel ID
> + * @ctrl : expected SCSW control flags
>   */
> -int wait_and_check_io_completion(int schid)
> +int wait_and_check_io_completion(int schid, uint32_t ctrl)
>  {
>  	wait_for_interrupt(PSW_MASK_IO);
> -	return check_io_completion(schid);
> +	return check_io_completion(schid, ctrl);
>  }
>  
>  /* check_io_completion:
>   * @schid: the subchannel ID
> + * @ctrl : expected SCSW control flags
>   *
> - * Makes the most common check to validate a successful I/O
> - * completion.
> + * If the ctrl parameter is not null check the IRB SCSW ctrl

I would say "not zero" instead of null, since it's an integer and not a
pointer.

with that fixed:

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> + * against the ctrl parameter.
> + * Otherwise, makes the most common check to validate a successful
> + * I/O completion.
>   * Only report failures.
>   */
> -int check_io_completion(int schid)
> +int check_io_completion(int schid, uint32_t ctrl)
>  {
>  	int ret = 0;
>  
> @@ -515,6 +519,13 @@ int check_io_completion(int schid)
>  		goto end;
>  	}
>  
> +	if (ctrl && (ctrl ^ irb.scsw.ctrl)) {
> +		report_info("%08x : %s", schid,
> dump_scsw_flags(irb.scsw.ctrl));
> +		report_info("expected : %s", dump_scsw_flags(ctrl));
> +		ret = -1;
> +		goto end;
> +	}
> +
>  	/* Verify that device status is valid */
>  	if (!(irb.scsw.ctrl & SCSW_SC_PENDING)) {
>  		report(0, "No status pending after interrupt. Subch
> Ctrl: %08x", diff --git a/s390x/css.c b/s390x/css.c
> index 16723f6..57dc340 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -95,7 +95,7 @@ static void test_sense(void)
>  		goto error;
>  	}
>  
> -	if (wait_and_check_io_completion(test_device_sid) < 0)
> +	if (wait_and_check_io_completion(test_device_sid, 0) < 0)
>  		goto error;
>  
>  	/* Test transfer completion */
> @@ -138,7 +138,7 @@ static void sense_id(void)
>  {
>  	assert(!start_ccw1_chain(test_device_sid, ccw));
>  
> -	assert(wait_and_check_io_completion(test_device_sid) >= 0);
> +	assert(wait_and_check_io_completion(test_device_sid, 0) >=
> 0); }
>  
>  static void css_init(void)