arch/arm64: elfcorehdr should be the first allocation
PATCH 1/1 arch/arm64: elfcorehdr should be the first allocation
From: Abhimanyu Saini <[email protected]>
elfcorehdr_addr is assigned by kexec-utils and device tree of dump kernel is fixed in chosen node with parameter "linux,elfcorehdr". So, memory should be first reserved for elfcorehdr, otherwise overlaps may happen with other memory allocations which were done before the allocation of elcorehdr in the crash kernel
--- arch/arm64/mm/init.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
arch/arm64/mm/init.c +4 -2
@@ -453,6 +453,10 @@ void __init arm64_memblock_init(void)* Register the kernel text, kernel data, initrd, and initial * pagetables with memblock. */ + +/* make this the first reservation so that there are no chances of +* overlap */ +reserve_elfcorehdr ();memblock_reserve (__pa_symbol (_text), _end - _text);#ifdef CONFIG_BLK_DEV_INITRD if (initrd_start) {
@@ -474,8 +478,6 @@ void __init arm64_memblock_init(void)reserve_crashkernel (); -reserve_elfcorehdr (); -dma_contiguous_reserve (arm64_dma_phys_limit);memblock_allow_resize ();
Signature · 1 line
2.14.1
PATCH 1/1 arch/arm64: elfcorehdr should be the first allocation
Quoted 7 lines · Prabhakar Kushwaha
From: Abhimanyu Saini <[email protected]>
elfcorehdr_addris assigned by kexec-utils and device tree of dump kernel is fixed in chosen node with parameter "linux,elfcorehdr". So, memory should be first reserved for elfcorehdr, otherwise overlaps may happen with other memory allocations which were done before the allocation of elcorehdr in the crash kernel
What happens in that case? Do you have a crash log we can include in the commit message?
Quoted 4 lines
Signed-off-by Guanhua <[email protected]>Signed-off-by Poonam Aggrwal <[email protected]>Signed-off-by Abhimanyu Saini <[email protected]>---
Really? How on Earth did you get three people co-developing this patch?
Quoted 19 lines
arch/arm64/mm/init.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)arch/arm64/mm/init.c +4
-0@@ -453,6 +453,10 @@ void __init arm64_memblock_init(void)* Register the kernel text, kernel data, initrd, and initial * pagetables with memblock. */ + +/* make this the first reservation so that there are no chances of +* overlap */ +reserve_elfcorehdr ();memblock_reserve (__pa_symbol (_text), _end - _text);#ifdef CONFIG_BLK_DEV_INITRD if (initrd_start) {
arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 477
Quoted 4 lines
arch/arm64/mm/init.c +0
-1@@ -474,8 +478,6 @@ void __init arm64_memblock_init(void)reserve_crashkernel (); -reserve_elfcorehdr ();
Why isn't this also a problem for reserve_crashkernel() or any other static reservations?
Will
PATCH 1/1 arch/arm64: elfcorehdr should be the first allocation
Quoted 11 lines · Will Deacon
Quoted 7 lines · Prabhakar Kushwaha
From: Abhimanyu Saini <[email protected]>
elfcorehdr_addris assigned by kexec-utils and device tree of dump kernel is fixed in chosen node with parameter "linux,elfcorehdr". So, memory should be first reserved for elfcorehdr, otherwise overlaps may happen with other memory allocations which were done before the allocation of elcorehdr in the crash kernelWhat happens in that case? Do you have a crash log we can include in the commit message?
In private discussions with Poonam, he said:
| The overlap here I observed was for the reserved-mem areas in the dtb. | And they were specific to NXP device.
Since I have not got any details since then, I'm not sure whether your patch is the way to go.
(I suspect that we might better fix the issue on kexec-tools side.)
Thanks,
-Takahiro AKASHI
Quoted 35 lines
Quoted 4 lines
Signed-off-by Guanhua <[email protected]>Signed-off-by Poonam Aggrwal <[email protected]>Signed-off-by Abhimanyu Saini <[email protected]>---
Really? How on Earth did you get three people co-developing this patch?
Quoted 19 lines
arch/arm64/mm/init.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)arch/arm64/mm/init.c +4
-0@@ -453,6 +453,10 @@ void __init arm64_memblock_init(void)* Register the kernel text, kernel data, initrd, and initial * pagetables with memblock. */ + +/* make this the first reservation so that there are no chances of +* overlap */ +reserve_elfcorehdr ();memblock_reserve (__pa_symbol (_text), _end - _text);#ifdef CONFIG_BLK_DEV_INITRD if (initrd_start) {arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 477
Quoted 4 lines
arch/arm64/mm/init.c +0
-1@@ -474,8 +478,6 @@ void __init arm64_memblock_init(void)reserve_crashkernel (); -reserve_elfcorehdr ();Why isn't this also a problem for
reserve_crashkernel()or any other static reservations?Will
PATCH 1/1 arch/arm64: elfcorehdr should be the first allocation
Thanks Takahiro and Will,
Please find responses below.
Regards
Poonam
Quoted 20 lines
-----Original Message-----
From: AKASHI Takahiro [mailto:takahiro.akashi at linaro.org]
Sent: Wednesday, December 13, 2017 4:17 PM
To: Abhimanyu Saini <[email protected]>
Cc: Will Deacon <[email protected]>; Prabhakar Kushwaha <[email protected]>; linux-arm-kernel at lists.infradead.org; Poonam Aggrwal <[email protected]>;G.h. Gao <[email protected]>; james.morse at arm.com
Subject: Re: [PATCH]arch/arm64: elfcorehdr should be the first allocationQuoted 9 lines · Will Deacon
Quoted 7 lines · Prabhakar Kushwaha
From: Abhimanyu Saini <[email protected]>
elfcorehdr_addris assigned by kexec-utils and device tree of dump kernel is fixed in chosen node with parameter "linux,elfcorehdr". So, memory should be first reserved for elfcorehdr, otherwise overlaps may happen with other memory allocations which were done before the allocation of elcorehdr in the crash kernel
Quoted 6 lines
Quoted 2 lines
What happens in that case? Do you have a crash log we can include in the commit message?
In private discussions with Poonam, he said:
| The overlap here I observed was for the reserved-mem areas in the dtb. | And they were specific to NXP device.
Thanks Takahiro for providing this information.
Yes the memory overlap happens because when the dump-capture kernel tries to reserve elfcoreheader at the specific address, it finds that the address has already been reserved for some other use by early_init_fdt_scan_reserved_mem(). Based on the "reserved-memory" node entries in the dts file , the overlap is seen to happen with one of the below alocations:
[ 0.000000] Reserved memory: created DMA memory pool at 0x00000000fb800000, size 4 MiB [ 0.000000] Reserved memory: initialized node qman-fqd, compatible id shared-dma-pool [ 0.000000] Reserved memory: created DMA memory pool at 0x00000000f8000000, size 32 MiB [ 0.000000] Reserved memory: initialized node qman-pfdr, compatible id shared-dma-pool
Quoted 5 lines
Since I have not got any details since then, I'm not sure whether your patch is the way to go.
(I suspect that we might better fix the issue on kexec-tools side.)
I may not have full understanding of kexec-tools, but I am not sure how will kexec tools be able to know what addresses will get assigned for the reserved-memory regions in the device tree.
Quoted 9 lines
Thanks,
-Takahiro AKASHIQuoted 6 lines
Quoted 4 lines
Signed-off-by Guanhua <[email protected]>Signed-off-by Poonam Aggrwal <[email protected]>Signed-off-by Abhimanyu Saini <[email protected]>---
Really? How on Earth did you get three people co-developing this patch?
: )Contribution was from all in terms of reproducing, testing on various platforms including the debug experiments. Just wanted to acknowledge the effort.
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
arch/arm64/mm/init.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)arch/arm64/mm/init.c index renamed from arch/arm64/mm/init.c +0
-0
5960bef0170d..551048cfcfff100644arch/arm64/mm/init.c +4
-0@@ -453,6 +453,10 @@ void __init arm64_memblock_init(void)* Register the kernel text, kernel data, initrd, and initial * pagetables with memblock. */ + +/* make this the first reservation so that there are no chances of +* overlap */ +reserve_elfcorehdr (); memblock_reserve(__pa_symbol(_text), _end - _text); #ifdef
CONFIG_BLK_DEV_INITRDarch/arm64/mm/init.c +0
-0if (initrd_start) {@@ -474,8 +478,6 @@ void __init arm64_memblock_init(void)
Quoted 6 lines
Quoted 6 lines
arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 477
Quoted 3 lines
arch/arm64/mm/init.c +0
-1reserve_crashkernel (); -reserve_elfcorehdr ();Why isn't this also a problem for
reserve_crashkernel()or any other static reservations?
Thanks, for raising this. Yes looking at the code flow this may also cause a problem, definitely when the start address of the crash kernel is specified in the bootargs. We mostly tested with just the size, so it was an allocation without a specific address.
Quoted 2 lines
Quoted 2 lines
Will
PATCH 1/1 arch/arm64: elfcorehdr should be the first allocation
Quoted 29 lines · Poonam Aggrwal
Thanks Takahiro and Will,
Please find responses below.
Regards
PoonamQuoted 23 lines
-----Original Message-----
From: AKASHI Takahiro [mailto:takahiro.akashi at linaro.org]
Sent: Wednesday, December 13, 2017 4:17 PM
To: Abhimanyu Saini <[email protected]>
Cc: Will Deacon <[email protected]>; Prabhakar Kushwaha <[email protected]>; linux-arm-kernel at lists.infradead.org; Poonam Aggrwal <[email protected]>;G.h. Gao <[email protected]>; james.morse at arm.com
Subject: Re: [PATCH]arch/arm64: elfcorehdr should be the first allocationQuoted 11 lines · Will Deacon
Quoted 7 lines · Prabhakar Kushwaha
From: Abhimanyu Saini <[email protected]>
elfcorehdr_addris assigned by kexec-utils and device tree of dump kernel is fixed in chosen node with parameter "linux,elfcorehdr". So, memory should be first reserved for elfcorehdr, otherwise overlaps may happen with other memory allocations which were done before the allocation of elcorehdr in the crash kernelWhat happens in that case? Do you have a crash log we can include in the commit message?
Quoted 6 lines
Quoted 3 lines
In private discussions with Poonam, he said:
| The overlap here I observed was for the reserved-mem areas in the dtb. | And they were specific to NXP device.Thanks Takahiro for providing this information.
Yes the memory overlap happens because when the dump-capture kernel tries to reserve elfcoreheader at the specific address, it finds that the address has already been reserved for some other use byearly_init_fdt_scan_reserved_mem(). Based on the "reserved-memory" node entries in the dts file , the overlap is seen to happen with one of the below alocations:
So the point is that reserve_elfcorehdr() should be placed before early_init_fdt_scan_reserved_mem() which may allocate memory dynamically, isn't it. I think it makes sense.
Lisewise, reserve_crashkernel(), which can also allocate memory statically in case of "crashkernel=SIZE at ADDRESS", should be.
(Even if an overrap might happen, this wouldn't prevent the system from booting though. We just can't use kdump in this case.)
Thanks,
-Takahiro AKASHI
Quoted 51 lines
[ 0.000000] Reserved memory: created DMA memory pool at 0x00000000fb800000, size 4 MiB [ 0.000000] Reserved memory: initialized node qman-fqd, compatible id shared-dma-pool [ 0.000000] Reserved memory: created DMA memory pool at 0x00000000f8000000, size 32 MiB [ 0.000000] Reserved memory: initialized node qman-pfdr, compatible id shared-dma-poolQuoted 5 lines
Since I have not got any details since then, I'm not sure whether your patch is the way to go.
(I suspect that we might better fix the issue on kexec-tools side.)I may not have full understanding of kexec-tools, but I am not sure how will kexec tools be able to know what addresses will get assigned for the reserved-memory regions in the device tree.
Quoted 9 lines
Thanks,
-Takahiro AKASHIQuoted 6 lines
Quoted 4 lines
Signed-off-by Guanhua <[email protected]>Signed-off-by Poonam Aggrwal <[email protected]>Signed-off-by Abhimanyu Saini <[email protected]>---
Really? How on Earth did you get three people co-developing this patch?
: )Contribution was from all in terms of reproducing, testing on various platforms including the debug experiments. Just wanted to acknowledge the effort.
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
arch/arm64/mm/init.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)arch/arm64/mm/init.c index renamed from arch/arm64/mm/init.c +0
-0
5960bef0170d..551048cfcfff100644arch/arm64/mm/init.c +4
-0@@ -453,6 +453,10 @@ void __init arm64_memblock_init(void)* Register the kernel text, kernel data, initrd, and initial * pagetables with memblock. */ + +/* make this the first reservation so that there are no chances of +* overlap */ +reserve_elfcorehdr (); memblock_reserve(__pa_symbol(_text), _end - _text); #ifdef
CONFIG_BLK_DEV_INITRDarch/arm64/mm/init.c +0
-0if (initrd_start) {@@ -474,8 +478,6 @@ void __init arm64_memblock_init(void)Quoted 6 lines
Quoted 6 lines
arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 477
Quoted 3 lines
arch/arm64/mm/init.c +0
-1reserve_crashkernel (); -reserve_elfcorehdr ();Why isn't this also a problem for
reserve_crashkernel()or any other static reservations?Thanks, for raising this. Yes looking at the code flow this may also cause a problem, definitely when the start address of the crash kernel is specified in the bootargs. We mostly tested with just the size, so it was an allocation without a specific address.
Quoted 2 lines
Quoted 2 lines
Will
PATCH 1/1 arch/arm64: elfcorehdr should be the first allocation
Quoted 53 lines
-----Original Message-----
From: AKASHI Takahiro [mailto:takahiro.akashi at linaro.org]
Sent: Monday, December 18, 2017 11:03 AM
To: Poonam Aggrwal <[email protected]>
Cc: Abhimanyu Saini <[email protected]>; Will Deacon <[email protected]>; Prabhakar Kushwaha <[email protected]>; linux-arm-kernel at lists.infradead.org;G.h. Gao <[email protected]>; james.morse at arm.com
Subject: Re: [PATCH]arch/arm64: elfcorehdr should be the first allocationQuoted 27 lines · Poonam Aggrwal
Thanks Takahiro and Will,
Please find responses below.
Regards
PoonamQuoted 21 lines
-----Original Message-----
From: AKASHI Takahiro [mailto:takahiro.akashi at linaro.org]
Sent: Wednesday, December 13, 2017 4:17 PM
To: Abhimanyu Saini <[email protected]>
Cc: Will Deacon <[email protected]>; Prabhakar Kushwaha <[email protected]>; linux-arm-kernel at lists.infradead.org; Poonam Aggrwal <[email protected]>;G.h. Gao <[email protected]>; james.morse at arm.com
Subject: Re: [PATCH]arch/arm64: elfcorehdr should be the first allocationQuoted 9 lines · Will Deacon
Quoted 7 lines · Prabhakar Kushwaha
From: Abhimanyu Saini <[email protected]>
elfcorehdr_addris assigned by kexec-utils and device tree of dump kernel is fixed in chosen node with parameter "linux,elfcorehdr". So, memory should be first reserved for elfcorehdr, otherwise overlaps may happen with other memory allocations which were done before the allocation of elcorehdr in the crash kernelQuoted 8 lines
Quoted 6 lines
Quoted 2 lines
What happens in that case? Do you have a crash log we can include in the commit message?
In private discussions with Poonam, he said:
| The overlap here I observed was for the reserved-mem areas in the dtb. | And they were specific to NXP device.Thanks Takahiro for providing this information.
Yes the memory overlap happens because when the dump-capture kernel triesto reserve elfcoreheader at the specific address, it finds that the address has already been reserved for some other use by
early_init_fdt_scan_reserved_mem().Quoted 1 line
Based on the "reserved-memory" node entries in the dts file , the overlap is
seen to happen with one of the below alocations:
Quoted 9 lines
So the point is that
reserve_elfcorehdr()should be placed beforeearly_init_fdt_scan_reserved_mem()which may allocate memory dynamically, isn't it. I think it makes sense.Lisewise,
reserve_crashkernel(), which can also allocate memory statically in case of "crashkernel=SIZE at ADDRESS", should be.
(Even if an overrap might happen, this wouldn't prevent the system from booting though. We just can't use kdump in this case.)
Right, so probably both the allocations (elfcorehdr and crash kernel) should go before the early_init_fdt_scan_reserved_mem. Should I send a re-spin?
Quoted 66 lines
Thanks,
-Takahiro AKASHIQuoted 1 line
[ 0.000000] Reserved memory: created DMA memory pool at
0x00000000fb800000, size 4 MiBQuoted 1 line
[ 0.000000] Reserved memory: initialized node qman-fqd, compatible idshared-dma-pool
Quoted 1 line
[ 0.000000] Reserved memory: created DMA memory pool at
0x00000000f8000000, size 32 MiBQuoted 1 line
[ 0.000000] Reserved memory: initialized node qman-pfdr, compatible idshared-dma-pool
Quoted 6 lines
Quoted 5 lines
Since I have not got any details since then, I'm not sure whether your patch is the way to go.
(I suspect that we might better fix the issue on kexec-tools side.)I may not have full understanding of kexec-tools, but I am not sure how will
kexec tools be able to know what addresses will get assigned for the reserved- memory regions in the device tree.
Quoted 10 lines
Quoted 9 lines
Thanks,
-Takahiro AKASHIQuoted 6 lines
Quoted 4 lines
Signed-off-by Guanhua <[email protected]>Signed-off-by Poonam Aggrwal <[email protected]>Signed-off-by Abhimanyu Saini <[email protected]>---
Really? How on Earth did you get three people co-developing this patch?
: )Contribution was from all in terms of reproducing, testing on various
platforms including the debug experiments. Just wanted to acknowledge the effort.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
arch/arm64/mm/init.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 457
Quoted 10 lines
arch/arm64/mm/init.c index renamed from arch/arm64/mm/init.c +0
-0
5960bef0170d..551048cfcfff100644arch/arm64/mm/init.c +2
-0@@ -453,6 +453,10 @@ void __init arm64_memblock_init(void)* Register the kernel text, kernel data, initrd, and initial * pagetables with memblock. */ + +/* make this the first reservation so that there are no chances of
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
arch/arm64/mm/init.c +2
-0+* overlap */ +reserve_elfcorehdr (); memblock_reserve(__pa_symbol(_text), _end - _text); #ifdef
CONFIG_BLK_DEV_INITRDarch/arm64/mm/init.c +0
-0if (initrd_start) {@@ -474,8 +478,6 @@ void __init arm64_memblock_init(void)Quoted 7 lines
Quoted 6 lines
Quoted 6 lines
arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 477
Quoted 3 lines
arch/arm64/mm/init.c +0
-1reserve_crashkernel (); -reserve_elfcorehdr ();Why isn't this also a problem for
reserve_crashkernel()or any other static reservations?Thanks, for raising this. Yes looking at the code flow this may also cause a
problem, definitely when the start address of the crash kernel is specified in the bootargs. We mostly tested with just the size, so it was an allocation without a specific address.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Will