Re: [PATCH v4 1/5] arm64: export memblock_reserve()d regions via /proc/iomem
Hi John, On 21/08/18 20:38, John Stultz wrote: > On Tue, Aug 21, 2018 at 3:22 AM, James Morse wrote: >> On 08/21/2018 05:39 AM, John Stultz wrote: >>> >>> Since this patch landed, on the HiKey board at bootup I'm seeing: >>> >>> [0.451884] WARNING: CPU: 1 PID: 1 at arch/arm64/kernel/setup.c:271 >>> reserve_memblock_reserved_regions+0xd4/0x13c > ... >>> From skimming the patch, it seems this is maybe expected? Or should >>> this warning raise eyebrows? I can't quite figure it out. >> >>> /proc/iomem now has: >>> ... >>> 0741-21ef : System RAM >>>1100-1113cfff : reserved >> >> >>> 21f0-21ff : reserved >> >> >> ^ This entry is what triggered the warning. >> >> It expects that meblock_reserved() memory is also described as memory. >> (memblock keeps them as separate lists, so its possible to reserve >> memory that doesn't exist... which it looks like your system is doing) > > So yea, I suspect the hikey dts isn't quite right here then. The DT mem-reserve code goes and memblock_removes() some regions, instead of marking them nomap. Given memblock has a for_each_resv_unavail_range() that explicitly walks reserved && !memory, it looks like this is expected, and its just me thinking this is strange. I will come up with a version of this patch that walks the 'System RAM' resources that were created during boot, and adds the memblock_reserved() regions to them, which should stop this happening. Thanks, James ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 1/5] arm64: export memblock_reserve()d regions via /proc/iomem
On Tue, Aug 21, 2018 at 3:22 AM, James Morse wrote: > On 08/21/2018 05:39 AM, John Stultz wrote: >> >> Since this patch landed, on the HiKey board at bootup I'm seeing: >> >> [0.451884] WARNING: CPU: 1 PID: 1 at arch/arm64/kernel/setup.c:271 >> reserve_memblock_reserved_regions+0xd4/0x13c ... >> From skimming the patch, it seems this is maybe expected? Or should >> this warning raise eyebrows? I can't quite figure it out. > > Ugh, sorry for the noise! This is warning that there is something wrong > with our assumptions about what types of memory exist. > > >> It seems to trigger on the pstore memory at 0x21f0-0x21ff. > > > ... pmem ... > >> >> /proc/iomem now has: >> ... >> 0741-21ef : System RAM >>1100-1113cfff : reserved > > >> 21f0-21ff : reserved > > > ^ This entry is what triggered the warning. > > It expects that meblock_reserved() memory is also described as memory. > (memblock keeps them as separate lists, so its possible to reserve > memory that doesn't exist... which it looks like your system is doing) So yea, I suspect the hikey dts isn't quite right here then. I've always thought how it setup the memory was a bit strange: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts#n30 So from your mail, I suspect the hole in the memory map for the pstore buffer isn't correct, and we should rework that. I'll give that a shot here and make sure pstore still works properly. thanks -john ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 1/5] arm64: export memblock_reserve()d regions via /proc/iomem
On 08/21/2018 11:22 AM, James Morse wrote:
On 08/21/2018 05:39 AM, John Stultz wrote:
On Sun, Jul 22, 2018 at 6:57 PM, AKASHI Takahiro
wrote:
From: James Morse
There has been some confusion around what is necessary to prevent kexec
overwriting important memory regions. memblock: reserve, or nomap?
Only memblock nomap regions are reported via /proc/iomem, kexec's
user-space doesn't know about memblock_reserve()d regions.
Until commit f56ab9a5b73ca ("efi/arm: Don't mark ACPI reclaim memory
as MEMBLOCK_NOMAP") the ACPI tables were nomap, now they are reserved
and thus possible for kexec to overwrite with the new kernel or initrd.
But this was always broken, as the UEFI memory map is also reserved
and not marked as nomap.
Exporting both nomap and reserved memblock types is a nuisance as
they live in different memblock structures which we can't walk at
the same time.
Take a second walk over memblock.reserved and add new 'reserved'
subnodes for the memblock_reserved() regions that aren't already
described by the existing code. (e.g. Kernel Code)
We use reserve_region_with_split() to find the gaps in existing named
regions. This handles the gap between 'kernel code' and 'kernel data'
which is memblock_reserve()d, but already partially described by
request_standard_resources(). e.g.:
| 8000-dfff : System RAM
| 8008-80ff : Kernel code
| 8100-8158 : reserved
| 8159-8237efff : Kernel data
| a000-dfff : Crash kernel
| e00f-f949 : System RAM
reserve_region_with_split needs kzalloc() which isn't available when
request_standard_resources() is called, use an initcall.
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 30ad2f085d1f..5b4fac434c84 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -241,6 +241,44 @@ static void __init request_standard_resources(void)
+static int __init reserve_memblock_reserved_regions(void)
+ for_each_reserved_mem_region(i, , ) {
+ if (end <= roundup_end)
+ continue; /* done already */
+
+ start = __pfn_to_phys(PFN_DOWN(start));
+ end = __pfn_to_phys(PFN_UP(end)) - 1;
+ roundup_end = end;
+
+ res = kzalloc(sizeof(*res), GFP_ATOMIC);
+ if (WARN_ON(!res))
+ return -ENOMEM;
+ res->start = start;
+ res->end = end;
+ res->name = "reserved";
+ res->flags = IORESOURCE_MEM;
+
+ mem = request_resource_conflict(_resource, res);
+ /*
+ * We expected memblock_reserve() regions to conflict with
+ * memory created by request_standard_resources().
+ */
+ if (WARN_ON_ONCE(!mem))
Since this patch landed, on the HiKey board at bootup I'm seeing:
[ 0.451884] WARNING: CPU: 1 PID: 1 at arch/arm64/kernel/setup.c:271
reserve_memblock_reserved_regions+0xd4/0x13c
From skimming the patch, it seems this is maybe expected? Or should
this warning raise eyebrows? I can't quite figure it out.
Ugh, sorry for the noise! This is warning that there is something wrong
with our assumptions about what types of memory exist.
It seems to trigger on the pstore memory at 0x21f0-0x21ff.
... pmem ...
So, this is a memblock_reserved() range that isn't actually memory.
This happens because your DT has carved these regions out of the memory
node, but added a named 'reserved-memory' region for them, just in case?
I'm not sure what it means if 'reserved-memory' is not also described as
memory
You do need the carve-out, otherwise this gets covered by the linear
map, and when you throw in that 'unbuffered' property you get both a
cacheable and uncacheable mapping of the same page.
Hmm, looks like its (even) more complicated than I thought, of_reserved_mem.c's
definition of 'nomap' is memblock_remove(), not memblock_mark_nomap().
This might be more common to all users of DTs memreserve.
Thanks,
James
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 1/5] arm64: export memblock_reserve()d regions via /proc/iomem
Hi John,
On 08/21/2018 05:39 AM, John Stultz wrote:
On Sun, Jul 22, 2018 at 6:57 PM, AKASHI Takahiro
wrote:
From: James Morse
There has been some confusion around what is necessary to prevent kexec
overwriting important memory regions. memblock: reserve, or nomap?
Only memblock nomap regions are reported via /proc/iomem, kexec's
user-space doesn't know about memblock_reserve()d regions.
Until commit f56ab9a5b73ca ("efi/arm: Don't mark ACPI reclaim memory
as MEMBLOCK_NOMAP") the ACPI tables were nomap, now they are reserved
and thus possible for kexec to overwrite with the new kernel or initrd.
But this was always broken, as the UEFI memory map is also reserved
and not marked as nomap.
Exporting both nomap and reserved memblock types is a nuisance as
they live in different memblock structures which we can't walk at
the same time.
Take a second walk over memblock.reserved and add new 'reserved'
subnodes for the memblock_reserved() regions that aren't already
described by the existing code. (e.g. Kernel Code)
We use reserve_region_with_split() to find the gaps in existing named
regions. This handles the gap between 'kernel code' and 'kernel data'
which is memblock_reserve()d, but already partially described by
request_standard_resources(). e.g.:
| 8000-dfff : System RAM
| 8008-80ff : Kernel code
| 8100-8158 : reserved
| 8159-8237efff : Kernel data
| a000-dfff : Crash kernel
| e00f-f949 : System RAM
reserve_region_with_split needs kzalloc() which isn't available when
request_standard_resources() is called, use an initcall.
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 30ad2f085d1f..5b4fac434c84 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -241,6 +241,44 @@ static void __init request_standard_resources(void)
+static int __init reserve_memblock_reserved_regions(void)
+ for_each_reserved_mem_region(i, , ) {
+ if (end <= roundup_end)
+ continue; /* done already */
+
+ start = __pfn_to_phys(PFN_DOWN(start));
+ end = __pfn_to_phys(PFN_UP(end)) - 1;
+ roundup_end = end;
+
+ res = kzalloc(sizeof(*res), GFP_ATOMIC);
+ if (WARN_ON(!res))
+ return -ENOMEM;
+ res->start = start;
+ res->end = end;
+ res->name = "reserved";
+ res->flags = IORESOURCE_MEM;
+
+ mem = request_resource_conflict(_resource, res);
+ /*
+* We expected memblock_reserve() regions to conflict with
+* memory created by request_standard_resources().
+*/
+ if (WARN_ON_ONCE(!mem))
+ continue;
+ kfree(res);
+
+ reserve_region_with_split(mem, start, end, "reserved");
+ }
+
+ return 0;
+}
+arch_initcall(reserve_memblock_reserved_regions);
+
Since this patch landed, on the HiKey board at bootup I'm seeing:
[0.451884] WARNING: CPU: 1 PID: 1 at arch/arm64/kernel/setup.c:271
reserve_memblock_reserved_regions+0xd4/0x13c
[0.451896] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
4.18.0-10758-ga534dc3 #709
[0.451903] Hardware name: HiKey Development Board (DT)
[0.451913] pstate: 8045 (Nzcv daif +PAN -UAO)
[0.451922] pc : reserve_memblock_reserved_regions+0xd4/0x13c
[0.451931] lr : reserve_memblock_reserved_regions+0xcc/0x13c
[0.451938] sp : ff8008053d30
[0.451945] x29: ff8008053d30 x28: ff8008ebe650
[0.451957] x27: ff8008ead060 x26: ff8008e113b0
[0.451969] x25: x24: 00488020
[0.451981] x23: 21ff x22: ff8008e0d860
[0.451993] x21: ff8008d74370 x20: ff8009019000
[0.452005] x19: ffc07507a400 x18: ff8009019a48
[0.452017] x17: x16:
[0.452028] x15: ff80890e973f x14: 0006
[0.452040] x13: x12:
[0.452051] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
[0.452063] x9 : x8 : ffc07507a480
[0.452074] x7 : x6 : ffc07c30
[0.452086] x5 : x4 : 21ff
[0.452097] x3 : 0001 x2 : 0001
[0.452109] x1 : x0 :
[0.452121] Call trace:
[0.452130] reserve_memblock_reserved_regions+0xd4/0x13c
[0.452140] do_one_initcall+0x78/0x150
[0.452148] kernel_init_freeable+0x198/0x258
[0.452159] kernel_init+0x10/0x108
[0.452170] ret_from_fork+0x10/0x18
[0.452181] ---[ end trace b4b78c443df3a750 ]---
From skimming the patch, it seems this is maybe expected? Or should
this warning raise eyebrows? I can't quite figure it out.
Ugh, sorry for the noise! This is warning that there is something wrong
with our assumptions about what types of
Re: [PATCH v4 1/5] arm64: export memblock_reserve()d regions via /proc/iomem
Hi John,
On Mon, Aug 20, 2018 at 09:39:01PM -0700, John Stultz wrote:
> On Sun, Jul 22, 2018 at 6:57 PM, AKASHI Takahiro
> wrote:
> > From: James Morse
> >
> > There has been some confusion around what is necessary to prevent kexec
> > overwriting important memory regions. memblock: reserve, or nomap?
> > Only memblock nomap regions are reported via /proc/iomem, kexec's
> > user-space doesn't know about memblock_reserve()d regions.
> >
> > Until commit f56ab9a5b73ca ("efi/arm: Don't mark ACPI reclaim memory
> > as MEMBLOCK_NOMAP") the ACPI tables were nomap, now they are reserved
> > and thus possible for kexec to overwrite with the new kernel or initrd.
> > But this was always broken, as the UEFI memory map is also reserved
> > and not marked as nomap.
> >
> > Exporting both nomap and reserved memblock types is a nuisance as
> > they live in different memblock structures which we can't walk at
> > the same time.
> >
> > Take a second walk over memblock.reserved and add new 'reserved'
> > subnodes for the memblock_reserved() regions that aren't already
> > described by the existing code. (e.g. Kernel Code)
> >
> > We use reserve_region_with_split() to find the gaps in existing named
> > regions. This handles the gap between 'kernel code' and 'kernel data'
> > which is memblock_reserve()d, but already partially described by
> > request_standard_resources(). e.g.:
> > | 8000-dfff : System RAM
> > | 8008-80ff : Kernel code
> > | 8100-8158 : reserved
> > | 8159-8237efff : Kernel data
> > | a000-dfff : Crash kernel
> > | e00f-f949 : System RAM
> >
> > reserve_region_with_split needs kzalloc() which isn't available when
> > request_standard_resources() is called, use an initcall.
> >
> > Reported-by: Bhupesh Sharma
> > Reported-by: Tyler Baicar
> > Suggested-by: Akashi Takahiro
> > Signed-off-by: James Morse
> > Fixes: d28f6df1305a ("arm64/kexec: Add core kexec support")
> > Reviewed-by: Ard Biesheuvel
> > CC: Mark Rutland
> > ---
> > arch/arm64/kernel/setup.c | 38 ++
> > 1 file changed, 38 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> > index 30ad2f085d1f..5b4fac434c84 100644
> > --- a/arch/arm64/kernel/setup.c
> > +++ b/arch/arm64/kernel/setup.c
> > @@ -241,6 +241,44 @@ static void __init request_standard_resources(void)
> > }
> > }
> >
> > +static int __init reserve_memblock_reserved_regions(void)
> > +{
> > + phys_addr_t start, end, roundup_end = 0;
> > + struct resource *mem, *res;
> > + u64 i;
> > +
> > + for_each_reserved_mem_region(i, , ) {
> > + if (end <= roundup_end)
> > + continue; /* done already */
> > +
> > + start = __pfn_to_phys(PFN_DOWN(start));
> > + end = __pfn_to_phys(PFN_UP(end)) - 1;
> > + roundup_end = end;
> > +
> > + res = kzalloc(sizeof(*res), GFP_ATOMIC);
> > + if (WARN_ON(!res))
> > + return -ENOMEM;
> > + res->start = start;
> > + res->end = end;
> > + res->name = "reserved";
> > + res->flags = IORESOURCE_MEM;
> > +
> > + mem = request_resource_conflict(_resource, res);
> > + /*
> > +* We expected memblock_reserve() regions to conflict with
> > +* memory created by request_standard_resources().
> > +*/
> > + if (WARN_ON_ONCE(!mem))
> > + continue;
> > + kfree(res);
> > +
> > + reserve_region_with_split(mem, start, end, "reserved");
> > + }
> > +
> > + return 0;
> > +}
> > +arch_initcall(reserve_memblock_reserved_regions);
> > +
>
> Since this patch landed, on the HiKey board at bootup I'm seeing:
>
> [0.451884] WARNING: CPU: 1 PID: 1 at arch/arm64/kernel/setup.c:271
> reserve_memblock_reserved_regions+0xd4/0x13c
> [0.451896] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
> 4.18.0-10758-ga534dc3 #709
> [0.451903] Hardware name: HiKey Development Board (DT)
> [0.451913] pstate: 8045 (Nzcv daif +PAN -UAO)
> [0.451922] pc : reserve_memblock_reserved_regions+0xd4/0x13c
> [0.451931] lr : reserve_memblock_reserved_regions+0xcc/0x13c
> [0.451938] sp : ff8008053d30
> [0.451945] x29: ff8008053d30 x28: ff8008ebe650
> [0.451957] x27: ff8008ead060 x26: ff8008e113b0
> [0.451969] x25: x24: 00488020
> [0.451981] x23: 21ff x22: ff8008e0d860
> [0.451993] x21: ff8008d74370 x20: ff8009019000
> [0.452005] x19: ffc07507a400 x18: ff8009019a48
> [0.452017] x17: x16:
> [0.452028] x15: ff80890e973f x14: 0006
> [0.452040] x13: x12:
> [0.452051] x11: 0101010101010101 x10:
[PATCH v4 1/5] arm64: export memblock_reserve()d regions via /proc/iomem
From: James Morse
There has been some confusion around what is necessary to prevent kexec
overwriting important memory regions. memblock: reserve, or nomap?
Only memblock nomap regions are reported via /proc/iomem, kexec's
user-space doesn't know about memblock_reserve()d regions.
Until commit f56ab9a5b73ca ("efi/arm: Don't mark ACPI reclaim memory
as MEMBLOCK_NOMAP") the ACPI tables were nomap, now they are reserved
and thus possible for kexec to overwrite with the new kernel or initrd.
But this was always broken, as the UEFI memory map is also reserved
and not marked as nomap.
Exporting both nomap and reserved memblock types is a nuisance as
they live in different memblock structures which we can't walk at
the same time.
Take a second walk over memblock.reserved and add new 'reserved'
subnodes for the memblock_reserved() regions that aren't already
described by the existing code. (e.g. Kernel Code)
We use reserve_region_with_split() to find the gaps in existing named
regions. This handles the gap between 'kernel code' and 'kernel data'
which is memblock_reserve()d, but already partially described by
request_standard_resources(). e.g.:
| 8000-dfff : System RAM
| 8008-80ff : Kernel code
| 8100-8158 : reserved
| 8159-8237efff : Kernel data
| a000-dfff : Crash kernel
| e00f-f949 : System RAM
reserve_region_with_split needs kzalloc() which isn't available when
request_standard_resources() is called, use an initcall.
Reported-by: Bhupesh Sharma
Reported-by: Tyler Baicar
Suggested-by: Akashi Takahiro
Signed-off-by: James Morse
Fixes: d28f6df1305a ("arm64/kexec: Add core kexec support")
Reviewed-by: Ard Biesheuvel
CC: Mark Rutland
---
arch/arm64/kernel/setup.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 30ad2f085d1f..5b4fac434c84 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -241,6 +241,44 @@ static void __init request_standard_resources(void)
}
}
+static int __init reserve_memblock_reserved_regions(void)
+{
+ phys_addr_t start, end, roundup_end = 0;
+ struct resource *mem, *res;
+ u64 i;
+
+ for_each_reserved_mem_region(i, , ) {
+ if (end <= roundup_end)
+ continue; /* done already */
+
+ start = __pfn_to_phys(PFN_DOWN(start));
+ end = __pfn_to_phys(PFN_UP(end)) - 1;
+ roundup_end = end;
+
+ res = kzalloc(sizeof(*res), GFP_ATOMIC);
+ if (WARN_ON(!res))
+ return -ENOMEM;
+ res->start = start;
+ res->end = end;
+ res->name = "reserved";
+ res->flags = IORESOURCE_MEM;
+
+ mem = request_resource_conflict(_resource, res);
+ /*
+* We expected memblock_reserve() regions to conflict with
+* memory created by request_standard_resources().
+*/
+ if (WARN_ON_ONCE(!mem))
+ continue;
+ kfree(res);
+
+ reserve_region_with_split(mem, start, end, "reserved");
+ }
+
+ return 0;
+}
+arch_initcall(reserve_memblock_reserved_regions);
+
u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
void __init setup_arch(char **cmdline_p)
--
2.18.0
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
