[PATCH v2 0/3] Enable support IPI_CPU_CRASH_STOP to be pseudo-NMI

[PATCH v2 0/3] Enable support IPI_CPU_CRASH_STOP to be pseudo-NMI

[Yuichi Ito]

This patchset enables IPI_CPU_CRASH_STOP IPI to be pseudo-NMI.
This allows kdump to collect system information even when the CPU is in
a HARDLOCKUP state.

Only IPI_CPU_CRASH_STOP uses NMI and the other IPIs remain normal IRQs.

The patch has been tested on FX1000.

It also uses some of Sumit's IPI patch set for NMI.[1]

[1] https://lore.kernel.org/lkml/1603983387-8738-3-git-send-email-sumit.garg@linaro.org/

$ echo 1 > /proc/sys/kernel/panic_on_rcu_stal
$ echo HARDLOCKUP > /sys/kernel/debug/provoke-crash/DIRECT
   : kernel panics and crash kernel boot
   : makedumpfile saves the system state at HARDLOCKUP in vmcore.

crash utility:
 #7 [fffffe00290afd30] lkdtm_HARDLOCKUP at fffffe0010857ee8
 #8 [fffffe00290afd40] direct_entry at fffffe0010857c94
 #9 [fffffe00290afd90] full_proxy_write at fffffe001055dea0
#10 [fffffe00290afdd0] vfs_write at fffffe001047533c
#11 [fffffe00290afe10] ksys_write at fffffe001047563c
#12 [fffffe00290afe60] __arm64_sys_write at fffffe00104756e8
#13 [fffffe00290afe70] do_el0_svc at fffffe00101590cc
#14 [fffffe00290afea0] el0_svc at fffffe0010147a30
#15 [fffffe00290afeb0] el0_sync_handler at fffffe001014835c
#16 [fffffe00290afff0] el0_sync at fffffe0010142c14

Changes in v1:
- Rebased to head of upstream master.
- Rebased to Sumit's latest IPIs patch-set [1].

[1] https://lore.kernel.org/lkml/1603983387-8738-3-git-send-email-sumit.garg@linaro.org/

- Add conditional branch of local_irq_disable(). 

Sumit Garg (1):
  irqchip/gic-v3: Enable support for SGIs to act as NMIs

Yuichi Ito (2):
  arch64: smp: Register IPI_CPU_CRASH_STOP IPI as pseudo-NMI
  arch64: smp: Disable priority masking when received NMI on PSR.I section

 arch/arm64/kernel/smp.c      | 44 +++++++++++++++++++++++++++++++++++---------
 drivers/irqchip/irq-gic-v3.c | 29 +++++++++++++++++++++--------
 2 files changed, 56 insertions(+), 17 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/3] irqchip/gic-v3: Enable support for SGIs to act as NMIs

[Yuichi Ito]

From: From: Sumit Garg <sumit.garg@linaro.org>

Add support to handle SGIs as pseudo NMIs. As SGIs or IPIs default to a
special flow handler: handle_percpu_devid_fasteoi_ipi(), so skip NMI
handler update in case of SGIs.

