Skip to contentSkip to conversation

arm64: kexec: Add machine_kexec_mask_interrupts to kexec path

PATCH New Sameer Goel 3 participants 4 messages Apr 19, 2018 – Apr 20, 2018 arm64

lore
Share & tools
lore

Sameer Goel

author
#0 lore raw

PATCH 1/1 arm64: kexec: Add machine_kexec_mask_interrupts to kexec path

In kdump code path SPIs at GIC level are explicitly cleared by calling machine_kexec_mask_interrupts. In case of kexec this functionality is delegated for most part to the device shutdown functions.
Add machine_kexec_mask_interrupts to machine_shutdown path to ensure that the irqs are masked at the gic level.

Signed-off-by Sameer Goel <[email protected]>

---
Porting code for masking irqs at gic from kdump shutdown to kexec path.

There was discussion about this functionality in [1].

[1] https://patchwork.kernel.org/patch/9817499/

 arch/arm64/include/asm/kexec.h    | 6 ++++++
 arch/arm64/kernel/machine_kexec.c | 2 +-
 arch/arm64/kernel/process.c       | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)
arch/arm64/include/asm/kexec.h +6 -0
index e17f052..f9ddb41 100644
@@ -93,6 +93,12 @@ static inline void crash_prepare_suspend(void) {}
 static inline void crash_post_resume(void) {}
 #endif
 
+#if defined(CONFIG_KEXEC)
+extern void machine_kexec_mask_interrupts(void);
+#else
+static inline void machine_kexec_mask_interrupts(void) {}
+#endif
+
 #endif /* __ASSEMBLY__ */
 
 #endif
arch/arm64/kernel/machine_kexec.c +1 -1
index f76ea92..b453180 100644
@@ -213,7 +213,7 @@ void machine_kexec(struct kimage *kimage)
 	BUG(); /* Should never get here. */
 }
 
-static void machine_kexec_mask_interrupts(void)
+void machine_kexec_mask_interrupts(void)
 {
 	unsigned int i;
 	struct irq_desc *desc;
arch/arm64/kernel/process.c +2 -0
index f08a2ed..9d0337e 100644
@@ -58,6 +58,7 @@
 #include <asm/mmu_context.h>
 #include <asm/processor.h>
 #include <asm/stacktrace.h>
+#include <asm/kexec.h>
 
 #ifdef CONFIG_CC_STACKPROTECTOR
 #include <linux/stackprotector.h>

@@ -107,6 +108,7 @@ void arch_cpu_idle_dead(void)
 void machine_shutdown(void)
 {
 	disable_nonboot_cpus();
+	machine_kexec_mask_interrupts();
 }
 
 /*
Signature · 1 line
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
#1 lore raw

PATCH 1/1 arm64: kexec: Add machine_kexec_mask_interrupts to kexec path

Quoted 6 lines · Sameer Goel

In kdump code path SPIs at GIC level are explicitly cleared by calling machine_kexec_mask_interrupts. In case of kexec this functionality is delegated for most part to the device shutdown functions.
Add machine_kexec_mask_interrupts to machine_shutdown path to ensure that the irqs are masked at the gic level.

I'm not sure that this change has visible effect, but anyway

Quoted 18 lines
Signed-off-by Sameer Goel <[email protected]>

---
Porting code for masking irqs at gic from kdump shutdown to kexec path.

There was discussion about this functionality in [1].

[1] https://patchwork.kernel.org/patch/9817499/

 arch/arm64/include/asm/kexec.h    | 6 ++++++
 arch/arm64/kernel/machine_kexec.c | 2 +-
 arch/arm64/kernel/process.c       | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)
arch/arm64/include/asm/kexec.h +0 -0
index e17f052..f9ddb41 100644

arch/arm64/include/asm/kexec.h static inline void crash_prepare_suspend(void) {} · line 96

Quoted 4 lines
arch/arm64/include/asm/kexec.h +1 -0
@@ -93,6 +93,12 @@ static inline void crash_prepare_suspend(void) {}
 static inline void crash_post_resume(void) {}
 #endif
 
+#if defined(CONFIG_KEXEC)

CONFIG_KEXEC_CORE would be better.

Thanks,
-Takahiro AKASHI

Quoted 44 lines
arch/arm64/include/asm/kexec.h +5 -0
+extern void machine_kexec_mask_interrupts(void);
+#else
+static inline void machine_kexec_mask_interrupts(void) {}
+#endif
+
 #endif /* __ASSEMBLY__ */
 
 #endif
arch/arm64/kernel/machine_kexec.c +1 -1
index f76ea92..b453180 100644
@@ -213,7 +213,7 @@ void machine_kexec(struct kimage *kimage)
 	BUG(); /* Should never get here. */
 }
 
-static void machine_kexec_mask_interrupts(void)
+void machine_kexec_mask_interrupts(void)
 {
 	unsigned int i;
 	struct irq_desc *desc;
arch/arm64/kernel/process.c +2 -0
index f08a2ed..9d0337e 100644
@@ -58,6 +58,7 @@
 #include <asm/mmu_context.h>
 #include <asm/processor.h>
 #include <asm/stacktrace.h>
+#include <asm/kexec.h>
 
 #ifdef CONFIG_CC_STACKPROTECTOR
 #include <linux/stackprotector.h>

@@ -107,6 +108,7 @@ void arch_cpu_idle_dead(void)
 void machine_shutdown(void)
 {
 	disable_nonboot_cpus();
+	machine_kexec_mask_interrupts();
 }
 
 /*
Signature · 1 line
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
reviewer
#2 lore raw

PATCH 1/1 arm64: kexec: Add machine_kexec_mask_interrupts to kexec path

In reply to Sameer Goel

Hi Sameer,

Quoted 6 lines · Sameer Goel

In kdump code path SPIs at GIC level are explicitly cleared by calling machine_kexec_mask_interrupts. In case of kexec this functionality is delegated for most part to the device shutdown functions.
Add machine_kexec_mask_interrupts to machine_shutdown path to ensure that the irqs are masked at the gic level.

I think that for kdump, the plan is to try to get irqchips to reset things in their probe paths (e.g. as that's the sanest place to poke APRn). Doing that would render machine_kexec_mask_interrupts() redundant for kexec.

Quoted 7 lines
Signed-off-by Sameer Goel <[email protected]>

---
Porting code for masking irqs at gic from kdump shutdown to kexec path.

There was discussion about this functionality in [1].

[1] https://patchwork.kernel.org/patch/9817499/

I don't follow why this is a problem. Nate mentions corrupting the new kernel -- has a problem been observed in practice?

The purgatory code is run with IRQs masked at the CPU, and the new kernel will initialise the GIC before enabling interrupts at the CPU. Once probed, the SMMU driver can handle spurious interrupts.

So AFAICT there is no problem today. Am I missing something?

When could an interrupt be taken that would be problematic?

Currently, I don't see that this is necessary. Given we'd like to remove machine_kexec_mask_interrupts() for the kdump case, I'd prefer that we don't mandate it in the usual kexec case.

Thanks,
Mark.

Quoted 64 lines
 arch/arm64/include/asm/kexec.h    | 6 ++++++
 arch/arm64/kernel/machine_kexec.c | 2 +-
 arch/arm64/kernel/process.c       | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)
arch/arm64/include/asm/kexec.h +6 -0
index e17f052..f9ddb41 100644
@@ -93,6 +93,12 @@ static inline void crash_prepare_suspend(void) {}
 static inline void crash_post_resume(void) {}
 #endif
 
+#if defined(CONFIG_KEXEC)
+extern void machine_kexec_mask_interrupts(void);
+#else
+static inline void machine_kexec_mask_interrupts(void) {}
+#endif
+
 #endif /* __ASSEMBLY__ */
 
 #endif
arch/arm64/kernel/machine_kexec.c +1 -1
index f76ea92..b453180 100644
@@ -213,7 +213,7 @@ void machine_kexec(struct kimage *kimage)
 	BUG(); /* Should never get here. */
 }
 
-static void machine_kexec_mask_interrupts(void)
+void machine_kexec_mask_interrupts(void)
 {
 	unsigned int i;
 	struct irq_desc *desc;
arch/arm64/kernel/process.c +2 -0
index f08a2ed..9d0337e 100644
@@ -58,6 +58,7 @@
 #include <asm/mmu_context.h>
 #include <asm/processor.h>
 #include <asm/stacktrace.h>
+#include <asm/kexec.h>
 
 #ifdef CONFIG_CC_STACKPROTECTOR
 #include <linux/stackprotector.h>

@@ -107,6 +108,7 @@ void arch_cpu_idle_dead(void)
 void machine_shutdown(void)
 {
 	disable_nonboot_cpus();
+	machine_kexec_mask_interrupts();
 }
 
 /*
Signature · 7 lines
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
author
#3 lore raw

PATCH 1/1 arm64: kexec: Add machine_kexec_mask_interrupts to kexec path

Quoted 10 lines · Mark Rutland

Hi Sameer,

Quoted 6 lines · Sameer Goel

In kdump code path SPIs at GIC level are explicitly cleared by calling machine_kexec_mask_interrupts. In case of kexec this functionality is delegated for most part to the device shutdown functions.
Add machine_kexec_mask_interrupts to machine_shutdown path to ensure that the irqs are masked at the gic level.

Quoted 4 lines

I think that for kdump, the plan is to try to get irqchips to reset things in their probe paths (e.g. as that's the sanest place to poke APRn). Doing that would render machine_kexec_mask_interrupts() redundant for kexec.

I agree the above proposed change in GIC will make machine_kexec_mask_interrupts() redundant. Can you please point me to the change if already posted.

Quoted 9 lines
Quoted 8 lines
Signed-off-by Sameer Goel <[email protected]>

---
Porting code for masking irqs at gic from kdump shutdown to kexec path.

There was discussion about this functionality in [1].

[1] https://patchwork.kernel.org/patch/9817499/

Quoted 8 lines

I don't follow why this is a problem. Nate mentions corrupting the new kernel -- has a problem been observed in practice?

The purgatory code is run with IRQs masked at the CPU, and the new kernel will initialise the GIC before enabling interrupts at the CPU. Once probed, the SMMU driver can handle spurious interrupts.

So AFAICT there is no problem today. Am I missing something?

This was something that was proposed when the driver owners were not clearing interrupts in their shutdown functions. So, more of a cleanup. From you description above it makes sense.

Thanks,
Sameer

Quoted 75 lines

When could an interrupt be taken that would be problematic?

Currently, I don't see that this is necessary. Given we'd like to remove machine_kexec_mask_interrupts() for the kdump case, I'd prefer that we don't mandate it in the usual kexec case.

Thanks,
Mark.

Quoted 64 lines
 arch/arm64/include/asm/kexec.h    | 6 ++++++
 arch/arm64/kernel/machine_kexec.c | 2 +-
 arch/arm64/kernel/process.c       | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)
arch/arm64/include/asm/kexec.h +6 -0
index e17f052..f9ddb41 100644
@@ -93,6 +93,12 @@ static inline void crash_prepare_suspend(void) {}
 static inline void crash_post_resume(void) {}
 #endif
 
+#if defined(CONFIG_KEXEC)
+extern void machine_kexec_mask_interrupts(void);
+#else
+static inline void machine_kexec_mask_interrupts(void) {}
+#endif
+
 #endif /* __ASSEMBLY__ */
 
 #endif
arch/arm64/kernel/machine_kexec.c +1 -1
index f76ea92..b453180 100644
@@ -213,7 +213,7 @@ void machine_kexec(struct kimage *kimage)
 	BUG(); /* Should never get here. */
 }
 
-static void machine_kexec_mask_interrupts(void)
+void machine_kexec_mask_interrupts(void)
 {
 	unsigned int i;
 	struct irq_desc *desc;
arch/arm64/kernel/process.c +2 -0
index f08a2ed..9d0337e 100644
@@ -58,6 +58,7 @@
 #include <asm/mmu_context.h>
 #include <asm/processor.h>
 #include <asm/stacktrace.h>
+#include <asm/kexec.h>
 
 #ifdef CONFIG_CC_STACKPROTECTOR
 #include <linux/stackprotector.h>

@@ -107,6 +108,7 @@ void arch_cpu_idle_dead(void)
 void machine_shutdown(void)
 {
 	disable_nonboot_cpus();
+	machine_kexec_mask_interrupts();
 }
 
 /*
Signature · 7 lines
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 1 line
 Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.