Re: [kvm-unit-tests PATCH v1 3/6] s390x: lib: css: upgrading IRQ handling

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

hum, I just saw I forgot to upgrade the report of the residual count.
This is not really relevant for this series.
Of course I will adapt it for the v2.


On 3/18/21 2:26 PM, Pierre Morel wrote:
> Until now we had very few usage of interrupts, to be able to handle
> several interrupts coming up asynchronously we need to take care
> to save the previous interrupt before handling the next one.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   lib/s390x/css.h     |  29 +++++++++++
>   lib/s390x/css_lib.c | 117 ++++++++++++++++++++++++++++++++++----------
>   2 files changed, 120 insertions(+), 26 deletions(-)
> 
> diff --git a/lib/s390x/css.h b/lib/s390x/css.h
> index 460b0bd..65fc335 100644
> --- a/lib/s390x/css.h
> +++ b/lib/s390x/css.h
> @@ -425,4 +425,33 @@ struct measurement_block_format1 {
>   	uint32_t irq_prio_delay_time;
>   };
>   
> +struct irq_entry {
> +	struct irq_entry *next;
> +	struct irb irb;
> +	uint32_t sid;
> +};
> +
> +struct irq_entry *alloc_irq(void);
> +struct irq_entry *get_irq(void);
> +void put_irq(struct irq_entry *irq);
> +
> +#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);
> +}
> +
>   #endif
> diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
> index f8db205..211c73c 100644
> --- a/lib/s390x/css_lib.c
> +++ b/lib/s390x/css_lib.c
> @@ -9,6 +9,8 @@
>    */
>   #include <libcflat.h>
>   #include <alloc_phys.h>
> +#include <util.h>
> +#include <alloc.h>
>   #include <asm/page.h>
>   #include <string.h>
>   #include <interrupt.h>
> @@ -22,6 +24,46 @@
>   struct schib schib;
>   struct chsc_scsc *chsc_scsc;
>   
> +static struct irq_entry *irqs;
> +
> +struct irq_entry *get_irq(void)
> +{
> +	struct irq_entry *irq = NULL;
> +
> +	if (irqs) {
> +		irq = irqs;
> +		irqs = irq->next;
> +	}
> +	return irq;
> +}
> +
> +void put_irq(struct irq_entry *irq)
> +{
> +	free(irq);
> +}
> +
> +static void save_irq(struct irq_entry *irq)
> +{
> +	struct irq_entry *e;
> +
> +	if (!irqs) {
> +		irqs = irq;
> +	} else {
> +		e = irqs;
> +		while (e && e->next)
> +			e = e->next;
> +		e->next = irq;
> +	}
> +}
> +
> +struct irq_entry *alloc_irq(void)
> +{
> +	struct irq_entry *irq;
> +
> +	irq = calloc(1, sizeof(*irq));
> +	return irq;
> +}
> +
>   static const char * const chsc_rsp_description[] = {
>   	"CHSC unknown error",
>   	"Command executed",
> @@ -422,38 +464,38 @@ static struct irb irb;
>   void css_irq_io(void)
>   {
>   	int ret = 0;
> -	char *flags;
> -	int sid;
> +	struct irq_entry *irq;
>   
>   	report_prefix_push("Interrupt");
> -	sid = lowcore_ptr->subsys_id_word;
> +	irq = alloc_irq();
> +	assert(irq);
> +
> +	irq->sid = lowcore_ptr->subsys_id_word;
>   	/* Lowlevel set the SID as interrupt parameter. */
> -	if (lowcore_ptr->io_int_param != sid) {
> +	if (lowcore_ptr->io_int_param != irq->sid) {
>   		report(0,
>   		       "io_int_param: %x differs from subsys_id_word: %x",
> -		       lowcore_ptr->io_int_param, sid);
> +		       lowcore_ptr->io_int_param, irq->sid);
>   		goto pop;
>   	}
>   	report_prefix_pop();
>   
>   	report_prefix_push("tsch");
> -	ret = tsch(sid, &irb);
> +	ret = tsch(irq->sid, &irq->irb);
>   	switch (ret) {
>   	case 1:
> -		dump_irb(&irb);
> -		flags = dump_scsw_flags(irb.scsw.ctrl);
> -		report(0,
> -		       "I/O interrupt, but tsch returns CC 1 for subchannel %08x. SCSW flags: %s",
> -		       sid, flags);
> +		report_info("no status pending on %08x : %s", irq->sid,
> +			    dump_scsw_flags(irq->irb.scsw.ctrl));
>   		break;
>   	case 2:
>   		report(0, "tsch returns unexpected CC 2");
>   		break;
>   	case 3:
> -		report(0, "tsch reporting sch %08x as not operational", sid);
> +		report(0, "tsch reporting sch %08x as not operational", irq->sid);
>   		break;
>   	case 0:
>   		/* Stay humble on success */
> +		save_irq(irq);
>   		break;
>   	}
>   pop:
> @@ -498,47 +540,70 @@ struct ccw1 *ccw_alloc(int code, void *data, int count, unsigned char flags)
>   int wait_and_check_io_completion(int schid)
>   {
>   	int ret = 0;
> -
> -	wait_for_interrupt(PSW_MASK_IO);
> +	struct irq_entry *irq = NULL;
>   
>   	report_prefix_push("check I/O completion");
>   
> -	if (lowcore_ptr->io_int_param != schid) {
> +	disable_io_irq();
> +	irq = get_irq();
> +	while (!irq) {
> +		wait_for_interrupt(PSW_MASK_IO);
> +		disable_io_irq();
> +		irq = get_irq();
> +		report_info("next try");
> +	}
> +	enable_io_irq();
> +
> +	assert(irq);
> +
> +	if (irq->sid != schid) {
>   		report(0, "interrupt parameter: expected %08x got %08x",
> -		       schid, lowcore_ptr->io_int_param);
> +		       schid, irq->sid);
>   		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",
> -		       irb.scsw.ctrl);
> -		ret = -1;
> +	if (!(irq->irb.scsw.ctrl & SCSW_SC_PENDING)) {
> +		ret = 0;
>   		goto end;
>   	}
>   
> -	if (!(irb.scsw.ctrl & (SCSW_SC_SECONDARY | SCSW_SC_PRIMARY))) {
> +	/* clear and halt pending are valid even without secondary or primary status */
> +	if (irq->irb.scsw.ctrl & (SCSW_FC_CLEAR | SCSW_FC_HALT)) {
> +		ret = 0;
> +		goto end;
> +	}
> +
> +	/* For start pending we need at least one of primary or secondary status */
> +	if (!(irq->irb.scsw.ctrl & (SCSW_SC_SECONDARY | SCSW_SC_PRIMARY))) {
>   		report(0, "Primary or secondary status missing. Subch Ctrl: %08x",
> -		       irb.scsw.ctrl);
> +		       irq->irb.scsw.ctrl);
>   		ret = -1;
>   		goto end;
>   	}
>   
> -	if (!(irb.scsw.dev_stat & (SCSW_DEVS_DEV_END | SCSW_DEVS_SCH_END))) {
> +	/* For start pending we also need to have device or channel end information */
> +	if (!(irq->irb.scsw.dev_stat & (SCSW_DEVS_DEV_END | SCSW_DEVS_SCH_END))) {
>   		report(0, "No device end or sch end. Dev. status: %02x",
> -		       irb.scsw.dev_stat);
> +		       irq->irb.scsw.dev_stat);
>   		ret = -1;
>   		goto end;
>   	}
>   
> -	if (irb.scsw.sch_stat & ~SCSW_SCHS_IL) {
> -		report_info("Unexpected Subch. status %02x", irb.scsw.sch_stat);
> +	/* We only accept the SubCHannel Status for Illegal Length */
> +	if (irq->irb.scsw.sch_stat & ~SCSW_SCHS_IL) {
> +		report_info("Unexpected Subch. status %02x",
> +			    irq->irb.scsw.sch_stat);
>   		ret = -1;
>   		goto end;
>   	}
>   
>   end:
> +	if (ret)
> +		dump_irb(&irq->irb);
> +
> +	put_irq(irq);
>   	report_prefix_pop();
>   	return ret;
>   }
> 

-- 
Pierre Morel
IBM Lab Boeblingen