linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: kexec: fix crashkernel= handling
@ 2016-03-29 10:10 Russell King
  2016-03-30  1:46 ` Dave Young
  2016-04-07 21:03 ` [PATCH v2] " Russell King
  0 siblings, 2 replies; 9+ messages in thread
From: Russell King @ 2016-03-29 10:10 UTC (permalink / raw)
  To: linux-arm-kernel

When the kernel crashkernel parameter is specified with just a size, we
are supposed to allocate a region from RAM to store the crashkernel.
However, ARM merely reserves physical address zero with no checking
that there is even RAM there.

Fix this by lifting similar code from x86, importing it to ARM with
the ARM specific parameters added.

Update the kdump documentation to reflect this change.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 Documentation/kdump/kdump.txt | 13 +++----------
 arch/arm/kernel/setup.c       | 26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/Documentation/kdump/kdump.txt b/Documentation/kdump/kdump.txt
index bc4bd5a44b88..88ff63d5fde3 100644
--- a/Documentation/kdump/kdump.txt
+++ b/Documentation/kdump/kdump.txt
@@ -263,12 +263,6 @@ been removed from the machine.
     crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
     range=start-[end]
 
-Please note, on arm, the offset is required.
-    crashkernel=<range1>:<size1>[,<range2>:<size2>,...]@offset
-    range=start-[end]
-
-    'start' is inclusive and 'end' is exclusive.
-
 For example:
 
     crashkernel=512M-2G:64M,2G-:128M
@@ -307,10 +301,9 @@ Boot into System Kernel
    on the memory consumption of the kdump system. In general this is not
    dependent on the memory size of the production system.
 
-   On arm, use "crashkernel=Y at X". Note that the start address of the kernel
-   will be aligned to 128MiB (0x08000000), so if the start address is not then
-   any space below the alignment point may be overwritten by the dump-capture kernel,
-   which means it is possible that the vmcore is not that precise as expected.
+   On arm, the use of "crashkernel=Y at X" is no longer necessary; the
+   kernel will automatically locate the crash kernel image within the
+   first 512MB of RAM if X is not given.
 
 
 Load the Dump-capture Kernel
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 7d0cba6f1cc5..5d8511c425f0 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -938,6 +938,13 @@ static int __init init_machine_late(void)
 late_initcall(init_machine_late);
 
 #ifdef CONFIG_KEXEC
