Received: by 2002:ac0:aed5:0:0:0:0:0 with SMTP id t21csp1147472imb; Sat, 2 Mar 2019 04:47:58 -0800 (PST) X-Google-Smtp-Source: APXvYqxqiU0aXrLQlFyvPTtasCjwPgthK4Zn8/VawKhQLjHLZGMthlMEb7lRROY18sH/yvf+2EOa X-Received: by 2002:aa7:8b09:: with SMTP id f9mr10683220pfd.168.1551530878898; Sat, 02 Mar 2019 04:47:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1551530878; cv=none; d=google.com; s=arc-20160816; b=CvXWAWq78EVTzbcf2S31Ouo9Mrc+0wKCDjaoiaCQEXMQNWkXkUM7uWJex+vy5c0dXL Aq5UKzNO44UNzeU1XzEFAWVXWC+d+3OnYu1TryPcGM5lrgMb0l8wQnih+RqrC7scRUmW oDJq45cyyI4yytmHSj8WxOZdLNFi8Z4SPfEX/aNtRcWrqFnbccYULkJbg+BmOwJjwNHQ A489zcFYP05mNWDNyxwrMVjZhtlJZzkmjQY9Ygb/yw9wng/ztDDKbRLdqd8Ck2/XG8o7 jLACggqlOSuOOD8T3ddwaNBjWxI1Po7VgEUd9UqEbN6j/D64BlxcZ1W6x7Q3lTLKR5VE 1HKA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from; bh=nGq8OthTd1AzGj4uJBwRPmi299whydr9zuTscH7iyis=; b=C8KHRNv14a/eEvXyBUafBgUkDUKsmbeerhth6eXDjFFGDd2dbOf1JuqykDF+/H3s0g Pmk4XOoE2GHOhlCWAytUXxZBSKLYpM+WJauioFcQczMkUZ4lBGW17BhvZPzmKSWSK7nd Q+oct5OtcnOwyuj+ooj091u10BJR5ZiNqQ7p7Qom3yOzkC6CQDqNuXpJGQ6y/OfdOkYk aEMDJlL/ntPTIRkmlliCKIr6rGFViSAWtBIzZdH5RiwS8RCXC5it2d3neKWj+R8efcOG 2SZq2VymNK9Ontr5/f+2TZQvVgPzrzv7dbZYy3uwfSpDOSMBmsXmvY7zrMaTqKcp0jJ4 9n/g== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id h3si554711pfj.137.2019.03.02.04.47.35; Sat, 02 Mar 2019 04:47:58 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726492AbfCBMqx (ORCPT + 99 others); Sat, 2 Mar 2019 07:46:53 -0500 Received: from ozlabs.org ([203.11.71.1]:54757 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726049AbfCBMqx (ORCPT ); Sat, 2 Mar 2019 07:46:53 -0500 Received: from authenticated.ozlabs.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail.ozlabs.org (Postfix) with ESMTPSA id 44BQyr35Vvz9s4Y; Sat, 2 Mar 2019 23:46:44 +1100 (AEDT) From: Michael Ellerman To: Will Deacon , linux-arch@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Will Deacon , "Paul E. McKenney" , Benjamin Herrenschmidt , Arnd Bergmann , Peter Zijlstra , Andrea Parri , Palmer Dabbelt , Daniel Lustig , David Howells , Alan Stern , Linus Torvalds , "Maciej W. Rozycki" , Paul Burton , Ingo Molnar , Yoshinori Sato , Rich Felker , Tony Luck Subject: Re: [PATCH 12/20] powerpc/mmiowb: Hook up mmwiob() implementation to asm-generic code In-Reply-To: <20190301140348.25175-13-will.deacon@arm.com> References: <20190301140348.25175-1-will.deacon@arm.com> <20190301140348.25175-13-will.deacon@arm.com> Date: Sat, 02 Mar 2019 23:46:41 +1100 Message-ID: <874l8ljvf2.fsf@concordia.ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Will, Will Deacon writes: > In a bid to kill off explicit mmiowb() usage in driver code, hook up > the asm-generic mmiowb() tracking code but provide a definition of > arch_mmiowb_state() so that the tracking data can remain in the paca > as it does at present > > This replaces the existing (flawed) implementation. > > Signed-off-by: Will Deacon > --- > arch/powerpc/Kconfig | 1 + > arch/powerpc/include/asm/Kbuild | 1 - > arch/powerpc/include/asm/io.h | 33 +++------------------------------ > arch/powerpc/include/asm/mmiowb.h | 20 ++++++++++++++++++++ > arch/powerpc/include/asm/paca.h | 6 +++++- > arch/powerpc/include/asm/spinlock.h | 17 ----------------- > arch/powerpc/xmon/xmon.c | 5 ++++- > 7 files changed, 33 insertions(+), 50 deletions(-) > create mode 100644 arch/powerpc/include/asm/mmiowb.h Thanks for fixing our bugs for us, I owe you some more beers :) I meant to reply to your previous series saying that we could just use more space in the paca, but you obviously worked that out yourself. I'll run this through our builders and do some boot tests but I looks good to me. Acked-by: Michael Ellerman cheers > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index 2890d36eb531..6979304475fd 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -134,6 +134,7 @@ config PPC > select ARCH_HAS_ELF_RANDOMIZE > select ARCH_HAS_FORTIFY_SOURCE > select ARCH_HAS_GCOV_PROFILE_ALL > + select ARCH_HAS_MMIOWB if PPC64 > select ARCH_HAS_PHYS_TO_DMA > select ARCH_HAS_PMEM_API if PPC64 > select ARCH_HAS_PTE_SPECIAL > diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild > index 57bd1f6660f4..77ff7fb24823 100644 > --- a/arch/powerpc/include/asm/Kbuild > +++ b/arch/powerpc/include/asm/Kbuild > @@ -8,7 +8,6 @@ generic-y += irq_regs.h > generic-y += irq_work.h > generic-y += local64.h > generic-y += mcs_spinlock.h > -generic-y += mmiowb.h > generic-y += preempt.h > generic-y += rwsem.h > generic-y += vtime.h > diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h > index 7f19fbd3ba55..828100476ba6 100644 > --- a/arch/powerpc/include/asm/io.h > +++ b/arch/powerpc/include/asm/io.h > @@ -34,14 +34,11 @@ extern struct pci_dev *isa_bridge_pcidev; > #include > #include > #include > +#include > #include > #include > #include > > -#ifdef CONFIG_PPC64 > -#include > -#endif > - > #define SIO_CONFIG_RA 0x398 > #define SIO_CONFIG_RD 0x399 > > @@ -107,12 +104,6 @@ extern bool isa_io_special; > * > */ > > -#ifdef CONFIG_PPC64 > -#define IO_SET_SYNC_FLAG() do { local_paca->io_sync = 1; } while(0) > -#else > -#define IO_SET_SYNC_FLAG() > -#endif > - > #define DEF_MMIO_IN_X(name, size, insn) \ > static inline u##size name(const volatile u##size __iomem *addr) \ > { \ > @@ -127,7 +118,7 @@ static inline void name(volatile u##size __iomem *addr, u##size val) \ > { \ > __asm__ __volatile__("sync;"#insn" %1,%y0" \ > : "=Z" (*addr) : "r" (val) : "memory"); \ > - IO_SET_SYNC_FLAG(); \ > + mmiowb_set_pending(); \ > } > > #define DEF_MMIO_IN_D(name, size, insn) \ > @@ -144,7 +135,7 @@ static inline void name(volatile u##size __iomem *addr, u##size val) \ > { \ > __asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \ > : "=m" (*addr) : "r" (val) : "memory"); \ > - IO_SET_SYNC_FLAG(); \ > + mmiowb_set_pending(); \ > } > > DEF_MMIO_IN_D(in_8, 8, lbz); > @@ -652,24 +643,6 @@ static inline void name at \ > > #include > > -#ifdef CONFIG_PPC32 > -#define mmiowb() > -#else > -/* > - * Enforce synchronisation of stores vs. spin_unlock > - * (this does it explicitly, though our implementation of spin_unlock > - * does it implicitely too) > - */ > -static inline void mmiowb(void) > -{ > - unsigned long tmp; > - > - __asm__ __volatile__("sync; li %0,0; stb %0,%1(13)" > - : "=&r" (tmp) : "i" (offsetof(struct paca_struct, io_sync)) > - : "memory"); > -} > -#endif /* !CONFIG_PPC32 */ > - > static inline void iosync(void) > { > __asm__ __volatile__ ("sync" : : : "memory"); > diff --git a/arch/powerpc/include/asm/mmiowb.h b/arch/powerpc/include/asm/mmiowb.h > new file mode 100644 > index 000000000000..b10180613507 > --- /dev/null > +++ b/arch/powerpc/include/asm/mmiowb.h > @@ -0,0 +1,20 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_POWERPC_MMIOWB_H > +#define _ASM_POWERPC_MMIOWB_H > + > +#ifdef CONFIG_MMIOWB > + > +#include > +#include > +#include > + > +#define arch_mmiowb_state() (&local_paca->mmiowb_state) > +#define mmiowb() mb() > + > +#else > +#define mmiowb() do { } while (0) > +#endif /* CONFIG_MMIOWB */ > + > +#include > + > +#endif /* _ASM_POWERPC_MMIOWB_H */ > diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h > index e843bc5d1a0f..134e912d403f 100644 > --- a/arch/powerpc/include/asm/paca.h > +++ b/arch/powerpc/include/asm/paca.h > @@ -34,6 +34,8 @@ > #include > #include > > +#include > + > register struct paca_struct *local_paca asm("r13"); > > #if defined(CONFIG_DEBUG_PREEMPT) && defined(CONFIG_SMP) > @@ -171,7 +173,6 @@ struct paca_struct { > u16 trap_save; /* Used when bad stack is encountered */ > u8 irq_soft_mask; /* mask for irq soft masking */ > u8 irq_happened; /* irq happened while soft-disabled */ > - u8 io_sync; /* writel() needs spin_unlock sync */ > u8 irq_work_pending; /* IRQ_WORK interrupt while soft-disable */ > u8 nap_state_lost; /* NV GPR values lost in power7_idle */ > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > @@ -264,6 +265,9 @@ struct paca_struct { > #ifdef CONFIG_STACKPROTECTOR > unsigned long canary; > #endif > +#ifdef CONFIG_MMIOWB > + struct mmiowb_state mmiowb_state; > +#endif > } ____cacheline_aligned; > > extern void copy_mm_to_paca(struct mm_struct *mm); > diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h > index 685c72310f5d..15b39c407c4e 100644 > --- a/arch/powerpc/include/asm/spinlock.h > +++ b/arch/powerpc/include/asm/spinlock.h > @@ -39,19 +39,6 @@ > #define LOCK_TOKEN 1 > #endif > > -#if defined(CONFIG_PPC64) && defined(CONFIG_SMP) > -#define CLEAR_IO_SYNC (get_paca()->io_sync = 0) > -#define SYNC_IO do { \ > - if (unlikely(get_paca()->io_sync)) { \ > - mb(); \ > - get_paca()->io_sync = 0; \ > - } \ > - } while (0) > -#else > -#define CLEAR_IO_SYNC > -#define SYNC_IO > -#endif > - > #ifdef CONFIG_PPC_PSERIES > #define vcpu_is_preempted vcpu_is_preempted > static inline bool vcpu_is_preempted(int cpu) > @@ -99,7 +86,6 @@ static inline unsigned long __arch_spin_trylock(arch_spinlock_t *lock) > > static inline int arch_spin_trylock(arch_spinlock_t *lock) > { > - CLEAR_IO_SYNC; > return __arch_spin_trylock(lock) == 0; > } > > @@ -130,7 +116,6 @@ extern void __rw_yield(arch_rwlock_t *lock); > > static inline void arch_spin_lock(arch_spinlock_t *lock) > { > - CLEAR_IO_SYNC; > while (1) { > if (likely(__arch_spin_trylock(lock) == 0)) > break; > @@ -148,7 +133,6 @@ void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned long flags) > { > unsigned long flags_dis; > > - CLEAR_IO_SYNC; > while (1) { > if (likely(__arch_spin_trylock(lock) == 0)) > break; > @@ -167,7 +151,6 @@ void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned long flags) > > static inline void arch_spin_unlock(arch_spinlock_t *lock) > { > - SYNC_IO; > __asm__ __volatile__("# arch_spin_unlock\n\t" > PPC_RELEASE_BARRIER: : :"memory"); > lock->slock = 0; > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index 757b8499aba2..de8e4693b176 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -2429,7 +2429,10 @@ static void dump_one_paca(int cpu) > DUMP(p, trap_save, "%#-*x"); > DUMP(p, irq_soft_mask, "%#-*x"); > DUMP(p, irq_happened, "%#-*x"); > - DUMP(p, io_sync, "%#-*x"); > +#ifdef CONFIG_MMIOWB > + DUMP(p, mmiowb_state.nesting_count, "%#-*x"); > + DUMP(p, mmiowb_state.mmiowb_pending, "%#-*x"); > +#endif > DUMP(p, irq_work_pending, "%#-*x"); > DUMP(p, nap_state_lost, "%#-*x"); > DUMP(p, sprg_vdso, "%#-*llx"); > -- > 2.11.0