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:   Tue, 04 Jul 2023 17:02:44 +0100
Message-ID: <86pm57wtp7.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Zenghui Yu <yuzenghui@huawei.com>
Cc:     <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Kunkun Jiang <jiangkunkun@huawei.com>,
        <wanghaibin.wang@huawei.com>
Subject: Re: [PATCH] irqchip/gic-v4.1: Properly lock VPEs when doing a directLPI invalidation
In-Reply-To: <ee9123ca-95fc-4482-cee1-965cfe6b89b4@huawei.com>
References: <20230617073242.3199746-1-maz@kernel.org>
        <b665e102-eba9-cd4f-5b4e-ccbd77444233@huawei.com>
        <86wmzgx1va.wl-maz@kernel.org>
        <ee9123ca-95fc-4482-cee1-965cfe6b89b4@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 Tue, 04 Jul 2023 16:42:32 +0100,
Zenghui Yu <yuzenghui@huawei.com> wrote:
> 
> Hi Marc,
> 
> On 2023/7/4 2:54, Marc Zyngier wrote:
> > On Thu, 29 Jun 2023 15:52:24 +0100,
> > Zenghui Yu <yuzenghui@huawei.com> wrote:
> >> 
> >> Hi Marc,
> >> 
> >> Nit: I think the Subject header can be changed to 'irqchip/gic-v4' as
> >> the bug it fixes only affects GICv4 HW. v4.1 is unaffected.
> > 
> > I'm not so sure.
> > 
> > v4.0 didn't allow direct invalidation of VPE doorbells (we had to use
> > the fake device hack), except for the HiSi special that implemented
> > DirectLPI despite the presence of multiple ITSs. It was a violation of
> > the architecture, but it really saved the day by making invalidations
> > cheap enough.
> 
> [ I should've mentioned that I had reproduced the bug and tested your
> patch on my 920, which is, yeah, a HiSi implementation of GICv4.0 with
> DirectLPI supported. But ]
> 
> > 
> > Only with v4.1 did we get architectural support for doorbell
> > invalidation via a register instead of a command for a fake device.
> > 
> > So as far as the architecture is concerned, this should only affect
> > v4.1. As a side effect, it also affect HiSi's v4.0 implementations.
> 
> ... iiuc the bug we're fixing is that we perform a register based
> invalidation for doorbells (via its_vpe_[un]mask_irq/its_vpe_send_inv),
> acquire and release the per-RD lock with a *race* against a concurrent
> its_map_vm(), which would modify the vpe->col_idx behind our backs and
> affect the lock we're looking for.
> 
> its_vpe_[un]mask_irq() are callbacks for the v4.0 irqchip, i.e.,
> its_vpe_irq_chip.
> 
> With v4.1, we switch to use its_vpe_4_1_irq_chip and invalidate
> doorbells by sending the new INVDB command (and shouldn't be affected by
> this bug).

Gah, of course you're right. And we hardly ever invalidate DBs with
v4.1 anyway since we can always say whether we want the doorbell or
not when exiting residency on the RD (based on trapping WFI or not).

Apologies for the noise, and thanks for putting me right!

	M.

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