Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 06 Jan 2023 11:55:54 +0000
Message-ID: <86pmbrop11.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Yipeng Zou <zouyipeng@huawei.com>
Cc:     <tglx@linutronix.de>, <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <hewenliang4@huawei.com>,
        <chris.zjh@huawei.com>, <liaochang1@huawei.com>
Subject: Re: [RFC PATCH] irqchip/gic-v3: wait irq done to set affinity
In-Reply-To: <20230106082136.68501-1-zouyipeng@huawei.com>
References: <20230106082136.68501-1-zouyipeng@huawei.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Fri, 06 Jan 2023 08:21:36 +0000,
Yipeng Zou <zouyipeng@huawei.com> wrote:
> 
> Recently we have some problem about gic set affinity in our test.
> 
> This patch just aim to make some discuss about this problem.
> 
> For now, the implementation of gic set affinity going to take effects
> immediately, and without check if any irq are being processed.
> 
> So, This leads to some problem, think about this scenario:
> 
> 1. First, we have an irq was generated by an device.
> 
> 2. In the processing of this irq(after handle event, before clear
> IRQD_IRQ_INPROGRESS flag), we modify the route and the gic takes effect
> immediately,at the same time the new one was generated again.

How is that possible?

If it is affected by GICD_IROUTERn (as your patch suggests), then it
is a SPI. If it is a SPI, it has an active state. Which means it
cannot fire again without a deactivation (EOI if EOImode=0, EOI+DIR if
EOImode=1) having taken place.

So either something has deactivated the interrupt without masking it
beforehand, or the active state is not honoured. Either way, this is
wrong.

> 
> 3. The new irq will be processing in other cpu which different form the
> old one.
> 
> 4. The new irq going to be discarded because of the flag IRQD_IRQ_INPROGRESS
> has been set.
> 
> I notice that if we set IRQF_ONESHOT when register the irq, this problem
> will gone.
> 
> But I'm also thinking about change the gic_set_affinity function, to wait
> current irq done on all cpus before gic_write_irouter.
> I'm not sure if that's appropriate.

The base architecture should guarantee that this is not a problem,
thanks to the active state. If that was a LPI (which do not have an
active state), that'd be a different problem. But this doesn't seem to
be the case here.

I'm afraid to say that what you describe seem like a bug of some sort,
either HW or SW.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.