+/*
+ * The crash region must be aligned to 128MB to avoid
+ * zImage relocating below the reserved region.
+ */
+#define CRASH_ALIGN	(128 << 20)
+#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))
+
 static inline unsigned long long get_total_mem(void)
 {
 	unsigned long total;
@@ -965,6 +972,25 @@ static void __init reserve_crashkernel(void)
 	if (ret)
 		return;
 
+	if (crash_base <= 0) {
+		crash_base = memblock_find_in_range(CRASH_ALIGN, CRASH_ADDR_MAX,
+						    crash_size, CRASH_ALIGN);
+		if (!crash_base) {
+			pr_err("crashkernel reservation failed - No suitable area found.\n");
+			return;
+		}
+	} else {
+		unsigned long long start;
+
+		start = memblock_find_in_range(crash_base,
+					       crash_base + crash_size,
+					       crash_size, SECTION_SIZE);
+		if (start != crash_base) {
+			pr_err("crashkernel reservation failed - memory is in use.\n");
+			return;
+		}
+	}
+
 	ret = memblock_reserve(crash_base, crash_size);
 	if (ret < 0) {
 		pr_warn("crashkernel reservation failed - memory is in use (0x%lx)\n",
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] ARM: kexec: fix crashkernel= handling
  2016-03-29 10:10 [PATCH] ARM: kexec: fix crashkernel= handling Russell King
@ 2016-03-30  1:46 ` Dave Young
  2016-03-30 12:39   ` Pratyush Anand
  2016-04-07 21:03 ` [PATCH v2] " Russell King
  1 sibling, 1 reply; 9+ messages in thread
From: Dave Young @ 2016-03-30  1:46 UTC (permalink / raw)
  To: linux-arm-kernel

Hi, Russell

A long standing issue, but nobody tried to do it. Thank you for bringing up.

On 03/29/16 at 11:10am, Russell King wrote:
> When the kernel crashkernel parameter is specified with just a size, we
> are supposed to allocate a region from RAM to store the crashkernel.
> However, ARM merely reserves physical address zero with no checking
> that there is even RAM there.
> 
> Fix this by lifting similar code from x86, importing it to ARM with
> the ARM specific parameters added.
> 
> Update the kdump documentation to reflect this change.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  Documentation/kdump/kdump.txt | 13 +++----------
>  arch/arm/kernel/setup.c       | 26 ++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/kdump/kdump.txt b/Documentation/kdump/kdump.txt
> index bc4bd5a44b88..88ff63d5fde3 100644
> --- a/Documentation/kdump/kdump.txt
> +++ b/Documentation/kdump/kdump.txt
> @@ -263,12 +263,6 @@ been removed from the machine.
>      crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
>      range=start-[end]
>  
> -Please note, on arm, the offset is required.
> -    crashkernel=<range1>:<size1>[,<range2>:<size2>,...]@offset
> -    range=start-[end]
> -
> -    'start' is inclusive and 'end' is exclusive.
> -
>  For example:
>  
>      crashkernel=512M-2G:64M,2G-:128M
> @@ -307,10 +301,9 @@ Boot into System Kernel
>     on the memory consumption of the kdump system. In general this is not
>     dependent on the memory size of the production system.
>  
> -   On arm, use "crashkernel=Y at X". Note that the start address of the kernel
> -   will be aligned to 128MiB (0x08000000), so if the start address is not then
> -   any space below the alignment point may be overwritten by the dump-capture kernel,
> -   which means it is possible that the vmcore is not that precise as expected.
> +   On arm, the use of "crashkernel=Y at X" is no longer necessary; the
> +   kernel will automatically locate the crash kernel image within the
> +   first 512MB of RAM if X is not given.
>  
>  
>  Load the Dump-capture Kernel
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 7d0cba6f1cc5..5d8511c425f0 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -938,6 +938,13 @@ static int __init init_machine_late(void)
>  late_initcall(init_machine_late);
>  
>  #ifdef CONFIG_KEXEC
> +/*
> + * The crash region must be aligned to 128MB to avoid
> + * zImage relocating below the reserved region.
> + */
> +#define CRASH_ALIGN	(128 << 20)
> +#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))

Any reason to limit crash mem within the first 512M only? What if one want to
reserve memory over 512M?

Thanks
Dave

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] ARM: kexec: fix crashkernel= handling
  2016-03-30  1:46 ` Dave Young
@ 2016-03-30 12:39   ` Pratyush Anand
  2016-03-30 13:05     ` Russell King - ARM Linux
  0 siblings, 1 reply; 9+ messages in thread
From: Pratyush Anand @ 2016-03-30 12:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 30/03/2016:09:46:38 AM, Dave Young wrote:
> Hi, Russell
> 
> A long standing issue, but nobody tried to do it. Thank you for bringing up.
> 
> On 03/29/16 at 11:10am, Russell King wrote:
> > When the kernel crashkernel parameter is specified with just a size, we
> > are supposed to allocate a region from RAM to store the crashkernel.
> > However, ARM merely reserves physical address zero with no checking
> > that there is even RAM there.
> > 
> > Fix this by lifting similar code from x86, importing it to ARM with
> > the ARM specific parameters added.
> > 
> > Update the kdump documentation to reflect this change.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > ---
> >  Documentation/kdump/kdump.txt | 13 +++----------
> >  arch/arm/kernel/setup.c       | 26 ++++++++++++++++++++++++++
> >  2 files changed, 29 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Documentation/kdump/kdump.txt b/Documentation/kdump/kdump.txt
> > index bc4bd5a44b88..88ff63d5fde3 100644
> > --- a/Documentation/kdump/kdump.txt
> > +++ b/Documentation/kdump/kdump.txt
> > @@ -263,12 +263,6 @@ been removed from the machine.
> >      crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
> >      range=start-[end]
> >  
> > -Please note, on arm, the offset is required.
> > -    crashkernel=<range1>:<size1>[,<range2>:<size2>,...]@offset
> > -    range=start-[end]
> > -
> > -    'start' is inclusive and 'end' is exclusive.
> > -
> >  For example:
> >  
> >      crashkernel=512M-2G:64M,2G-:128M
> > @@ -307,10 +301,9 @@ Boot into System Kernel
> >     on the memory consumption of the kdump system. In general this is not
> >     dependent on the memory size of the production system.
> >  
> > -   On arm, use "crashkernel=Y at X". Note that the start address of the kernel
> > -   will be aligned to 128MiB (0x08000000), so if the start address is not then
> > -   any space below the alignment point may be overwritten by the dump-capture kernel,
> > -   which means it is possible that the vmcore is not that precise as expected.
> > +   On arm, the use of "crashkernel=Y at X" is no longer necessary; the
> > +   kernel will automatically locate the crash kernel image within the
> > +   first 512MB of RAM if X is not given.
> >  
> >  
> >  Load the Dump-capture Kernel
> > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > index 7d0cba6f1cc5..5d8511c425f0 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -938,6 +938,13 @@ static int __init init_machine_late(void)
> >  late_initcall(init_machine_late);
> >  
> >  #ifdef CONFIG_KEXEC
> > +/*
> > + * The crash region must be aligned to 128MB to avoid
> > + * zImage relocating below the reserved region.
> > + */
> > +#define CRASH_ALIGN	(128 << 20)
> > +#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))
> 
> Any reason to limit crash mem within the first 512M only? What if one want to
> reserve memory over 512M?

When crash base is not give, then may be it can be just checked if memblock
region is memory and not reserved, then allow to reserve. That might help to
remove 512M restriction.

~Pratyush

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] ARM: kexec: fix crashkernel= handling
  2016-03-30 12:39   ` Pratyush Anand
@ 2016-03-30 13:05     ` Russell King - ARM Linux
  2016-03-30 13:27       ` Vivek Goyal
  0 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2016-03-30 13:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 30, 2016 at 06:09:22PM +0530, Pratyush Anand wrote:
> On 30/03/2016:09:46:38 AM, Dave Young wrote:
> > Hi, Russell
> > 
> > A long standing issue, but nobody tried to do it. Thank you for bringing up.
> > 
> > On 03/29/16 at 11:10am, Russell King wrote:
> > > When the kernel crashkernel parameter is specified with just a size, we
> > > are supposed to allocate a region from RAM to store the crashkernel.
> > > However, ARM merely reserves physical address zero with no checking
> > > that there is even RAM there.
> > > 
> > > Fix this by lifting similar code from x86, importing it to ARM with
> > > the ARM specific parameters added.
> > > 
> > > Update the kdump documentation to reflect this change.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > ---
> > >  Documentation/kdump/kdump.txt | 13 +++----------
> > >  arch/arm/kernel/setup.c       | 26 ++++++++++++++++++++++++++
> > >  2 files changed, 29 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/Documentation/kdump/kdump.txt b/Documentation/kdump/kdump.txt
> > > index bc4bd5a44b88..88ff63d5fde3 100644
> > > --- a/Documentation/kdump/kdump.txt
> > > +++ b/Documentation/kdump/kdump.txt
> > > @@ -263,12 +263,6 @@ been removed from the machine.
> > >      crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
> > >      range=start-[end]
> > >  
> > > -Please note, on arm, the offset is required.
> > > -    crashkernel=<range1>:<size1>[,<range2>:<size2>,...]@offset
> > > -    range=start-[end]
> > > -
> > > -    'start' is inclusive and 'end' is exclusive.
> > > -
> > >  For example:
> > >  
> > >      crashkernel=512M-2G:64M,2G-:128M
> > > @@ -307,10 +301,9 @@ Boot into System Kernel
> > >     on the memory consumption of the kdump system. In general this is not
> > >     dependent on the memory size of the production system.
> > >  
> > > -   On arm, use "crashkernel=Y at X". Note that the start address of the kernel
> > > -   will be aligned to 128MiB (0x08000000), so if the start address is not then
> > > -   any space below the alignment point may be overwritten by the dump-capture kernel,
> > > -   which means it is possible that the vmcore is not that precise as expected.
> > > +   On arm, the use of "crashkernel=Y at X" is no longer necessary; the
> > > +   kernel will automatically locate the crash kernel image within the
> > > +   first 512MB of RAM if X is not given.
> > >  
> > >  
> > >  Load the Dump-capture Kernel
> > > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > > index 7d0cba6f1cc5..5d8511c425f0 100644
> > > --- a/arch/arm/kernel/setup.c
> > > +++ b/arch/arm/kernel/setup.c
> > > @@ -938,6 +938,13 @@ static int __init init_machine_late(void)
> > >  late_initcall(init_machine_late);
> > >  
> > >  #ifdef CONFIG_KEXEC
> > > +/*
> > > + * The crash region must be aligned to 128MB to avoid
> > > + * zImage relocating below the reserved region.
> > > + */
> > > +#define CRASH_ALIGN	(128 << 20)
> > > +#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))
> > 
> > Any reason to limit crash mem within the first 512M only? What if one want to
> > reserve memory over 512M?
> 
> When crash base is not give, then may be it can be just checked if memblock
> region is memory and not reserved, then allow to reserve. That might help to
> remove 512M restriction.

... and then I'll have to update the commit text.

You may notice that I say that this is mostly taken from the x86
implementation.  The x86 implementation also has this 512MB
allocation limit, to prevent it being placed too high in physical
memory.

This limit only applies for the case where the user hasn't specified
the base.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] ARM: kexec: fix crashkernel= handling
  2016-03-30 13:05     ` Russell King - ARM Linux
@ 2016-03-30 13:27       ` Vivek Goyal
  2016-04-01 13:25         ` Russell King - ARM Linux
  0 siblings, 1 reply; 9+ messages in thread
From: Vivek Goyal @ 2016-03-30 13:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 30, 2016 at 02:05:30PM +0100, Russell King - ARM Linux wrote:
> On Wed, Mar 30, 2016 at 06:09:22PM +0530, Pratyush Anand wrote:
> > On 30/03/2016:09:46:38 AM, Dave Young wrote:
> > > Hi, Russell
> > > 
> > > A long standing issue, but nobody tried to do it. Thank you for bringing up.
> > > 
> > > On 03/29/16 at 11:10am, Russell King wrote:
> > > > When the kernel crashkernel parameter is specified with just a size, we
> > > > are supposed to allocate a region from RAM to store the crashkernel.
> > > > However, ARM merely reserves physical address zero with no checking
> > > > that there is even RAM there.
> > > > 
> > > > Fix this by lifting similar code from x86, importing it to ARM with
> > > > the ARM specific parameters added.
> > > > 
> > > > Update the kdump documentation to reflect this change.
> > > > 
> > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > > ---
> > > >  Documentation/kdump/kdump.txt | 13 +++----------
> > > >  arch/arm/kernel/setup.c       | 26 ++++++++++++++++++++++++++
> > > >  2 files changed, 29 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/Documentation/kdump/kdump.txt b/Documentation/kdump/kdump.txt
> > > > index bc4bd5a44b88..88ff63d5fde3 100644
> > > > --- a/Documentation/kdump/kdump.txt
> > > > +++ b/Documentation/kdump/kdump.txt
> > > > @@ -263,12 +263,6 @@ been removed from the machine.
> > > >      crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
> > > >      range=start-[end]
> > > >  
> > > > -Please note, on arm, the offset is required.
> > > > -    crashkernel=<range1>:<size1>[,<range2>:<size2>,...]@offset
> > > > -    range=start-[end]
> > > > -
> > > > -    'start' is inclusive and 'end' is exclusive.
> > > > -
> > > >  For example:
> > > >  
> > > >      crashkernel=512M-2G:64M,2G-:128M
> > > > @@ -307,10 +301,9 @@ Boot into System Kernel
> > > >     on the memory consumption of the kdump system. In general this is not
> > > >     dependent on the memory size of the production system.
> > > >  
> > > > -   On arm, use "crashkernel=Y at X". Note that the start address of the kernel
> > > > -   will be aligned to 128MiB (0x08000000), so if the start address is not then
> > > > -   any space below the alignment point may be overwritten by the dump-capture kernel,
> > > > -   which means it is possible that the vmcore is not that precise as expected.
> > > > +   On arm, the use of "crashkernel=Y at X" is no longer necessary; the
> > > > +   kernel will automatically locate the crash kernel image within the
> > > > +   first 512MB of RAM if X is not given.
> > > >  
> > > >  
> > > >  Load the Dump-capture Kernel
> > > > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > > > index 7d0cba6f1cc5..5d8511c425f0 100644
> > > > --- a/arch/arm/kernel/setup.c
> > > > +++ b/arch/arm/kernel/setup.c
> > > > @@ -938,6 +938,13 @@ static int __init init_machine_late(void)
> > > >  late_initcall(init_machine_late);
> > > >  
> > > >  #ifdef CONFIG_KEXEC
> > > > +/*
> > > > + * The crash region must be aligned to 128MB to avoid
> > > > + * zImage relocating below the reserved region.
> > > > + */
> > > > +#define CRASH_ALIGN	(128 << 20)
> > > > +#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))
> > > 
> > > Any reason to limit crash mem within the first 512M only? What if one want to
> > > reserve memory over 512M?
> > 
> > When crash base is not give, then may be it can be just checked if memblock
> > region is memory and not reserved, then allow to reserve. That might help to
> > remove 512M restriction.
> 
> ... and then I'll have to update the commit text.
> 
> You may notice that I say that this is mostly taken from the x86
> implementation.  The x86 implementation also has this 512MB
> allocation limit, to prevent it being placed too high in physical
> memory.

IIRC, x86 had this limitation as they could not support any higher. But
if ARM can support higher, it would be good to allow that.

Thanks
Vivek

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] ARM: kexec: fix crashkernel= handling
  2016-03-30 13:27       ` Vivek Goyal
@ 2016-04-01 13:25         ` Russell King - ARM Linux
  2016-04-06  6:57           ` Dave Young
  0 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2016-04-01 13:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 30, 2016 at 09:27:08AM -0400, Vivek Goyal wrote:
> On Wed, Mar 30, 2016 at 02:05:30PM +0100, Russell King - ARM Linux wrote:
> > On Wed, Mar 30, 2016 at 06:09:22PM +0530, Pratyush Anand wrote:
> > > On 30/03/2016:09:46:38 AM, Dave Young wrote:
> > > > Hi, Russell
> > > > 
> > > > A long standing issue, but nobody tried to do it. Thank you for bringing up.
> > > > 
> > > > On 03/29/16 at 11:10am, Russell King wrote:
> > > > > When the kernel crashkernel parameter is specified with just a size, we
> > > > > are supposed to allocate a region from RAM to store the crashkernel.
> > > > > However, ARM merely reserves physical address zero with no checking
> > > > > that there is even RAM there.
> > > > > 
> > > > > Fix this by lifting similar code from x86, importing it to ARM with
> > > > > the ARM specific parameters added.
> > > > > 
> > > > > Update the kdump documentation to reflect this change.
> > > > > 
> > > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > > > ---
> > > > >  Documentation/kdump/kdump.txt | 13 +++----------
> > > > >  arch/arm/kernel/setup.c       | 26 ++++++++++++++++++++++++++
> > > > >  2 files changed, 29 insertions(+), 10 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/kdump/kdump.txt b/Documentation/kdump/kdump.txt
> > > > > index bc4bd5a44b88..88ff63d5fde3 100644
> > > > > --- a/Documentation/kdump/kdump.txt
> > > > > +++ b/Documentation/kdump/kdump.txt
> > > > > @@ -263,12 +263,6 @@ been removed from the machine.
> > > > >      crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
> > > > >      range=start-[end]
> > > > >  
> > > > > -Please note, on arm, the offset is required.
> > > > > -    crashkernel=<range1>:<size1>[,<range2>:<size2>,...]@offset
> > > > > -    range=start-[end]
> > > > > -
> > > > > -    'start' is inclusive and 'end' is exclusive.
> > > > > -
> > > > >  For example:
> > > > >  
> > > > >      crashkernel=512M-2G:64M,2G-:128M
> > > > > @@ -307,10 +301,9 @@ Boot into System Kernel
> > > > >     on the memory consumption of the kdump system. In general this is not
> > > > >     dependent on the memory size of the production system.
> > > > >  
> > > > > -   On arm, use "crashkernel=Y at X". Note that the start address of the kernel
> > > > > -   will be aligned to 128MiB (0x08000000), so if the start address is not then
> > > > > -   any space below the alignment point may be overwritten by the dump-capture kernel,
> > > > > -   which means it is possible that the vmcore is not that precise as expected.
> > > > > +   On arm, the use of "crashkernel=Y at X" is no longer necessary; the
> > > > > +   kernel will automatically locate the crash kernel image within the
> > > > > +   first 512MB of RAM if X is not given.
> > > > >  
> > > > >  
> > > > >  Load the Dump-capture Kernel
> > > > > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > > > > index 7d0cba6f1cc5..5d8511c425f0 100644
> > > > > --- a/arch/arm/kernel/setup.c
> > > > > +++ b/arch/arm/kernel/setup.c
> > > > > @@ -938,6 +938,13 @@ static int __init init_machine_late(void)
> > > > >  late_initcall(init_machine_late);
> > > > >  
> > > > >  #ifdef CONFIG_KEXEC
> > > > > +/*
> > > > > + * The crash region must be aligned to 128MB to avoid
> > > > > + * zImage relocating below the reserved region.
> > > > > + */
> > > > > +#define CRASH_ALIGN	(128 << 20)
> > > > > +#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))
> > > > 
> > > > Any reason to limit crash mem within the first 512M only? What if one want to
> > > > reserve memory over 512M?
> > > 
> > > When crash base is not give, then may be it can be just checked if memblock
> > > region is memory and not reserved, then allow to reserve. That might help to
> > > remove 512M restriction.
> > 
> > ... and then I'll have to update the commit text.
> > 
> > You may notice that I say that this is mostly taken from the x86
> > implementation.  The x86 implementation also has this 512MB
> > allocation limit, to prevent it being placed too high in physical
> > memory.
> 
> IIRC, x86 had this limitation as they could not support any higher. But
> if ARM can support higher, it would be good to allow that.

Well, if we want to remove it, we then need to sort out a method of
specifying a limit on the address - where platforms physical memory
bridges the 4GB CPU-accessible limit, the crashkernel region must be
allocated below that so that it is boot-time accessible.

Some patches also have boot-time only aliases of RAM, with the normal
alias above 4GB (eg, TI Keystone2) where the running view of RAM is
at 0x800000000, but it has a non-coherent boot alias at 0x80000000.

I've patches which resolve some of the issues there, and making that
change would make this patch dependent on those changes.  So, I
recommend that this patch remains as-is for the time being, and this
issue is addressed in a later patch after the Keystone2 idmap_to_phys()
patches, similar to:

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 0a12fcf1aff6..74781e6d4e87 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -962,7 +962,6 @@ late_initcall(init_machine_late);
  * zImage relocating below the reserved region.
  */
 #define CRASH_ALIGN	(128 << 20)
-#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))
 
 static inline unsigned long long get_total_mem(void)
 {
@@ -992,7 +991,8 @@ static void __init reserve_crashkernel(void)
 		return;
 
 	if (crash_base <= 0) {
-		crash_base = memblock_find_in_range(CRASH_ALIGN, CRASH_ADDR_MAX,
+		unsigned long long crash_max = idmap_to_phys((u32)~0);
+		crash_base = memblock_find_in_range(CRASH_ALIGN, crash_max,
 						    crash_size, CRASH_ALIGN);
 		if (!crash_base) {
 			pr_err("crashkernel reservation failed - No suitable area found.\n");

Right now, I don't want to tie this facility to TI Keystone2 support
as what this patch is doing is something that the ARM kexec support
should have been doing since it was first introduced, so folk may
want to backport this change to stable trees.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] ARM: kexec: fix crashkernel= handling
  2016-04-01 13:25         ` Russell King - ARM Linux
@ 2016-04-06  6:57           ` Dave Young
  0 siblings, 0 replies; 9+ messages in thread
From: Dave Young @ 2016-04-06  6:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/01/16 at 02:25pm, Russell King - ARM Linux wrote:

[snip]

> Well, if we want to remove it, we then need to sort out a method of
> specifying a limit on the address - where platforms physical memory
> bridges the 4GB CPU-accessible limit, the crashkernel region must be
> allocated below that so that it is boot-time accessible.
> 
> Some patches also have boot-time only aliases of RAM, with the normal
> alias above 4GB (eg, TI Keystone2) where the running view of RAM is
> at 0x800000000, but it has a non-coherent boot alias at 0x80000000.
> 
> I've patches which resolve some of the issues there, and making that
> change would make this patch dependent on those changes.  So, I
> recommend that this patch remains as-is for the time being, and this
> issue is addressed in a later patch after the Keystone2 idmap_to_phys()
> patches, similar to:
> 
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 0a12fcf1aff6..74781e6d4e87 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -962,7 +962,6 @@ late_initcall(init_machine_late);
>   * zImage relocating below the reserved region.
>   */
>  #define CRASH_ALIGN	(128 << 20)
> -#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))
>  
>  static inline unsigned long long get_total_mem(void)
>  {
> @@ -992,7 +991,8 @@ static void __init reserve_crashkernel(void)
>  		return;
>  
>  	if (crash_base <= 0) {
> -		crash_base = memblock_find_in_range(CRASH_ALIGN, CRASH_ADDR_MAX,
> +		unsigned long long crash_max = idmap_to_phys((u32)~0);
> +		crash_base = memblock_find_in_range(CRASH_ALIGN, crash_max,
>  						    crash_size, CRASH_ALIGN);
>  		if (!crash_base) {
>  			pr_err("crashkernel reservation failed - No suitable area found.\n");
> 
> Right now, I don't want to tie this facility to TI Keystone2 support
> as what this patch is doing is something that the ARM kexec support
> should have been doing since it was first introduced, so folk may
> want to backport this change to stable trees.

Is it possible for PHYS_OFFSET + (512 << 20) be larger than 4G assume that phys_addr_t is 32bit, if so it can be trunked to a small value then
the max will be wrong?

Otherwise I think use it temprarily is fine.

Thanks
Dave

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] ARM: kexec: fix crashkernel= handling
  2016-03-29 10:10 [PATCH] ARM: kexec: fix crashkernel= handling Russell King
  2016-03-30  1:46 ` Dave Young
