From: "Heiko Stübner" <heiko@sntech.de> To: Atish Patra <atishp@atishpatra.org> Cc: Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Albert Ou <aou@eecs.berkeley.edu>, linux-riscv <linux-riscv@lists.infradead.org>, devicetree <devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>, Rob Herring <robh+dt@kernel.org>, Wei Fu <wefu@redhat.com>, liush <liush@allwinnertech.com>, Guo Ren <guoren@kernel.org>, Anup Patel <anup@brainfault.org>, Drew Fustini <drew@beagleboard.org>, Christoph Hellwig <hch@lst.de>, Arnd Bergmann <arnd@arndb.de>, Chen-Yu Tsai <wens@csie.org>, Maxime Ripard <maxime@cerno.tech>, Daniel Lustig <dlustig@nvidia.com>, Greg Favor <gfavor@ventanamicro.com>, Andrea Mondelli <andrea.mondelli@huawei.com>, Jonathan Behrens <behrensj@mit.edu>, Xinhaoqu <xinhaoqu@huawei.com>, Bill Huffman <huffman@cadence.com>, Nick Kossifidis <mick@ics.forth.gr>, Allen Baum <allen.baum@esperantotech.com>, Josh Scheid <jscheid@ventanamicro.com>, Richard Trauben <rtrauben@gmail.com>, Samuel Holland <samuel@sholland.org>, Christoph Muellner <cmuellner@linux.com>, Philipp Tomsich <philipp.tomsich@vrull.eu> Subject: Re: [PATCH v5 01/14] riscv: only use IPIs to handle cache-flushes on remote cpus Date: Mon, 24 Jan 2022 13:30:06 +0100 [thread overview] Message-ID: <10651919.N8281ZbHTu@diego> (raw) In-Reply-To: <CAOnJCU+NR_hOrvS_+B+OKXeg4s+uh37gYWGVTs_kDd3LQDVEkQ@mail.gmail.com> Hi Atish, Am Samstag, 22. Januar 2022, 04:45:52 CET schrieb Atish Patra: > On Fri, Jan 21, 2022 at 8:37 AM Heiko Stuebner <heiko@sntech.de> wrote: > > > > Right now, the flush_icache functions always use the SBI remote-fence > > when SBI is available, leaving using IPIs as a fallback mechanism. > > > > IPIs on the other hand are more flexible, as the ipi_ops are initially > > set to go through SBI but later will be overwritten to go through the > > ACLINT/CLINT. > > > > In a discussion we had, Nick was of the opinion that "In general we > > should prefer doing IPIs on S-mode through CLINT instead of going > > through SBI/M-mode, > > Yes. Once Anup's ACLINT drivers are merged, that should be the > preferred approach. > > https://github.com/avpatel/linux/commit/416c667fd77d6f1fc310cbf727ec127aaf96cae2 > > >so IMHO we should only be using > > on_each_cpu_mask(ipi_remote_fence_i) on flush_icache_all()/ > > flush_icache_mm() and remove any explicit calls to sbi_remote_fence_i(), > > That's a bit confusing because we will be using SBI calls for all other fences > while using IPIs for fence.i > > > because this way we continue using SBI for doing remote fences even after > > CLINT/ACLINT driver is registered, instead of using direct IPIs through > > CLINT/ACLINT." > > > > So follow this suggestion and just do ipi calls to have the proper kernel > > parts do them, > > > > This also fixes the null-ptr dereference happening when flush_icache_all() > > is called before sbi_init(). > > > > IMHO, this series should only fix the null-ptr dereference issue. > The IPI based fence (for all) should only be disabled along with the > ACLINT driver > that actually enables S-mode IPIs. ok, I'll roll this back to simply fixing the null-ptr issue. Meanwhile I even found a nicer solution without actually touching the cachflush code. Without sbi_init() we can assume that we're still before smp bringup, so the local_flush_icache_all() in flush_icache_all() will do the trick just fine and sbi_remote_fence_i() simply just needs to be an empty function until sbi is initialized. Heiko > > > Suggested-by: Nick Kossifidis <mick@ics.forth.gr> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > --- > > arch/riscv/mm/cacheflush.c | 8 +------- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > > index 6cb7d96ad9c7..c35375cd52ec 100644 > > --- a/arch/riscv/mm/cacheflush.c > > +++ b/arch/riscv/mm/cacheflush.c > > @@ -17,11 +17,7 @@ static void ipi_remote_fence_i(void *info) > > void flush_icache_all(void) > > { > > local_flush_icache_all(); > > - > > - if (IS_ENABLED(CONFIG_RISCV_SBI)) > > - sbi_remote_fence_i(NULL); > > - else > > - on_each_cpu(ipi_remote_fence_i, NULL, 1); > > + on_each_cpu(ipi_remote_fence_i, NULL, 1); > > } > > EXPORT_SYMBOL(flush_icache_all); > > > > @@ -66,8 +62,6 @@ void flush_icache_mm(struct mm_struct *mm, bool local) > > * with flush_icache_deferred(). > > */ > > smp_mb(); > > - } else if (IS_ENABLED(CONFIG_RISCV_SBI)) { > > - sbi_remote_fence_i(&others); > > } else { > > on_each_cpu_mask(&others, ipi_remote_fence_i, NULL, 1); > > } > > -- > > 2.30.2 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > >
WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de> To: Atish Patra <atishp@atishpatra.org> Cc: Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Albert Ou <aou@eecs.berkeley.edu>, linux-riscv <linux-riscv@lists.infradead.org>, devicetree <devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>, Rob Herring <robh+dt@kernel.org>, Wei Fu <wefu@redhat.com>, liush <liush@allwinnertech.com>, Guo Ren <guoren@kernel.org>, Anup Patel <anup@brainfault.org>, Drew Fustini <drew@beagleboard.org>, Christoph Hellwig <hch@lst.de>, Arnd Bergmann <arnd@arndb.de>, Chen-Yu Tsai <wens@csie.org>, Maxime Ripard <maxime@cerno.tech>, Daniel Lustig <dlustig@nvidia.com>, Greg Favor <gfavor@ventanamicro.com>, Andrea Mondelli <andrea.mondelli@huawei.com>, Jonathan Behrens <behrensj@mit.edu>, Xinhaoqu <xinhaoqu@huawei.com>, Bill Huffman <huffman@cadence.com>, Nick Kossifidis <mick@ics.forth.gr>, Allen Baum <allen.baum@esperantotech.com>, Josh Scheid <jscheid@ventanamicro.com>, Richard Trauben <rtrauben@gmail.com>, Samuel Holland <samuel@sholland.org>, Christoph Muellner <cmuellner@linux.com>, Philipp Tomsich <philipp.tomsich@vrull.eu> Subject: Re: [PATCH v5 01/14] riscv: only use IPIs to handle cache-flushes on remote cpus Date: Mon, 24 Jan 2022 13:30:06 +0100 [thread overview] Message-ID: <10651919.N8281ZbHTu@diego> (raw) In-Reply-To: <CAOnJCU+NR_hOrvS_+B+OKXeg4s+uh37gYWGVTs_kDd3LQDVEkQ@mail.gmail.com> Hi Atish, Am Samstag, 22. Januar 2022, 04:45:52 CET schrieb Atish Patra: > On Fri, Jan 21, 2022 at 8:37 AM Heiko Stuebner <heiko@sntech.de> wrote: > > > > Right now, the flush_icache functions always use the SBI remote-fence > > when SBI is available, leaving using IPIs as a fallback mechanism. > > > > IPIs on the other hand are more flexible, as the ipi_ops are initially > > set to go through SBI but later will be overwritten to go through the > > ACLINT/CLINT. > > > > In a discussion we had, Nick was of the opinion that "In general we > > should prefer doing IPIs on S-mode through CLINT instead of going > > through SBI/M-mode, > > Yes. Once Anup's ACLINT drivers are merged, that should be the > preferred approach. > > https://github.com/avpatel/linux/commit/416c667fd77d6f1fc310cbf727ec127aaf96cae2 > > >so IMHO we should only be using > > on_each_cpu_mask(ipi_remote_fence_i) on flush_icache_all()/ > > flush_icache_mm() and remove any explicit calls to sbi_remote_fence_i(), > > That's a bit confusing because we will be using SBI calls for all other fences > while using IPIs for fence.i > > > because this way we continue using SBI for doing remote fences even after > > CLINT/ACLINT driver is registered, instead of using direct IPIs through > > CLINT/ACLINT." > > > > So follow this suggestion and just do ipi calls to have the proper kernel > > parts do them, > > > > This also fixes the null-ptr dereference happening when flush_icache_all() > > is called before sbi_init(). > > > > IMHO, this series should only fix the null-ptr dereference issue. > The IPI based fence (for all) should only be disabled along with the > ACLINT driver > that actually enables S-mode IPIs. ok, I'll roll this back to simply fixing the null-ptr issue. Meanwhile I even found a nicer solution without actually touching the cachflush code. Without sbi_init() we can assume that we're still before smp bringup, so the local_flush_icache_all() in flush_icache_all() will do the trick just fine and sbi_remote_fence_i() simply just needs to be an empty function until sbi is initialized. Heiko > > > Suggested-by: Nick Kossifidis <mick@ics.forth.gr> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > > --- > > arch/riscv/mm/cacheflush.c | 8 +------- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > > index 6cb7d96ad9c7..c35375cd52ec 100644 > > --- a/arch/riscv/mm/cacheflush.c > > +++ b/arch/riscv/mm/cacheflush.c > > @@ -17,11 +17,7 @@ static void ipi_remote_fence_i(void *info) > > void flush_icache_all(void) > > { > > local_flush_icache_all(); > > - > > - if (IS_ENABLED(CONFIG_RISCV_SBI)) > > - sbi_remote_fence_i(NULL); > > - else > > - on_each_cpu(ipi_remote_fence_i, NULL, 1); > > + on_each_cpu(ipi_remote_fence_i, NULL, 1); > > } > > EXPORT_SYMBOL(flush_icache_all); > > > > @@ -66,8 +62,6 @@ void flush_icache_mm(struct mm_struct *mm, bool local) > > * with flush_icache_deferred(). > > */ > > smp_mb(); > > - } else if (IS_ENABLED(CONFIG_RISCV_SBI)) { > > - sbi_remote_fence_i(&others); > > } else { > > on_each_cpu_mask(&others, ipi_remote_fence_i, NULL, 1); > > } > > -- > > 2.30.2 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2022-01-24 12:30 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-21 16:36 [PATCH v5 00/14] riscv: support for svpbmt and D1 memory types Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 01/14] riscv: only use IPIs to handle cache-flushes on remote cpus Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-22 3:45 ` Atish Patra 2022-01-22 3:45 ` Atish Patra 2022-01-24 12:30 ` Heiko Stübner [this message] 2022-01-24 12:30 ` Heiko Stübner 2022-01-22 4:10 ` Anup Patel 2022-01-22 4:10 ` Anup Patel 2022-01-21 16:36 ` [PATCH v5 02/14] riscv: integrate alternatives better into the main architecture Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 03/14] riscv: allow different stages with alternatives Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 04/14] riscv: implement module alternatives Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 05/14] riscv: implement ALTERNATIVE_2 macro Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 06/14] riscv: extend concatenated alternatives-lines to the same length Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 07/14] riscv: prevent compressed instructions in alternatives Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 08/14] riscv: move boot alternatives to a slightly earlier position Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 09/14] riscv: Fix accessing pfn bits in PTEs for non-32bit variants Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 10/14] riscv: add cpufeature handling via alternatives Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 11/14] dt-bindings: riscv: add MMU Standard Extensions support for Svpbmt Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-02-04 22:33 ` Rob Herring 2022-02-04 22:33 ` Rob Herring 2022-02-07 13:39 ` Heiko Stuebner 2022-02-07 13:39 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 12/14] riscv: add RISC-V Svpbmt extension supports Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 13/14] riscv: remove FIXMAP_PAGE_IO and fall back to its default value Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-21 16:36 ` [PATCH v5 14/14] riscv: add memory-type errata for T-Head Heiko Stuebner 2022-01-21 16:36 ` Heiko Stuebner 2022-01-24 7:22 ` [PATCH v5 00/14] riscv: support for svpbmt and D1 memory types Christoph Hellwig 2022-01-24 7:22 ` Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=10651919.N8281ZbHTu@diego \ --to=heiko@sntech.de \ --cc=allen.baum@esperantotech.com \ --cc=andrea.mondelli@huawei.com \ --cc=anup@brainfault.org \ --cc=aou@eecs.berkeley.edu \ --cc=arnd@arndb.de \ --cc=atishp@atishpatra.org \ --cc=behrensj@mit.edu \ --cc=cmuellner@linux.com \ --cc=devicetree@vger.kernel.org \ --cc=dlustig@nvidia.com \ --cc=drew@beagleboard.org \ --cc=gfavor@ventanamicro.com \ --cc=guoren@kernel.org \ --cc=hch@lst.de \ --cc=huffman@cadence.com \ --cc=jscheid@ventanamicro.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=liush@allwinnertech.com \ --cc=maxime@cerno.tech \ --cc=mick@ics.forth.gr \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ --cc=philipp.tomsich@vrull.eu \ --cc=robh+dt@kernel.org \ --cc=rtrauben@gmail.com \ --cc=samuel@sholland.org \ --cc=wefu@redhat.com \ --cc=wens@csie.org \ --cc=xinhaoqu@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.