[Skiboot] [PATCH v2 3/5] hw/psi-p9: Mask OPAL-owned LSIs without handlers

[Cédric Le Goater]

On 05/09/2019 05:08, Oliver O'Halloran wrote:
> Some versions of Swift have the TPM interrupt line of the second chip
> pulled up instead of down. This causes the PSI's external (TPM) interrupt
> to constantly re-fire since it's an LSI and the interrupt signal is
> constantly active. There's nothing that can be done to clear the underlying
> interrupt condition so we to ensure that it's masked.
> 
> The problem isn't really specific to the external interrupt and will
> occur for any of the PSI interrupts that don't have an actual handler
> (FSP, global error, and sometimes the external). When one of these is
> delivered to OPAL we should log that it happened and mask it to prevent
> re-firing.
> 
> Cc: Cédric Le Goater <clg at kaod.org>
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
> ---
> v2: Use __xive_source_mask() to mask the interrupt rather than
>     opencoding the ESB manipulation in the PSI driver.

We can get rid of xive_source_mask() and rename it to __xive_source_mask().

I would remove the test on platform.external_irq as it is not used on P9 
and it adds extra noise in the code. 

Anyhow,

Reviewed-by: Cédric Le Goater <clg at kaod.org>

Thanks,

C.

> 
>     look up the interrupt name from the name array rather than
>     using the name callback.
> ---
>  hw/psi.c | 35 +++++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/psi.c b/hw/psi.c
> index 99ec06ac488a..6c28eb0447ad 100644
> --- a/hw/psi.c
> +++ b/hw/psi.c
> @@ -536,6 +536,29 @@ static char *psi_p9_irq_name(struct irq_source *is, uint32_t isn)
>  	return strdup(p9_psi_int_names[idx]);
>  }
>  
> +static void psi_p9_mask_unhandled_irq(struct irq_source *is, uint32_t isn)
> +{
> +	int idx = isn - psi->interrupt;
> +	struct psi *psi = is->data;
> +	const char *name;
> +
> +	if (idx < ARRAY_SIZE(p9_psi_int_names))
> +		name = p9_psi_int_names[idx];
> +	else
> +		name = "unknown irq";
> +
> +	prerror("PSI: Masking unhandled LSI %s (idx: %d)!\n", name, idx);
> +
> +	/*
> +	 * All the PSI interrupts are LSIs and will be constantly re-fired
> +	 * unless the underlying interrupt condition is cleared. If we don't
> +	 * have a handler for the interrupt then it needs to be masked to
> +	 * prevent the IRQ from locking up the thread which handles it.
> +	 */
> +	__xive_source_mask(is, isn);
> +
> +}
> +
>  static void psihb_p9_interrupt(struct irq_source *is, uint32_t isn)
>  {
>  	struct psi *psi = is->data;
> @@ -548,21 +571,17 @@ static void psihb_p9_interrupt(struct irq_source *is, uint32_t isn)
>  	case P9_PSI_IRQ_OCC:
>  		occ_p9_interrupt(psi->chip_id);
>  		break;
> -	case P9_PSI_IRQ_FSI:
> -		printf("PSI: FSI irq received\n");
> -		break;
>  	case P9_PSI_IRQ_LPCHC:
>  		lpc_interrupt(psi->chip_id);
>  		break;
>  	case P9_PSI_IRQ_LOCAL_ERR:
>  		prd_psi_interrupt(psi->chip_id);
>  		break;
> -	case P9_PSI_IRQ_GLOBAL_ERR:
> -		printf("PSI: Global error irq received\n");
> -		break;
>  	case P9_PSI_IRQ_EXTERNAL:
>  		if (platform.external_irq)
>  			platform.external_irq(psi->chip_id);
> +		else
> +			psi_p9_mask_unhandled_irq(is, isn);
>  		break;
>  	case P9_PSI_IRQ_LPC_SIRQ0:
>  	case P9_PSI_IRQ_LPC_SIRQ1:
> @@ -580,6 +599,9 @@ static void psihb_p9_interrupt(struct irq_source *is, uint32_t isn)
>  	case P9_PSI_IRQ_PSU:
>  		p9_sbe_interrupt(psi->chip_id);
>  		break;
> +
> +	default:
> +		psi_p9_mask_unhandled_irq(is, isn);
>  	}
>  }
>  
> @@ -614,6 +636,7 @@ static const struct irq_source_ops psi_p9_irq_ops = {
>  	.interrupt = psihb_p9_interrupt,
>  	.attributes = psi_p9_irq_attributes,
>  	.name = psi_p9_irq_name,
> +	.mask = psi_p9_mask_unhandled_irq,
>  };
>  
>  void psi_set_external_irq_policy(bool policy)
>