[PATCH] arm64: kdump: retain reserved memory regions

[PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

This is a fix against the issue that crash dump kernel may hang up
during booting, which can happen on any ACPI-based system with "ACPI
Reclaim Memory."

	<kicking off kdump after panic>
	Bye!
	   (snip...)
	ACPI: Core revision 20170728
	pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
	Internal error: Oops: 96000021 [#1] SMP
	Modules linked in:
	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
	task: ffff000008d05180 task.stack: ffff000008cc0000
	PC is at acpi_ns_lookup+0x25c/0x3c0
	LR is at acpi_ds_load1_begin_op+0xa4/0x294
	   (snip...)
	Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
	Call trace:
	   (snip...)
	[<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
	[<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
	[<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
	[<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
	[<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
	[<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
	[<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
	[<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
	[<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
	[<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
	[<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
	[<ffff000008badc20>] acpi_early_init+0x9c/0xd0
	[<ffff000008b70d50>] start_kernel+0x3b4/0x43c
	Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
	---[ end trace c46ed37f9651c58e ]---
	Kernel panic - not syncing: Fatal exception
	Rebooting in 10 seconds..

(diagnosis)
* This fault is a data abort, alignment fault (ESR=0x96000021)
  during reading out ACPI table.
* Initial ACPI tables are normally stored in system ram and marked as
  "ACPI Reclaim memory" by the firmware.
* After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
  memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
  removing NOMAP bit and they are instead "memblock-reserved".
* When crash dump kernel boots up, it tries to accesses ACPI tables by
  ioremap'ing them (through acpi_os_ioremap()).
* Since those regions are not included in device tree's
  "usable-memory-range" and so not recognized as part of crash dump
  kernel's system ram, ioremap() will create a non-cacheable mapping here.
* ACPI accessor/helper functions are compiled in without unaligned access
  support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
  panic when accessing ACPI tables.

With this patch, all the reserved memory regions, as well as NOMAP-
attributed ones which are presumably ACPI runtime code and data, are set
to be retained in system ram even if they are outside of usable memory
range specified by device tree blob. Accordingly, ACPI tables are mapped
as cacheable and can be safely accessed without causing unaligned access
faults.

Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 arch/arm64/mm/init.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 2d5a443b205c..e4a8b64a09b1 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -352,11 +352,23 @@ static void __init fdt_enforce_memory_region(void)
 	struct memblock_region reg = {
 		.size = 0,
 	};
+	u64 idx;
+	phys_addr_t start, end;
 
 	of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
 
-	if (reg.size)
-		memblock_cap_memory_range(reg.base, reg.size);
+	if (reg.size) {
+retry:
+		/* exclude usable & !reserved memory */
+		for_each_free_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE,
+					&start, &end, NULL) {
+			memblock_remove(start, end - start);
+			goto retry;
+		}
+
+		/* add back fdt's usable memory */
+		memblock_add(reg.base, reg.size);
+	}
 }
 
 void __init arm64_memblock_init(void)
-- 
2.15.1

[PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

This is a fix against the issue that crash dump kernel may hang up
during booting, which can happen on any ACPI-based system with "ACPI
Reclaim Memory."

	<kicking off kdump after panic>
	Bye!
	   (snip...)
	ACPI: Core revision 20170728
	pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
	Internal error: Oops: 96000021 [#1] SMP
	Modules linked in:
	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
	task: ffff000008d05180 task.stack: ffff000008cc0000
	PC is at acpi_ns_lookup+0x25c/0x3c0
	LR is at acpi_ds_load1_begin_op+0xa4/0x294
	   (snip...)
	Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
	Call trace:
	   (snip...)
	[<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
	[<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
	[<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
	[<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
	[<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
	[<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
	[<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
	[<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
	[<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
	[<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
	[<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
	[<ffff000008badc20>] acpi_early_init+0x9c/0xd0
	[<ffff000008b70d50>] start_kernel+0x3b4/0x43c
	Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
	---[ end trace c46ed37f9651c58e ]---
	Kernel panic - not syncing: Fatal exception
	Rebooting in 10 seconds..

(diagnosis)
* This fault is a data abort, alignment fault (ESR=0x96000021)
  during reading out ACPI table.
* Initial ACPI tables are normally stored in system ram and marked as
  "ACPI Reclaim memory" by the firmware.
* After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
  memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
  removing NOMAP bit and they are instead "memblock-reserved".
* When crash dump kernel boots up, it tries to accesses ACPI tables by
  ioremap'ing them (through acpi_os_ioremap()).
* Since those regions are not included in device tree's
  "usable-memory-range" and so not recognized as part of crash dump
  kernel's system ram, ioremap() will create a non-cacheable mapping here.
* ACPI accessor/helper functions are compiled in without unaligned access
  support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
  panic when accessing ACPI tables.

With this patch, all the reserved memory regions, as well as NOMAP-
attributed ones which are presumably ACPI runtime code and data, are set
to be retained in system ram even if they are outside of usable memory
range specified by device tree blob. Accordingly, ACPI tables are mapped
as cacheable and can be safely accessed without causing unaligned access
faults.

Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 arch/arm64/mm/init.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 2d5a443b205c..e4a8b64a09b1 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -352,11 +352,23 @@ static void __init fdt_enforce_memory_region(void)
 	struct memblock_region reg = {
 		.size = 0,
 	};
+	u64 idx;
+	phys_addr_t start, end;
 
 	of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
 
-	if (reg.size)
-		memblock_cap_memory_range(reg.base, reg.size);
+	if (reg.size) {
+retry:
+		/* exclude usable & !reserved memory */
+		for_each_free_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE,
+					&start, &end, NULL) {
+			memblock_remove(start, end - start);
+			goto retry;
+		}
+
+		/* add back fdt's usable memory */
+		memblock_add(reg.base, reg.size);
+	}
 }
 
 void __init arm64_memblock_init(void)
-- 
2.15.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[Ard Biesheuvel]

On 10 January 2018 at 10:09, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> This is a fix against the issue that crash dump kernel may hang up
> during booting, which can happen on any ACPI-based system with "ACPI
> Reclaim Memory."
>
>         <kicking off kdump after panic>
>         Bye!
>            (snip...)
>         ACPI: Core revision 20170728
>         pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
>         Internal error: Oops: 96000021 [#1] SMP
>         Modules linked in:
>         CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
>         task: ffff000008d05180 task.stack: ffff000008cc0000
>         PC is at acpi_ns_lookup+0x25c/0x3c0
>         LR is at acpi_ds_load1_begin_op+0xa4/0x294
>            (snip...)
>         Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
>         Call trace:
>            (snip...)
>         [<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
>         [<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
>         [<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
>         [<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
>         [<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
>         [<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
>         [<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
>         [<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
>         [<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
>         [<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
>         [<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
>         [<ffff000008badc20>] acpi_early_init+0x9c/0xd0
>         [<ffff000008b70d50>] start_kernel+0x3b4/0x43c
>         Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
>         ---[ end trace c46ed37f9651c58e ]---
>         Kernel panic - not syncing: Fatal exception
>         Rebooting in 10 seconds..
>
> (diagnosis)
> * This fault is a data abort, alignment fault (ESR=0x96000021)
>   during reading out ACPI table.
> * Initial ACPI tables are normally stored in system ram and marked as
>   "ACPI Reclaim memory" by the firmware.
> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>   removing NOMAP bit and they are instead "memblock-reserved".
> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>   ioremap'ing them (through acpi_os_ioremap()).
> * Since those regions are not included in device tree's
>   "usable-memory-range" and so not recognized as part of crash dump
>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> * ACPI accessor/helper functions are compiled in without unaligned access
>   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
>   panic when accessing ACPI tables.
>
> With this patch, all the reserved memory regions, as well as NOMAP-
> attributed ones which are presumably ACPI runtime code and data, are set
> to be retained in system ram even if they are outside of usable memory
> range specified by device tree blob. Accordingly, ACPI tables are mapped
> as cacheable and can be safely accessed without causing unaligned access
> faults.
>
> Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm64/mm/init.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 2d5a443b205c..e4a8b64a09b1 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -352,11 +352,23 @@ static void __init fdt_enforce_memory_region(void)
>         struct memblock_region reg = {
>                 .size = 0,
>         };
> +       u64 idx;
> +       phys_addr_t start, end;
>
>         of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
>
> -       if (reg.size)
> -               memblock_cap_memory_range(reg.base, reg.size);

Given that memblock_cap_memory_range() was introduced by you for
kdump, is there any way to handle it there?
If not, should we remove it?

> +       if (reg.size) {
> +retry:
> +               /* exclude usable & !reserved memory */
> +               for_each_free_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE,
> +                                       &start, &end, NULL) {
> +                       memblock_remove(start, end - start);
> +                       goto retry;
> +               }
> +
> +               /* add back fdt's usable memory */
> +               memblock_add(reg.base, reg.size);
> +       }
>  }
>
>  void __init arm64_memblock_init(void)
> --
> 2.15.1
>

Re: [PATCH] arm64: kdump: retain reserved memory regions

[Ard Biesheuvel]

On 10 January 2018 at 10:09, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> This is a fix against the issue that crash dump kernel may hang up
> during booting, which can happen on any ACPI-based system with "ACPI
> Reclaim Memory."
>
>         <kicking off kdump after panic>
>         Bye!
>            (snip...)
>         ACPI: Core revision 20170728
>         pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
>         Internal error: Oops: 96000021 [#1] SMP
>         Modules linked in:
>         CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
>         task: ffff000008d05180 task.stack: ffff000008cc0000
>         PC is at acpi_ns_lookup+0x25c/0x3c0
>         LR is at acpi_ds_load1_begin_op+0xa4/0x294
>            (snip...)
>         Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
>         Call trace:
>            (snip...)
>         [<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
>         [<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
>         [<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
>         [<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
>         [<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
>         [<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
>         [<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
>         [<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
>         [<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
>         [<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
>         [<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
>         [<ffff000008badc20>] acpi_early_init+0x9c/0xd0
>         [<ffff000008b70d50>] start_kernel+0x3b4/0x43c
>         Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
>         ---[ end trace c46ed37f9651c58e ]---
>         Kernel panic - not syncing: Fatal exception
>         Rebooting in 10 seconds..
>
> (diagnosis)
> * This fault is a data abort, alignment fault (ESR=0x96000021)
>   during reading out ACPI table.
> * Initial ACPI tables are normally stored in system ram and marked as
>   "ACPI Reclaim memory" by the firmware.
> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>   removing NOMAP bit and they are instead "memblock-reserved".
> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>   ioremap'ing them (through acpi_os_ioremap()).
> * Since those regions are not included in device tree's
>   "usable-memory-range" and so not recognized as part of crash dump
>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> * ACPI accessor/helper functions are compiled in without unaligned access
>   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
>   panic when accessing ACPI tables.
>
> With this patch, all the reserved memory regions, as well as NOMAP-
> attributed ones which are presumably ACPI runtime code and data, are set
> to be retained in system ram even if they are outside of usable memory
> range specified by device tree blob. Accordingly, ACPI tables are mapped
> as cacheable and can be safely accessed without causing unaligned access
> faults.
>
> Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm64/mm/init.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 2d5a443b205c..e4a8b64a09b1 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -352,11 +352,23 @@ static void __init fdt_enforce_memory_region(void)
>         struct memblock_region reg = {
>                 .size = 0,
>         };
> +       u64 idx;
> +       phys_addr_t start, end;
>
>         of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
>
> -       if (reg.size)
> -               memblock_cap_memory_range(reg.base, reg.size);

Given that memblock_cap_memory_range() was introduced by you for
kdump, is there any way to handle it there?
If not, should we remove it?

> +       if (reg.size) {
> +retry:
> +               /* exclude usable & !reserved memory */
> +               for_each_free_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE,
> +                                       &start, &end, NULL) {
> +                       memblock_remove(start, end - start);
> +                       goto retry;
> +               }
> +
> +               /* add back fdt's usable memory */
> +               memblock_add(reg.base, reg.size);
> +       }
>  }
>
>  void __init arm64_memblock_init(void)
> --
> 2.15.1
>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[James Morse]

Hi Akashi,

On 10/01/18 10:09, AKASHI Takahiro wrote:
> This is a fix against the issue that crash dump kernel may hang up
> during booting, which can happen on any ACPI-based system with "ACPI
> Reclaim Memory."
> 
> 	<kicking off kdump after panic>
> 	Bye!
> 	   (snip...)
> 	ACPI: Core revision 20170728
> 	pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> 	Internal error: Oops: 96000021 [#1] SMP
> 	Modules linked in:
> 	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> 	task: ffff000008d05180 task.stack: ffff000008cc0000
> 	PC is at acpi_ns_lookup+0x25c/0x3c0
> 	LR is at acpi_ds_load1_begin_op+0xa4/0x294
> 	   (snip...)
> 	Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> 	Call trace:
> 	   (snip...)
> 	[<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> 	[<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> 	[<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> 	[<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> 	[<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> 	[<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> 	[<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> 	[<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> 	[<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> 	[<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> 	[<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> 	[<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> 	[<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> 	Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> 	---[ end trace c46ed37f9651c58e ]---
> 	Kernel panic - not syncing: Fatal exception
> 	Rebooting in 10 seconds..
> 
> (diagnosis)
> * This fault is a data abort, alignment fault (ESR=0x96000021)
>   during reading out ACPI table.
> * Initial ACPI tables are normally stored in system ram and marked as
>   "ACPI Reclaim memory" by the firmware.
> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>   removing NOMAP bit and they are instead "memblock-reserved".
> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>   ioremap'ing them (through acpi_os_ioremap()).
> * Since those regions are not included in device tree's
>   "usable-memory-range" and so not recognized as part of crash dump
>   kernel's system ram, ioremap() will create a non-cacheable mapping here.

Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
what we pulled into memblock, which is different during kdump.

Is an alternative to teach acpi_os_ioremap() to ask
efi_mem_attributes() directly for the attributes to use?
(e.g. arch_apei_get_mem_attribute())


> * ACPI accessor/helper functions are compiled in without unaligned access
>   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
>   panic when accessing ACPI tables.
> 
> With this patch, all the reserved memory regions, as well as NOMAP-
> attributed ones which are presumably ACPI runtime code and data, are set
> to be retained in system ram even if they are outside of usable memory
> range specified by device tree blob. Accordingly, ACPI tables are mapped
> as cacheable and can be safely accessed without causing unaligned access
> faults.


Thanks,

James

Re: [PATCH] arm64: kdump: retain reserved memory regions

[James Morse]

Hi Akashi,

On 10/01/18 10:09, AKASHI Takahiro wrote:
> This is a fix against the issue that crash dump kernel may hang up
> during booting, which can happen on any ACPI-based system with "ACPI
> Reclaim Memory."
> 
> 	<kicking off kdump after panic>
> 	Bye!
> 	   (snip...)
> 	ACPI: Core revision 20170728
> 	pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> 	Internal error: Oops: 96000021 [#1] SMP
> 	Modules linked in:
> 	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> 	task: ffff000008d05180 task.stack: ffff000008cc0000
> 	PC is at acpi_ns_lookup+0x25c/0x3c0
> 	LR is at acpi_ds_load1_begin_op+0xa4/0x294
> 	   (snip...)
> 	Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> 	Call trace:
> 	   (snip...)
> 	[<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> 	[<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> 	[<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> 	[<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> 	[<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> 	[<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> 	[<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> 	[<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> 	[<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> 	[<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> 	[<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> 	[<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> 	[<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> 	Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> 	---[ end trace c46ed37f9651c58e ]---
> 	Kernel panic - not syncing: Fatal exception
> 	Rebooting in 10 seconds..
> 
> (diagnosis)
> * This fault is a data abort, alignment fault (ESR=0x96000021)
>   during reading out ACPI table.
> * Initial ACPI tables are normally stored in system ram and marked as
>   "ACPI Reclaim memory" by the firmware.
> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>   removing NOMAP bit and they are instead "memblock-reserved".
> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>   ioremap'ing them (through acpi_os_ioremap()).
> * Since those regions are not included in device tree's
>   "usable-memory-range" and so not recognized as part of crash dump
>   kernel's system ram, ioremap() will create a non-cacheable mapping here.

Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
what we pulled into memblock, which is different during kdump.

Is an alternative to teach acpi_os_ioremap() to ask
efi_mem_attributes() directly for the attributes to use?
(e.g. arch_apei_get_mem_attribute())


> * ACPI accessor/helper functions are compiled in without unaligned access
>   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
>   panic when accessing ACPI tables.
> 
> With this patch, all the reserved memory regions, as well as NOMAP-
> attributed ones which are presumably ACPI runtime code and data, are set
> to be retained in system ram even if they are outside of usable memory
> range specified by device tree blob. Accordingly, ACPI tables are mapped
> as cacheable and can be safely accessed without causing unaligned access
> faults.


Thanks,

James

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

On Wed, Jan 10, 2018 at 11:09:32AM +0000, Ard Biesheuvel wrote:
> On 10 January 2018 at 10:09, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > This is a fix against the issue that crash dump kernel may hang up
> > during booting, which can happen on any ACPI-based system with "ACPI
> > Reclaim Memory."
> >
> >         <kicking off kdump after panic>
> >         Bye!
> >            (snip...)
> >         ACPI: Core revision 20170728
> >         pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> >         Internal error: Oops: 96000021 [#1] SMP
> >         Modules linked in:
> >         CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> >         task: ffff000008d05180 task.stack: ffff000008cc0000
> >         PC is at acpi_ns_lookup+0x25c/0x3c0
> >         LR is at acpi_ds_load1_begin_op+0xa4/0x294
> >            (snip...)
> >         Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> >         Call trace:
> >            (snip...)
> >         [<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> >         [<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> >         [<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> >         [<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> >         [<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> >         [<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> >         [<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> >         [<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> >         [<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> >         [<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> >         [<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> >         [<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> >         [<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> >         Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> >         ---[ end trace c46ed37f9651c58e ]---
> >         Kernel panic - not syncing: Fatal exception
> >         Rebooting in 10 seconds..
> >
> > (diagnosis)
> > * This fault is a data abort, alignment fault (ESR=0x96000021)
> >   during reading out ACPI table.
> > * Initial ACPI tables are normally stored in system ram and marked as
> >   "ACPI Reclaim memory" by the firmware.
> > * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> >   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> >   removing NOMAP bit and they are instead "memblock-reserved".
> > * When crash dump kernel boots up, it tries to accesses ACPI tables by
> >   ioremap'ing them (through acpi_os_ioremap()).
> > * Since those regions are not included in device tree's
> >   "usable-memory-range" and so not recognized as part of crash dump
> >   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> > * ACPI accessor/helper functions are compiled in without unaligned access
> >   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
> >   panic when accessing ACPI tables.
> >
> > With this patch, all the reserved memory regions, as well as NOMAP-
> > attributed ones which are presumably ACPI runtime code and data, are set
> > to be retained in system ram even if they are outside of usable memory
> > range specified by device tree blob. Accordingly, ACPI tables are mapped
> > as cacheable and can be safely accessed without causing unaligned access
> > faults.
> >
> > Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  arch/arm64/mm/init.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index 2d5a443b205c..e4a8b64a09b1 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -352,11 +352,23 @@ static void __init fdt_enforce_memory_region(void)
> >         struct memblock_region reg = {
> >                 .size = 0,
> >         };
> > +       u64 idx;
> > +       phys_addr_t start, end;
> >
> >         of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
> >
> > -       if (reg.size)
> > -               memblock_cap_memory_range(reg.base, reg.size);
> 
> Given that memblock_cap_memory_range() was introduced by you for
> kdump, is there any way to handle it there?

Indeed, but I'm not sure that the new semantics of this function
is quite generic.

> If not, should we remove it?

I prefer to remove it.

Thanks,
-Takahiro AKASHI

> > +       if (reg.size) {
> > +retry:
> > +               /* exclude usable & !reserved memory */
> > +               for_each_free_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE,
> > +                                       &start, &end, NULL) {
> > +                       memblock_remove(start, end - start);
> > +                       goto retry;
> > +               }
> > +
> > +               /* add back fdt's usable memory */
> > +               memblock_add(reg.base, reg.size);
> > +       }
> >  }
> >
> >  void __init arm64_memblock_init(void)
> > --
> > 2.15.1
> >

Re: [PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

On Wed, Jan 10, 2018 at 11:09:32AM +0000, Ard Biesheuvel wrote:
> On 10 January 2018 at 10:09, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > This is a fix against the issue that crash dump kernel may hang up
> > during booting, which can happen on any ACPI-based system with "ACPI
> > Reclaim Memory."
> >
> >         <kicking off kdump after panic>
> >         Bye!
> >            (snip...)
> >         ACPI: Core revision 20170728
> >         pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> >         Internal error: Oops: 96000021 [#1] SMP
> >         Modules linked in:
> >         CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> >         task: ffff000008d05180 task.stack: ffff000008cc0000
> >         PC is at acpi_ns_lookup+0x25c/0x3c0
> >         LR is at acpi_ds_load1_begin_op+0xa4/0x294
> >            (snip...)
> >         Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> >         Call trace:
> >            (snip...)
> >         [<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> >         [<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> >         [<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> >         [<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> >         [<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> >         [<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> >         [<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> >         [<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> >         [<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> >         [<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> >         [<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> >         [<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> >         [<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> >         Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> >         ---[ end trace c46ed37f9651c58e ]---
> >         Kernel panic - not syncing: Fatal exception
> >         Rebooting in 10 seconds..
> >
> > (diagnosis)
> > * This fault is a data abort, alignment fault (ESR=0x96000021)
> >   during reading out ACPI table.
> > * Initial ACPI tables are normally stored in system ram and marked as
> >   "ACPI Reclaim memory" by the firmware.
> > * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> >   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> >   removing NOMAP bit and they are instead "memblock-reserved".
> > * When crash dump kernel boots up, it tries to accesses ACPI tables by
> >   ioremap'ing them (through acpi_os_ioremap()).
> > * Since those regions are not included in device tree's
> >   "usable-memory-range" and so not recognized as part of crash dump
> >   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> > * ACPI accessor/helper functions are compiled in without unaligned access
> >   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
> >   panic when accessing ACPI tables.
> >
> > With this patch, all the reserved memory regions, as well as NOMAP-
> > attributed ones which are presumably ACPI runtime code and data, are set
> > to be retained in system ram even if they are outside of usable memory
> > range specified by device tree blob. Accordingly, ACPI tables are mapped
> > as cacheable and can be safely accessed without causing unaligned access
> > faults.
> >
> > Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  arch/arm64/mm/init.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index 2d5a443b205c..e4a8b64a09b1 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -352,11 +352,23 @@ static void __init fdt_enforce_memory_region(void)
> >         struct memblock_region reg = {
> >                 .size = 0,
> >         };
> > +       u64 idx;
> > +       phys_addr_t start, end;
> >
> >         of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
> >
> > -       if (reg.size)
> > -               memblock_cap_memory_range(reg.base, reg.size);
> 
> Given that memblock_cap_memory_range() was introduced by you for
> kdump, is there any way to handle it there?

Indeed, but I'm not sure that the new semantics of this function
is quite generic.

> If not, should we remove it?

I prefer to remove it.

Thanks,
-Takahiro AKASHI

> > +       if (reg.size) {
> > +retry:
> > +               /* exclude usable & !reserved memory */
> > +               for_each_free_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE,
> > +                                       &start, &end, NULL) {
> > +                       memblock_remove(start, end - start);
> > +                       goto retry;
> > +               }
> > +
> > +               /* add back fdt's usable memory */
> > +               memblock_add(reg.base, reg.size);
> > +       }
> >  }
> >
> >  void __init arm64_memblock_init(void)
> > --
> > 2.15.1
> >

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

James,

On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
> Hi Akashi,
> 
> On 10/01/18 10:09, AKASHI Takahiro wrote:
> > This is a fix against the issue that crash dump kernel may hang up
> > during booting, which can happen on any ACPI-based system with "ACPI
> > Reclaim Memory."
> > 
> > 	<kicking off kdump after panic>
> > 	Bye!
> > 	   (snip...)
> > 	ACPI: Core revision 20170728
> > 	pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> > 	Internal error: Oops: 96000021 [#1] SMP
> > 	Modules linked in:
> > 	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> > 	task: ffff000008d05180 task.stack: ffff000008cc0000
> > 	PC is at acpi_ns_lookup+0x25c/0x3c0
> > 	LR is at acpi_ds_load1_begin_op+0xa4/0x294
> > 	   (snip...)
> > 	Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> > 	Call trace:
> > 	   (snip...)
> > 	[<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> > 	[<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> > 	[<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> > 	[<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> > 	[<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> > 	[<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> > 	[<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> > 	[<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> > 	[<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> > 	[<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> > 	[<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> > 	[<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> > 	[<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> > 	Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> > 	---[ end trace c46ed37f9651c58e ]---
> > 	Kernel panic - not syncing: Fatal exception
> > 	Rebooting in 10 seconds..
> > 
> > (diagnosis)
> > * This fault is a data abort, alignment fault (ESR=0x96000021)
> >   during reading out ACPI table.
> > * Initial ACPI tables are normally stored in system ram and marked as
> >   "ACPI Reclaim memory" by the firmware.
> > * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> >   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> >   removing NOMAP bit and they are instead "memblock-reserved".
> > * When crash dump kernel boots up, it tries to accesses ACPI tables by
> >   ioremap'ing them (through acpi_os_ioremap()).
> > * Since those regions are not included in device tree's
> >   "usable-memory-range" and so not recognized as part of crash dump
> >   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> 
> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
> what we pulled into memblock, which is different during kdump.
> 
> Is an alternative to teach acpi_os_ioremap() to ask
> efi_mem_attributes() directly for the attributes to use?
> (e.g. arch_apei_get_mem_attribute())

I didn't think of this approach.
Do you mean a change like the patch below?
(I'm still debugging this code since the kernel fails to boot.)

Thanks,
-Takahiro AKASHI

> 
> > * ACPI accessor/helper functions are compiled in without unaligned access
> >   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
> >   panic when accessing ACPI tables.
> > 
> > With this patch, all the reserved memory regions, as well as NOMAP-
> > attributed ones which are presumably ACPI runtime code and data, are set
> > to be retained in system ram even if they are outside of usable memory
> > range specified by device tree blob. Accordingly, ACPI tables are mapped
> > as cacheable and can be safely accessed without causing unaligned access
> > faults.
> 
> 
> Thanks,
> 
> James
===8<===
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 32f465a80e4e..6953aaaf2bfa 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -16,6 +16,7 @@
 #include <linux/psci.h>
 
 #include <asm/cputype.h>
+#include <asm/io.h>
 #include <asm/smp_plat.h>
 #include <asm/tlbflush.h>
 
@@ -29,18 +30,13 @@
 
 /* Basic configuration for ACPI */
 #ifdef	CONFIG_ACPI
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
+
 /* ACPI table mapping after acpi_permanent_mmap is set */
 static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
 					    acpi_size size)
 {
-	/*
-	 * EFI's reserve_regions() call adds memory with the WB attribute
-	 * to memblock via early_init_dt_add_memory_arch().
-	 */
-	if (!memblock_is_memory(phys))
-		return ioremap(phys, size);
-
-	return ioremap_cache(phys, size);
+	return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
 }
 #define acpi_os_ioremap acpi_os_ioremap
 
@@ -125,7 +121,10 @@ static inline const char *acpi_get_enable_method(int cpu)
  * for compatibility.
  */
 #define acpi_disable_cmcff 1
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
+static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+	return __acpi_get_mem_attribute(addr);
+}
 #endif /* CONFIG_ACPI_APEI */
 
 #ifdef CONFIG_ACPI_NUMA
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..43e9d8371f88 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -239,8 +239,7 @@ void __init acpi_boot_table_init(void)
 	}
 }
 
-#ifdef CONFIG_ACPI_APEI
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 {
 	/*
 	 * According to "Table 8 Map: EFI memory types to AArch64 memory
@@ -261,4 +260,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_NC);
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }
-#endif

Re: [PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

James,

On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
> Hi Akashi,
> 
> On 10/01/18 10:09, AKASHI Takahiro wrote:
> > This is a fix against the issue that crash dump kernel may hang up
> > during booting, which can happen on any ACPI-based system with "ACPI
> > Reclaim Memory."
> > 
> > 	<kicking off kdump after panic>
> > 	Bye!
> > 	   (snip...)
> > 	ACPI: Core revision 20170728
> > 	pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> > 	Internal error: Oops: 96000021 [#1] SMP
> > 	Modules linked in:
> > 	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> > 	task: ffff000008d05180 task.stack: ffff000008cc0000
> > 	PC is at acpi_ns_lookup+0x25c/0x3c0
> > 	LR is at acpi_ds_load1_begin_op+0xa4/0x294
> > 	   (snip...)
> > 	Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> > 	Call trace:
> > 	   (snip...)
> > 	[<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> > 	[<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> > 	[<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> > 	[<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> > 	[<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> > 	[<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> > 	[<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> > 	[<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> > 	[<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> > 	[<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> > 	[<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> > 	[<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> > 	[<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> > 	Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> > 	---[ end trace c46ed37f9651c58e ]---
> > 	Kernel panic - not syncing: Fatal exception
> > 	Rebooting in 10 seconds..
> > 
> > (diagnosis)
> > * This fault is a data abort, alignment fault (ESR=0x96000021)
> >   during reading out ACPI table.
> > * Initial ACPI tables are normally stored in system ram and marked as
> >   "ACPI Reclaim memory" by the firmware.
> > * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> >   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> >   removing NOMAP bit and they are instead "memblock-reserved".
> > * When crash dump kernel boots up, it tries to accesses ACPI tables by
> >   ioremap'ing them (through acpi_os_ioremap()).
> > * Since those regions are not included in device tree's
> >   "usable-memory-range" and so not recognized as part of crash dump
> >   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> 
> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
> what we pulled into memblock, which is different during kdump.
> 
> Is an alternative to teach acpi_os_ioremap() to ask
> efi_mem_attributes() directly for the attributes to use?
> (e.g. arch_apei_get_mem_attribute())

I didn't think of this approach.
Do you mean a change like the patch below?
(I'm still debugging this code since the kernel fails to boot.)

Thanks,
-Takahiro AKASHI

> 
> > * ACPI accessor/helper functions are compiled in without unaligned access
> >   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
> >   panic when accessing ACPI tables.
> > 
> > With this patch, all the reserved memory regions, as well as NOMAP-
> > attributed ones which are presumably ACPI runtime code and data, are set
> > to be retained in system ram even if they are outside of usable memory
> > range specified by device tree blob. Accordingly, ACPI tables are mapped
> > as cacheable and can be safely accessed without causing unaligned access
> > faults.
> 
> 
> Thanks,
> 
> James
===8<===
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 32f465a80e4e..6953aaaf2bfa 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -16,6 +16,7 @@
 #include <linux/psci.h>
 
 #include <asm/cputype.h>
+#include <asm/io.h>
 #include <asm/smp_plat.h>
 #include <asm/tlbflush.h>
 
@@ -29,18 +30,13 @@
 
 /* Basic configuration for ACPI */
 #ifdef	CONFIG_ACPI
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
+
 /* ACPI table mapping after acpi_permanent_mmap is set */
 static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
 					    acpi_size size)
 {
-	/*
-	 * EFI's reserve_regions() call adds memory with the WB attribute
-	 * to memblock via early_init_dt_add_memory_arch().
-	 */
-	if (!memblock_is_memory(phys))
-		return ioremap(phys, size);
-
-	return ioremap_cache(phys, size);
+	return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
 }
 #define acpi_os_ioremap acpi_os_ioremap
 
@@ -125,7 +121,10 @@ static inline const char *acpi_get_enable_method(int cpu)
  * for compatibility.
  */
 #define acpi_disable_cmcff 1
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
+static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+	return __acpi_get_mem_attribute(addr);
+}
 #endif /* CONFIG_ACPI_APEI */
 
 #ifdef CONFIG_ACPI_NUMA
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..43e9d8371f88 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -239,8 +239,7 @@ void __init acpi_boot_table_init(void)
 	}
 }
 
-#ifdef CONFIG_ACPI_APEI
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 {
 	/*
 	 * According to "Table 8 Map: EFI memory types to AArch64 memory
@@ -261,4 +260,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_NC);
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }
-#endif

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[James Morse]

Hi Akashi,

On 11/01/18 11:38, AKASHI Takahiro wrote:
> On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
>> On 10/01/18 10:09, AKASHI Takahiro wrote:
>>> This is a fix against the issue that crash dump kernel may hang up
>>> during booting, which can happen on any ACPI-based system with "ACPI
>>> Reclaim Memory."

>>> (diagnosis)
>>> * This fault is a data abort, alignment fault (ESR=0x96000021)
>>>   during reading out ACPI table.
>>> * Initial ACPI tables are normally stored in system ram and marked as
>>>   "ACPI Reclaim memory" by the firmware.
>>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>>>   removing NOMAP bit and they are instead "memblock-reserved".
>>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>>>   ioremap'ing them (through acpi_os_ioremap()).
>>> * Since those regions are not included in device tree's
>>>   "usable-memory-range" and so not recognized as part of crash dump
>>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
>>
>> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
>> what we pulled into memblock, which is different during kdump.
>>
>> Is an alternative to teach acpi_os_ioremap() to ask
>> efi_mem_attributes() directly for the attributes to use?
>> (e.g. arch_apei_get_mem_attribute())
> 
> I didn't think of this approach.
> Do you mean a change like the patch below?

Yes. Aha, you can pretty much re-use the helper directly.

It was just a suggestion, removing the extra abstraction that is causing the bug
could be cleaner ...

> (I'm still debugging this code since the kernel fails to boot.)

... but might be too fragile.

There are points during boot when the EFI memory map isn't mapped. I think that
helper will return 'device memory' for everything when this happens.



Thanks,

James

Re: [PATCH] arm64: kdump: retain reserved memory regions

[James Morse]

Hi Akashi,

On 11/01/18 11:38, AKASHI Takahiro wrote:
> On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
>> On 10/01/18 10:09, AKASHI Takahiro wrote:
>>> This is a fix against the issue that crash dump kernel may hang up
>>> during booting, which can happen on any ACPI-based system with "ACPI
>>> Reclaim Memory."

>>> (diagnosis)
>>> * This fault is a data abort, alignment fault (ESR=0x96000021)
>>>   during reading out ACPI table.
>>> * Initial ACPI tables are normally stored in system ram and marked as
>>>   "ACPI Reclaim memory" by the firmware.
>>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>>>   removing NOMAP bit and they are instead "memblock-reserved".
>>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>>>   ioremap'ing them (through acpi_os_ioremap()).
>>> * Since those regions are not included in device tree's
>>>   "usable-memory-range" and so not recognized as part of crash dump
>>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
>>
>> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
>> what we pulled into memblock, which is different during kdump.
>>
>> Is an alternative to teach acpi_os_ioremap() to ask
>> efi_mem_attributes() directly for the attributes to use?
>> (e.g. arch_apei_get_mem_attribute())
> 
> I didn't think of this approach.
> Do you mean a change like the patch below?

Yes. Aha, you can pretty much re-use the helper directly.

It was just a suggestion, removing the extra abstraction that is causing the bug
could be cleaner ...

> (I'm still debugging this code since the kernel fails to boot.)

... but might be too fragile.

There are points during boot when the EFI memory map isn't mapped. I think that
helper will return 'device memory' for everything when this happens.



Thanks,

James

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[Goel, Sameer]

Hi Akashi,
 Thanks for posting the patch. I tested the patch on Qualcomm Centriq system.
Thanks,
Sameer

On 1/10/2018 3:09 AM, AKASHI Takahiro wrote:
> This is a fix against the issue that crash dump kernel may hang up
> during booting, which can happen on any ACPI-based system with "ACPI
> Reclaim Memory."
> 
> 	<kicking off kdump after panic>
> 	Bye!
> 	   (snip...)
> 	ACPI: Core revision 20170728
> 	pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> 	Internal error: Oops: 96000021 [#1] SMP
> 	Modules linked in:
> 	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> 	task: ffff000008d05180 task.stack: ffff000008cc0000
> 	PC is at acpi_ns_lookup+0x25c/0x3c0
> 	LR is at acpi_ds_load1_begin_op+0xa4/0x294
> 	   (snip...)
> 	Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> 	Call trace:
> 	   (snip...)
> 	[<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> 	[<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> 	[<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> 	[<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> 	[<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> 	[<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> 	[<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> 	[<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> 	[<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> 	[<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> 	[<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> 	[<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> 	[<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> 	Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> 	---[ end trace c46ed37f9651c58e ]---
> 	Kernel panic - not syncing: Fatal exception
> 	Rebooting in 10 seconds..
> 
> (diagnosis)
> * This fault is a data abort, alignment fault (ESR=0x96000021)
>   during reading out ACPI table.
> * Initial ACPI tables are normally stored in system ram and marked as
>   "ACPI Reclaim memory" by the firmware.
> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>   removing NOMAP bit and they are instead "memblock-reserved".
> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>   ioremap'ing them (through acpi_os_ioremap()).
> * Since those regions are not included in device tree's
>   "usable-memory-range" and so not recognized as part of crash dump
>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> * ACPI accessor/helper functions are compiled in without unaligned access
>   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
>   panic when accessing ACPI tables.
> 
> With this patch, all the reserved memory regions, as well as NOMAP-
> attributed ones which are presumably ACPI runtime code and data, are set
> to be retained in system ram even if they are outside of usable memory
> range specified by device tree blob. Accordingly, ACPI tables are mapped
> as cacheable and can be safely accessed without causing unaligned access
> faults.
> 
> Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm64/mm/init.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 2d5a443b205c..e4a8b64a09b1 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -352,11 +352,23 @@ static void __init fdt_enforce_memory_region(void)
>  	struct memblock_region reg = {
>  		.size = 0,
>  	};
> +	u64 idx;
> +	phys_addr_t start, end;
>  
>  	of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
>  
> -	if (reg.size)
> -		memblock_cap_memory_range(reg.base, reg.size);
> +	if (reg.size) {
> +retry:
> +		/* exclude usable & !reserved memory */
> +		for_each_free_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE,
> +					&start, &end, NULL) {
> +			memblock_remove(start, end - start);
> +			goto retry;
> +		}
> +
> +		/* add back fdt's usable memory */
> +		memblock_add(reg.base, reg.size);
> +	}
>  }
>  
>  void __init arm64_memblock_init(void)
> 

-- 
 Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] arm64: kdump: retain reserved memory regions

[Goel, Sameer]

Hi Akashi,
 Thanks for posting the patch. I tested the patch on Qualcomm Centriq system.
Thanks,
Sameer

On 1/10/2018 3:09 AM, AKASHI Takahiro wrote:
> This is a fix against the issue that crash dump kernel may hang up
> during booting, which can happen on any ACPI-based system with "ACPI
> Reclaim Memory."
> 
> 	<kicking off kdump after panic>
> 	Bye!
> 	   (snip...)
> 	ACPI: Core revision 20170728
> 	pud=000000002e7d0003, *pmd=000000002e7c0003, *pte=00e8000039710707
> 	Internal error: Oops: 96000021 [#1] SMP
> 	Modules linked in:
> 	CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc6 #1
> 	task: ffff000008d05180 task.stack: ffff000008cc0000
> 	PC is at acpi_ns_lookup+0x25c/0x3c0
> 	LR is at acpi_ds_load1_begin_op+0xa4/0x294
> 	   (snip...)
> 	Process swapper/0 (pid: 0, stack limit = 0xffff000008cc0000)
> 	Call trace:
> 	   (snip...)
> 	[<ffff0000084a6764>] acpi_ns_lookup+0x25c/0x3c0
> 	[<ffff00000849b4f8>] acpi_ds_load1_begin_op+0xa4/0x294
> 	[<ffff0000084ad4ac>] acpi_ps_build_named_op+0xc4/0x198
> 	[<ffff0000084ad6cc>] acpi_ps_create_op+0x14c/0x270
> 	[<ffff0000084acfa8>] acpi_ps_parse_loop+0x188/0x5c8
> 	[<ffff0000084ae048>] acpi_ps_parse_aml+0xb0/0x2b8
> 	[<ffff0000084a8e10>] acpi_ns_one_complete_parse+0x144/0x184
> 	[<ffff0000084a8e98>] acpi_ns_parse_table+0x48/0x68
> 	[<ffff0000084a82cc>] acpi_ns_load_table+0x4c/0xdc
> 	[<ffff0000084b32f8>] acpi_tb_load_namespace+0xe4/0x264
> 	[<ffff000008baf9b4>] acpi_load_tables+0x48/0xc0
> 	[<ffff000008badc20>] acpi_early_init+0x9c/0xd0
> 	[<ffff000008b70d50>] start_kernel+0x3b4/0x43c
> 	Code: b9008fb9 2a000318 36380054 32190318 (b94002c0)
> 	---[ end trace c46ed37f9651c58e ]---
> 	Kernel panic - not syncing: Fatal exception
> 	Rebooting in 10 seconds..
> 
> (diagnosis)
> * This fault is a data abort, alignment fault (ESR=0x96000021)
>   during reading out ACPI table.
> * Initial ACPI tables are normally stored in system ram and marked as
>   "ACPI Reclaim memory" by the firmware.
> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>   removing NOMAP bit and they are instead "memblock-reserved".
> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>   ioremap'ing them (through acpi_os_ioremap()).
> * Since those regions are not included in device tree's
>   "usable-memory-range" and so not recognized as part of crash dump
>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> * ACPI accessor/helper functions are compiled in without unaligned access
>   support (ACPI_MISALIGNMENT_NOT_SUPPORTED), eventually ending up a fatal
>   panic when accessing ACPI tables.
> 
> With this patch, all the reserved memory regions, as well as NOMAP-
> attributed ones which are presumably ACPI runtime code and data, are set
> to be retained in system ram even if they are outside of usable memory
> range specified by device tree blob. Accordingly, ACPI tables are mapped
> as cacheable and can be safely accessed without causing unaligned access
> faults.
> 
> Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm64/mm/init.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 2d5a443b205c..e4a8b64a09b1 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -352,11 +352,23 @@ static void __init fdt_enforce_memory_region(void)
>  	struct memblock_region reg = {
>  		.size = 0,
>  	};
> +	u64 idx;
> +	phys_addr_t start, end;
>  
>  	of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
>  
> -	if (reg.size)
> -		memblock_cap_memory_range(reg.base, reg.size);
> +	if (reg.size) {
> +retry:
> +		/* exclude usable & !reserved memory */
> +		for_each_free_mem_range(idx, NUMA_NO_NODE, MEMBLOCK_NONE,
> +					&start, &end, NULL) {
> +			memblock_remove(start, end - start);
> +			goto retry;
> +		}
> +
> +		/* add back fdt's usable memory */
> +		memblock_add(reg.base, reg.size);
> +	}
>  }
>  
>  void __init arm64_memblock_init(void)
> 

-- 
 Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

James,

On Fri, Jan 19, 2018 at 11:39:58AM +0000, James Morse wrote:
> Hi Akashi,
> 
> On 11/01/18 11:38, AKASHI Takahiro wrote:
> > On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
> >> On 10/01/18 10:09, AKASHI Takahiro wrote:
> >>> This is a fix against the issue that crash dump kernel may hang up
> >>> during booting, which can happen on any ACPI-based system with "ACPI
> >>> Reclaim Memory."
> 
> >>> (diagnosis)
> >>> * This fault is a data abort, alignment fault (ESR=0x96000021)
> >>>   during reading out ACPI table.
> >>> * Initial ACPI tables are normally stored in system ram and marked as
> >>>   "ACPI Reclaim memory" by the firmware.
> >>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> >>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> >>>   removing NOMAP bit and they are instead "memblock-reserved".
> >>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
> >>>   ioremap'ing them (through acpi_os_ioremap()).
> >>> * Since those regions are not included in device tree's
> >>>   "usable-memory-range" and so not recognized as part of crash dump
> >>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> >>
> >> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
> >> what we pulled into memblock, which is different during kdump.
> >>
> >> Is an alternative to teach acpi_os_ioremap() to ask
> >> efi_mem_attributes() directly for the attributes to use?
> >> (e.g. arch_apei_get_mem_attribute())
> > 
> > I didn't think of this approach.
> > Do you mean a change like the patch below?
> 
> Yes. Aha, you can pretty much re-use the helper directly.
> 
> It was just a suggestion, removing the extra abstraction that is causing the bug
> could be cleaner ...
> 
> > (I'm still debugging this code since the kernel fails to boot.)
> 
> ... but might be too fragile.
> 
> There are points during boot when the EFI memory map isn't mapped.

Right, this was a problem for my patch.
Attached is the revised and workable one.
Efi_memmap_init_late() may alternatively be called in acpi_early_init() or
even in acpi_os_ioremap(), but either way it looks a bit odd.

Thanks,
-Takahiro AKASHI

> I think that
> helper will return 'device memory' for everything when this happens.
> 
> 
> Thanks,
> 
> James
===8<===
>From c88f4c8106ba7a918c835b1cdf538b1d21019863 Mon Sep 17 00:00:00 2001
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
Date: Mon, 29 Jan 2018 15:07:43 +0900
Subject: [PATCH] arm64: kdump: make acpi_os_ioremap() more generic

---
 arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
 arch/arm64/kernel/acpi.c      |  7 ++-----
 init/main.c                   |  4 ++++
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 32f465a80e4e..d53c95f4e1a9 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -12,10 +12,12 @@
 #ifndef _ASM_ACPI_H
 #define _ASM_ACPI_H
 
+#include <linux/efi.h>
 #include <linux/memblock.h>
 #include <linux/psci.h>
 
 #include <asm/cputype.h>
+#include <asm/io.h>
 #include <asm/smp_plat.h>
 #include <asm/tlbflush.h>
 
@@ -29,18 +31,22 @@
 
 /* Basic configuration for ACPI */
 #ifdef	CONFIG_ACPI
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
+
 /* ACPI table mapping after acpi_permanent_mmap is set */
 static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
 					    acpi_size size)
 {
+	/* For normal memory we already have a cacheable mapping. */
+	if (memblock_is_map_memory(phys))
+		return (void __iomem *)__phys_to_virt(phys);
+
 	/*
-	 * EFI's reserve_regions() call adds memory with the WB attribute
-	 * to memblock via early_init_dt_add_memory_arch().
+	 * We should still honor the memory's attribute here because
+	 * crash dump kernel possibly excludes some ACPI (reclaim)
+	 * regions from memblock list.
 	 */
-	if (!memblock_is_memory(phys))
-		return ioremap(phys, size);
-
-	return ioremap_cache(phys, size);
+	return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
 }
 #define acpi_os_ioremap acpi_os_ioremap
 
@@ -125,7 +131,10 @@ static inline const char *acpi_get_enable_method(int cpu)
  * for compatibility.
  */
 #define acpi_disable_cmcff 1
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
+static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+	return __acpi_get_mem_attribute(addr);
+}
 #endif /* CONFIG_ACPI_APEI */
 
 #ifdef CONFIG_ACPI_NUMA
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..f94bdf7be439 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -31,10 +31,9 @@
 #include <asm/cpu_ops.h>
 #include <asm/smp_plat.h>
 
-#ifdef CONFIG_ACPI_APEI
+/* CONFIG_ACPI_APEI */
 # include <linux/efi.h>
 # include <asm/pgtable.h>
-#endif
 
 int acpi_noirq = 1;		/* skip ACPI IRQ initialization */
 int acpi_disabled = 1;
@@ -239,8 +238,7 @@ void __init acpi_boot_table_init(void)
 	}
 }
 
-#ifdef CONFIG_ACPI_APEI
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 {
 	/*
 	 * According to "Table 8 Map: EFI memory types to AArch64 memory
@@ -261,4 +259,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_NC);
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }
-#endif
diff --git a/init/main.c b/init/main.c
index a8100b954839..a479ece2bae9 100644
--- a/init/main.c
+++ b/init/main.c
@@ -674,6 +674,10 @@ asmlinkage __visible void __init start_kernel(void)
 	debug_objects_mem_init();
 	setup_per_cpu_pageset();
 	numa_policy_init();
+#if defined(CONFIG_ARM64) && defined(CONFIG_EFI)
+	efi_memmap_init_late(efi.memmap.phys_map,
+			efi.memmap.nr_map * efi.memmap.desc_size);
+#endif
 	acpi_early_init();
 	if (late_time_init)
 		late_time_init();
-- 
2.15.1

Re: [PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

James,

On Fri, Jan 19, 2018 at 11:39:58AM +0000, James Morse wrote:
> Hi Akashi,
> 
> On 11/01/18 11:38, AKASHI Takahiro wrote:
> > On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
> >> On 10/01/18 10:09, AKASHI Takahiro wrote:
> >>> This is a fix against the issue that crash dump kernel may hang up
> >>> during booting, which can happen on any ACPI-based system with "ACPI
> >>> Reclaim Memory."
> 
> >>> (diagnosis)
> >>> * This fault is a data abort, alignment fault (ESR=0x96000021)
> >>>   during reading out ACPI table.
> >>> * Initial ACPI tables are normally stored in system ram and marked as
> >>>   "ACPI Reclaim memory" by the firmware.
> >>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> >>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> >>>   removing NOMAP bit and they are instead "memblock-reserved".
> >>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
> >>>   ioremap'ing them (through acpi_os_ioremap()).
> >>> * Since those regions are not included in device tree's
> >>>   "usable-memory-range" and so not recognized as part of crash dump
> >>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> >>
> >> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
> >> what we pulled into memblock, which is different during kdump.
> >>
> >> Is an alternative to teach acpi_os_ioremap() to ask
> >> efi_mem_attributes() directly for the attributes to use?
> >> (e.g. arch_apei_get_mem_attribute())
> > 
> > I didn't think of this approach.
> > Do you mean a change like the patch below?
> 
> Yes. Aha, you can pretty much re-use the helper directly.
> 
> It was just a suggestion, removing the extra abstraction that is causing the bug
> could be cleaner ...
> 
> > (I'm still debugging this code since the kernel fails to boot.)
> 
> ... but might be too fragile.
> 
> There are points during boot when the EFI memory map isn't mapped.

Right, this was a problem for my patch.
Attached is the revised and workable one.
Efi_memmap_init_late() may alternatively be called in acpi_early_init() or
even in acpi_os_ioremap(), but either way it looks a bit odd.

Thanks,
-Takahiro AKASHI

> I think that
> helper will return 'device memory' for everything when this happens.
> 
> 
> Thanks,
> 
> James
===8<===
From c88f4c8106ba7a918c835b1cdf538b1d21019863 Mon Sep 17 00:00:00 2001
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
Date: Mon, 29 Jan 2018 15:07:43 +0900
Subject: [PATCH] arm64: kdump: make acpi_os_ioremap() more generic

---
 arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
 arch/arm64/kernel/acpi.c      |  7 ++-----
 init/main.c                   |  4 ++++
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 32f465a80e4e..d53c95f4e1a9 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -12,10 +12,12 @@
 #ifndef _ASM_ACPI_H
 #define _ASM_ACPI_H
 
+#include <linux/efi.h>
 #include <linux/memblock.h>
 #include <linux/psci.h>
 
 #include <asm/cputype.h>
+#include <asm/io.h>
 #include <asm/smp_plat.h>
 #include <asm/tlbflush.h>
 
@@ -29,18 +31,22 @@
 
 /* Basic configuration for ACPI */
 #ifdef	CONFIG_ACPI
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
+
 /* ACPI table mapping after acpi_permanent_mmap is set */
 static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
 					    acpi_size size)
 {
+	/* For normal memory we already have a cacheable mapping. */
+	if (memblock_is_map_memory(phys))
+		return (void __iomem *)__phys_to_virt(phys);
+
 	/*
-	 * EFI's reserve_regions() call adds memory with the WB attribute
-	 * to memblock via early_init_dt_add_memory_arch().
+	 * We should still honor the memory's attribute here because
+	 * crash dump kernel possibly excludes some ACPI (reclaim)
+	 * regions from memblock list.
 	 */
-	if (!memblock_is_memory(phys))
-		return ioremap(phys, size);
-
-	return ioremap_cache(phys, size);
+	return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
 }
 #define acpi_os_ioremap acpi_os_ioremap
 
@@ -125,7 +131,10 @@ static inline const char *acpi_get_enable_method(int cpu)
  * for compatibility.
  */
 #define acpi_disable_cmcff 1
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
+static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+{
+	return __acpi_get_mem_attribute(addr);
+}
 #endif /* CONFIG_ACPI_APEI */
 
 #ifdef CONFIG_ACPI_NUMA
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..f94bdf7be439 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -31,10 +31,9 @@
 #include <asm/cpu_ops.h>
 #include <asm/smp_plat.h>
 
-#ifdef CONFIG_ACPI_APEI
+/* CONFIG_ACPI_APEI */
 # include <linux/efi.h>
 # include <asm/pgtable.h>
-#endif
 
 int acpi_noirq = 1;		/* skip ACPI IRQ initialization */
 int acpi_disabled = 1;
@@ -239,8 +238,7 @@ void __init acpi_boot_table_init(void)
 	}
 }
 
-#ifdef CONFIG_ACPI_APEI
-pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
+pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 {
 	/*
 	 * According to "Table 8 Map: EFI memory types to AArch64 memory
@@ -261,4 +259,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_NC);
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }
-#endif
diff --git a/init/main.c b/init/main.c
index a8100b954839..a479ece2bae9 100644
--- a/init/main.c
+++ b/init/main.c
@@ -674,6 +674,10 @@ asmlinkage __visible void __init start_kernel(void)
 	debug_objects_mem_init();
 	setup_per_cpu_pageset();
 	numa_policy_init();
+#if defined(CONFIG_ARM64) && defined(CONFIG_EFI)
+	efi_memmap_init_late(efi.memmap.phys_map,
+			efi.memmap.nr_map * efi.memmap.desc_size);
+#endif
 	acpi_early_init();
 	if (late_time_init)
 		late_time_init();
-- 
2.15.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[Ard Biesheuvel]

On 29 January 2018 at 08:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> James,
>
> On Fri, Jan 19, 2018 at 11:39:58AM +0000, James Morse wrote:
>> Hi Akashi,
>>
>> On 11/01/18 11:38, AKASHI Takahiro wrote:
>> > On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
>> >> On 10/01/18 10:09, AKASHI Takahiro wrote:
>> >>> This is a fix against the issue that crash dump kernel may hang up
>> >>> during booting, which can happen on any ACPI-based system with "ACPI
>> >>> Reclaim Memory."
>>
>> >>> (diagnosis)
>> >>> * This fault is a data abort, alignment fault (ESR=0x96000021)
>> >>>   during reading out ACPI table.
>> >>> * Initial ACPI tables are normally stored in system ram and marked as
>> >>>   "ACPI Reclaim memory" by the firmware.
>> >>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>> >>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>> >>>   removing NOMAP bit and they are instead "memblock-reserved".
>> >>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>> >>>   ioremap'ing them (through acpi_os_ioremap()).
>> >>> * Since those regions are not included in device tree's
>> >>>   "usable-memory-range" and so not recognized as part of crash dump
>> >>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
>> >>
>> >> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
>> >> what we pulled into memblock, which is different during kdump.
>> >>
>> >> Is an alternative to teach acpi_os_ioremap() to ask
>> >> efi_mem_attributes() directly for the attributes to use?
>> >> (e.g. arch_apei_get_mem_attribute())
>> >
>> > I didn't think of this approach.
>> > Do you mean a change like the patch below?
>>
>> Yes. Aha, you can pretty much re-use the helper directly.
>>
>> It was just a suggestion, removing the extra abstraction that is causing the bug
>> could be cleaner ...
>>
>> > (I'm still debugging this code since the kernel fails to boot.)
>>
>> ... but might be too fragile.
>>
>> There are points during boot when the EFI memory map isn't mapped.
>
> Right, this was a problem for my patch.
> Attached is the revised and workable one.
> Efi_memmap_init_late() may alternatively be called in acpi_early_init() or
> even in acpi_os_ioremap(), but either way it looks a bit odd.
>

Akashi-san,

efi_memmap_init_late() is currently being called from
arm_enable_runtime_services(), which is an early initcall. If that is
too late for acpi_early_init(), we could perhaps move the call
forward, i.e., sth like

---------8<------------
diff --git a/drivers/firmware/efi/arm-runtime.c
b/drivers/firmware/efi/arm-runtime.c
index 6f60d659b323..e835d3b20af6 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -117,7 +117,7 @@ static bool __init efi_virtmap_init(void)
  * non-early mapping of the UEFI system table and virtual mappings for all
  * EFI_MEMORY_RUNTIME regions.
  */
-static int __init arm_enable_runtime_services(void)
+void __init efi_enter_virtual_mode(void)
 {
        u64 mapsize;

@@ -156,7 +156,6 @@ static int __init arm_enable_runtime_services(void)

        return 0;
 }
-early_initcall(arm_enable_runtime_services);

 void efi_virtmap_load(void)
 {
diff --git a/init/main.c b/init/main.c
index a8100b954839..2d0927768e2d 100644
--- a/init/main.c
+++ b/init/main.c
@@ -674,6 +674,9 @@ asmlinkage __visible void __init start_kernel(void)
        debug_objects_mem_init();
        setup_per_cpu_pageset();
        numa_policy_init();
+       if (IS_ENABLED(CONFIG_EFI) &&
+           (IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)))
+               efi_enter_virtual_mode();
        acpi_early_init();
        if (late_time_init)
                late_time_init();
---------8<------------

would be reasonable imo. Also, I think it is justifiable to make ACPI
depend on UEFI on arm64, which is notably different from x86.

(I know 'efi_enter_virtual_mode' is not entirely accurate here, given
that we call SetVirtualAddressMap from the UEFI stub on ARM, but it is
still close enough, given that one could argue that EFI is not in
'virtual mode' until the mappings are in place)



> ===8<===
> From c88f4c8106ba7a918c835b1cdf538b1d21019863 Mon Sep 17 00:00:00 2001
> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Date: Mon, 29 Jan 2018 15:07:43 +0900
> Subject: [PATCH] arm64: kdump: make acpi_os_ioremap() more generic
>
> ---
>  arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
>  arch/arm64/kernel/acpi.c      |  7 ++-----
>  init/main.c                   |  4 ++++
>  3 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index 32f465a80e4e..d53c95f4e1a9 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -12,10 +12,12 @@
>  #ifndef _ASM_ACPI_H
>  #define _ASM_ACPI_H
>
> +#include <linux/efi.h>
>  #include <linux/memblock.h>
>  #include <linux/psci.h>
>
>  #include <asm/cputype.h>
> +#include <asm/io.h>
>  #include <asm/smp_plat.h>
>  #include <asm/tlbflush.h>
>
> @@ -29,18 +31,22 @@
>
>  /* Basic configuration for ACPI */
>  #ifdef CONFIG_ACPI
> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
> +
>  /* ACPI table mapping after acpi_permanent_mmap is set */
>  static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
>                                             acpi_size size)
>  {
> +       /* For normal memory we already have a cacheable mapping. */
> +       if (memblock_is_map_memory(phys))
> +               return (void __iomem *)__phys_to_virt(phys);
> +
>         /*
> -        * EFI's reserve_regions() call adds memory with the WB attribute
> -        * to memblock via early_init_dt_add_memory_arch().
> +        * We should still honor the memory's attribute here because
> +        * crash dump kernel possibly excludes some ACPI (reclaim)
> +        * regions from memblock list.
>          */
> -       if (!memblock_is_memory(phys))
> -               return ioremap(phys, size);
> -
> -       return ioremap_cache(phys, size);
> +       return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
>  }
>  #define acpi_os_ioremap acpi_os_ioremap
>
> @@ -125,7 +131,10 @@ static inline const char *acpi_get_enable_method(int cpu)
>   * for compatibility.
>   */
>  #define acpi_disable_cmcff 1
> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> +{
> +       return __acpi_get_mem_attribute(addr);
> +}
>  #endif /* CONFIG_ACPI_APEI */
>
>  #ifdef CONFIG_ACPI_NUMA
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index b3162715ed78..f94bdf7be439 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -31,10 +31,9 @@
>  #include <asm/cpu_ops.h>
>  #include <asm/smp_plat.h>
>
> -#ifdef CONFIG_ACPI_APEI
> +/* CONFIG_ACPI_APEI */
>  # include <linux/efi.h>
>  # include <asm/pgtable.h>
> -#endif
>
>  int acpi_noirq = 1;            /* skip ACPI IRQ initialization */
>  int acpi_disabled = 1;
> @@ -239,8 +238,7 @@ void __init acpi_boot_table_init(void)
>         }
>  }
>
> -#ifdef CONFIG_ACPI_APEI
> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
>  {
>         /*
>          * According to "Table 8 Map: EFI memory types to AArch64 memory
> @@ -261,4 +259,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>                 return __pgprot(PROT_NORMAL_NC);
>         return __pgprot(PROT_DEVICE_nGnRnE);
>  }
> -#endif
> diff --git a/init/main.c b/init/main.c
> index a8100b954839..a479ece2bae9 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -674,6 +674,10 @@ asmlinkage __visible void __init start_kernel(void)
>         debug_objects_mem_init();
>         setup_per_cpu_pageset();
>         numa_policy_init();
> +#if defined(CONFIG_ARM64) && defined(CONFIG_EFI)
> +       efi_memmap_init_late(efi.memmap.phys_map,
> +                       efi.memmap.nr_map * efi.memmap.desc_size);
> +#endif
>         acpi_early_init();
>         if (late_time_init)
>                 late_time_init();
> --
> 2.15.1
>

Re: [PATCH] arm64: kdump: retain reserved memory regions

[Ard Biesheuvel]

On 29 January 2018 at 08:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> James,
>
> On Fri, Jan 19, 2018 at 11:39:58AM +0000, James Morse wrote:
>> Hi Akashi,
>>
>> On 11/01/18 11:38, AKASHI Takahiro wrote:
>> > On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
>> >> On 10/01/18 10:09, AKASHI Takahiro wrote:
>> >>> This is a fix against the issue that crash dump kernel may hang up
>> >>> during booting, which can happen on any ACPI-based system with "ACPI
>> >>> Reclaim Memory."
>>
>> >>> (diagnosis)
>> >>> * This fault is a data abort, alignment fault (ESR=0x96000021)
>> >>>   during reading out ACPI table.
>> >>> * Initial ACPI tables are normally stored in system ram and marked as
>> >>>   "ACPI Reclaim memory" by the firmware.
>> >>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>> >>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>> >>>   removing NOMAP bit and they are instead "memblock-reserved".
>> >>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>> >>>   ioremap'ing them (through acpi_os_ioremap()).
>> >>> * Since those regions are not included in device tree's
>> >>>   "usable-memory-range" and so not recognized as part of crash dump
>> >>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
>> >>
>> >> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
>> >> what we pulled into memblock, which is different during kdump.
>> >>
>> >> Is an alternative to teach acpi_os_ioremap() to ask
>> >> efi_mem_attributes() directly for the attributes to use?
>> >> (e.g. arch_apei_get_mem_attribute())
>> >
>> > I didn't think of this approach.
>> > Do you mean a change like the patch below?
>>
>> Yes. Aha, you can pretty much re-use the helper directly.
>>
>> It was just a suggestion, removing the extra abstraction that is causing the bug
>> could be cleaner ...
>>
>> > (I'm still debugging this code since the kernel fails to boot.)
>>
>> ... but might be too fragile.
>>
>> There are points during boot when the EFI memory map isn't mapped.
>
> Right, this was a problem for my patch.
> Attached is the revised and workable one.
> Efi_memmap_init_late() may alternatively be called in acpi_early_init() or
> even in acpi_os_ioremap(), but either way it looks a bit odd.
>

Akashi-san,

efi_memmap_init_late() is currently being called from
arm_enable_runtime_services(), which is an early initcall. If that is
too late for acpi_early_init(), we could perhaps move the call
forward, i.e., sth like

---------8<------------
diff --git a/drivers/firmware/efi/arm-runtime.c
b/drivers/firmware/efi/arm-runtime.c
index 6f60d659b323..e835d3b20af6 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -117,7 +117,7 @@ static bool __init efi_virtmap_init(void)
  * non-early mapping of the UEFI system table and virtual mappings for all
  * EFI_MEMORY_RUNTIME regions.
  */
-static int __init arm_enable_runtime_services(void)
+void __init efi_enter_virtual_mode(void)
 {
        u64 mapsize;

@@ -156,7 +156,6 @@ static int __init arm_enable_runtime_services(void)

        return 0;
 }
-early_initcall(arm_enable_runtime_services);

 void efi_virtmap_load(void)
 {
diff --git a/init/main.c b/init/main.c
index a8100b954839..2d0927768e2d 100644
--- a/init/main.c
+++ b/init/main.c
@@ -674,6 +674,9 @@ asmlinkage __visible void __init start_kernel(void)
        debug_objects_mem_init();
        setup_per_cpu_pageset();
        numa_policy_init();
+       if (IS_ENABLED(CONFIG_EFI) &&
+           (IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)))
+               efi_enter_virtual_mode();
        acpi_early_init();
        if (late_time_init)
                late_time_init();
---------8<------------

would be reasonable imo. Also, I think it is justifiable to make ACPI
depend on UEFI on arm64, which is notably different from x86.

(I know 'efi_enter_virtual_mode' is not entirely accurate here, given
that we call SetVirtualAddressMap from the UEFI stub on ARM, but it is
still close enough, given that one could argue that EFI is not in
'virtual mode' until the mappings are in place)



> ===8<===
> From c88f4c8106ba7a918c835b1cdf538b1d21019863 Mon Sep 17 00:00:00 2001
> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Date: Mon, 29 Jan 2018 15:07:43 +0900
> Subject: [PATCH] arm64: kdump: make acpi_os_ioremap() more generic
>
> ---
>  arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
>  arch/arm64/kernel/acpi.c      |  7 ++-----
>  init/main.c                   |  4 ++++
>  3 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index 32f465a80e4e..d53c95f4e1a9 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -12,10 +12,12 @@
>  #ifndef _ASM_ACPI_H
>  #define _ASM_ACPI_H
>
> +#include <linux/efi.h>
>  #include <linux/memblock.h>
>  #include <linux/psci.h>
>
>  #include <asm/cputype.h>
> +#include <asm/io.h>
>  #include <asm/smp_plat.h>
>  #include <asm/tlbflush.h>
>
> @@ -29,18 +31,22 @@
>
>  /* Basic configuration for ACPI */
>  #ifdef CONFIG_ACPI
> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
> +
>  /* ACPI table mapping after acpi_permanent_mmap is set */
>  static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
>                                             acpi_size size)
>  {
> +       /* For normal memory we already have a cacheable mapping. */
> +       if (memblock_is_map_memory(phys))
> +               return (void __iomem *)__phys_to_virt(phys);
> +
>         /*
> -        * EFI's reserve_regions() call adds memory with the WB attribute
> -        * to memblock via early_init_dt_add_memory_arch().
> +        * We should still honor the memory's attribute here because
> +        * crash dump kernel possibly excludes some ACPI (reclaim)
> +        * regions from memblock list.
>          */
> -       if (!memblock_is_memory(phys))
> -               return ioremap(phys, size);
> -
> -       return ioremap_cache(phys, size);
> +       return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
>  }
>  #define acpi_os_ioremap acpi_os_ioremap
>
> @@ -125,7 +131,10 @@ static inline const char *acpi_get_enable_method(int cpu)
>   * for compatibility.
>   */
>  #define acpi_disable_cmcff 1
> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> +{
> +       return __acpi_get_mem_attribute(addr);
> +}
>  #endif /* CONFIG_ACPI_APEI */
>
>  #ifdef CONFIG_ACPI_NUMA
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index b3162715ed78..f94bdf7be439 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -31,10 +31,9 @@
>  #include <asm/cpu_ops.h>
>  #include <asm/smp_plat.h>
>
> -#ifdef CONFIG_ACPI_APEI
> +/* CONFIG_ACPI_APEI */
>  # include <linux/efi.h>
>  # include <asm/pgtable.h>
> -#endif
>
>  int acpi_noirq = 1;            /* skip ACPI IRQ initialization */
>  int acpi_disabled = 1;
> @@ -239,8 +238,7 @@ void __init acpi_boot_table_init(void)
>         }
>  }
>
> -#ifdef CONFIG_ACPI_APEI
> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
>  {
>         /*
>          * According to "Table 8 Map: EFI memory types to AArch64 memory
> @@ -261,4 +259,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>                 return __pgprot(PROT_NORMAL_NC);
>         return __pgprot(PROT_DEVICE_nGnRnE);
>  }
> -#endif
> diff --git a/init/main.c b/init/main.c
> index a8100b954839..a479ece2bae9 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -674,6 +674,10 @@ asmlinkage __visible void __init start_kernel(void)
>         debug_objects_mem_init();
>         setup_per_cpu_pageset();
>         numa_policy_init();
> +#if defined(CONFIG_ARM64) && defined(CONFIG_EFI)
> +       efi_memmap_init_late(efi.memmap.phys_map,
> +                       efi.memmap.nr_map * efi.memmap.desc_size);
> +#endif
>         acpi_early_init();
>         if (late_time_init)
>                 late_time_init();
> --
> 2.15.1
>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[Bhupesh Sharma]

Hi Ard, Akashi,

On Mon, Jan 29, 2018 at 5:41 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 29 January 2018 at 08:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>> James,
>>
>> On Fri, Jan 19, 2018 at 11:39:58AM +0000, James Morse wrote:
>>> Hi Akashi,
>>>
>>> On 11/01/18 11:38, AKASHI Takahiro wrote:
>>> > On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
>>> >> On 10/01/18 10:09, AKASHI Takahiro wrote:
>>> >>> This is a fix against the issue that crash dump kernel may hang up
>>> >>> during booting, which can happen on any ACPI-based system with "ACPI
>>> >>> Reclaim Memory."
>>>
>>> >>> (diagnosis)
>>> >>> * This fault is a data abort, alignment fault (ESR=0x96000021)
>>> >>>   during reading out ACPI table.
>>> >>> * Initial ACPI tables are normally stored in system ram and marked as
>>> >>>   "ACPI Reclaim memory" by the firmware.
>>> >>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>>> >>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>>> >>>   removing NOMAP bit and they are instead "memblock-reserved".
>>> >>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>>> >>>   ioremap'ing them (through acpi_os_ioremap()).
>>> >>> * Since those regions are not included in device tree's
>>> >>>   "usable-memory-range" and so not recognized as part of crash dump
>>> >>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
>>> >>
>>> >> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
>>> >> what we pulled into memblock, which is different during kdump.
>>> >>
>>> >> Is an alternative to teach acpi_os_ioremap() to ask
>>> >> efi_mem_attributes() directly for the attributes to use?
>>> >> (e.g. arch_apei_get_mem_attribute())
>>> >
>>> > I didn't think of this approach.
>>> > Do you mean a change like the patch below?
>>>
>>> Yes. Aha, you can pretty much re-use the helper directly.
>>>
>>> It was just a suggestion, removing the extra abstraction that is causing the bug
>>> could be cleaner ...
>>>
>>> > (I'm still debugging this code since the kernel fails to boot.)
>>>
>>> ... but might be too fragile.
>>>
>>> There are points during boot when the EFI memory map isn't mapped.
>>
>> Right, this was a problem for my patch.
>> Attached is the revised and workable one.
>> Efi_memmap_init_late() may alternatively be called in acpi_early_init() or
>> even in acpi_os_ioremap(), but either way it looks a bit odd.
>>
>
> Akashi-san,
>
> efi_memmap_init_late() is currently being called from
> arm_enable_runtime_services(), which is an early initcall. If that is
> too late for acpi_early_init(), we could perhaps move the call
> forward, i.e., sth like
>
> ---------8<------------
> diff --git a/drivers/firmware/efi/arm-runtime.c
> b/drivers/firmware/efi/arm-runtime.c
> index 6f60d659b323..e835d3b20af6 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -117,7 +117,7 @@ static bool __init efi_virtmap_init(void)
>   * non-early mapping of the UEFI system table and virtual mappings for all
>   * EFI_MEMORY_RUNTIME regions.
>   */
> -static int __init arm_enable_runtime_services(void)
> +void __init efi_enter_virtual_mode(void)
>  {
>         u64 mapsize;
>
> @@ -156,7 +156,6 @@ static int __init arm_enable_runtime_services(void)
>
>         return 0;
>  }
> -early_initcall(arm_enable_runtime_services);
>
>  void efi_virtmap_load(void)
>  {
> diff --git a/init/main.c b/init/main.c
> index a8100b954839..2d0927768e2d 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -674,6 +674,9 @@ asmlinkage __visible void __init start_kernel(void)
>         debug_objects_mem_init();
>         setup_per_cpu_pageset();
>         numa_policy_init();
> +       if (IS_ENABLED(CONFIG_EFI) &&
> +           (IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)))
> +               efi_enter_virtual_mode();
>         acpi_early_init();
>         if (late_time_init)
>                 late_time_init();
> ---------8<------------
>
> would be reasonable imo. Also, I think it is justifiable to make ACPI
> depend on UEFI on arm64, which is notably different from x86.
>
> (I know 'efi_enter_virtual_mode' is not entirely accurate here, given
> that we call SetVirtualAddressMap from the UEFI stub on ARM, but it is
> still close enough, given that one could argue that EFI is not in
> 'virtual mode' until the mappings are in place)
>
>
>
>> ===8<===
>> From c88f4c8106ba7a918c835b1cdf538b1d21019863 Mon Sep 17 00:00:00 2001
>> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> Date: Mon, 29 Jan 2018 15:07:43 +0900
>> Subject: [PATCH] arm64: kdump: make acpi_os_ioremap() more generic
>>
>> ---
>>  arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
>>  arch/arm64/kernel/acpi.c      |  7 ++-----
>>  init/main.c                   |  4 ++++
>>  3 files changed, 22 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index 32f465a80e4e..d53c95f4e1a9 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -12,10 +12,12 @@
>>  #ifndef _ASM_ACPI_H
>>  #define _ASM_ACPI_H
>>
>> +#include <linux/efi.h>
>>  #include <linux/memblock.h>
>>  #include <linux/psci.h>
>>
>>  #include <asm/cputype.h>
>> +#include <asm/io.h>
>>  #include <asm/smp_plat.h>
>>  #include <asm/tlbflush.h>
>>
>> @@ -29,18 +31,22 @@
>>
>>  /* Basic configuration for ACPI */
>>  #ifdef CONFIG_ACPI
>> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
>> +
>>  /* ACPI table mapping after acpi_permanent_mmap is set */
>>  static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
>>                                             acpi_size size)
>>  {
>> +       /* For normal memory we already have a cacheable mapping. */
>> +       if (memblock_is_map_memory(phys))
>> +               return (void __iomem *)__phys_to_virt(phys);
>> +
>>         /*
>> -        * EFI's reserve_regions() call adds memory with the WB attribute
>> -        * to memblock via early_init_dt_add_memory_arch().
>> +        * We should still honor the memory's attribute here because
>> +        * crash dump kernel possibly excludes some ACPI (reclaim)
>> +        * regions from memblock list.
>>          */
>> -       if (!memblock_is_memory(phys))
>> -               return ioremap(phys, size);
>> -
>> -       return ioremap_cache(phys, size);
>> +       return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
>>  }
>>  #define acpi_os_ioremap acpi_os_ioremap
>>
>> @@ -125,7 +131,10 @@ static inline const char *acpi_get_enable_method(int cpu)
>>   * for compatibility.
>>   */
>>  #define acpi_disable_cmcff 1
>> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
>> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>> +{
>> +       return __acpi_get_mem_attribute(addr);
>> +}
>>  #endif /* CONFIG_ACPI_APEI */
>>
>>  #ifdef CONFIG_ACPI_NUMA
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index b3162715ed78..f94bdf7be439 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -31,10 +31,9 @@
>>  #include <asm/cpu_ops.h>
>>  #include <asm/smp_plat.h>
>>
>> -#ifdef CONFIG_ACPI_APEI
>> +/* CONFIG_ACPI_APEI */
>>  # include <linux/efi.h>
>>  # include <asm/pgtable.h>
>> -#endif
>>
>>  int acpi_noirq = 1;            /* skip ACPI IRQ initialization */
>>  int acpi_disabled = 1;
>> @@ -239,8 +238,7 @@ void __init acpi_boot_table_init(void)
>>         }
>>  }
>>
>> -#ifdef CONFIG_ACPI_APEI
>> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
>>  {
>>         /*
>>          * According to "Table 8 Map: EFI memory types to AArch64 memory
>> @@ -261,4 +259,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>>                 return __pgprot(PROT_NORMAL_NC);
>>         return __pgprot(PROT_DEVICE_nGnRnE);
>>  }
>> -#endif
>> diff --git a/init/main.c b/init/main.c
>> index a8100b954839..a479ece2bae9 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -674,6 +674,10 @@ asmlinkage __visible void __init start_kernel(void)
>>         debug_objects_mem_init();
>>         setup_per_cpu_pageset();
>>         numa_policy_init();
>> +#if defined(CONFIG_ARM64) && defined(CONFIG_EFI)
>> +       efi_memmap_init_late(efi.memmap.phys_map,
>> +                       efi.memmap.nr_map * efi.memmap.desc_size);
>> +#endif
>>         acpi_early_init();
>>         if (late_time_init)
>>                 late_time_init();
>> --
>> 2.15.1
>>

I tested Ard's patch (on top of Akashi's proposed changes) on the
huawei taishan machine (where I originally found the problem) and I
can confirm that I am able to boot the kdump kernel properly and also
save the crashcore dump on local disk.

Also as Ard mentioned, 'efi_enter_virtual_mode' is probably not the
best name for the proposed function as we have already called
'SetVirtualAddressMap', but I cannot think of anything better. If
there are other opinions we can consider the same, otherwise may be we
can formalize this and queue it up as crashkernel is bricked on arm64
machines which support acpi boot machines without the same (and
several kdump users are affected because of the same).

Please feel free to add:

Tested-by: Bhupesh Sharma <bhsharma@redhat.com>

Regards,
Bhupesh

Re: [PATCH] arm64: kdump: retain reserved memory regions

[Bhupesh Sharma]

Hi Ard, Akashi,

On Mon, Jan 29, 2018 at 5:41 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 29 January 2018 at 08:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>> James,
>>
>> On Fri, Jan 19, 2018 at 11:39:58AM +0000, James Morse wrote:
>>> Hi Akashi,
>>>
>>> On 11/01/18 11:38, AKASHI Takahiro wrote:
>>> > On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
>>> >> On 10/01/18 10:09, AKASHI Takahiro wrote:
>>> >>> This is a fix against the issue that crash dump kernel may hang up
>>> >>> during booting, which can happen on any ACPI-based system with "ACPI
>>> >>> Reclaim Memory."
>>>
>>> >>> (diagnosis)
>>> >>> * This fault is a data abort, alignment fault (ESR=0x96000021)
>>> >>>   during reading out ACPI table.
>>> >>> * Initial ACPI tables are normally stored in system ram and marked as
>>> >>>   "ACPI Reclaim memory" by the firmware.
>>> >>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
>>> >>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
>>> >>>   removing NOMAP bit and they are instead "memblock-reserved".
>>> >>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
>>> >>>   ioremap'ing them (through acpi_os_ioremap()).
>>> >>> * Since those regions are not included in device tree's
>>> >>>   "usable-memory-range" and so not recognized as part of crash dump
>>> >>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
>>> >>
>>> >> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
>>> >> what we pulled into memblock, which is different during kdump.
>>> >>
>>> >> Is an alternative to teach acpi_os_ioremap() to ask
>>> >> efi_mem_attributes() directly for the attributes to use?
>>> >> (e.g. arch_apei_get_mem_attribute())
>>> >
>>> > I didn't think of this approach.
>>> > Do you mean a change like the patch below?
>>>
>>> Yes. Aha, you can pretty much re-use the helper directly.
>>>
>>> It was just a suggestion, removing the extra abstraction that is causing the bug
>>> could be cleaner ...
>>>
>>> > (I'm still debugging this code since the kernel fails to boot.)
>>>
>>> ... but might be too fragile.
>>>
>>> There are points during boot when the EFI memory map isn't mapped.
>>
>> Right, this was a problem for my patch.
>> Attached is the revised and workable one.
>> Efi_memmap_init_late() may alternatively be called in acpi_early_init() or
>> even in acpi_os_ioremap(), but either way it looks a bit odd.
>>
>
> Akashi-san,
>
> efi_memmap_init_late() is currently being called from
> arm_enable_runtime_services(), which is an early initcall. If that is
> too late for acpi_early_init(), we could perhaps move the call
> forward, i.e., sth like
>
> ---------8<------------
> diff --git a/drivers/firmware/efi/arm-runtime.c
> b/drivers/firmware/efi/arm-runtime.c
> index 6f60d659b323..e835d3b20af6 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -117,7 +117,7 @@ static bool __init efi_virtmap_init(void)
>   * non-early mapping of the UEFI system table and virtual mappings for all
>   * EFI_MEMORY_RUNTIME regions.
>   */
> -static int __init arm_enable_runtime_services(void)
> +void __init efi_enter_virtual_mode(void)
>  {
>         u64 mapsize;
>
> @@ -156,7 +156,6 @@ static int __init arm_enable_runtime_services(void)
>
>         return 0;
>  }
> -early_initcall(arm_enable_runtime_services);
>
>  void efi_virtmap_load(void)
>  {
> diff --git a/init/main.c b/init/main.c
> index a8100b954839..2d0927768e2d 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -674,6 +674,9 @@ asmlinkage __visible void __init start_kernel(void)
>         debug_objects_mem_init();
>         setup_per_cpu_pageset();
>         numa_policy_init();
> +       if (IS_ENABLED(CONFIG_EFI) &&
> +           (IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)))
> +               efi_enter_virtual_mode();
>         acpi_early_init();
>         if (late_time_init)
>                 late_time_init();
> ---------8<------------
>
> would be reasonable imo. Also, I think it is justifiable to make ACPI
> depend on UEFI on arm64, which is notably different from x86.
>
> (I know 'efi_enter_virtual_mode' is not entirely accurate here, given
> that we call SetVirtualAddressMap from the UEFI stub on ARM, but it is
> still close enough, given that one could argue that EFI is not in
> 'virtual mode' until the mappings are in place)
>
>
>
>> ===8<===
>> From c88f4c8106ba7a918c835b1cdf538b1d21019863 Mon Sep 17 00:00:00 2001
>> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> Date: Mon, 29 Jan 2018 15:07:43 +0900
>> Subject: [PATCH] arm64: kdump: make acpi_os_ioremap() more generic
>>
>> ---
>>  arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
>>  arch/arm64/kernel/acpi.c      |  7 ++-----
>>  init/main.c                   |  4 ++++
>>  3 files changed, 22 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index 32f465a80e4e..d53c95f4e1a9 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -12,10 +12,12 @@
>>  #ifndef _ASM_ACPI_H
>>  #define _ASM_ACPI_H
>>
>> +#include <linux/efi.h>
>>  #include <linux/memblock.h>
>>  #include <linux/psci.h>
>>
>>  #include <asm/cputype.h>
>> +#include <asm/io.h>
>>  #include <asm/smp_plat.h>
>>  #include <asm/tlbflush.h>
>>
>> @@ -29,18 +31,22 @@
>>
>>  /* Basic configuration for ACPI */
>>  #ifdef CONFIG_ACPI
>> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
>> +
>>  /* ACPI table mapping after acpi_permanent_mmap is set */
>>  static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
>>                                             acpi_size size)
>>  {
>> +       /* For normal memory we already have a cacheable mapping. */
>> +       if (memblock_is_map_memory(phys))
>> +               return (void __iomem *)__phys_to_virt(phys);
>> +
>>         /*
>> -        * EFI's reserve_regions() call adds memory with the WB attribute
>> -        * to memblock via early_init_dt_add_memory_arch().
>> +        * We should still honor the memory's attribute here because
>> +        * crash dump kernel possibly excludes some ACPI (reclaim)
>> +        * regions from memblock list.
>>          */
>> -       if (!memblock_is_memory(phys))
>> -               return ioremap(phys, size);
>> -
>> -       return ioremap_cache(phys, size);
>> +       return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
>>  }
>>  #define acpi_os_ioremap acpi_os_ioremap
>>
>> @@ -125,7 +131,10 @@ static inline const char *acpi_get_enable_method(int cpu)
>>   * for compatibility.
>>   */
>>  #define acpi_disable_cmcff 1
>> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
>> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>> +{
>> +       return __acpi_get_mem_attribute(addr);
>> +}
>>  #endif /* CONFIG_ACPI_APEI */
>>
>>  #ifdef CONFIG_ACPI_NUMA
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index b3162715ed78..f94bdf7be439 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -31,10 +31,9 @@
>>  #include <asm/cpu_ops.h>
>>  #include <asm/smp_plat.h>
>>
>> -#ifdef CONFIG_ACPI_APEI
>> +/* CONFIG_ACPI_APEI */
>>  # include <linux/efi.h>
>>  # include <asm/pgtable.h>
>> -#endif
>>
>>  int acpi_noirq = 1;            /* skip ACPI IRQ initialization */
>>  int acpi_disabled = 1;
>> @@ -239,8 +238,7 @@ void __init acpi_boot_table_init(void)
>>         }
>>  }
>>
>> -#ifdef CONFIG_ACPI_APEI
>> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
>>  {
>>         /*
>>          * According to "Table 8 Map: EFI memory types to AArch64 memory
>> @@ -261,4 +259,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>>                 return __pgprot(PROT_NORMAL_NC);
>>         return __pgprot(PROT_DEVICE_nGnRnE);
>>  }
>> -#endif
>> diff --git a/init/main.c b/init/main.c
>> index a8100b954839..a479ece2bae9 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -674,6 +674,10 @@ asmlinkage __visible void __init start_kernel(void)
>>         debug_objects_mem_init();
>>         setup_per_cpu_pageset();
>>         numa_policy_init();
>> +#if defined(CONFIG_ARM64) && defined(CONFIG_EFI)
>> +       efi_memmap_init_late(efi.memmap.phys_map,
>> +                       efi.memmap.nr_map * efi.memmap.desc_size);
>> +#endif
>>         acpi_early_init();
>>         if (late_time_init)
>>                 late_time_init();
>> --
>> 2.15.1
>>

I tested Ard's patch (on top of Akashi's proposed changes) on the
huawei taishan machine (where I originally found the problem) and I
can confirm that I am able to boot the kdump kernel properly and also
save the crashcore dump on local disk.

Also as Ard mentioned, 'efi_enter_virtual_mode' is probably not the
best name for the proposed function as we have already called
'SetVirtualAddressMap', but I cannot think of anything better. If
there are other opinions we can consider the same, otherwise may be we
can formalize this and queue it up as crashkernel is bricked on arm64
machines which support acpi boot machines without the same (and
several kdump users are affected because of the same).

Please feel free to add:

Tested-by: Bhupesh Sharma <bhsharma@redhat.com>

Regards,
Bhupesh

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

Ard, Bhupesh,

Thank you for the comments.
I will re-post a revised patch soon after running some tests.

But I'm still wondering whether my original approach[1] may be
useful in other (non-ACPI/efi) cases given that the current
memblock_cap_memory_range() has kinda flaw that any memory
reserved by firmware can be ignored at crash dump kernel.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553098.html

Thanks,
-Takahiro AKASHI

On Wed, Jan 31, 2018 at 11:20:20AM +0530, Bhupesh Sharma wrote:
> Hi Ard, Akashi,
> 
> On Mon, Jan 29, 2018 at 5:41 PM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> > On 29 January 2018 at 08:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >> James,
> >>
> >> On Fri, Jan 19, 2018 at 11:39:58AM +0000, James Morse wrote:
> >>> Hi Akashi,
> >>>
> >>> On 11/01/18 11:38, AKASHI Takahiro wrote:
> >>> > On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
> >>> >> On 10/01/18 10:09, AKASHI Takahiro wrote:
> >>> >>> This is a fix against the issue that crash dump kernel may hang up
> >>> >>> during booting, which can happen on any ACPI-based system with "ACPI
> >>> >>> Reclaim Memory."
> >>>
> >>> >>> (diagnosis)
> >>> >>> * This fault is a data abort, alignment fault (ESR=0x96000021)
> >>> >>>   during reading out ACPI table.
> >>> >>> * Initial ACPI tables are normally stored in system ram and marked as
> >>> >>>   "ACPI Reclaim memory" by the firmware.
> >>> >>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> >>> >>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> >>> >>>   removing NOMAP bit and they are instead "memblock-reserved".
> >>> >>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
> >>> >>>   ioremap'ing them (through acpi_os_ioremap()).
> >>> >>> * Since those regions are not included in device tree's
> >>> >>>   "usable-memory-range" and so not recognized as part of crash dump
> >>> >>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> >>> >>
> >>> >> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
> >>> >> what we pulled into memblock, which is different during kdump.
> >>> >>
> >>> >> Is an alternative to teach acpi_os_ioremap() to ask
> >>> >> efi_mem_attributes() directly for the attributes to use?
> >>> >> (e.g. arch_apei_get_mem_attribute())
> >>> >
> >>> > I didn't think of this approach.
> >>> > Do you mean a change like the patch below?
> >>>
> >>> Yes. Aha, you can pretty much re-use the helper directly.
> >>>
> >>> It was just a suggestion, removing the extra abstraction that is causing the bug
> >>> could be cleaner ...
> >>>
> >>> > (I'm still debugging this code since the kernel fails to boot.)
> >>>
> >>> ... but might be too fragile.
> >>>
> >>> There are points during boot when the EFI memory map isn't mapped.
> >>
> >> Right, this was a problem for my patch.
> >> Attached is the revised and workable one.
> >> Efi_memmap_init_late() may alternatively be called in acpi_early_init() or
> >> even in acpi_os_ioremap(), but either way it looks a bit odd.
> >>
> >
> > Akashi-san,
> >
> > efi_memmap_init_late() is currently being called from
> > arm_enable_runtime_services(), which is an early initcall. If that is
> > too late for acpi_early_init(), we could perhaps move the call
> > forward, i.e., sth like
> >
> > ---------8<------------
> > diff --git a/drivers/firmware/efi/arm-runtime.c
> > b/drivers/firmware/efi/arm-runtime.c
> > index 6f60d659b323..e835d3b20af6 100644
> > --- a/drivers/firmware/efi/arm-runtime.c
> > +++ b/drivers/firmware/efi/arm-runtime.c
> > @@ -117,7 +117,7 @@ static bool __init efi_virtmap_init(void)
> >   * non-early mapping of the UEFI system table and virtual mappings for all
> >   * EFI_MEMORY_RUNTIME regions.
> >   */
> > -static int __init arm_enable_runtime_services(void)
> > +void __init efi_enter_virtual_mode(void)
> >  {
> >         u64 mapsize;
> >
> > @@ -156,7 +156,6 @@ static int __init arm_enable_runtime_services(void)
> >
> >         return 0;
> >  }
> > -early_initcall(arm_enable_runtime_services);
> >
> >  void efi_virtmap_load(void)
> >  {
> > diff --git a/init/main.c b/init/main.c
> > index a8100b954839..2d0927768e2d 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -674,6 +674,9 @@ asmlinkage __visible void __init start_kernel(void)
> >         debug_objects_mem_init();
> >         setup_per_cpu_pageset();
> >         numa_policy_init();
> > +       if (IS_ENABLED(CONFIG_EFI) &&
> > +           (IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)))
> > +               efi_enter_virtual_mode();
> >         acpi_early_init();
> >         if (late_time_init)
> >                 late_time_init();
> > ---------8<------------
> >
> > would be reasonable imo. Also, I think it is justifiable to make ACPI
> > depend on UEFI on arm64, which is notably different from x86.
> >
> > (I know 'efi_enter_virtual_mode' is not entirely accurate here, given
> > that we call SetVirtualAddressMap from the UEFI stub on ARM, but it is
> > still close enough, given that one could argue that EFI is not in
> > 'virtual mode' until the mappings are in place)
> >
> >
> >
> >> ===8<===
> >> From c88f4c8106ba7a918c835b1cdf538b1d21019863 Mon Sep 17 00:00:00 2001
> >> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >> Date: Mon, 29 Jan 2018 15:07:43 +0900
> >> Subject: [PATCH] arm64: kdump: make acpi_os_ioremap() more generic
> >>
> >> ---
> >>  arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
> >>  arch/arm64/kernel/acpi.c      |  7 ++-----
> >>  init/main.c                   |  4 ++++
> >>  3 files changed, 22 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> >> index 32f465a80e4e..d53c95f4e1a9 100644
> >> --- a/arch/arm64/include/asm/acpi.h
> >> +++ b/arch/arm64/include/asm/acpi.h
> >> @@ -12,10 +12,12 @@
> >>  #ifndef _ASM_ACPI_H
> >>  #define _ASM_ACPI_H
> >>
> >> +#include <linux/efi.h>
> >>  #include <linux/memblock.h>
> >>  #include <linux/psci.h>
> >>
> >>  #include <asm/cputype.h>
> >> +#include <asm/io.h>
> >>  #include <asm/smp_plat.h>
> >>  #include <asm/tlbflush.h>
> >>
> >> @@ -29,18 +31,22 @@
> >>
> >>  /* Basic configuration for ACPI */
> >>  #ifdef CONFIG_ACPI
> >> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
> >> +
> >>  /* ACPI table mapping after acpi_permanent_mmap is set */
> >>  static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
> >>                                             acpi_size size)
> >>  {
> >> +       /* For normal memory we already have a cacheable mapping. */
> >> +       if (memblock_is_map_memory(phys))
> >> +               return (void __iomem *)__phys_to_virt(phys);
> >> +
> >>         /*
> >> -        * EFI's reserve_regions() call adds memory with the WB attribute
> >> -        * to memblock via early_init_dt_add_memory_arch().
> >> +        * We should still honor the memory's attribute here because
> >> +        * crash dump kernel possibly excludes some ACPI (reclaim)
> >> +        * regions from memblock list.
> >>          */
> >> -       if (!memblock_is_memory(phys))
> >> -               return ioremap(phys, size);
> >> -
> >> -       return ioremap_cache(phys, size);
> >> +       return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
> >>  }
> >>  #define acpi_os_ioremap acpi_os_ioremap
> >>
> >> @@ -125,7 +131,10 @@ static inline const char *acpi_get_enable_method(int cpu)
> >>   * for compatibility.
> >>   */
> >>  #define acpi_disable_cmcff 1
> >> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
> >> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> >> +{
> >> +       return __acpi_get_mem_attribute(addr);
> >> +}
> >>  #endif /* CONFIG_ACPI_APEI */
> >>
> >>  #ifdef CONFIG_ACPI_NUMA
> >> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> >> index b3162715ed78..f94bdf7be439 100644
> >> --- a/arch/arm64/kernel/acpi.c
> >> +++ b/arch/arm64/kernel/acpi.c
> >> @@ -31,10 +31,9 @@
> >>  #include <asm/cpu_ops.h>
> >>  #include <asm/smp_plat.h>
> >>
> >> -#ifdef CONFIG_ACPI_APEI
> >> +/* CONFIG_ACPI_APEI */
> >>  # include <linux/efi.h>
> >>  # include <asm/pgtable.h>
> >> -#endif
> >>
> >>  int acpi_noirq = 1;            /* skip ACPI IRQ initialization */
> >>  int acpi_disabled = 1;
> >> @@ -239,8 +238,7 @@ void __init acpi_boot_table_init(void)
> >>         }
> >>  }
> >>
> >> -#ifdef CONFIG_ACPI_APEI
> >> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> >> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
> >>  {
> >>         /*
> >>          * According to "Table 8 Map: EFI memory types to AArch64 memory
> >> @@ -261,4 +259,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> >>                 return __pgprot(PROT_NORMAL_NC);
> >>         return __pgprot(PROT_DEVICE_nGnRnE);
> >>  }
> >> -#endif
> >> diff --git a/init/main.c b/init/main.c
> >> index a8100b954839..a479ece2bae9 100644
> >> --- a/init/main.c
> >> +++ b/init/main.c
> >> @@ -674,6 +674,10 @@ asmlinkage __visible void __init start_kernel(void)
> >>         debug_objects_mem_init();
> >>         setup_per_cpu_pageset();
> >>         numa_policy_init();
> >> +#if defined(CONFIG_ARM64) && defined(CONFIG_EFI)
> >> +       efi_memmap_init_late(efi.memmap.phys_map,
> >> +                       efi.memmap.nr_map * efi.memmap.desc_size);
> >> +#endif
> >>         acpi_early_init();
> >>         if (late_time_init)
> >>                 late_time_init();
> >> --
> >> 2.15.1
> >>
> 
> I tested Ard's patch (on top of Akashi's proposed changes) on the
> huawei taishan machine (where I originally found the problem) and I
> can confirm that I am able to boot the kdump kernel properly and also
> save the crashcore dump on local disk.
> 
> Also as Ard mentioned, 'efi_enter_virtual_mode' is probably not the
> best name for the proposed function as we have already called
> 'SetVirtualAddressMap', but I cannot think of anything better. If
> there are other opinions we can consider the same, otherwise may be we
> can formalize this and queue it up as crashkernel is bricked on arm64
> machines which support acpi boot machines without the same (and
> several kdump users are affected because of the same).
> 
> Please feel free to add:
> 
> Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
> 
> Regards,
> Bhupesh

Re: [PATCH] arm64: kdump: retain reserved memory regions

[AKASHI Takahiro]

Ard, Bhupesh,

Thank you for the comments.
I will re-post a revised patch soon after running some tests.

But I'm still wondering whether my original approach[1] may be
useful in other (non-ACPI/efi) cases given that the current
memblock_cap_memory_range() has kinda flaw that any memory
reserved by firmware can be ignored at crash dump kernel.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553098.html

Thanks,
-Takahiro AKASHI

On Wed, Jan 31, 2018 at 11:20:20AM +0530, Bhupesh Sharma wrote:
> Hi Ard, Akashi,
> 
> On Mon, Jan 29, 2018 at 5:41 PM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> > On 29 January 2018 at 08:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >> James,
> >>
> >> On Fri, Jan 19, 2018 at 11:39:58AM +0000, James Morse wrote:
> >>> Hi Akashi,
> >>>
> >>> On 11/01/18 11:38, AKASHI Takahiro wrote:
> >>> > On Wed, Jan 10, 2018 at 11:26:55AM +0000, James Morse wrote:
> >>> >> On 10/01/18 10:09, AKASHI Takahiro wrote:
> >>> >>> This is a fix against the issue that crash dump kernel may hang up
> >>> >>> during booting, which can happen on any ACPI-based system with "ACPI
> >>> >>> Reclaim Memory."
> >>>
> >>> >>> (diagnosis)
> >>> >>> * This fault is a data abort, alignment fault (ESR=0x96000021)
> >>> >>>   during reading out ACPI table.
> >>> >>> * Initial ACPI tables are normally stored in system ram and marked as
> >>> >>>   "ACPI Reclaim memory" by the firmware.
> >>> >>> * After the commit f56ab9a5b73c ("efi/arm: Don't mark ACPI reclaim
> >>> >>>   memory as MEMBLOCK_NOMAP"), those regions' attribute were changed
> >>> >>>   removing NOMAP bit and they are instead "memblock-reserved".
> >>> >>> * When crash dump kernel boots up, it tries to accesses ACPI tables by
> >>> >>>   ioremap'ing them (through acpi_os_ioremap()).
> >>> >>> * Since those regions are not included in device tree's
> >>> >>>   "usable-memory-range" and so not recognized as part of crash dump
> >>> >>>   kernel's system ram, ioremap() will create a non-cacheable mapping here.
> >>> >>
> >>> >> Ugh, because acpi_os_ioremap() looks at the efi memory map through the prism of
> >>> >> what we pulled into memblock, which is different during kdump.
> >>> >>
> >>> >> Is an alternative to teach acpi_os_ioremap() to ask
> >>> >> efi_mem_attributes() directly for the attributes to use?
> >>> >> (e.g. arch_apei_get_mem_attribute())
> >>> >
> >>> > I didn't think of this approach.
> >>> > Do you mean a change like the patch below?
> >>>
> >>> Yes. Aha, you can pretty much re-use the helper directly.
> >>>
> >>> It was just a suggestion, removing the extra abstraction that is causing the bug
> >>> could be cleaner ...
> >>>
> >>> > (I'm still debugging this code since the kernel fails to boot.)
> >>>
> >>> ... but might be too fragile.
> >>>
> >>> There are points during boot when the EFI memory map isn't mapped.
> >>
> >> Right, this was a problem for my patch.
> >> Attached is the revised and workable one.
> >> Efi_memmap_init_late() may alternatively be called in acpi_early_init() or
> >> even in acpi_os_ioremap(), but either way it looks a bit odd.
> >>
> >
> > Akashi-san,
> >
> > efi_memmap_init_late() is currently being called from
> > arm_enable_runtime_services(), which is an early initcall. If that is
> > too late for acpi_early_init(), we could perhaps move the call
> > forward, i.e., sth like
> >
> > ---------8<------------
> > diff --git a/drivers/firmware/efi/arm-runtime.c
> > b/drivers/firmware/efi/arm-runtime.c
> > index 6f60d659b323..e835d3b20af6 100644
> > --- a/drivers/firmware/efi/arm-runtime.c
> > +++ b/drivers/firmware/efi/arm-runtime.c
> > @@ -117,7 +117,7 @@ static bool __init efi_virtmap_init(void)
> >   * non-early mapping of the UEFI system table and virtual mappings for all
> >   * EFI_MEMORY_RUNTIME regions.
> >   */
> > -static int __init arm_enable_runtime_services(void)
> > +void __init efi_enter_virtual_mode(void)
> >  {
> >         u64 mapsize;
> >
> > @@ -156,7 +156,6 @@ static int __init arm_enable_runtime_services(void)
> >
> >         return 0;
> >  }
> > -early_initcall(arm_enable_runtime_services);
> >
> >  void efi_virtmap_load(void)
> >  {
> > diff --git a/init/main.c b/init/main.c
> > index a8100b954839..2d0927768e2d 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -674,6 +674,9 @@ asmlinkage __visible void __init start_kernel(void)
> >         debug_objects_mem_init();
> >         setup_per_cpu_pageset();
> >         numa_policy_init();
> > +       if (IS_ENABLED(CONFIG_EFI) &&
> > +           (IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)))
> > +               efi_enter_virtual_mode();
> >         acpi_early_init();
> >         if (late_time_init)
> >                 late_time_init();
> > ---------8<------------
> >
> > would be reasonable imo. Also, I think it is justifiable to make ACPI
> > depend on UEFI on arm64, which is notably different from x86.
> >
> > (I know 'efi_enter_virtual_mode' is not entirely accurate here, given
> > that we call SetVirtualAddressMap from the UEFI stub on ARM, but it is
> > still close enough, given that one could argue that EFI is not in
> > 'virtual mode' until the mappings are in place)
> >
> >
> >
> >> ===8<===
> >> From c88f4c8106ba7a918c835b1cdf538b1d21019863 Mon Sep 17 00:00:00 2001
> >> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >> Date: Mon, 29 Jan 2018 15:07:43 +0900
> >> Subject: [PATCH] arm64: kdump: make acpi_os_ioremap() more generic
> >>
> >> ---
> >>  arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
> >>  arch/arm64/kernel/acpi.c      |  7 ++-----
> >>  init/main.c                   |  4 ++++
> >>  3 files changed, 22 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> >> index 32f465a80e4e..d53c95f4e1a9 100644
> >> --- a/arch/arm64/include/asm/acpi.h
> >> +++ b/arch/arm64/include/asm/acpi.h
> >> @@ -12,10 +12,12 @@
> >>  #ifndef _ASM_ACPI_H
> >>  #define _ASM_ACPI_H
> >>
> >> +#include <linux/efi.h>
> >>  #include <linux/memblock.h>
> >>  #include <linux/psci.h>
> >>
> >>  #include <asm/cputype.h>
> >> +#include <asm/io.h>
> >>  #include <asm/smp_plat.h>
> >>  #include <asm/tlbflush.h>
> >>
> >> @@ -29,18 +31,22 @@
> >>
> >>  /* Basic configuration for ACPI */
> >>  #ifdef CONFIG_ACPI
> >> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr);
> >> +
> >>  /* ACPI table mapping after acpi_permanent_mmap is set */
> >>  static inline void __iomem *acpi_os_ioremap(acpi_physical_address phys,
> >>                                             acpi_size size)
> >>  {
> >> +       /* For normal memory we already have a cacheable mapping. */
> >> +       if (memblock_is_map_memory(phys))
> >> +               return (void __iomem *)__phys_to_virt(phys);
> >> +
> >>         /*
> >> -        * EFI's reserve_regions() call adds memory with the WB attribute
> >> -        * to memblock via early_init_dt_add_memory_arch().
> >> +        * We should still honor the memory's attribute here because
> >> +        * crash dump kernel possibly excludes some ACPI (reclaim)
> >> +        * regions from memblock list.
> >>          */
> >> -       if (!memblock_is_memory(phys))
> >> -               return ioremap(phys, size);
> >> -
> >> -       return ioremap_cache(phys, size);
> >> +       return __ioremap(phys, size, __acpi_get_mem_attribute(phys));
> >>  }
> >>  #define acpi_os_ioremap acpi_os_ioremap
> >>
> >> @@ -125,7 +131,10 @@ static inline const char *acpi_get_enable_method(int cpu)
> >>   * for compatibility.
> >>   */
> >>  #define acpi_disable_cmcff 1
> >> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr);
> >> +static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> >> +{
> >> +       return __acpi_get_mem_attribute(addr);
> >> +}
> >>  #endif /* CONFIG_ACPI_APEI */
> >>
> >>  #ifdef CONFIG_ACPI_NUMA
> >> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> >> index b3162715ed78..f94bdf7be439 100644
> >> --- a/arch/arm64/kernel/acpi.c
> >> +++ b/arch/arm64/kernel/acpi.c
> >> @@ -31,10 +31,9 @@
> >>  #include <asm/cpu_ops.h>
> >>  #include <asm/smp_plat.h>
> >>
> >> -#ifdef CONFIG_ACPI_APEI
> >> +/* CONFIG_ACPI_APEI */
> >>  # include <linux/efi.h>
> >>  # include <asm/pgtable.h>
> >> -#endif
> >>
> >>  int acpi_noirq = 1;            /* skip ACPI IRQ initialization */
> >>  int acpi_disabled = 1;
> >> @@ -239,8 +238,7 @@ void __init acpi_boot_table_init(void)
> >>         }
> >>  }
> >>
> >> -#ifdef CONFIG_ACPI_APEI
> >> -pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> >> +pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
> >>  {
> >>         /*
> >>          * According to "Table 8 Map: EFI memory types to AArch64 memory
> >> @@ -261,4 +259,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
> >>                 return __pgprot(PROT_NORMAL_NC);
> >>         return __pgprot(PROT_DEVICE_nGnRnE);
> >>  }
> >> -#endif
> >> diff --git a/init/main.c b/init/main.c
> >> index a8100b954839..a479ece2bae9 100644
> >> --- a/init/main.c
> >> +++ b/init/main.c
> >> @@ -674,6 +674,10 @@ asmlinkage __visible void __init start_kernel(void)
> >>         debug_objects_mem_init();
> >>         setup_per_cpu_pageset();
> >>         numa_policy_init();
> >> +#if defined(CONFIG_ARM64) && defined(CONFIG_EFI)
> >> +       efi_memmap_init_late(efi.memmap.phys_map,
> >> +                       efi.memmap.nr_map * efi.memmap.desc_size);
> >> +#endif
> >>         acpi_early_init();
> >>         if (late_time_init)
> >>                 late_time_init();
> >> --
> >> 2.15.1
> >>
> 
> I tested Ard's patch (on top of Akashi's proposed changes) on the
> huawei taishan machine (where I originally found the problem) and I
> can confirm that I am able to boot the kdump kernel properly and also
> save the crashcore dump on local disk.
> 
> Also as Ard mentioned, 'efi_enter_virtual_mode' is probably not the
> best name for the proposed function as we have already called
> 'SetVirtualAddressMap', but I cannot think of anything better. If
> there are other opinions we can consider the same, otherwise may be we
> can formalize this and queue it up as crashkernel is bricked on arm64
> machines which support acpi boot machines without the same (and
> several kdump users are affected because of the same).
> 
> Please feel free to add:
> 
> Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
> 
> Regards,
> Bhupesh

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec