[Skiboot] [PATCH] xive: Fix potential for lost IPIs when manipulating CPPR

[Michael Neuling]

This is upstream as of 50e1921f98


On Fri, 2017-04-21 at 08:59 +1000, Benjamin Herrenschmidt wrote:
> We don't trigger the IPI in set_mfrr() if the CPPR of the
> destination is more favored than the MFRR. However, we fail
> to re-evaluate it when the CPPR later changes.
> 
> This fixes it. The way this is implemented can lead to
> spurious IPIs but these are harmless.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> ---
>  hw/xive.c | 35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/xive.c b/hw/xive.c
> index 1e2648c..eb75f5f 100644
> --- a/hw/xive.c
> +++ b/hw/xive.c
> @@ -3066,6 +3066,24 @@ static inline uint8_t opal_xive_check_pending(struct
> xive_cpu_state *xs,
>  	return xs->pending & mask;
>  }
>  
> +static void opal_xive_update_cppr(struct xive_cpu_state *xs, u8 cppr)
> +{
> +	/* Peform the update */
> +	xs->cppr = cppr;
> +	out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr);
> +
> +	/* Trigger the IPI if it's still more favored than the CPPR
> +	 *
> +	 * This can lead to a bunch of spurrious retriggers if the
> +	 * IPI is queued up behind other interrupts but that's not
> +	 * a big deal and keeps the code simpler
> +	 */
> +	if (xs->mfrr < cppr) {
> +		xive_ipi_trigger(xs->xive, GIRQ_TO_IDX(xs->ipi_irq));
> +		xs->ipi_sent = true;
> +	}
> +}
> +
>  static int64_t opal_xive_eoi(uint32_t xirr)
>  {
>  	struct cpu_thread *c = this_cpu();
> @@ -3150,13 +3168,6 @@ static int64_t opal_xive_eoi(uint32_t xirr)
>  		/* Is it an IPI ? */
>  		if (special_ipi) {
>  			xive_ipi_eoi(src_x, idx);
> -
> -			/* Check mfrr and eventually re-trigger. We check
> -			 * against the new CPPR since we are about to update
> -			 * the HW.
> -			 */
> -			if (xs->mfrr < cppr)
> -				xive_ipi_trigger(src_x, idx);
>  		} else {
>  			/* Otherwise go through the source mechanism */
>  			xive_vdbg(src_x, "EOI of IDX %x in EXT range\n",
> idx);
> @@ -3167,8 +3178,7 @@ static int64_t opal_xive_eoi(uint32_t xirr)
>  	}
>  
>  	/* Finally restore CPPR */
> -	xs->cppr = cppr;
> -	out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr);
> +	opal_xive_update_cppr(xs, cppr);
>  
>  	xive_cpu_vdbg(c, "  pending=0x%x cppr=%d\n", xs->pending, cppr);
>  
> @@ -3297,8 +3307,7 @@ static int64_t opal_xive_get_xirr(uint32_t *out_xirr,
> bool just_poll)
>  
>  	} else {
>  		/* Nothing was active, this is a fluke, restore CPPR */
> -		xs->cppr = old_cppr;
> -		out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, old_cppr);
> +		opal_xive_update_cppr(xs, old_cppr);
>  		xive_cpu_vdbg(c, "  nothing active, restored CPPR to %d\n",
>  			      old_cppr);
>  	}
> @@ -3328,9 +3337,7 @@ static int64_t opal_xive_set_cppr(uint8_t cppr)
>  	xive_cpu_vdbg(c, "CPPR setting to %d\n", cppr);
>  
>  	lock(&xs->lock);
> -	c->xstate->cppr = cppr;
> -	out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr);
> -
> +	opal_xive_update_cppr(xs, cppr);
>  	unlock(&xs->lock);
>  
>  	return OPAL_SUCCESS;
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot