linux-kernel - Re: [PATCH v11 4/6] arm64: smp: Add arch support for backtrace using pseudo-NMI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 25 Aug 2023 17:27:27 -0500
From:   Stephen Boyd <swboyd@...omium.org>
To:     Catalin Marinas <catalin.marinas@....com>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Douglas Anderson <dianders@...omium.org>,
        Marc Zyngier <maz@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Sumit Garg <sumit.garg@...aro.org>,
        Will Deacon <will@...nel.org>
Cc:     "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        Ard Biesheuvel <ardb@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        kgdb-bugreport@...ts.sourceforge.net,
        Lecopzer Chen <lecopzer.chen@...iatek.com>,
        linux-perf-users@...r.kernel.org,
        Masayoshi Mizuma <msys.mizuma@...il.com>,
        Chen-Yu Tsai <wens@...e.org>,
        linux-arm-kernel@...ts.infradead.org, ito-yuichi@...itsu.com,
        Peter Zijlstra <peterz@...radead.org>,
        D Scott Phillips <scott@...amperecomputing.com>,
        Ingo Molnar <mingo@...nel.org>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Valentin Schneider <vschneid@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v11 4/6] arm64: smp: Add arch support for backtrace using pseudo-NMI

Quoting Douglas Anderson (2023-08-24 08:30:30)
> diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
> index fac08e18bcd5..50ce8b697ff3 100644
> --- a/arch/arm64/include/asm/irq.h
> +++ b/arch/arm64/include/asm/irq.h
> @@ -6,6 +6,9 @@
>
>  #include <asm-generic/irq.h>
>
> +void arch_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu);

Some nits, but otherwise

Reviewed-by: Stephen Boyd <swboyd@...omium.org>

> +#define arch_trigger_cpumask_backtrace arch_trigger_cpumask_backtrace
> +
>  struct pt_regs;
>
>  int set_handle_irq(void (*handle_irq)(struct pt_regs *));
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index a5848f1ef817..c8896cbc5327 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -72,12 +73,18 @@ enum ipi_msg_type {
>         IPI_CPU_CRASH_STOP,
>         IPI_TIMER,
>         IPI_IRQ_WORK,
> -       NR_IPI
> +       NR_IPI,
> +       /*
> +        * Any enum >= NR_IPI and < MAX_IPI is special and not tracable
> +        * with trace_ipi_*
> +        */
> +       IPI_CPU_BACKTRACE = NR_IPI,
> +       MAX_IPI
>  };
>
>  static int ipi_irq_base __read_mostly;
>  static int nr_ipi __read_mostly = NR_IPI;
> -static struct irq_desc *ipi_desc[NR_IPI] __read_mostly;
> +static struct irq_desc *ipi_desc[MAX_IPI] __read_mostly;

Side note: it would be nice to mark ipi_desc as __ro_after_init. Same
for nr_ipi and ipi_irq_base.

>
>  static void ipi_setup(int cpu);
>
> @@ -845,6 +852,22 @@ static void __noreturn ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs
>  #endif
>  }
>
> +static void arm64_backtrace_ipi(cpumask_t *mask)
> +{
> +       __ipi_send_mask(ipi_desc[IPI_CPU_BACKTRACE], mask);
> +}
> +
> +void arch_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu)

Can this be 'bool exclude_self' instead of int? That matches all other
implementations from what I can tell.

> +{
> +       /*
> +        * NOTE: though nmi_trigger_cpumask_backtrace has "nmi_" in the name,

USe nmi_trigger_cpumask_backtrace() to indicate function.

> +        * nothing about it truly needs to be implemented using an NMI, it's
> +        * just that it's _allowed_ to work with NMIs. If ipi_should_be_nmi()
> +        * returned false our backtrace attempt will just use a regular IPI.
> +        */
> +       nmi_trigger_cpumask_backtrace(mask, exclude_cpu, arm64_backtrace_ipi);