diff mbox series

[v9,6/7] kgdb: Provide a stub kgdb_nmicallback() if !CONFIG_KGDB

Message ID 20230601143109.v9.6.Ia3aeac89bb6751b682237e76e5ba594318e4b1aa@changeid
State New
Headers
Series arm64: Add debug IPI for backtraces / kgdb; try to use NMI for it |

Commit Message

Doug Anderson June 1, 2023, 9:31 p.m. UTC 
  To save architectures from needing to wrap the call in #ifdefs, add a
stub no-op version of kgdb_nmicallback(), which returns 1 if it didn't
handle anything.

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
In v9 this is the only kgdb dependency. I'm assuming it could go
through the arm64 tree? If that's not a good idea, we could always
change the patch ("arm64: kgdb: Roundup cpus using IPI as NMI") not to
depend on it by only calling kgdb_nmicallback() if CONFIG_KGDB is not
defined.

Changes in v9:
- Added missing "inline"

Changes in v8:
- "Provide a stub kgdb_nmicallback() if !CONFIG_KGDB" new for v8

 include/linux/kgdb.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Doug Anderson June 15, 2023, 6:14 p.m. UTC | #1 
Daniel,

On Thu, Jun 1, 2023 at 2:37 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> To save architectures from needing to wrap the call in #ifdefs, add a
> stub no-op version of kgdb_nmicallback(), which returns 1 if it didn't
> handle anything.
>
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> In v9 this is the only kgdb dependency. I'm assuming it could go
> through the arm64 tree? If that's not a good idea, we could always
> change the patch ("arm64: kgdb: Roundup cpus using IPI as NMI") not to
> depend on it by only calling kgdb_nmicallback() if CONFIG_KGDB is not
> defined.
>
> Changes in v9:
> - Added missing "inline"
>
> Changes in v8:
> - "Provide a stub kgdb_nmicallback() if !CONFIG_KGDB" new for v8
>
>  include/linux/kgdb.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> index 258cdde8d356..76e891ee9e37 100644
> --- a/include/linux/kgdb.h
> +++ b/include/linux/kgdb.h
> @@ -365,5 +365,6 @@ extern void kgdb_free_init_mem(void);
>  #define dbg_late_init()
>  static inline void kgdb_panic(const char *msg) {}
>  static inline void kgdb_free_init_mem(void) { }
> +static inline int kgdb_nmicallback(int cpu, void *regs) { return 1; }

What do you think about landing just ${SUBJECT} patch in kgdb right
now so it can end up in v6.5-rc1? It seems like this series is
currently blocked on Mark getting a spare moment and it seems unlikely
that'll happen this cycle. If we at least land the kgdb patch then it
would make things all that much easier to land in the next cycle. The
kgdb patch feels like it can make sense on its own...

-Doug
Daniel Thompson June 26, 2023, 2:30 p.m. UTC | #2 
On Thu, Jun 15, 2023 at 11:14:18AM -0700, Doug Anderson wrote:
> Daniel,
>
> On Thu, Jun 1, 2023 at 2:37 PM Douglas Anderson <dianders@chromium.org> wrote:
> >
> > To save architectures from needing to wrap the call in #ifdefs, add a
> > stub no-op version of kgdb_nmicallback(), which returns 1 if it didn't
> > handle anything.
> >
> > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > In v9 this is the only kgdb dependency. I'm assuming it could go
> > through the arm64 tree? If that's not a good idea, we could always
> > change the patch ("arm64: kgdb: Roundup cpus using IPI as NMI") not to
> > depend on it by only calling kgdb_nmicallback() if CONFIG_KGDB is not
> > defined.
> >
> > Changes in v9:
> > - Added missing "inline"
> >
> > Changes in v8:
> > - "Provide a stub kgdb_nmicallback() if !CONFIG_KGDB" new for v8
> >
> >  include/linux/kgdb.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> > index 258cdde8d356..76e891ee9e37 100644
> > --- a/include/linux/kgdb.h
> > +++ b/include/linux/kgdb.h
> > @@ -365,5 +365,6 @@ extern void kgdb_free_init_mem(void);
> >  #define dbg_late_init()
> >  static inline void kgdb_panic(const char *msg) {}
> >  static inline void kgdb_free_init_mem(void) { }
> > +static inline int kgdb_nmicallback(int cpu, void *regs) { return 1; }
>
> What do you think about landing just ${SUBJECT} patch in kgdb right
> now so it can end up in v6.5-rc1? It seems like this series is
> currently blocked on Mark getting a spare moment and it seems unlikely
> that'll happen this cycle. If we at least land the kgdb patch then it
> would make things all that much easier to land in the next cycle. The
> kgdb patch feels like it can make sense on its own...

Yes, grabbing this one should be fine!


Daniel.
Mark Rutland Aug. 7, 2023, 10:27 a.m. UTC | #3 
On Thu, Jun 01, 2023 at 02:31:50PM -0700, Douglas Anderson wrote:
> To save architectures from needing to wrap the call in #ifdefs, add a
> stub no-op version of kgdb_nmicallback(), which returns 1 if it didn't
> handle anything.
> 
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> In v9 this is the only kgdb dependency. I'm assuming it could go
> through the arm64 tree? If that's not a good idea, we could always
> change the patch ("arm64: kgdb: Roundup cpus using IPI as NMI") not to
> depend on it by only calling kgdb_nmicallback() if CONFIG_KGDB is not
> defined.
> 
> Changes in v9:
> - Added missing "inline"
> 
> Changes in v8:
> - "Provide a stub kgdb_nmicallback() if !CONFIG_KGDB" new for v8
> 
>  include/linux/kgdb.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> index 258cdde8d356..76e891ee9e37 100644
> --- a/include/linux/kgdb.h
> +++ b/include/linux/kgdb.h
> @@ -365,5 +365,6 @@ extern void kgdb_free_init_mem(void);
>  #define dbg_late_init()
>  static inline void kgdb_panic(const char *msg) {}
>  static inline void kgdb_free_init_mem(void) { }
> +static inline int kgdb_nmicallback(int cpu, void *regs) { return 1; }

Is '1' an error?

Can we return an actual error code if so? It makes it *much* clearer to anyone
looking at the code.

Thanks,
Mark.

>  #endif /* ! CONFIG_KGDB */
>  #endif /* _KGDB_H_ */
> -- 
> 2.41.0.rc2.161.g9c6817b8e7-goog
>
Mark Rutland Aug. 7, 2023, 10:29 a.m. UTC | #4 
On Mon, Aug 07, 2023 at 11:27:00AM +0100, Mark Rutland wrote:
> On Thu, Jun 01, 2023 at 02:31:50PM -0700, Douglas Anderson wrote:
> > +static inline int kgdb_nmicallback(int cpu, void *regs) { return 1; }
> 
> Is '1' an error?
> 
> Can we return an actual error code if so? It makes it *much* clearer to anyone
> looking at the code.

Never mind; I see this was already queued. Sorry for the noise.

Thanks,
Mark.
diff mbox series

Patch

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 258cdde8d356..76e891ee9e37 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -365,5 +365,6 @@  extern void kgdb_free_init_mem(void);
 #define dbg_late_init()
 static inline void kgdb_panic(const char *msg) {}
 static inline void kgdb_free_init_mem(void) { }
+static inline int kgdb_nmicallback(int cpu, void *regs) { return 1; }
 #endif /* ! CONFIG_KGDB */
 #endif /* _KGDB_H_ */