Messages in this thread | | | From | Barry Song <> | Date | Mon, 21 Feb 2022 09:20:52 +1300 | Subject | Re: [PATCH] irqchip/gic-v3: use dsb(ishst) to synchronize data to smp before issuing ipi |
| |
On Mon, Feb 21, 2022 at 4:21 AM Marc Zyngier <maz@kernel.org> wrote: > > On 2022-02-20 15:04, Russell King (Oracle) wrote: > > On Sat, Feb 19, 2022 at 05:55:49AM +0800, 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); > > > > On ARM, wmb() is a dsb(st) followed by other operations which may > > include a sync operation at the L2 cache, and SoC specific barriers > > for the bus. Hopefully, nothing will break if the sledge hammer is > > replaced by the tack hammer. > > The saving grace is that ARMv8 forbids (as per D4.4.11) these > SW-visible, > non architected caches (something like PL310 simply isn't allowed). > Given > that GICv3 requires ARMv8 the first place, we should be OK. > > As for SoC-specific bus barriers, I don't know of any that would affect > an ARMv8 based SoC. But I'm always prepared to be badly surprised...2
i assume we have been safe since dsb(ish) has been widely used for smp data visibility in arm64: arch/arm64/include/asm/assembler.h: dsb ish arch/arm64/include/asm/cacheflush.h: dsb(ish); arch/arm64/include/asm/pgtable.h: dsb(ishst); arch/arm64/include/asm/pgtable.h: dsb(ishst); arch/arm64/include/asm/pgtable.h: dsb(ishst); arch/arm64/include/asm/pgtable.h: dsb(ishst); arch/arm64/include/asm/pgtable.h: * is doing a dsb(ishst), but that penalises the fastpath. arch/arm64/include/asm/smp.h: dsb(ishst); arch/arm64/include/asm/tlbflush.h: "dsb ish\n tlbi " #op, \ arch/arm64/include/asm/tlbflush.h: "dsb ish\n tlbi " #op ", %0", \ arch/arm64/include/asm/tlbflush.h: dsb(ishst); arch/arm64/include/asm/tlbflush.h: dsb(ish); arch/arm64/include/asm/tlbflush.h: dsb(ishst); arch/arm64/include/asm/tlbflush.h: dsb(ish); arch/arm64/include/asm/tlbflush.h: dsb(ishst); arch/arm64/include/asm/tlbflush.h: dsb(ish); arch/arm64/include/asm/tlbflush.h: dsb(ishst); arch/arm64/include/asm/tlbflush.h: dsb(ish); arch/arm64/include/asm/tlbflush.h: dsb(ishst); arch/arm64/include/asm/tlbflush.h: dsb(ish); arch/arm64/include/asm/tlbflush.h: dsb(ishst); arch/arm64/include/asm/tlbflush.h: dsb(ish); arch/arm64/kernel/alternative.c: dsb(ish); arch/arm64/kernel/entry.S: dsb ish arch/arm64/kernel/head.S: dsb ishst // Make zero page visible to PTW arch/arm64/kernel/hibernate-asm.S: dsb ish /* wait for PoU cleaning to finish */ arch/arm64/kernel/hibernate-asm.S: dsb ish arch/arm64/kernel/mte.c: dsb(ish); arch/arm64/kernel/process.c: dsb(ish); arch/arm64/kernel/sys_compat.c: dsb(ish); arch/arm64/kvm/hyp/nvhe/tlb.c: dsb(ishst); arch/arm64/kvm/hyp/nvhe/tlb.c: dsb(ish); arch/arm64/kvm/hyp/nvhe/tlb.c: dsb(ish); arch/arm64/kvm/hyp/nvhe/tlb.c: dsb(ishst); arch/arm64/kvm/hyp/nvhe/tlb.c: dsb(ish); arch/arm64/kvm/hyp/nvhe/tlb.c: dsb(ishst); arch/arm64/kvm/hyp/nvhe/tlb.c: dsb(ish); arch/arm64/kvm/hyp/pgtable.c: dsb(ishst); arch/arm64/kvm/hyp/pgtable.c: dsb(ishst); arch/arm64/kvm/hyp/pgtable.c: dsb(ishst); arch/arm64/kvm/hyp/pgtable.c: dsb(ish); arch/arm64/kvm/hyp/pgtable.c: dsb(ishst); arch/arm64/kvm/hyp/pgtable.c: dsb(ishst); arch/arm64/kvm/hyp/pgtable.c: dsb(ishst); arch/arm64/kvm/hyp/vhe/tlb.c: dsb(ishst); arch/arm64/kvm/hyp/vhe/tlb.c: dsb(ish); arch/arm64/kvm/hyp/vhe/tlb.c: dsb(ish); arch/arm64/kvm/hyp/vhe/tlb.c: dsb(ishst); arch/arm64/kvm/hyp/vhe/tlb.c: dsb(ish); arch/arm64/kvm/hyp/vhe/tlb.c: dsb(ishst); arch/arm64/kvm/hyp/vhe/tlb.c: dsb(ish); arch/arm64/mm/cache.S: dsb ishst arch/arm64/mm/cache.S: dsb ishst arch/arm64/mm/kasan_init.c: dsb(ishst); arch/arm64/mm/mmu.c: * We need dsb(ishst) here to ensure the page-table-walker sees arch/arm64/mm/mmu.c: dsb(ishst); arch/arm64/mm/proc.S: dsb ish drivers/irqchip/irq-gic-v3-its.c: dsb(ishst); drivers/irqchip/irq-gic-v3-its.c: dsb(ishst);
Plus, is it even a problem to arm since arm only requires soc-level barrier for system-level memory barrier rather than smp-level barrier?
#if defined(CONFIG_ARM_DMA_MEM_BUFFERABLE) || defined(CONFIG_SMP) #define mb() __arm_heavy_mb() #define rmb() dsb() #define wmb() __arm_heavy_mb(st) #define dma_rmb() dmb(osh) #define dma_wmb() dmb(oshst) #else #define mb() barrier() #define rmb() barrier() #define wmb() barrier() #define dma_rmb() barrier() #define dma_wmb() barrier() #endif
#define __smp_mb() dmb(ish) #define __smp_rmb() __smp_mb() #define __smp_wmb() dmb(ishst)
I am not seeing arm requires soc-level mb for smp-level barrier though the mandatory barriers are using heavy_mb which has soc-level mb.
In this particular case, we are asking the data visibility for smp-level not system-level. I am not quite sure Russell's concern is relevant.
> > M. > -- > Jazz is not dead. It just smells funny...
Thanks Barry
| |