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;
MIME-Version: 1.0
Date:   Sun, 20 Feb 2022 15:04:53 +0000
From:   Marc Zyngier <maz@kernel.org>
To:     Ard Biesheuvel <ardb@kernel.org>
Cc:     Barry Song <21cnbao@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Will Deacon <will@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linuxarm <linuxarm@huawei.com>,
        Barry Song <song.bao.hua@hisilicon.com>
Subject: Re: [PATCH] irqchip/gic-v3: use dsb(ishst) to synchronize data to smp
 before issuing ipi
In-Reply-To: <CAMj1kXF14uEM=eTsqKR8MBWYS_yebMQF5kca86gGyuXkjs+GRg@mail.gmail.com>
References: <20220218215549.4274-1-song.bao.hua@hisilicon.com>
 <6432e7e97b828d887da8794c150161c4@kernel.org>
 <CAMj1kXF14uEM=eTsqKR8MBWYS_yebMQF5kca86gGyuXkjs+GRg@mail.gmail.com>
User-Agent: Roundcube Webmail/1.4.13
Message-ID: <49ee858267c75144c601b1e42d4f4c28@kernel.org>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022-02-20 13:30, Ard Biesheuvel wrote:
> On Sat, 19 Feb 2022 at 10:57, Marc Zyngier <maz@kernel.org> wrote:
>> 
>> On 2022-02-18 21:55, Barry Song wrote:
>> > dsb(ishst) should be enough here as we only need to guarantee the
>> > visibility of data to other CPUs in smp inner domain before we
>> > send the ipi.
>> >
>> > Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
>> > ---
>> >  drivers/irqchip/irq-gic-v3.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/irqchip/irq-gic-v3.c
>> > b/drivers/irqchip/irq-gic-v3.c
>> > index 5e935d97207d..0efe1a9a9f3b 100644
>> > --- a/drivers/irqchip/irq-gic-v3.c
>> > +++ b/drivers/irqchip/irq-gic-v3.c
>> > @@ -1211,7 +1211,7 @@ static void gic_ipi_send_mask(struct irq_data
>> > *d, const struct cpumask *mask)
>> >        * Ensure that stores to Normal memory are visible to the
>> >        * other CPUs before issuing the IPI.
>> >        */
>> > -     wmb();
>> > +     dsb(ishst);
>> >
>> >       for_each_cpu(cpu, mask) {
>> >               u64 cluster_id = MPIDR_TO_SGI_CLUSTER_ID(cpu_logical_map(cpu));
>> 
>> I'm not opposed to that change, but I'm pretty curious whether this
>> makes
>> any visible difference in practice. Could you measure the effect of 
>> this
>> change
>> for any sort of IPI heavy workload?
>> 
> 
> Does this have to be a DSB ?

We can use a DMB ISHST for the other interrupt controllers that use a
MMIO access to signal the IPI, as we need to order the previous writes 
with
the MMIO access (and DMB fits that bill).

For GICv3 when ICC_SRE_EL1.SRE==1, we need to order a set of writes with
a system register access with side effects, and the only barrier that
allows us to do that is DSB.

It is a bit unfortunate that the relative lightweight nature of the 
sysreg
CPU interface is encumbered by fairly heavy barriers (the most horrible 
one
being the DSB SY on each read of IAR to be able to synchronise the CPU 
interface
and the redistributor).

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...