Re: [kvm-unit-tests PATCH 4/7] s390x: css: registering IRQ

[Thomas Huth <thuth@redhat.com>]

On 27/08/2021 12.17, Pierre Morel wrote:
> Registering IRQ for the CSS level.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   lib/s390x/css.h     | 21 +++++++++++++++++++++
>   lib/s390x/css_lib.c | 27 +++++++++++++++++++++++++--
>   2 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
> index 2005f4d7..0422f2e7 100644
> --- a/lib/s390x/css.h
> +++ b/lib/s390x/css.h
> @@ -402,4 +402,25 @@ struct measurement_block_format1 {
>   	uint32_t irq_prio_delay_time;
>   };
>   
> +#include <asm/arch_def.h>
> +static inline void disable_io_irq(void)
> +{
> +	uint64_t mask;
> +
> +	mask = extract_psw_mask();
> +	mask &= ~PSW_MASK_IO;
> +	load_psw_mask(mask);
> +}
> +
> +static inline void enable_io_irq(void)
> +{
> +	uint64_t mask;
> +
> +	mask = extract_psw_mask();
> +	mask |= PSW_MASK_IO;
> +	load_psw_mask(mask);
> +}
> +
> +int register_css_irq_func(void (*f)(void));
> +int unregister_css_irq_func(void (*f)(void));
>   #endif
> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
> index 484f9c41..a89fc93c 100644
> --- a/lib/s390x/css_lib.c
> +++ b/lib/s390x/css_lib.c
> @@ -350,8 +350,29 @@ bool css_disable_mb(int schid)
>   	return retry_count > 0;
>   }
>   
> -static struct irb irb;
> +static void (*css_irq_func)(void);
> +
> +int register_css_irq_func(void (*f)(void))
> +{
> +	if (css_irq_func)
> +		return -1;
> +	css_irq_func = f;
> +	assert(register_io_int_func(css_irq_io) == 0);

It's unlikely that we ever disable assert() in the k-u-t, but anyway, I'd 
prefer not to put function calls within assert() statements, just in case. 
So could you please replace this with:

	rc = register_io_int_func(css_irq_io);
	assert(rc == 0);

?

> +	enable_io_isc(0x80 >> IO_SCH_ISC);
> +	return 0;
> +}
>   
> +int unregister_css_irq_func(void (*f)(void))
> +{
> +	if (css_irq_func != f)
> +		return -1;
> +	enable_io_isc(0);
> +	unregister_io_int_func(css_irq_io);
> +	css_irq_func = NULL;
> +	return 0;
> +}
> +
> +static struct irb irb;
>   void css_irq_io(void)
>   {
>   	int ret = 0;
> @@ -386,7 +407,9 @@ void css_irq_io(void)
>   		report(0, "tsch reporting sch %08x as not operational", sid);
>   		break;
>   	case 0:
> -		/* Stay humble on success */
> +		/* Call upper level IRQ routine */
> +		if (css_irq_func)
> +			css_irq_func();
>   		break;
>   	}
>   pop:
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>