Subject: Re: [PATCH 07/31] powerpc/xive: Fix xive_irq_set_affinity for MSI

From: Thomas Gleixner <[email-masked]>
Date: Fri, 14 May 2021 22:48:35 +0200

On Fri, Apr 30 2021 at 10:03, Cédric Le Goater wrote:
> The MSI affinity is automanaged and it can be set before starting the
> associated IRQ.
>
> ( Should we simply remove the irqd_is_started() test ? )

If the hardware can handle it properly.

But see:

  cffb717ceb8e ("powerpc/xive: Ensure active irqd when setting affinity")

which introduced that condition. It mutters something about migration of
shutdown interrupts:

       [  123.053037264,3] XIVE[ IC 00  ] ISN 2 lead to invalid IVE !
       [   77.885859] xive: Error -6 reconfiguring irq 17
       [   77.885862] IRQ17: set affinity failed(-6).

Not that I can decode that :)

Non-managed interrupts have the sequence:

      startup()
      set_affinity()

which is historical and an earlier attempt to flip it caused havoc in
some places.

With managed we needed to make sure that the affinity is set correctly
right at start. So it needs to be done the other way round and it turned
out that for MSI this works.

I have no idea, whether that might make the above issue reappear or
not. If so, then we need some extra state to make it work.

The root cause which triggered the problem got fixed, so there should be
no issue _if_ this was specifically related to that CPU unplug case.

> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 96737938e8e3..3485baf9ec8c 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -710,7 +710,7 @@ static int xive_irq_set_affinity(struct irq_data *d,
>  		return -EINVAL;
>  
>  	/* Don't do anything if the interrupt isn't started */
> -	if (!irqd_is_started(d))
> +	if (!irqd_is_started(d) && !irqd_affinity_is_managed(d))
>  		return IRQ_SET_MASK_OK;
>  
>  	/*

Thanks,

        tglx