| Message ID | 20200501161938.2949443-1-daniel.thompson@linaro.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | ab8ad279ceac4fc78ae4dcf1a26326e05695e537 |
| Headers | show |
| Series | arm64: cacheflush: Fix KGDB trap detection | expand |
Hi, On Fri, May 1, 2020 at 9:20 AM Daniel Thompson <daniel.thompson@linaro.org> wrote: > > flush_icache_range() contains a bodge to avoid issuing IPIs when the kgdb > trap handler is running because issuing IPIs is unsafe (and unnecessary) > in this exection context. However the current test is flawed: it both s/exection/execution/ > over-matches (could skip the IPI when the kgdb trap is not running) and > under-matches (does not skip the IPI for all kgdb cache operations). Maybe explain why? I believe this is because "kgdb_connected" signifies that a host "gdb" is connected. * If we're sitting at the kdb prompt "kgdb_connected" won't be set but we might still try to do something with a breakpoint with interrupts off. * If we are currently _not_ stopped in the debugger but the host "gdb" is connected then "kgdb_connected" will continue to be set. In this context we will exit early if any other callers happen to try to cache flush with interrupts disabled. > Fix by replacing the ad-hoc check with the proper kgdb macro. This also > allows us to drop the #ifdef wrapper. > > Fixes: 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache for kernel mappings") > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> > --- > arch/arm64/include/asm/cacheflush.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Other than the suggestions to the commit message: Reviewed-by: Douglas Anderson <dianders@chromium.org>
diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h index e6cca3d4acf7..ce50c1f1f1ea 100644 --- a/arch/arm64/include/asm/cacheflush.h +++ b/arch/arm64/include/asm/cacheflush.h @@ -79,7 +79,7 @@ static inline void flush_icache_range(unsigned long start, unsigned long end) * IPI all online CPUs so that they undergo a context synchronization * event and are forced to refetch the new instructions. */ -#ifdef CONFIG_KGDB + /* * KGDB performs cache maintenance with interrupts disabled, so we * will deadlock trying to IPI the secondary CPUs. In theory, we can @@ -89,9 +89,9 @@ static inline void flush_icache_range(unsigned long start, unsigned long end) * the patching operation, so we don't need extra IPIs here anyway. * In which case, add a KGDB-specific bodge and return early. */ - if (kgdb_connected && irqs_disabled()) + if (in_dbg_master()) return; -#endif + kick_all_cpus_sync(); }
flush_icache_range() contains a bodge to avoid issuing IPIs when the kgdb trap handler is running because issuing IPIs is unsafe (and unnecessary) in this exection context. However the current test is flawed: it both over-matches (could skip the IPI when the kgdb trap is not running) and under-matches (does not skip the IPI for all kgdb cache operations). Fix by replacing the ad-hoc check with the proper kgdb macro. This also allows us to drop the #ifdef wrapper. Fixes: 3b8c9f1cdfc5 ("arm64: IPI each CPU after invalidating the I-cache for kernel mappings") Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> --- arch/arm64/include/asm/cacheflush.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) base-commit: 6a8b55ed4056ea5559ebe4f6a4b247f627870d4c -- 2.25.1