@ 2016-04-07 21:03 ` Russell King
  2016-04-08  2:12   ` Dave Young
  1 sibling, 1 reply; 9+ messages in thread
From: Russell King @ 2016-04-07 21:03 UTC (permalink / raw)
  To: linux-arm-kernel

When the kernel crashkernel parameter is specified with just a size, we
are supposed to allocate a region from RAM to store the crashkernel.
However, ARM merely reserves physical address zero with no checking that
there is even RAM there.

Fix this by lifting similar code from x86, importing it to ARM with the
ARM specific parameters added.  In the absence of any platform specific
information, we allocate the crashkernel region from the first 512MB of
physical memory.

Update the kdump documentation to reflect this change.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 Documentation/kdump/kdump.txt | 13 +++----------
 arch/arm/kernel/setup.c       | 29 +++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/Documentation/kdump/kdump.txt b/Documentation/kdump/kdump.txt
index bc4bd5a44b88..88ff63d5fde3 100644
--- a/Documentation/kdump/kdump.txt
+++ b/Documentation/kdump/kdump.txt
@@ -263,12 +263,6 @@ been removed from the machine.
     crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
     range=start-[end]
 
-Please note, on arm, the offset is required.
-    crashkernel=<range1>:<size1>[,<range2>:<size2>,...]@offset
-    range=start-[end]
-
-    'start' is inclusive and 'end' is exclusive.
-
 For example:
 
     crashkernel=512M-2G:64M,2G-:128M
@@ -307,10 +301,9 @@ Boot into System Kernel
    on the memory consumption of the kdump system. In general this is not
    dependent on the memory size of the production system.
 
-   On arm, use "crashkernel=Y at X". Note that the start address of the kernel
-   will be aligned to 128MiB (0x08000000), so if the start address is not then
-   any space below the alignment point may be overwritten by the dump-capture kernel,
-   which means it is possible that the vmcore is not that precise as expected.
+   On arm, the use of "crashkernel=Y at X" is no longer necessary; the
+   kernel will automatically locate the crash kernel image within the
+   first 512MB of RAM if X is not given.
 
 
 Load the Dump-capture Kernel
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 139791ed473d..77b54c461c52 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -938,6 +938,13 @@ static int __init init_machine_late(void)
 late_initcall(init_machine_late);
 
 #ifdef CONFIG_KEXEC
+/*
+ * The crash region must be aligned to 128MB to avoid
+ * zImage relocating below the reserved region.
+ */
+#define CRASH_ALIGN	(128 << 20)
+#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))
+
 static inline unsigned long long get_total_mem(void)
 {
 	unsigned long total;
@@ -965,6 +972,28 @@ static void __init reserve_crashkernel(void)
 	if (ret)
 		return;
 
+	if (crash_base <= 0) {
+		unsigned long long crash_max = CRASH_ADDR_MAX;
+		if (crash_max > (u32)~0)
+			crash_max = (u32)~0;
+		crash_base = memblock_find_in_range(CRASH_ALIGN, crash_max,
+						    crash_size, CRASH_ALIGN);
+		if (!crash_base) {
+			pr_err("crashkernel reservation failed - No suitable area found.\n");
+			return;
+		}
+	} else {
+		unsigned long long start;
+
+		start = memblock_find_in_range(crash_base,
+					       crash_base + crash_size,
+					       crash_size, SECTION_SIZE);
+		if (start != crash_base) {
+			pr_err("crashkernel reservation failed - memory is in use.\n");
+			return;
+		}
+	}
+
 	ret = memblock_reserve(crash_base, crash_size);
 	if (ret < 0) {
 		pr_warn("crashkernel reservation failed - memory is in use (0x%lx)\n",
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] ARM: kexec: fix crashkernel= handling
  2016-04-07 21:03 ` [PATCH v2] " Russell King
@ 2016-04-08  2:12   ` Dave Young
  0 siblings, 0 replies; 9+ messages in thread
From: Dave Young @ 2016-04-08  2:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/07/16 at 10:03pm, Russell King wrote:
> When the kernel crashkernel parameter is specified with just a size, we
> are supposed to allocate a region from RAM to store the crashkernel.
> However, ARM merely reserves physical address zero with no checking that
> there is even RAM there.
> 
> Fix this by lifting similar code from x86, importing it to ARM with the
> ARM specific parameters added.  In the absence of any platform specific
> information, we allocate the crashkernel region from the first 512MB of
> physical memory.
> 
> Update the kdump documentation to reflect this change.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  Documentation/kdump/kdump.txt | 13 +++----------
>  arch/arm/kernel/setup.c       | 29 +++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/kdump/kdump.txt b/Documentation/kdump/kdump.txt
> index bc4bd5a44b88..88ff63d5fde3 100644
> --- a/Documentation/kdump/kdump.txt
> +++ b/Documentation/kdump/kdump.txt
> @@ -263,12 +263,6 @@ been removed from the machine.
>      crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
>      range=start-[end]
>  
> -Please note, on arm, the offset is required.
> -    crashkernel=<range1>:<size1>[,<range2>:<size2>,...]@offset
> -    range=start-[end]
> -
> -    'start' is inclusive and 'end' is exclusive.
> -
>  For example:
>  
>      crashkernel=512M-2G:64M,2G-:128M
> @@ -307,10 +301,9 @@ Boot into System Kernel
>     on the memory consumption of the kdump system. In general this is not
>     dependent on the memory size of the production system.
>  
> -   On arm, use "crashkernel=Y at X". Note that the start address of the kernel
> -   will be aligned to 128MiB (0x08000000), so if the start address is not then
> -   any space below the alignment point may be overwritten by the dump-capture kernel,
> -   which means it is possible that the vmcore is not that precise as expected.
> +   On arm, the use of "crashkernel=Y at X" is no longer necessary; the
> +   kernel will automatically locate the crash kernel image within the
> +   first 512MB of RAM if X is not given.
>  
>  
>  Load the Dump-capture Kernel
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 139791ed473d..77b54c461c52 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -938,6 +938,13 @@ static int __init init_machine_late(void)
>  late_initcall(init_machine_late);
>  
>  #ifdef CONFIG_KEXEC
> +/*
> + * The crash region must be aligned to 128MB to avoid
> + * zImage relocating below the reserved region.
> + */
> +#define CRASH_ALIGN	(128 << 20)
> +#define CRASH_ADDR_MAX	(PHYS_OFFSET + (512 << 20))
> +
>  static inline unsigned long long get_total_mem(void)
>  {
>  	unsigned long total;
> @@ -965,6 +972,28 @@ static void __init reserve_crashkernel(void)
>  	if (ret)
>  		return;
>  
> +	if (crash_base <= 0) {
> +		unsigned long long crash_max = CRASH_ADDR_MAX;
> +		if (crash_max > (u32)~0)
> +			crash_max = (u32)~0;
> +		crash_base = memblock_find_in_range(CRASH_ALIGN, crash_max,
> +						    crash_size, CRASH_ALIGN);
> +		if (!crash_base) {
> +			pr_err("crashkernel reservation failed - No suitable area found.\n");
> +			return;
> +		}
> +	} else {
> +		unsigned long long start;
> +
> +		start = memblock_find_in_range(crash_base,
> +					       crash_base + crash_size,
> +					       crash_size, SECTION_SIZE);
> +		if (start != crash_base) {
> +			pr_err("crashkernel reservation failed - memory is in use.\n");
> +			return;
> +		}
> +	}
> +
>  	ret = memblock_reserve(crash_base, crash_size);
>  	if (ret < 0) {
>  		pr_warn("crashkernel reservation failed - memory is in use (0x%lx)\n",

Reviewed-by: Dave Young <dyoung@redhat.com>

Thanks
Dave

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-04-08  2:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-29 10:10 [PATCH] ARM: kexec: fix crashkernel= handling Russell King
2016-03-30  1:46 ` Dave Young
2016-03-30 12:39   ` Pratyush Anand
2016-03-30 13:05     ` Russell King - ARM Linux
2016-03-30 13:27       ` Vivek Goyal
2016-04-01 13:25         ` Russell King - ARM Linux
2016-04-06  6:57           ` Dave Young
2016-04-07 21:03 ` [PATCH v2] " Russell King
2016-04-08  2:12   ` Dave Young

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).