[PATCH] irqchip/gic-v4.1: Disallow setting affinity for virtual SGIs

[PATCH] irqchip/gic-v4.1: Disallow setting affinity for virtual SGIs

[Zenghui Yu]

Running a guest on the GICv4.1-implemented board, we will get the
following warning:

[   59.062120] genirq: irq_chip GICv4.1-sgi did not update eff. affinity mask of irq 46

It may be caused by irqbalance (or other userspace tools) which tries to
change the affinity of virtual SGIs on the host. One way to "fix" it is
to update the effective_affinity value in irq_set_affinity callback. But
as the comment above says, "There is no notion of affinity for virtual
SGIs, at least not on the host", doing so only makes things confusing.

Given the vSGIs are private to the specified vPE, changing the affinity
on host is actually meaningless and achieves nothing. Let's just forbid
it.

Reported-by: Nianyao Tang <tangnianyao@huawei.com>
Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---

Hi Marc,

This just restores the behavior of your v5 [*]. I wonder that what's the
reason to change it to 'return IRQ_SET_MASK_OK' in v6? What I've missed
here?

[*] https://lore.kernel.org/kvm/20200304203330.4967-9-maz@kernel.org/

Thanks.

 drivers/irqchip/irq-gic-v3-its.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 54d142ccc63a..101c3e52c769 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4033,9 +4033,8 @@ static int its_sgi_set_affinity(struct irq_data *d,
 	/*
 	 * There is no notion of affinity for virtual SGIs, at least
 	 * not on the host (since they can only be targetting a vPE).
-	 * Tell the kernel we've done whatever it asked for.
 	 */
-	return IRQ_SET_MASK_OK;
+	return -EINVAL;
 }
 
 static int its_sgi_set_irqchip_state(struct irq_data *d,
-- 
2.19.1

Re: [PATCH] irqchip/gic-v4.1: Disallow setting affinity for virtual SGIs

[Marc Zyngier]

Hi Zenghui,

On Sat, 11 Apr 2020 10:10:32 +0100,
Zenghui Yu <yuzenghui@huawei.com> wrote:
> 
> Running a guest on the GICv4.1-implemented board, we will get the
> following warning:
> 
> [   59.062120] genirq: irq_chip GICv4.1-sgi did not update eff. affinity mask of irq 46
> 
> It may be caused by irqbalance (or other userspace tools) which tries to
> change the affinity of virtual SGIs on the host. One way to "fix" it is
> to update the effective_affinity value in irq_set_affinity callback. But
> as the comment above says, "There is no notion of affinity for virtual
> SGIs, at least not on the host", doing so only makes things confusing.
> 
> Given the vSGIs are private to the specified vPE, changing the affinity
> on host is actually meaningless and achieves nothing. Let's just forbid
> it.
> 
> Reported-by: Nianyao Tang <tangnianyao@huawei.com>
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> ---
> 
> Hi Marc,
> 
> This just restores the behavior of your v5 [*]. I wonder that what's the
> reason to change it to 'return IRQ_SET_MASK_OK' in v6? What I've missed
> here?
> 
> [*] https://lore.kernel.org/kvm/20200304203330.4967-9-maz@kernel.org/

Not allowing the affinity move results in the kernel screaming when
playing with CPU hotplug (it really wants to move the interrupt
around). Which is why I dropped the -EINVAL, therefore introducing
another bug. I fixed it with this patch[1], which I was planning to
post after -rc1.

Let me know what you think

Thanks,

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/gic-v4.1-fixes-5.7&id=26bf9895ebe8dbe4ff509aebc71b5990919a740d

-- 
Jazz is not dead, it just smells funny.

Re: [PATCH] irqchip/gic-v4.1: Disallow setting affinity for virtual SGIs

[Zenghui Yu]

Hi Marc,

On 2020/4/11 17:41, Marc Zyngier wrote:
> Hi Zenghui,
> 
> On Sat, 11 Apr 2020 10:10:32 +0100,
> Zenghui Yu <yuzenghui@huawei.com> wrote:
>>
>> Running a guest on the GICv4.1-implemented board, we will get the
>> following warning:
>>
>> [   59.062120] genirq: irq_chip GICv4.1-sgi did not update eff. affinity mask of irq 46
>>
>> It may be caused by irqbalance (or other userspace tools) which tries to
>> change the affinity of virtual SGIs on the host. One way to "fix" it is
>> to update the effective_affinity value in irq_set_affinity callback. But
>> as the comment above says, "There is no notion of affinity for virtual
>> SGIs, at least not on the host", doing so only makes things confusing.
>>
>> Given the vSGIs are private to the specified vPE, changing the affinity
>> on host is actually meaningless and achieves nothing. Let's just forbid
>> it.
>>
>> Reported-by: Nianyao Tang <tangnianyao@huawei.com>
>> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
>> ---
>>
>> Hi Marc,
>>
>> This just restores the behavior of your v5 [*]. I wonder that what's the
>> reason to change it to 'return IRQ_SET_MASK_OK' in v6? What I've missed
>> here?
>>
>> [*] https://lore.kernel.org/kvm/20200304203330.4967-9-maz@kernel.org/
> 
> Not allowing the affinity move results in the kernel screaming when
> playing with CPU hotplug (it really wants to move the interrupt
> around). Which is why I dropped the -EINVAL, therefore introducing
> another bug. I fixed it with this patch[1], which I was planning to
> post after -rc1.

I didn't realize the CPU hotplug case. Please take your approach to
fix it. (As mentioned, this was also one way I planned to fix it.)

> 
> Let me know what you think

TBH, I'm not very familiar with the IRQ core behavior on CPU hotplug.
I will read further and comment on your formal patch (please cc me),
but now spinning on some other things...


Thanks,
Zenghui