From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A47BECD1284 for ; Thu, 4 Apr 2024 07:56:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/pDrL8wEWpP0BEic3Vd8jZR1P/erDTKAOcP9B3DvM6Y=; b=jVE3Bvc1uMLAbS 2OcQav8zNe6m6YhRqkaP2i0Zaq2OBTIpfGDXhDV9NuzQnM+tM4tyoHvK3/X27QOS1bnW1iue2k1Qv zpym/X4M/wn33bPAb0dED/AuNzTS2i8LDz2Ki/4HyHPtmJHurdJ/Qs9GmooxjDJjlKHUBpM1yJtOK zBAIci7MGNkZiGTXmCV59rMK6XJYIs6Nws6g1ZMSuyROL4R4MM89cZ9w+MJZVXW3swaq1r82oyHRl 8hq7cPBUkhpDlXFKnxjino+wN1isKgipzGc2cbs5kGIB6u8LRtJadqOA/Llk9kcfhyM2VfTEx99xt u52FB/TbIp6mRlv2VZ6g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rsHxU-00000001iyj-46PD; Thu, 04 Apr 2024 07:56:29 +0000 Received: from relay2-d.mail.gandi.net ([2001:4b98:dc4:8::222]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rsHxQ-00000001ixX-3wC4 for linux-riscv@lists.infradead.org; Thu, 04 Apr 2024 07:56:26 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 35CCF40005; Thu, 4 Apr 2024 07:56:18 +0000 (UTC) Message-ID: Date: Thu, 4 Apr 2024 09:56:17 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 03/13] riscv: Use IPIs for remote cache/TLB flushes by default Content-Language: en-US To: Samuel Holland , Palmer Dabbelt , linux-riscv@lists.infradead.org Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Alexandre Ghiti , Jisheng Zhang , Yunhui Cui , Anup Patel References: <20240327045035.368512-1-samuel.holland@sifive.com> <20240327045035.368512-4-samuel.holland@sifive.com> From: Alexandre Ghiti In-Reply-To: <20240327045035.368512-4-samuel.holland@sifive.com> X-GND-Sasl: alex@ghiti.fr X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240404_005625_458052_03F407D2 X-CRM114-Status: GOOD ( 31.73 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On 27/03/2024 05:49, Samuel Holland wrote: > An IPI backend is always required in an SMP configuration, but an SBI > implementation is not. For example, SBI will be unavailable when the > kernel runs in M mode. For this reason, consider IPI delivery of cache > and TLB flushes to be the base case, and any other implementation (such > as the SBI remote fence extension) to be an optimization. > > Generally, if IPIs can be delivered without firmware assistance, they > are assumed to be faster than SBI calls due to the SBI context switch > overhead. However, when SBI is used as the IPI backend, then the context > switch cost must be paid anyway, and performing the cache/TLB flush > directly in the SBI implementation is more efficient than injecting an > interrupt to S-mode. This is the only existing scenario where > riscv_ipi_set_virq_range() is called with use_for_rfence set to false. > > sbi_ipi_init() already checks riscv_ipi_have_virq_range(), so it only > calls riscv_ipi_set_virq_range() when no other IPI device is available. > This allows moving the static key and dropping the use_for_rfence > parameter. This decouples the static key from the irqchip driver probe > order. > > Furthermore, the static branch only makes sense when CONFIG_RISCV_SBI is > enabled. Optherwise, IPIs must be used. Add a fallback definition of > riscv_use_sbi_for_rfence() which handles this case and removes the need > to check CONFIG_RISCV_SBI elsewhere, such as in cacheflush.c. > > Reviewed-by: Anup Patel > Signed-off-by: Samuel Holland > --- > > Changes in v6: > - Clarify the commit message for patch 3 based on ML discussion > > Changes in v5: > - Also switch to riscv_use_sbi_for_rfence() in asm/pgalloc.h > > Changes in v4: > - New patch for v4 > > arch/riscv/include/asm/pgalloc.h | 7 ++++--- > arch/riscv/include/asm/sbi.h | 4 ++++ > arch/riscv/include/asm/smp.h | 15 ++------------- > arch/riscv/kernel/sbi-ipi.c | 11 ++++++++++- > arch/riscv/kernel/smp.c | 11 +---------- > arch/riscv/mm/cacheflush.c | 5 ++--- > arch/riscv/mm/tlbflush.c | 31 ++++++++++++++----------------- > drivers/clocksource/timer-clint.c | 2 +- > 8 files changed, 38 insertions(+), 48 deletions(-) > > diff --git a/arch/riscv/include/asm/pgalloc.h b/arch/riscv/include/asm/pgalloc.h > index b34587da8882..f52264304f77 100644 > --- a/arch/riscv/include/asm/pgalloc.h > +++ b/arch/riscv/include/asm/pgalloc.h > @@ -8,6 +8,7 @@ > #define _ASM_RISCV_PGALLOC_H > > #include > +#include > #include > > #ifdef CONFIG_MMU > @@ -17,10 +18,10 @@ > > static inline void riscv_tlb_remove_ptdesc(struct mmu_gather *tlb, void *pt) > { > - if (riscv_use_ipi_for_rfence()) > - tlb_remove_page_ptdesc(tlb, pt); > - else > + if (riscv_use_sbi_for_rfence()) > tlb_remove_ptdesc(tlb, pt); > + else > + tlb_remove_page_ptdesc(tlb, pt); > } > > static inline void pmd_populate_kernel(struct mm_struct *mm, > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > index 6e68f8dff76b..ea84392ca9d7 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -375,8 +375,12 @@ unsigned long riscv_cached_marchid(unsigned int cpu_id); > unsigned long riscv_cached_mimpid(unsigned int cpu_id); > > #if IS_ENABLED(CONFIG_SMP) && IS_ENABLED(CONFIG_RISCV_SBI) > +DECLARE_STATIC_KEY_FALSE(riscv_sbi_for_rfence); > +#define riscv_use_sbi_for_rfence() \ > + static_branch_unlikely(&riscv_sbi_for_rfence) > void sbi_ipi_init(void); > #else > +static inline bool riscv_use_sbi_for_rfence(void) { return false; } > static inline void sbi_ipi_init(void) { } > #endif > > diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h > index 0d555847cde6..7ac80e9f2288 100644 > --- a/arch/riscv/include/asm/smp.h > +++ b/arch/riscv/include/asm/smp.h > @@ -49,12 +49,7 @@ void riscv_ipi_disable(void); > bool riscv_ipi_have_virq_range(void); > > /* Set the IPI interrupt numbers for arch (called by irqchip drivers) */ > -void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence); > - > -/* Check if we can use IPIs for remote FENCEs */ > -DECLARE_STATIC_KEY_FALSE(riscv_ipi_for_rfence); > -#define riscv_use_ipi_for_rfence() \ > - static_branch_unlikely(&riscv_ipi_for_rfence) > +void riscv_ipi_set_virq_range(int virq, int nr); > > /* Check other CPUs stop or not */ > bool smp_crash_stop_failed(void); > @@ -104,16 +99,10 @@ static inline bool riscv_ipi_have_virq_range(void) > return false; > } > > -static inline void riscv_ipi_set_virq_range(int virq, int nr, > - bool use_for_rfence) > +static inline void riscv_ipi_set_virq_range(int virq, int nr) > { > } > > -static inline bool riscv_use_ipi_for_rfence(void) > -{ > - return false; > -} > - > #endif /* CONFIG_SMP */ > > #if defined(CONFIG_HOTPLUG_CPU) && (CONFIG_SMP) > diff --git a/arch/riscv/kernel/sbi-ipi.c b/arch/riscv/kernel/sbi-ipi.c > index a4559695ce62..1026e22955cc 100644 > --- a/arch/riscv/kernel/sbi-ipi.c > +++ b/arch/riscv/kernel/sbi-ipi.c > @@ -13,6 +13,9 @@ > #include > #include > > +DEFINE_STATIC_KEY_FALSE(riscv_sbi_for_rfence); > +EXPORT_SYMBOL_GPL(riscv_sbi_for_rfence); > + > static int sbi_ipi_virq; > > static void sbi_ipi_handle(struct irq_desc *desc) > @@ -72,6 +75,12 @@ void __init sbi_ipi_init(void) > "irqchip/sbi-ipi:starting", > sbi_ipi_starting_cpu, NULL); > > - riscv_ipi_set_virq_range(virq, BITS_PER_BYTE, false); > + riscv_ipi_set_virq_range(virq, BITS_PER_BYTE); > pr_info("providing IPIs using SBI IPI extension\n"); > + > + /* > + * Use the SBI remote fence extension to avoid > + * the extra context switch needed to handle IPIs. > + */ > + static_branch_enable(&riscv_sbi_for_rfence); > } > diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c > index 45dd4035416e..8e6eb64459af 100644 > --- a/arch/riscv/kernel/smp.c > +++ b/arch/riscv/kernel/smp.c > @@ -171,10 +171,7 @@ bool riscv_ipi_have_virq_range(void) > return (ipi_virq_base) ? true : false; > } > > -DEFINE_STATIC_KEY_FALSE(riscv_ipi_for_rfence); > -EXPORT_SYMBOL_GPL(riscv_ipi_for_rfence); > - > -void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence) > +void riscv_ipi_set_virq_range(int virq, int nr) > { > int i, err; > > @@ -197,12 +194,6 @@ void riscv_ipi_set_virq_range(int virq, int nr, bool use_for_rfence) > > /* Enabled IPIs for boot CPU immediately */ > riscv_ipi_enable(); > - > - /* Update RFENCE static key */ > - if (use_for_rfence) > - static_branch_enable(&riscv_ipi_for_rfence); > - else > - static_branch_disable(&riscv_ipi_for_rfence); > } > > static const char * const ipi_names[] = { > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > index bc61ee5975e4..d76fc73e594b 100644 > --- a/arch/riscv/mm/cacheflush.c > +++ b/arch/riscv/mm/cacheflush.c > @@ -21,7 +21,7 @@ void flush_icache_all(void) > { > local_flush_icache_all(); > > - if (IS_ENABLED(CONFIG_RISCV_SBI) && !riscv_use_ipi_for_rfence()) > + if (riscv_use_sbi_for_rfence()) > sbi_remote_fence_i(NULL); > else > on_each_cpu(ipi_remote_fence_i, NULL, 1); > @@ -69,8 +69,7 @@ void flush_icache_mm(struct mm_struct *mm, bool local) > * with flush_icache_deferred(). > */ > smp_mb(); > - } else if (IS_ENABLED(CONFIG_RISCV_SBI) && > - !riscv_use_ipi_for_rfence()) { > + } else if (riscv_use_sbi_for_rfence()) { > sbi_remote_fence_i(&others); > } else { > on_each_cpu_mask(&others, ipi_remote_fence_i, NULL, 1); > diff --git a/arch/riscv/mm/tlbflush.c b/arch/riscv/mm/tlbflush.c > index 893566e004b7..0435605b07d0 100644 > --- a/arch/riscv/mm/tlbflush.c > +++ b/arch/riscv/mm/tlbflush.c > @@ -79,10 +79,10 @@ static void __ipi_flush_tlb_all(void *info) > > void flush_tlb_all(void) > { > - if (riscv_use_ipi_for_rfence()) > - on_each_cpu(__ipi_flush_tlb_all, NULL, 1); > - else > + if (riscv_use_sbi_for_rfence()) > sbi_remote_sfence_vma_asid(NULL, 0, FLUSH_TLB_MAX_SIZE, FLUSH_TLB_NO_ASID); > + else > + on_each_cpu(__ipi_flush_tlb_all, NULL, 1); > } > > struct flush_tlb_range_data { > @@ -103,7 +103,6 @@ static void __flush_tlb_range(struct cpumask *cmask, unsigned long asid, > unsigned long start, unsigned long size, > unsigned long stride) > { > - struct flush_tlb_range_data ftd; > bool broadcast; > > if (cpumask_empty(cmask)) > @@ -119,20 +118,18 @@ static void __flush_tlb_range(struct cpumask *cmask, unsigned long asid, > broadcast = true; > } > > - if (broadcast) { > - if (riscv_use_ipi_for_rfence()) { > - ftd.asid = asid; > - ftd.start = start; > - ftd.size = size; > - ftd.stride = stride; > - on_each_cpu_mask(cmask, > - __ipi_flush_tlb_range_asid, > - &ftd, 1); > - } else > - sbi_remote_sfence_vma_asid(cmask, > - start, size, asid); > - } else { > + if (!broadcast) { > local_flush_tlb_range_asid(start, size, stride, asid); > + } else if (riscv_use_sbi_for_rfence()) { > + sbi_remote_sfence_vma_asid(cmask, start, size, asid); > + } else { > + struct flush_tlb_range_data ftd; > + > + ftd.asid = asid; > + ftd.start = start; > + ftd.size = size; > + ftd.stride = stride; > + on_each_cpu_mask(cmask, __ipi_flush_tlb_range_asid, &ftd, 1); > } > > if (cmask != cpu_online_mask) > diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c > index 09fd292eb83d..0bdd9d7ec545 100644 > --- a/drivers/clocksource/timer-clint.c > +++ b/drivers/clocksource/timer-clint.c > @@ -251,7 +251,7 @@ static int __init clint_timer_init_dt(struct device_node *np) > } > > irq_set_chained_handler(clint_ipi_irq, clint_ipi_interrupt); > - riscv_ipi_set_virq_range(rc, BITS_PER_BYTE, true); > + riscv_ipi_set_virq_range(rc, BITS_PER_BYTE); > clint_clear_ipi(); > #endif > Reviewed-by: Alexandre Ghiti _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv