Re: [PATCH v29 4/9] arm64: kdump: implement machine_crash_shutdown()
Will, On Mon, Jan 23, 2017 at 05:46:34PM +, Will Deacon wrote: > On Thu, Jan 12, 2017 at 12:01:11PM +, Will Deacon wrote: > > On Thu, Jan 12, 2017 at 01:21:44PM +0900, AKASHI Takahiro wrote: > > > On Wed, Jan 11, 2017 at 10:54:05AM +, Will Deacon wrote: > > > > On Wed, Jan 11, 2017 at 03:36:28PM +0900, AKASHI Takahiro wrote: > > > > > On Tue, Jan 10, 2017 at 11:32:48AM +, Will Deacon wrote: > > > > > > On Wed, Dec 28, 2016 at 01:36:01PM +0900, AKASHI Takahiro wrote: > > > > > > > @@ -22,6 +25,7 @@ > > > > > > > extern const unsigned char arm64_relocate_new_kernel[]; > > > > > > > extern const unsigned long arm64_relocate_new_kernel_size; > > > > > > > > > > > > > > +static bool in_crash_kexec; > > > > > > > > > > > > Do you actually need this bool? Why not call kexec_crash_loaded() > > > > > > instead? > > > > > > > > > > The two have different meanings: > > > > > "in_crash_kexec" indicates that kdump is taking place, while > > > > > kexec_crash_loaded() tells us only whether crash dump kernel has been > > > > > loaded or not. > > > > > > > > > > It is crucial to distinguish them especially for machine_kexec() > > > > > which can be called on normal kexec even if kdump has been set up. > > > > > > > > Ah, I see. So how about just doing: > > > > > > > > if (kimage == kexec_crash_image) > > > > > > > > in machine_kexec? > > > > > > Yeah, it should work. > > > Do you want to merge the following hunk, > > > or expect that I will re-send the whole patch series > > > (with other changes if any)? > > > > Thanks, I'll fold it in and shout if I run into any problems. My plan is > > to queue this for 4.11. > > Given the DT discussion with Mark, I assume you'll post a new version with > this rolled in. Yes, I will! -Takahiro AKASHI > Will ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v29 4/9] arm64: kdump: implement machine_crash_shutdown()
On Thu, Jan 12, 2017 at 12:01:11PM +, Will Deacon wrote: > On Thu, Jan 12, 2017 at 01:21:44PM +0900, AKASHI Takahiro wrote: > > On Wed, Jan 11, 2017 at 10:54:05AM +, Will Deacon wrote: > > > On Wed, Jan 11, 2017 at 03:36:28PM +0900, AKASHI Takahiro wrote: > > > > On Tue, Jan 10, 2017 at 11:32:48AM +, Will Deacon wrote: > > > > > On Wed, Dec 28, 2016 at 01:36:01PM +0900, AKASHI Takahiro wrote: > > > > > > @@ -22,6 +25,7 @@ > > > > > > extern const unsigned char arm64_relocate_new_kernel[]; > > > > > > extern const unsigned long arm64_relocate_new_kernel_size; > > > > > > > > > > > > +static bool in_crash_kexec; > > > > > > > > > > Do you actually need this bool? Why not call kexec_crash_loaded() > > > > > instead? > > > > > > > > The two have different meanings: > > > > "in_crash_kexec" indicates that kdump is taking place, while > > > > kexec_crash_loaded() tells us only whether crash dump kernel has been > > > > loaded or not. > > > > > > > > It is crucial to distinguish them especially for machine_kexec() > > > > which can be called on normal kexec even if kdump has been set up. > > > > > > Ah, I see. So how about just doing: > > > > > > if (kimage == kexec_crash_image) > > > > > > in machine_kexec? > > > > Yeah, it should work. > > Do you want to merge the following hunk, > > or expect that I will re-send the whole patch series > > (with other changes if any)? > > Thanks, I'll fold it in and shout if I run into any problems. My plan is > to queue this for 4.11. Given the DT discussion with Mark, I assume you'll post a new version with this rolled in. Will ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v29 4/9] arm64: kdump: implement machine_crash_shutdown()
On Thu, Jan 12, 2017 at 01:21:44PM +0900, AKASHI Takahiro wrote: > On Wed, Jan 11, 2017 at 10:54:05AM +, Will Deacon wrote: > > On Wed, Jan 11, 2017 at 03:36:28PM +0900, AKASHI Takahiro wrote: > > > On Tue, Jan 10, 2017 at 11:32:48AM +, Will Deacon wrote: > > > > On Wed, Dec 28, 2016 at 01:36:01PM +0900, AKASHI Takahiro wrote: > > > > > @@ -22,6 +25,7 @@ > > > > > extern const unsigned char arm64_relocate_new_kernel[]; > > > > > extern const unsigned long arm64_relocate_new_kernel_size; > > > > > > > > > > +static bool in_crash_kexec; > > > > > > > > Do you actually need this bool? Why not call kexec_crash_loaded() > > > > instead? > > > > > > The two have different meanings: > > > "in_crash_kexec" indicates that kdump is taking place, while > > > kexec_crash_loaded() tells us only whether crash dump kernel has been > > > loaded or not. > > > > > > It is crucial to distinguish them especially for machine_kexec() > > > which can be called on normal kexec even if kdump has been set up. > > > > Ah, I see. So how about just doing: > > > > if (kimage == kexec_crash_image) > > > > in machine_kexec? > > Yeah, it should work. > Do you want to merge the following hunk, > or expect that I will re-send the whole patch series > (with other changes if any)? Thanks, I'll fold it in and shout if I run into any problems. My plan is to queue this for 4.11. Will ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v29 4/9] arm64: kdump: implement machine_crash_shutdown()
On Wed, Jan 11, 2017 at 10:54:05AM +, Will Deacon wrote:
> On Wed, Jan 11, 2017 at 03:36:28PM +0900, AKASHI Takahiro wrote:
> > On Tue, Jan 10, 2017 at 11:32:48AM +, Will Deacon wrote:
> > > On Wed, Dec 28, 2016 at 01:36:01PM +0900, AKASHI Takahiro wrote:
> > > > @@ -22,6 +25,7 @@
> > > > extern const unsigned char arm64_relocate_new_kernel[];
> > > > extern const unsigned long arm64_relocate_new_kernel_size;
> > > >
> > > > +static bool in_crash_kexec;
> > >
> > > Do you actually need this bool? Why not call kexec_crash_loaded() instead?
> >
> > The two have different meanings:
> > "in_crash_kexec" indicates that kdump is taking place, while
> > kexec_crash_loaded() tells us only whether crash dump kernel has been
> > loaded or not.
> >
> > It is crucial to distinguish them especially for machine_kexec()
> > which can be called on normal kexec even if kdump has been set up.
>
> Ah, I see. So how about just doing:
>
> if (kimage == kexec_crash_image)
>
> in machine_kexec?
Yeah, it should work.
Do you want to merge the following hunk,
or expect that I will re-send the whole patch series
(with other changes if any)?
-Takahiro AkASHI
> Will
===8<==
diff --git a/arch/arm64/kernel/machine_kexec.c
b/arch/arm64/kernel/machine_kexec.c
index 994fe0bc5cc0..c0fc3d458195 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -26,7 +26,6 @@
extern const unsigned char arm64_relocate_new_kernel[];
extern const unsigned long arm64_relocate_new_kernel_size;
-static bool in_crash_kexec;
static unsigned long kimage_start;
/**
@@ -154,7 +153,7 @@ void machine_kexec(struct kimage *kimage)
* New cpus may have become stuck_in_kernel after we loaded the image.
*/
BUG_ON((cpus_are_stuck_in_kernel() || (num_online_cpus() > 1)) &&
- !WARN_ON(in_crash_kexec));
+ !WARN_ON(kimage == kexec_crash_image));
reboot_code_buffer_phys = page_to_phys(kimage->control_code_page);
reboot_code_buffer = phys_to_virt(reboot_code_buffer_phys);
@@ -206,8 +205,8 @@ void machine_kexec(struct kimage *kimage)
* relocation is complete.
*/
- cpu_soft_restart(!in_crash_kexec, reboot_code_buffer_phys, kimage->head,
- kimage_start, 0);
+ cpu_soft_restart(kimage != kexec_crash_image,
+ reboot_code_buffer_phys, kimage->head, kimage_start, 0);
BUG(); /* Should never get here. */
}
@@ -250,8 +249,6 @@ void machine_crash_shutdown(struct pt_regs *regs)
{
local_irq_disable();
- in_crash_kexec = true;
-
/* shutdown non-crashing cpus */
smp_send_crash_stop();
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v29 4/9] arm64: kdump: implement machine_crash_shutdown()
On Wed, Jan 11, 2017 at 03:36:28PM +0900, AKASHI Takahiro wrote: > On Tue, Jan 10, 2017 at 11:32:48AM +, Will Deacon wrote: > > On Wed, Dec 28, 2016 at 01:36:01PM +0900, AKASHI Takahiro wrote: > > > @@ -22,6 +25,7 @@ > > > extern const unsigned char arm64_relocate_new_kernel[]; > > > extern const unsigned long arm64_relocate_new_kernel_size; > > > > > > +static bool in_crash_kexec; > > > > Do you actually need this bool? Why not call kexec_crash_loaded() instead? > > The two have different meanings: > "in_crash_kexec" indicates that kdump is taking place, while > kexec_crash_loaded() tells us only whether crash dump kernel has been > loaded or not. > > It is crucial to distinguish them especially for machine_kexec() > which can be called on normal kexec even if kdump has been set up. Ah, I see. So how about just doing: if (kimage == kexec_crash_image) in machine_kexec? Will ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v29 4/9] arm64: kdump: implement machine_crash_shutdown()
Hi Will, On Tue, Jan 10, 2017 at 11:32:48AM +, Will Deacon wrote: > On Wed, Dec 28, 2016 at 01:36:01PM +0900, AKASHI Takahiro wrote: > > Primary kernel calls machine_crash_shutdown() to shut down non-boot cpus > > and save registers' status in per-cpu ELF notes before starting crash > > dump kernel. See kernel_kexec(). > > Even if not all secondary cpus have shut down, we do kdump anyway. > > > > As we don't have to make non-boot(crashed) cpus offline (to preserve > > correct status of cpus at crash dump) before shutting down, this patch > > also adds a variant of smp_send_stop(). > > > > Signed-off-by: AKASHI Takahiro > > Reviewed-by: James Morse > > Acked-by: Catalin Marinas > > --- > > arch/arm64/include/asm/hardirq.h | 2 +- > > arch/arm64/include/asm/kexec.h| 42 +- > > arch/arm64/include/asm/smp.h | 2 ++ > > arch/arm64/kernel/machine_kexec.c | 56 -- > > arch/arm64/kernel/smp.c | 63 > > +++ > > 5 files changed, 160 insertions(+), 5 deletions(-) > > [...] > > > diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h > > index d050d720a1b4..cea009f2657d 100644 > > --- a/arch/arm64/include/asm/smp.h > > +++ b/arch/arm64/include/asm/smp.h > > @@ -148,6 +148,8 @@ static inline void cpu_panic_kernel(void) > > */ > > bool cpus_are_stuck_in_kernel(void); > > > > +extern void smp_send_crash_stop(void); > > + > > #endif /* ifndef __ASSEMBLY__ */ > > > > #endif /* ifndef __ASM_SMP_H */ > > diff --git a/arch/arm64/kernel/machine_kexec.c > > b/arch/arm64/kernel/machine_kexec.c > > index bc96c8a7fc79..c60346d33bb1 100644 > > --- a/arch/arm64/kernel/machine_kexec.c > > +++ b/arch/arm64/kernel/machine_kexec.c > > @@ -9,6 +9,9 @@ > > * published by the Free Software Foundation. > > */ > > > > +#include > > +#include > > +#include > > #include > > #include > > > > @@ -22,6 +25,7 @@ > > extern const unsigned char arm64_relocate_new_kernel[]; > > extern const unsigned long arm64_relocate_new_kernel_size; > > > > +static bool in_crash_kexec; > > Do you actually need this bool? Why not call kexec_crash_loaded() instead? The two have different meanings: "in_crash_kexec" indicates that kdump is taking place, while kexec_crash_loaded() tells us only whether crash dump kernel has been loaded or not. It is crucial to distinguish them especially for machine_kexec() which can be called on normal kexec even if kdump has been set up. Thanks, -Takahiro AKASHI > Will ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v29 4/9] arm64: kdump: implement machine_crash_shutdown()
On Wed, Dec 28, 2016 at 01:36:01PM +0900, AKASHI Takahiro wrote: > Primary kernel calls machine_crash_shutdown() to shut down non-boot cpus > and save registers' status in per-cpu ELF notes before starting crash > dump kernel. See kernel_kexec(). > Even if not all secondary cpus have shut down, we do kdump anyway. > > As we don't have to make non-boot(crashed) cpus offline (to preserve > correct status of cpus at crash dump) before shutting down, this patch > also adds a variant of smp_send_stop(). > > Signed-off-by: AKASHI Takahiro > Reviewed-by: James Morse > Acked-by: Catalin Marinas > --- > arch/arm64/include/asm/hardirq.h | 2 +- > arch/arm64/include/asm/kexec.h| 42 +- > arch/arm64/include/asm/smp.h | 2 ++ > arch/arm64/kernel/machine_kexec.c | 56 -- > arch/arm64/kernel/smp.c | 63 > +++ > 5 files changed, 160 insertions(+), 5 deletions(-) [...] > diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h > index d050d720a1b4..cea009f2657d 100644 > --- a/arch/arm64/include/asm/smp.h > +++ b/arch/arm64/include/asm/smp.h > @@ -148,6 +148,8 @@ static inline void cpu_panic_kernel(void) > */ > bool cpus_are_stuck_in_kernel(void); > > +extern void smp_send_crash_stop(void); > + > #endif /* ifndef __ASSEMBLY__ */ > > #endif /* ifndef __ASM_SMP_H */ > diff --git a/arch/arm64/kernel/machine_kexec.c > b/arch/arm64/kernel/machine_kexec.c > index bc96c8a7fc79..c60346d33bb1 100644 > --- a/arch/arm64/kernel/machine_kexec.c > +++ b/arch/arm64/kernel/machine_kexec.c > @@ -9,6 +9,9 @@ > * published by the Free Software Foundation. > */ > > +#include > +#include > +#include > #include > #include > > @@ -22,6 +25,7 @@ > extern const unsigned char arm64_relocate_new_kernel[]; > extern const unsigned long arm64_relocate_new_kernel_size; > > +static bool in_crash_kexec; Do you actually need this bool? Why not call kexec_crash_loaded() instead? Will ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v29 4/9] arm64: kdump: implement machine_crash_shutdown()
Primary kernel calls machine_crash_shutdown() to shut down non-boot cpus
and save registers' status in per-cpu ELF notes before starting crash
dump kernel. See kernel_kexec().
Even if not all secondary cpus have shut down, we do kdump anyway.
As we don't have to make non-boot(crashed) cpus offline (to preserve
correct status of cpus at crash dump) before shutting down, this patch
also adds a variant of smp_send_stop().
Signed-off-by: AKASHI Takahiro
Reviewed-by: James Morse
Acked-by: Catalin Marinas
---
arch/arm64/include/asm/hardirq.h | 2 +-
arch/arm64/include/asm/kexec.h| 42 +-
arch/arm64/include/asm/smp.h | 2 ++
arch/arm64/kernel/machine_kexec.c | 56 --
arch/arm64/kernel/smp.c | 63 +++
5 files changed, 160 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
index 8740297dac77..1473fc2f7ab7 100644
--- a/arch/arm64/include/asm/hardirq.h
+++ b/arch/arm64/include/asm/hardirq.h
@@ -20,7 +20,7 @@
#include
#include
-#define NR_IPI 6
+#define NR_IPI 7
typedef struct {
unsigned int __softirq_pending;
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 04744dc5fb61..f40ace1fa21a 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -40,7 +40,47 @@
static inline void crash_setup_regs(struct pt_regs *newregs,
struct pt_regs *oldregs)
{
- /* Empty routine needed to avoid build errors. */
+ if (oldregs) {
+ memcpy(newregs, oldregs, sizeof(*newregs));
+ } else {
+ u64 tmp1, tmp2;
+
+ __asm__ __volatile__ (
+ "stp x0, x1, [%2, #16 * 0]\n"
+ "stp x2, x3, [%2, #16 * 1]\n"
+ "stp x4, x5, [%2, #16 * 2]\n"
+ "stp x6, x7, [%2, #16 * 3]\n"
+ "stp x8, x9, [%2, #16 * 4]\n"
+ "stpx10, x11, [%2, #16 * 5]\n"
+ "stpx12, x13, [%2, #16 * 6]\n"
+ "stpx14, x15, [%2, #16 * 7]\n"
+ "stpx16, x17, [%2, #16 * 8]\n"
+ "stpx18, x19, [%2, #16 * 9]\n"
+ "stpx20, x21, [%2, #16 * 10]\n"
+ "stpx22, x23, [%2, #16 * 11]\n"
+ "stpx24, x25, [%2, #16 * 12]\n"
+ "stpx26, x27, [%2, #16 * 13]\n"
+ "stpx28, x29, [%2, #16 * 14]\n"
+ "mov %0, sp\n"
+ "stpx30, %0, [%2, #16 * 15]\n"
+
+ "/* faked current PSTATE */\n"
+ "mrs %0, CurrentEL\n"
+ "mrs %1, SPSEL\n"
+ "orr %0, %0, %1\n"
+ "mrs %1, DAIF\n"
+ "orr %0, %0, %1\n"
+ "mrs %1, NZCV\n"
+ "orr %0, %0, %1\n"
+ /* pc */
+ "adr %1, 1f\n"
+ "1:\n"
+ "stp %1, %0, [%2, #16 * 16]\n"
+ : "=&r" (tmp1), "=&r" (tmp2)
+ : "r" (newregs)
+ : "memory"
+ );
+ }
}
#endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
index d050d720a1b4..cea009f2657d 100644
--- a/arch/arm64/include/asm/smp.h
+++ b/arch/arm64/include/asm/smp.h
@@ -148,6 +148,8 @@ static inline void cpu_panic_kernel(void)
*/
bool cpus_are_stuck_in_kernel(void);
+extern void smp_send_crash_stop(void);
+
#endif /* ifndef __ASSEMBLY__ */
#endif /* ifndef __ASM_SMP_H */
diff --git a/arch/arm64/kernel/machine_kexec.c
b/arch/arm64/kernel/machine_kexec.c
index bc96c8a7fc79..c60346d33bb1 100644
--- a/arch/arm64/kernel/machine_kexec.c
+++ b/arch/arm64/kernel/machine_kexec.c
@@ -9,6 +9,9 @@
* published by the Free Software Foundation.
*/
+#include
+#include
+#include
#include
#include
@@ -22,6 +25,7 @@
extern const unsigned char arm64_relocate_new_kernel[];
extern const unsigned long arm64_relocate_new_kernel_size;
+static bool in_crash_kexec;
static unsigned long kimage_start;
/**
@@ -148,7 +152,8 @@ void machine_kexec(struct kimage *kimage)
/*
* New cpus may have become stuck_in_kernel after we loaded the image.
*/
- BUG_ON(cpus_are_stuck_in_kernel() || (num_online_cpus() > 1));
+ BUG_ON((cpus_are_stuck_in_kernel() || (num_online_cpus() > 1)) &&
+ !WARN_ON(in_crash_kexec));
reboot_code_buffer_phys = page_to_phys(kimage->control_code_page);
reboot_code_buffer = phys_to_virt(reboot_code_buffer_phys);
@@ -200,13 +205,58 @@