Also, enable NMI support prior to gic_smp_init() as allocation of SGIs
as IRQs/NMIs happen as part of this routine.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/irqchip/irq-gic-v3.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 16fecc0..7010ae2 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -461,6 +461,7 @@ static u32 gic_get_ppi_index(struct irq_data *d)
 static int gic_irq_nmi_setup(struct irq_data *d)
 {
 	struct irq_desc *desc = irq_to_desc(d->irq);
+	u32 idx;
 
 	if (!gic_supports_nmi())
 		return -EINVAL;
@@ -478,16 +479,22 @@ static int gic_irq_nmi_setup(struct irq_data *d)
 		return -EINVAL;
 
 	/* desc lock should already be held */
-	if (gic_irq_in_rdist(d)) {
-		u32 idx = gic_get_ppi_index(d);
+	switch (get_intid_range(d)) {
+	case SGI_RANGE:
+		break;
+	case PPI_RANGE:
+	case EPPI_RANGE:
+		idx = gic_get_ppi_index(d);
 
 		/* Setting up PPI as NMI, only switch handler for first NMI */
 		if (!refcount_inc_not_zero(&ppi_nmi_refs[idx])) {
 			refcount_set(&ppi_nmi_refs[idx], 1);
 			desc->handle_irq = handle_percpu_devid_fasteoi_nmi;
 		}
-	} else {
+		break;
+	default:
 		desc->handle_irq = handle_fasteoi_nmi;
+		break;
 	}
 
 	gic_irq_set_prio(d, GICD_INT_NMI_PRI);
@@ -498,6 +505,7 @@ static int gic_irq_nmi_setup(struct irq_data *d)
 static void gic_irq_nmi_teardown(struct irq_data *d)
 {
 	struct irq_desc *desc = irq_to_desc(d->irq);
+	u32 idx;
 
 	if (WARN_ON(!gic_supports_nmi()))
 		return;
@@ -515,14 +523,20 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
 		return;
 
 	/* desc lock should already be held */
-	if (gic_irq_in_rdist(d)) {
-		u32 idx = gic_get_ppi_index(d);
+	switch (get_intid_range(d)) {
+	case SGI_RANGE:
+		break;
+	case PPI_RANGE:
+	case EPPI_RANGE:
+		idx = gic_get_ppi_index(d);
 
 		/* Tearing down NMI, only switch handler for last NMI */
 		if (refcount_dec_and_test(&ppi_nmi_refs[idx]))
 			desc->handle_irq = handle_percpu_devid_irq;
-	} else {
+		break;
+	default:
 		desc->handle_irq = handle_fasteoi_irq;
+		break;
 	}
 
 	gic_irq_set_prio(d, GICD_INT_DEF_PRI);
@@ -1708,6 +1722,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
 
 	gic_dist_init();
 	gic_cpu_init();
+	gic_enable_nmi_support();
 	gic_smp_init();
 	gic_cpu_pm_init();
 
@@ -1719,8 +1734,6 @@ static int __init gic_init_bases(void __iomem *dist_base,
 			gicv2m_init(handle, gic_data.domain);
 	}
 
-	gic_enable_nmi_support();
-
 	return 0;
 
 out_free:
-- 
1.8.3.1

[PATCH v2 2/3] arm64: smp: Register IPI_CPU_CRASH_STOP IPI as pseudo-NMI

[Yuichi Ito]

Register IPI_CPU_CRASH_STOP IPI as pseudo-NMI.
For systems that do not support pseudo-NMI, register as a normal IRQ.

Signed-off-by: Yuichi Ito <ito-yuichi@fujitsu.com>
---
 arch/arm64/kernel/smp.c | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 82e75fc..fd59bc7 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -80,6 +80,8 @@ enum ipi_msg_type {
 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 int ipi_crash_stop = -1;
+static int ipi_crash_stop_enable_nmi;
 
 static void ipi_setup(int cpu);
 
@@ -960,8 +962,16 @@ static void ipi_setup(int cpu)
 	if (WARN_ON_ONCE(!ipi_irq_base))
 		return;
 
-	for (i = 0; i < nr_ipi; i++)
-		enable_percpu_irq(ipi_irq_base + i, 0);
+	for (i = 0; i < nr_ipi; i++) {
+		if (ipi_irq_base + i == ipi_crash_stop) {
+			if (!prepare_percpu_nmi(ipi_irq_base + i)) {
+				enable_percpu_nmi(ipi_irq_base + i, 0);
+				ipi_crash_stop_enable_nmi = 1;
+			} else
+				pr_crit("CPU%u: IPI_CPU_CRASH_STOP cannot be enabled NMI.\n", cpu);
+		} else
+			enable_percpu_irq(ipi_irq_base + i, 0);
+	}
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
@@ -972,24 +982,38 @@ static void ipi_teardown(int cpu)
 	if (WARN_ON_ONCE(!ipi_irq_base))
 		return;
 
-	for (i = 0; i < nr_ipi; i++)
-		disable_percpu_irq(ipi_irq_base + i);
+	for (i = 0; i < nr_ipi; i++) {
+		if (ipi_irq_base + i == ipi_crash_stop) {
+			if (ipi_crash_stop_enable_nmi) {
+				disable_percpu_nmi(ipi_irq_base + i);
+				teardown_percpu_nmi(ipi_irq_base + i);
+			}
+		} else
+			disable_percpu_irq(ipi_irq_base + i);
+	}
 }
 #endif
 
 void __init set_smp_ipi_range(int ipi_base, int n)
 {
-	int i;
+	int i, ret;
 
 	WARN_ON(n < NR_IPI);
 	nr_ipi = min(n, NR_IPI);
 
+	ret = request_percpu_nmi(ipi_base + IPI_CPU_CRASH_STOP,
+				ipi_handler, "IPI", &cpu_number);
+	if (!ret)
+		ipi_crash_stop = ipi_base + IPI_CPU_CRASH_STOP;
+
 	for (i = 0; i < nr_ipi; i++) {
 		int err;
 
-		err = request_percpu_irq(ipi_base + i, ipi_handler,
-					 "IPI", &cpu_number);
-		WARN_ON(err);
+		if (ipi_base + i != ipi_crash_stop) {
+			err = request_percpu_irq(ipi_base + i, ipi_handler,
+						"IPI", &cpu_number);
+			WARN_ON(err);
+		}
 
 		ipi_desc[i] = irq_to_desc(ipi_base + i);
 		irq_set_status_flags(ipi_base + i, IRQ_HIDDEN);
-- 
1.8.3.1

[PATCH v2 3/3] arm64: smp: Disable priority masking when NMI is enable on PSR.I section

[Yuichi Ito]

It should be prohibitted to use priority masking in NMI context.

Using local_irq_disable() under the above conditions causes a WARNING.
Then, there will be also a mismatch between the PSR.I values and PMR GIC_PRIO_PSR_I_SET.

Signed-off-by: Yuichi Ito <ito-yuichi@fujitsu.com>
---
 arch/arm64/kernel/smp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index fd59bc7..3c49f06 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -872,7 +872,9 @@ static void ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs)
 
 	atomic_dec(&waiting_for_crash_ipi);
 
-	local_irq_disable();
+	if(!in_nmi())
+		local_irq_disable();
+
 	sdei_mask_local_cpu();
 
 	if (IS_ENABLED(CONFIG_HOTPLUG_CPU))
-- 
1.8.3.1

RE: [PATCH v2 0/3] Enable support IPI_CPU_CRASH_STOP to be pseudo-NMI

[ito-yuichi]

Hi Marc, Sumit

What should I do to merge this patch.
I would appreciate if you have any advice.

I have not tested it with ThunderX2 yet.

Best regards,

Yuichi Ito

> -----Original Message-----
> From: Yuichi Ito <ito-yuichi@fujitsu.com>
> Sent: Wednesday, November 4, 2020 5:06 PM
> To: maz@kernel.org; sumit.garg@linaro.org; tglx@linutronix.de;
> jason@lakedaemon.net; catalin.marinas@arm.com; will@kernel.org
> Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Ito,
> Yuichi/伊藤 有一 <ito-yuichi@fujitsu.com>
> Subject: [PATCH v2 0/3] Enable support IPI_CPU_CRASH_STOP to be
> pseudo-NMI
> 
> This patchset enables IPI_CPU_CRASH_STOP IPI to be pseudo-NMI.
> This allows kdump to collect system information even when the CPU is in a
> HARDLOCKUP state.
> 
> Only IPI_CPU_CRASH_STOP uses NMI and the other IPIs remain normal
> IRQs.
> 
> The patch has been tested on FX1000.
> 
> It also uses some of Sumit's IPI patch set for NMI.[1]
> 
> [1]
> https://lore.kernel.org/lkml/1603983387-8738-3-git-send-email-sumit.garg@l
> inaro.org/
> 
> $ echo 1 > /proc/sys/kernel/panic_on_rcu_stal
> $ echo HARDLOCKUP > /sys/kernel/debug/provoke-crash/DIRECT
>    : kernel panics and crash kernel boot
>    : makedumpfile saves the system state at HARDLOCKUP in vmcore.
> 
> crash utility:
>  #7 [fffffe00290afd30] lkdtm_HARDLOCKUP at fffffe0010857ee8
>  #8 [fffffe00290afd40] direct_entry at fffffe0010857c94
>  #9 [fffffe00290afd90] full_proxy_write at fffffe001055dea0
> #10 [fffffe00290afdd0] vfs_write at fffffe001047533c
> #11 [fffffe00290afe10] ksys_write at fffffe001047563c
> #12 [fffffe00290afe60] __arm64_sys_write at fffffe00104756e8
> #13 [fffffe00290afe70] do_el0_svc at fffffe00101590cc
> #14 [fffffe00290afea0] el0_svc at fffffe0010147a30
> #15 [fffffe00290afeb0] el0_sync_handler at fffffe001014835c
> #16 [fffffe00290afff0] el0_sync at fffffe0010142c14
> 
> Changes in v1:
> - Rebased to head of upstream master.
> - Rebased to Sumit's latest IPIs patch-set [1].
> 
> [1]
> https://lore.kernel.org/lkml/1603983387-8738-3-git-send-email-sumit.garg@l
> inaro.org/
> 
> - Add conditional branch of local_irq_disable().
> 
> Sumit Garg (1):
>   irqchip/gic-v3: Enable support for SGIs to act as NMIs
> 
> Yuichi Ito (2):
>   arch64: smp: Register IPI_CPU_CRASH_STOP IPI as pseudo-NMI
>   arch64: smp: Disable priority masking when received NMI on PSR.I section
> 
>  arch/arm64/kernel/smp.c      | 44
> +++++++++++++++++++++++++++++++++++---------
>  drivers/irqchip/irq-gic-v3.c | 29 +++++++++++++++++++++--------
>  2 files changed, 56 insertions(+), 17 deletions(-)
> 
> --
> 1.8.3.1

Re: [PATCH v2 1/3] irqchip/gic-v3: Enable support for SGIs to act as NMIs

[Masayoshi Mizuma]

Hi Yuichi-san,

This patch is under review here:
https://lore.kernel.org/linux-arm-kernel/1604317487-14543-3-git-send-email-sumit.garg@linaro.org/

So, it would be great if you could send your feedback to the
thread; testing, code review.

Thanks,
Masa

On Wed, Nov 04, 2020 at 05:05:37PM +0900, Yuichi Ito wrote:
> From: From: Sumit Garg <sumit.garg@linaro.org>
> 
> Add support to handle SGIs as pseudo NMIs. As SGIs or IPIs default to a
> special flow handler: handle_percpu_devid_fasteoi_ipi(), so skip NMI
> handler update in case of SGIs.
> 
> Also, enable NMI support prior to gic_smp_init() as allocation of SGIs
> as IRQs/NMIs happen as part of this routine.
> 
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
>  drivers/irqchip/irq-gic-v3.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 16fecc0..7010ae2 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -461,6 +461,7 @@ static u32 gic_get_ppi_index(struct irq_data *d)
>  static int gic_irq_nmi_setup(struct irq_data *d)
>  {
>  	struct irq_desc *desc = irq_to_desc(d->irq);
> +	u32 idx;
>  
>  	if (!gic_supports_nmi())
>  		return -EINVAL;
> @@ -478,16 +479,22 @@ static int gic_irq_nmi_setup(struct irq_data *d)
>  		return -EINVAL;
>  
>  	/* desc lock should already be held */
> -	if (gic_irq_in_rdist(d)) {
> -		u32 idx = gic_get_ppi_index(d);
> +	switch (get_intid_range(d)) {
> +	case SGI_RANGE:
> +		break;
> +	case PPI_RANGE:
> +	case EPPI_RANGE:
> +		idx = gic_get_ppi_index(d);
>  
>  		/* Setting up PPI as NMI, only switch handler for first NMI */
>  		if (!refcount_inc_not_zero(&ppi_nmi_refs[idx])) {
>  			refcount_set(&ppi_nmi_refs[idx], 1);
>  			desc->handle_irq = handle_percpu_devid_fasteoi_nmi;
>  		}
> -	} else {
> +		break;
> +	default:
>  		desc->handle_irq = handle_fasteoi_nmi;
> +		break;
>  	}
>  
>  	gic_irq_set_prio(d, GICD_INT_NMI_PRI);
> @@ -498,6 +505,7 @@ static int gic_irq_nmi_setup(struct irq_data *d)
>  static void gic_irq_nmi_teardown(struct irq_data *d)
>  {
>  	struct irq_desc *desc = irq_to_desc(d->irq);
> +	u32 idx;
>  
>  	if (WARN_ON(!gic_supports_nmi()))
>  		return;
> @@ -515,14 +523,20 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
>  		return;
>  
>  	/* desc lock should already be held */
> -	if (gic_irq_in_rdist(d)) {
> -		u32 idx = gic_get_ppi_index(d);
> +	switch (get_intid_range(d)) {
> +	case SGI_RANGE:
> +		break;
> +	case PPI_RANGE:
> +	case EPPI_RANGE:
> +		idx = gic_get_ppi_index(d);
>  
>  		/* Tearing down NMI, only switch handler for last NMI */
>  		if (refcount_dec_and_test(&ppi_nmi_refs[idx]))
>  			desc->handle_irq = handle_percpu_devid_irq;
> -	} else {
> +		break;
> +	default:
>  		desc->handle_irq = handle_fasteoi_irq;
> +		break;
>  	}
>  
>  	gic_irq_set_prio(d, GICD_INT_DEF_PRI);
> @@ -1708,6 +1722,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
>  
>  	gic_dist_init();
>  	gic_cpu_init();
> +	gic_enable_nmi_support();
>  	gic_smp_init();
>  	gic_cpu_pm_init();
>  
> @@ -1719,8 +1734,6 @@ static int __init gic_init_bases(void __iomem *dist_base,
>  			gicv2m_init(handle, gic_data.domain);
>  	}
>  
> -	gic_enable_nmi_support();
> -
>  	return 0;
>  
>  out_free:
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel