Re: [kvm-unit-tests PATCH v4 3/3] lib: s390x: better smp interrupt checks

[Janosch Frank <frankja@linux.ibm.com>]

On 7/13/22 19:40, Claudio Imbrenda wrote:
> Use per-CPU flags and callbacks for Program and Extern interrupts,
> instead of global variables.
> 
> This allows for more accurate error handling; a CPU waiting for an
> interrupt will not have it "stolen" by a different CPU that was not
> supposed to wait for one, and now two CPUs can wait for interrupts at
> the same time.
> 
> This will significantly improve error reporting and debugging when
> things go wrong.
> 
> Both program interrupts and external interrupts are now CPU-bound, even
> though some external interrupts are floating (notably, the SCLP
> interrupt). In those cases, the testcases should mask interrupts and/or
> expect them appropriately according to need.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Small nit below

> ---
>   lib/s390x/asm/arch_def.h  | 17 ++++++++-
>   lib/s390x/asm/interrupt.h |  3 +-
>   lib/s390x/smp.h           |  8 +---
>   lib/s390x/interrupt.c     | 77 +++++++++++++++++++++++++++++++--------
>   lib/s390x/smp.c           | 11 ++++++
>   s390x/skrf.c              |  2 +-
>   6 files changed, 92 insertions(+), 26 deletions(-)
> 
> diff --git a/lib/s390x/asm/arch_def.h b/lib/s390x/asm/arch_def.h
> index 358ef82e..6e664f62 100644
> --- a/lib/s390x/asm/arch_def.h
> +++ b/lib/s390x/asm/arch_def.h
> @@ -41,6 +41,18 @@ struct psw {
>   	uint64_t	addr;
>   };
>   
> +struct cpu {
> +	struct lowcore *lowcore;
> +	uint64_t *stack;
> +	void (*pgm_cleanup_func)(struct stack_frame_int *);
> +	void (*ext_cleanup_func)(struct stack_frame_int *);
> +	uint16_t addr;
> +	uint16_t idx;
> +	bool active;
> +	bool pgm_int_expected;
> +	bool ext_int_expected;
> +};
> +
>   #define AS_PRIM				0
>   #define AS_ACCR				1
>   #define AS_SECN				2
> @@ -125,7 +137,8 @@ struct lowcore {
>   	uint8_t		pad_0x0280[0x0308 - 0x0280];	/* 0x0280 */
>   	uint64_t	sw_int_crs[16];			/* 0x0308 */
>   	struct psw	sw_int_psw;			/* 0x0388 */
> -	uint8_t		pad_0x0310[0x11b0 - 0x0398];	/* 0x0398 */
> +	struct cpu *	this_cpu;			/* 0x0398 */

Move the * to the variable name

> +	uint8_t		pad_0x03a0[0x11b0 - 0x03a0];	/* 0x03a0 */
>   	uint64_t	mcck_ext_sa_addr;		/* 0x11b0 */
>   	uint8_t		pad_0x11b8[0x1200 - 0x11b8];	/* 0x11b8 */
>   	uint64_t	fprs_sa[16];			/* 0x1200 */
> @@ -148,6 +161,8 @@ _Static_assert(sizeof(struct lowcore) == 0x1900, "Lowcore size");