[PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[AKASHI Takahiro]

This patch series is a set of bug fixes to address kexec/kdump
failures which are sometimes observed on ACPI-only system and reported
in LAK-ML before.

In short, the phenomena are:
1. kexec'ed kernel can fail to boot because some ACPI table is corrupted
   by a new kernel (or other data) being loaded into System RAM. Currently
   kexec may possibly allocate space ignoring such "reserved" regions.
   We will see no messages after "Bye!"

2. crash dump (kdump) kernel can fail to boot and get into panic due to
   an alignment fault when accessing ACPI tables. This can happen because
   those tables are not always properly aligned while they are mapped
   non-cacheable (ioremap'ed) as they are not recognized as part of System
   RAM under the current implementation.

After discussing several possibilities to address those issues,
the agreed approach, in my understanding, is
* to add resource entries for every "reserved", i.e. memblock_reserve(),
  regions to /proc/iomem.
  (NOMAP regions, also marked as "reserved," remains at top-level for
  backward compatibility. User-space can tell the difference between
  reserved-system-ram and reserved-address-space.)
* For case (1), user space (kexec-tools) should rule out such regions
  in searching for free space for loaded data.
* For case (2), the kernel should access ACPI tables by mapping
  them with appropriate memory attributes described in UEFI memory map.
  (This means that it doesn't require any changes in /proc/iomem, and
  hence user space.)

Please find past discussions about /proc/iomem in [1].
--- more words from James ---
Our attempts to fix this just in the kernel reached a dead end, because Kdump
needs to include reserved-system-ram, whereas kexec has to avoid it. User-space
needs to be able to tell reserved-system-ram and reserved-address-space apart.
Hence we need to expose that information, and pick it up in user-space.

Patched-kernel and unpatch-user-space will work the same way it does today, as
the additional reserved regions are ignored by user-space.

Unpatched-kernel and patched-user-space will also work the same way it does
today as the additional reserved regions are missing.
--->8---

Patch#1 addresses kexec case, for which you are also required to update
user space. See necessary patches in [2]. If you want to review Patch#1,
please also take a look at and review [2].

Patch#2, #3 and #4 address the kdump case above.


Changes in v3.1 (2018, July 10, 2018)
* add Ard's patch[4] to this patch set

Changes in v3 (2018, July 9, 2018)
* drop the v2's patch#3, preferring [4]

Changes in v2 (2018, June 19, 2018)
* re-organise v1's patch#2 and #3 into v2's #2, #3 and #4
  not to break bisect

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/565980.html
[2] https://git.linaro.org/people/takahiro.akashi/kexec-tools.git arm64/resv_mem
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-April/573655.html
[4] https://marc.info/?l=linux-efi&m=152930773507524&w=2

AKASHI Takahiro (2):
  efi/arm: map UEFI memory map even w/o runtime services enabled
  arm64: acpi: fix alignment fault in accessing ACPI

Ard Biesheuvel (1):
  efi/arm: preserve early mapping of UEFI memory map longer for BGRT

James Morse (1):
  arm64: export memblock_reserve()d regions via /proc/iomem

 arch/arm64/include/asm/acpi.h      | 23 ++++++++++++------
 arch/arm64/kernel/acpi.c           | 11 +++------
 arch/arm64/kernel/setup.c          | 38 ++++++++++++++++++++++++++++++
 drivers/firmware/efi/arm-init.c    |  1 -
 drivers/firmware/efi/arm-runtime.c | 16 +++++++------
 5 files changed, 66 insertions(+), 23 deletions(-)

-- 
2.17.0

[PATCH v3.1 0/4] arm64: kexec, kdump: fix boot failures on acpi-only system

[AKASHI Takahiro]

This patch series is a set of bug fixes to address kexec/kdump
failures which are sometimes observed on ACPI-only system and reported
in LAK-ML before.

In short, the phenomena are:
1. kexec'ed kernel can fail to boot because some ACPI table is corrupted
   by a new kernel (or other data) being loaded into System RAM. Currently
   kexec may possibly allocate space ignoring such "reserved" regions.
   We will see no messages after "Bye!"

2. crash dump (kdump) kernel can fail to boot and get into panic due to
   an alignment fault when accessing ACPI tables. This can happen because
   those tables are not always properly aligned while they are mapped
   non-cacheable (ioremap'ed) as they are not recognized as part of System
   RAM under the current implementation.

After discussing several possibilities to address those issues,
the agreed approach, in my understanding, is
* to add resource entries for every "reserved", i.e. memblock_reserve(),
  regions to /proc/iomem.
  (NOMAP regions, also marked as "reserved," remains at top-level for
  backward compatibility. User-space can tell the difference between
  reserved-system-ram and reserved-address-space.)
* For case (1), user space (kexec-tools) should rule out such regions
  in searching for free space for loaded data.
* For case (2), the kernel should access ACPI tables by mapping
  them with appropriate memory attributes described in UEFI memory map.
  (This means that it doesn't require any changes in /proc/iomem, and
  hence user space.)

Please find past discussions about /proc/iomem in [1].
--- more words from James ---
Our attempts to fix this just in the kernel reached a dead end, because Kdump
needs to include reserved-system-ram, whereas kexec has to avoid it. User-space
needs to be able to tell reserved-system-ram and reserved-address-space apart.
Hence we need to expose that information, and pick it up in user-space.

Patched-kernel and unpatch-user-space will work the same way it does today, as
the additional reserved regions are ignored by user-space.

Unpatched-kernel and patched-user-space will also work the same way it does
today as the additional reserved regions are missing.
--->8---

Patch#1 addresses kexec case, for which you are also required to update
user space. See necessary patches in [2]. If you want to review Patch#1,
please also take a look at and review [2].

Patch#2, #3 and #4 address the kdump case above.


Changes in v3.1 (2018, July 10, 2018)
* add Ard's patch[4] to this patch set

Changes in v3 (2018, July 9, 2018)
* drop the v2's patch#3, preferring [4]

Changes in v2 (2018, June 19, 2018)
* re-organise v1's patch#2 and #3 into v2's #2, #3 and #4
  not to break bisect

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/565980.html
[2] https://git.linaro.org/people/takahiro.akashi/kexec-tools.git arm64/resv_mem
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-April/573655.html
[4] https://marc.info/?l=linux-efi&m=152930773507524&w=2

AKASHI Takahiro (2):
  efi/arm: map UEFI memory map even w/o runtime services enabled
  arm64: acpi: fix alignment fault in accessing ACPI

Ard Biesheuvel (1):
  efi/arm: preserve early mapping of UEFI memory map longer for BGRT

James Morse (1):
  arm64: export memblock_reserve()d regions via /proc/iomem

 arch/arm64/include/asm/acpi.h      | 23 ++++++++++++------
 arch/arm64/kernel/acpi.c           | 11 +++------
 arch/arm64/kernel/setup.c          | 38 ++++++++++++++++++++++++++++++
 drivers/firmware/efi/arm-init.c    |  1 -
 drivers/firmware/efi/arm-runtime.c | 16 +++++++------
 5 files changed, 66 insertions(+), 23 deletions(-)

-- 
2.17.0

[PATCH v3.1 0/4] arm64: kexec, kdump: fix boot failures on acpi-only system

[AKASHI Takahiro]

This patch series is a set of bug fixes to address kexec/kdump
failures which are sometimes observed on ACPI-only system and reported
in LAK-ML before.

In short, the phenomena are:
1. kexec'ed kernel can fail to boot because some ACPI table is corrupted
   by a new kernel (or other data) being loaded into System RAM. Currently
   kexec may possibly allocate space ignoring such "reserved" regions.
   We will see no messages after "Bye!"

2. crash dump (kdump) kernel can fail to boot and get into panic due to
   an alignment fault when accessing ACPI tables. This can happen because
   those tables are not always properly aligned while they are mapped
   non-cacheable (ioremap'ed) as they are not recognized as part of System
   RAM under the current implementation.

After discussing several possibilities to address those issues,
the agreed approach, in my understanding, is
* to add resource entries for every "reserved", i.e. memblock_reserve(),
  regions to /proc/iomem.
  (NOMAP regions, also marked as "reserved," remains at top-level for
  backward compatibility. User-space can tell the difference between
  reserved-system-ram and reserved-address-space.)
* For case (1), user space (kexec-tools) should rule out such regions
  in searching for free space for loaded data.
* For case (2), the kernel should access ACPI tables by mapping
  them with appropriate memory attributes described in UEFI memory map.
  (This means that it doesn't require any changes in /proc/iomem, and
  hence user space.)

Please find past discussions about /proc/iomem in [1].
--- more words from James ---
Our attempts to fix this just in the kernel reached a dead end, because Kdump
needs to include reserved-system-ram, whereas kexec has to avoid it. User-space
needs to be able to tell reserved-system-ram and reserved-address-space apart.
Hence we need to expose that information, and pick it up in user-space.

Patched-kernel and unpatch-user-space will work the same way it does today, as
the additional reserved regions are ignored by user-space.

Unpatched-kernel and patched-user-space will also work the same way it does
today as the additional reserved regions are missing.
--->8---

Patch#1 addresses kexec case, for which you are also required to update
user space. See necessary patches in [2]. If you want to review Patch#1,
please also take a look at and review [2].

Patch#2, #3 and #4 address the kdump case above.


Changes in v3.1 (2018, July 10, 2018)
* add Ard's patch[4] to this patch set

Changes in v3 (2018, July 9, 2018)
* drop the v2's patch#3, preferring [4]

Changes in v2 (2018, June 19, 2018)
* re-organise v1's patch#2 and #3 into v2's #2, #3 and #4
  not to break bisect

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/565980.html
[2] https://git.linaro.org/people/takahiro.akashi/kexec-tools.git arm64/resv_mem
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-April/573655.html
[4] https://marc.info/?l=linux-efi&m=152930773507524&w=2

AKASHI Takahiro (2):
  efi/arm: map UEFI memory map even w/o runtime services enabled
  arm64: acpi: fix alignment fault in accessing ACPI

Ard Biesheuvel (1):
  efi/arm: preserve early mapping of UEFI memory map longer for BGRT

James Morse (1):
  arm64: export memblock_reserve()d regions via /proc/iomem

 arch/arm64/include/asm/acpi.h      | 23 ++++++++++++------
 arch/arm64/kernel/acpi.c           | 11 +++------
 arch/arm64/kernel/setup.c          | 38 ++++++++++++++++++++++++++++++
 drivers/firmware/efi/arm-init.c    |  1 -
 drivers/firmware/efi/arm-runtime.c | 16 +++++++------
 5 files changed, 66 insertions(+), 23 deletions(-)

-- 
2.17.0


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

[PATCH v3.1 1/4] arm64: export memblock_reserve()d regions via /proc/iomem

[AKASHI Takahiro]

From: James Morse <james.morse@arm.com>

There has been some confusion around what is necessary to prevent kexec
overwriting important memory regions. memblock: reserve, or nomap?
Only memblock nomap regions are reported via /proc/iomem, kexec's
user-space doesn't know about memblock_reserve()d regions.

Until commit f56ab9a5b73ca ("efi/arm: Don't mark ACPI reclaim memory
as MEMBLOCK_NOMAP") the ACPI tables were nomap, now they are reserved
and thus possible for kexec to overwrite with the new kernel or initrd.
But this was always broken, as the UEFI memory map is also reserved
and not marked as nomap.

Exporting both nomap and reserved memblock types is a nuisance as
they live in different memblock structures which we can't walk at
the same time.

Take a second walk over memblock.reserved and add new 'reserved'
subnodes for the memblock_reserved() regions that aren't already
described by the existing code. (e.g. Kernel Code)

We use reserve_region_with_split() to find the gaps in existing named
regions. This handles the gap between 'kernel code' and 'kernel data'
which is memblock_reserve()d, but already partially described by
request_standard_resources(). e.g.:
| 80000000-dfffffff : System RAM
|   80080000-80ffffff : Kernel code
|   81000000-8158ffff : reserved
|   81590000-8237efff : Kernel data
|   a0000000-dfffffff : Crash kernel
| e00f0000-f949ffff : System RAM

reserve_region_with_split needs kzalloc() which isn't available when
request_standard_resources() is called, use an initcall.

Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
Reported-by: Tyler Baicar <tbaicar@codeaurora.org>
Suggested-by: Akashi Takahiro <takahiro.akashi@linaro.org>
Signed-off-by: James Morse <james.morse@arm.com>
Fixes: d28f6df1305a ("arm64/kexec: Add core kexec support")
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
CC: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/setup.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 30ad2f085d1f..5b4fac434c84 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -241,6 +241,44 @@ static void __init request_standard_resources(void)
 	}
 }
 
+static int __init reserve_memblock_reserved_regions(void)
+{
+	phys_addr_t start, end, roundup_end = 0;
+	struct resource *mem, *res;
+	u64 i;
+
+	for_each_reserved_mem_region(i, &start, &end) {
+		if (end <= roundup_end)
+			continue; /* done already */
+
+		start = __pfn_to_phys(PFN_DOWN(start));
+		end = __pfn_to_phys(PFN_UP(end)) - 1;
+		roundup_end = end;
+
+		res = kzalloc(sizeof(*res), GFP_ATOMIC);
+		if (WARN_ON(!res))
+			return -ENOMEM;
+		res->start = start;
+		res->end = end;
+		res->name  = "reserved";
+		res->flags = IORESOURCE_MEM;
+
+		mem = request_resource_conflict(&iomem_resource, res);
+		/*
+		 * We expected memblock_reserve() regions to conflict with
+		 * memory created by request_standard_resources().
+		 */
+		if (WARN_ON_ONCE(!mem))
+			continue;
+		kfree(res);
+
+		reserve_region_with_split(mem, start, end, "reserved");
+	}
+
+	return 0;
+}
+arch_initcall(reserve_memblock_reserved_regions);
+
 u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
 
 void __init setup_arch(char **cmdline_p)
-- 
2.17.0

[PATCH v3.1 1/4] arm64: export memblock_reserve()d regions via /proc/iomem

[AKASHI Takahiro]

From: James Morse <james.morse@arm.com>

There has been some confusion around what is necessary to prevent kexec
overwriting important memory regions. memblock: reserve, or nomap?
Only memblock nomap regions are reported via /proc/iomem, kexec's
user-space doesn't know about memblock_reserve()d regions.

Until commit f56ab9a5b73ca ("efi/arm: Don't mark ACPI reclaim memory
as MEMBLOCK_NOMAP") the ACPI tables were nomap, now they are reserved
and thus possible for kexec to overwrite with the new kernel or initrd.
But this was always broken, as the UEFI memory map is also reserved
and not marked as nomap.

Exporting both nomap and reserved memblock types is a nuisance as
they live in different memblock structures which we can't walk at
the same time.

Take a second walk over memblock.reserved and add new 'reserved'
subnodes for the memblock_reserved() regions that aren't already
described by the existing code. (e.g. Kernel Code)

We use reserve_region_with_split() to find the gaps in existing named
regions. This handles the gap between 'kernel code' and 'kernel data'
which is memblock_reserve()d, but already partially described by
request_standard_resources(). e.g.:
| 80000000-dfffffff : System RAM
|   80080000-80ffffff : Kernel code
|   81000000-8158ffff : reserved
|   81590000-8237efff : Kernel data
|   a0000000-dfffffff : Crash kernel
| e00f0000-f949ffff : System RAM

reserve_region_with_split needs kzalloc() which isn't available when
request_standard_resources() is called, use an initcall.

Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
Reported-by: Tyler Baicar <tbaicar@codeaurora.org>
Suggested-by: Akashi Takahiro <takahiro.akashi@linaro.org>
Signed-off-by: James Morse <james.morse@arm.com>
Fixes: d28f6df1305a ("arm64/kexec: Add core kexec support")
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
CC: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/setup.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 30ad2f085d1f..5b4fac434c84 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -241,6 +241,44 @@ static void __init request_standard_resources(void)
 	}
 }
 
+static int __init reserve_memblock_reserved_regions(void)
+{
+	phys_addr_t start, end, roundup_end = 0;
+	struct resource *mem, *res;
+	u64 i;
+
+	for_each_reserved_mem_region(i, &start, &end) {
+		if (end <= roundup_end)
+			continue; /* done already */
+
+		start = __pfn_to_phys(PFN_DOWN(start));
+		end = __pfn_to_phys(PFN_UP(end)) - 1;
+		roundup_end = end;
+
+		res = kzalloc(sizeof(*res), GFP_ATOMIC);
+		if (WARN_ON(!res))
+			return -ENOMEM;
+		res->start = start;
+		res->end = end;
+		res->name  = "reserved";
+		res->flags = IORESOURCE_MEM;
+
+		mem = request_resource_conflict(&iomem_resource, res);
+		/*
+		 * We expected memblock_reserve() regions to conflict with
+		 * memory created by request_standard_resources().
+		 */
+		if (WARN_ON_ONCE(!mem))
+			continue;
+		kfree(res);
+
+		reserve_region_with_split(mem, start, end, "reserved");
+	}
+
+	return 0;
+}
+arch_initcall(reserve_memblock_reserved_regions);
+
 u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
 
 void __init setup_arch(char **cmdline_p)
-- 
2.17.0

[PATCH v3.1 1/4] arm64: export memblock_reserve()d regions via /proc/iomem

[AKASHI Takahiro]

From: James Morse <james.morse@arm.com>

There has been some confusion around what is necessary to prevent kexec
overwriting important memory regions. memblock: reserve, or nomap?
Only memblock nomap regions are reported via /proc/iomem, kexec's
user-space doesn't know about memblock_reserve()d regions.

Until commit f56ab9a5b73ca ("efi/arm: Don't mark ACPI reclaim memory
as MEMBLOCK_NOMAP") the ACPI tables were nomap, now they are reserved
and thus possible for kexec to overwrite with the new kernel or initrd.
But this was always broken, as the UEFI memory map is also reserved
and not marked as nomap.

Exporting both nomap and reserved memblock types is a nuisance as
they live in different memblock structures which we can't walk at
the same time.

Take a second walk over memblock.reserved and add new 'reserved'
subnodes for the memblock_reserved() regions that aren't already
described by the existing code. (e.g. Kernel Code)

We use reserve_region_with_split() to find the gaps in existing named
regions. This handles the gap between 'kernel code' and 'kernel data'
which is memblock_reserve()d, but already partially described by
request_standard_resources(). e.g.:
| 80000000-dfffffff : System RAM
|   80080000-80ffffff : Kernel code
|   81000000-8158ffff : reserved
|   81590000-8237efff : Kernel data
|   a0000000-dfffffff : Crash kernel
| e00f0000-f949ffff : System RAM

reserve_region_with_split needs kzalloc() which isn't available when
request_standard_resources() is called, use an initcall.

Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
Reported-by: Tyler Baicar <tbaicar@codeaurora.org>
Suggested-by: Akashi Takahiro <takahiro.akashi@linaro.org>
Signed-off-by: James Morse <james.morse@arm.com>
Fixes: d28f6df1305a ("arm64/kexec: Add core kexec support")
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
CC: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/setup.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 30ad2f085d1f..5b4fac434c84 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -241,6 +241,44 @@ static void __init request_standard_resources(void)
 	}
 }
 
+static int __init reserve_memblock_reserved_regions(void)
+{
+	phys_addr_t start, end, roundup_end = 0;
+	struct resource *mem, *res;
+	u64 i;
+
+	for_each_reserved_mem_region(i, &start, &end) {
+		if (end <= roundup_end)
+			continue; /* done already */
+
+		start = __pfn_to_phys(PFN_DOWN(start));
+		end = __pfn_to_phys(PFN_UP(end)) - 1;
+		roundup_end = end;
+
+		res = kzalloc(sizeof(*res), GFP_ATOMIC);
+		if (WARN_ON(!res))
+			return -ENOMEM;
+		res->start = start;
+		res->end = end;
+		res->name  = "reserved";
+		res->flags = IORESOURCE_MEM;
+
+		mem = request_resource_conflict(&iomem_resource, res);
+		/*
+		 * We expected memblock_reserve() regions to conflict with
+		 * memory created by request_standard_resources().
+		 */
+		if (WARN_ON_ONCE(!mem))
+			continue;
+		kfree(res);
+
+		reserve_region_with_split(mem, start, end, "reserved");
+	}
+
+	return 0;
+}
+arch_initcall(reserve_memblock_reserved_regions);
+
 u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
 
 void __init setup_arch(char **cmdline_p)
-- 
2.17.0


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

[PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[AKASHI Takahiro]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

The BGRT code validates the contents of the table against the UEFI
memory map, and so it expects it to be mapped when the code runs.

On ARM, this is currently not the case, since we tear down the early
mapping after efi_init() completes, and only create the permanent
mapping in arm_enable_runtime_services(), which executes as an early
initcall, but still leaves a window where the UEFI memory map is not
mapped.

So move the call to efi_memmap_unmap() from efi_init() to
arm_enable_runtime_services().

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/arm-init.c    | 1 -
 drivers/firmware/efi/arm-runtime.c | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index b5214c143fee..388a929baf95 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -259,7 +259,6 @@ void __init efi_init(void)
 
 	reserve_regions();
 	efi_esrt_init();
-	efi_memmap_unmap();
 
 	memblock_reserve(params.mmap & PAGE_MASK,
 			 PAGE_ALIGN(params.mmap_size +
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 5889cbea60b8..59a8c0ec94d5 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
 		return 0;
 	}
 
+	efi_memmap_unmap();
+
 	if (efi_runtime_disabled()) {
 		pr_info("EFI runtime services will be disabled.\n");
 		return 0;
-- 
2.17.0

[PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[AKASHI Takahiro]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

The BGRT code validates the contents of the table against the UEFI
memory map, and so it expects it to be mapped when the code runs.

On ARM, this is currently not the case, since we tear down the early
mapping after efi_init() completes, and only create the permanent
mapping in arm_enable_runtime_services(), which executes as an early
initcall, but still leaves a window where the UEFI memory map is not
mapped.

So move the call to efi_memmap_unmap() from efi_init() to
arm_enable_runtime_services().

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/arm-init.c    | 1 -
 drivers/firmware/efi/arm-runtime.c | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index b5214c143fee..388a929baf95 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -259,7 +259,6 @@ void __init efi_init(void)
 
 	reserve_regions();
 	efi_esrt_init();
-	efi_memmap_unmap();
 
 	memblock_reserve(params.mmap & PAGE_MASK,
 			 PAGE_ALIGN(params.mmap_size +
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 5889cbea60b8..59a8c0ec94d5 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
 		return 0;
 	}
 
+	efi_memmap_unmap();
+
 	if (efi_runtime_disabled()) {
 		pr_info("EFI runtime services will be disabled.\n");
 		return 0;
-- 
2.17.0

[PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[AKASHI Takahiro]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

The BGRT code validates the contents of the table against the UEFI
memory map, and so it expects it to be mapped when the code runs.

On ARM, this is currently not the case, since we tear down the early
mapping after efi_init() completes, and only create the permanent
mapping in arm_enable_runtime_services(), which executes as an early
initcall, but still leaves a window where the UEFI memory map is not
mapped.

So move the call to efi_memmap_unmap() from efi_init() to
arm_enable_runtime_services().

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/arm-init.c    | 1 -
 drivers/firmware/efi/arm-runtime.c | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index b5214c143fee..388a929baf95 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -259,7 +259,6 @@ void __init efi_init(void)
 
 	reserve_regions();
 	efi_esrt_init();
-	efi_memmap_unmap();
 
 	memblock_reserve(params.mmap & PAGE_MASK,
 			 PAGE_ALIGN(params.mmap_size +
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 5889cbea60b8..59a8c0ec94d5 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
 		return 0;
 	}
 
+	efi_memmap_unmap();
+
 	if (efi_runtime_disabled()) {
 		pr_info("EFI runtime services will be disabled.\n");
 		return 0;
-- 
2.17.0


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

[PATCH v3.1 3/4] efi/arm: map UEFI memory map even w/o runtime services enabled

[AKASHI Takahiro]

Under the current implementation, UEFI memory map will be mapped and made
available in virtual mappings only if runtime services are enabled.
But in a later patch, we want to use UEFI memory map in acpi_os_ioremap()
to create mappings of ACPI tables using memory attributes described in
UEFI memory map.
See the following commit:
    arm64: acpi: fix alignment fault in accessing ACPI tables

So, as a first step, arm_enter_runtime_services() is modified, alongside
Ard's patch[1], so that UEFI memory map will not be freed even if
efi=noruntime.

[1] https://marc.info/?l=linux-efi&m=152930773507524&w=2

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/arm-runtime.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 59a8c0ec94d5..a00934d263c5 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -117,6 +117,13 @@ static int __init arm_enable_runtime_services(void)
 
 	efi_memmap_unmap();
 
+	mapsize = efi.memmap.desc_size * efi.memmap.nr_map;
+
+	if (efi_memmap_init_late(efi.memmap.phys_map, mapsize)) {
+		pr_err("Failed to remap EFI memory map\n");
+		return 0;
+	}
+
 	if (efi_runtime_disabled()) {
 		pr_info("EFI runtime services will be disabled.\n");
 		return 0;
@@ -129,13 +136,6 @@ static int __init arm_enable_runtime_services(void)
 
 	pr_info("Remapping and enabling EFI services.\n");
 
-	mapsize = efi.memmap.desc_size * efi.memmap.nr_map;
-
-	if (efi_memmap_init_late(efi.memmap.phys_map, mapsize)) {
-		pr_err("Failed to remap EFI memory map\n");
-		return -ENOMEM;
-	}
-
 	if (!efi_virtmap_init()) {
 		pr_err("UEFI virtual mapping missing or invalid -- runtime services will not be available\n");
 		return -ENOMEM;
-- 
2.17.0

[PATCH v3.1 3/4] efi/arm: map UEFI memory map even w/o runtime services enabled

[AKASHI Takahiro]

Under the current implementation, UEFI memory map will be mapped and made
available in virtual mappings only if runtime services are enabled.
But in a later patch, we want to use UEFI memory map in acpi_os_ioremap()
to create mappings of ACPI tables using memory attributes described in
UEFI memory map.
See the following commit:
    arm64: acpi: fix alignment fault in accessing ACPI tables

So, as a first step, arm_enter_runtime_services() is modified, alongside
Ard's patch[1], so that UEFI memory map will not be freed even if
efi=noruntime.

[1] https://marc.info/?l=linux-efi&m=152930773507524&w=2

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/arm-runtime.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 59a8c0ec94d5..a00934d263c5 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -117,6 +117,13 @@ static int __init arm_enable_runtime_services(void)
 
 	efi_memmap_unmap();
 
+	mapsize = efi.memmap.desc_size * efi.memmap.nr_map;
+
+	if (efi_memmap_init_late(efi.memmap.phys_map, mapsize)) {
+		pr_err("Failed to remap EFI memory map\n");
+		return 0;
+	}
+
 	if (efi_runtime_disabled()) {
 		pr_info("EFI runtime services will be disabled.\n");
 		return 0;
@@ -129,13 +136,6 @@ static int __init arm_enable_runtime_services(void)
 
 	pr_info("Remapping and enabling EFI services.\n");
 
-	mapsize = efi.memmap.desc_size * efi.memmap.nr_map;
-
-	if (efi_memmap_init_late(efi.memmap.phys_map, mapsize)) {
-		pr_err("Failed to remap EFI memory map\n");
-		return -ENOMEM;
-	}
-
 	if (!efi_virtmap_init()) {
 		pr_err("UEFI virtual mapping missing or invalid -- runtime services will not be available\n");
 		return -ENOMEM;
-- 
2.17.0

[PATCH v3.1 3/4] efi/arm: map UEFI memory map even w/o runtime services enabled

[AKASHI Takahiro]

Under the current implementation, UEFI memory map will be mapped and made
available in virtual mappings only if runtime services are enabled.
But in a later patch, we want to use UEFI memory map in acpi_os_ioremap()
to create mappings of ACPI tables using memory attributes described in
UEFI memory map.
See the following commit:
    arm64: acpi: fix alignment fault in accessing ACPI tables

So, as a first step, arm_enter_runtime_services() is modified, alongside
Ard's patch[1], so that UEFI memory map will not be freed even if
efi=noruntime.

[1] https://marc.info/?l=linux-efi&m=152930773507524&w=2

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/arm-runtime.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 59a8c0ec94d5..a00934d263c5 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -117,6 +117,13 @@ static int __init arm_enable_runtime_services(void)
 
 	efi_memmap_unmap();
 
+	mapsize = efi.memmap.desc_size * efi.memmap.nr_map;
+
+	if (efi_memmap_init_late(efi.memmap.phys_map, mapsize)) {
+		pr_err("Failed to remap EFI memory map\n");
+		return 0;
+	}
+
 	if (efi_runtime_disabled()) {
 		pr_info("EFI runtime services will be disabled.\n");
 		return 0;
@@ -129,13 +136,6 @@ static int __init arm_enable_runtime_services(void)
 
 	pr_info("Remapping and enabling EFI services.\n");
 
-	mapsize = efi.memmap.desc_size * efi.memmap.nr_map;
-
-	if (efi_memmap_init_late(efi.memmap.phys_map, mapsize)) {
-		pr_err("Failed to remap EFI memory map\n");
-		return -ENOMEM;
-	}
-
 	if (!efi_virtmap_init()) {
 		pr_err("UEFI virtual mapping missing or invalid -- runtime services will not be available\n");
 		return -ENOMEM;
-- 
2.17.0


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

[PATCH v3.1 4/4] arm64: acpi: fix alignment fault in accessing ACPI

[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."

(kernel messages after panic kicked off kdump)
	   (snip...)
	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 are differently handled
  as they are "memblock-reserved", without NOMAP bit.
* So they are now excluded from device tree's "usable-memory-range"
  which kexec-tools determines based on a current view of /proc/iomem.
* When crash dump kernel boots up, it tries to accesses ACPI tables by
  mapping them with ioremap(), not ioremap_cache(), in acpi_os_ioremap()
  since they are no longer part of mapped system ram.
* Given that ACPI accessor/helper functions are compiled in without
  unaligned access support (ACPI_MISALIGNMENT_NOT_SUPPORTED),
  any unaligned access to ACPI tables can cause a fatal panic.

With this patch, acpi_os_ioremap() always honors memory attribute
information provided by the firmware (EFI) and retaining cacheability
allows the kernel safe access to ACPI tables.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: James Morse <james.morse@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reported-by and Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
---
 arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
 arch/arm64/kernel/acpi.c      | 11 +++--------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 0db62a4cbce2..68bc18cb2b85 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
 
@@ -129,7 +135,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 7b09487ff8fb..ed46dc188b22 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -18,6 +18,7 @@
 #include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <linux/cpumask.h>
+#include <linux/efi.h>
 #include <linux/efi-bgrt.h>
 #include <linux/init.h>
 #include <linux/irq.h>
@@ -29,13 +30,9 @@
 
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
+#include <asm/pgtable.h>
 #include <asm/smp_plat.h>
 
-#ifdef CONFIG_ACPI_APEI
-# include <linux/efi.h>
-# include <asm/pgtable.h>
-#endif
-
 int acpi_noirq = 1;		/* skip ACPI IRQ initialization */
 int acpi_disabled = 1;
 EXPORT_SYMBOL(acpi_disabled);
@@ -239,8 +236,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 +257,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_NC);
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }
-#endif
-- 
2.17.0

[PATCH v3.1 4/4] arm64: acpi: fix alignment fault in accessing ACPI

[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."

(kernel messages after panic kicked off kdump)
	   (snip...)
	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 are differently handled
  as they are "memblock-reserved", without NOMAP bit.
* So they are now excluded from device tree's "usable-memory-range"
  which kexec-tools determines based on a current view of /proc/iomem.
* When crash dump kernel boots up, it tries to accesses ACPI tables by
  mapping them with ioremap(), not ioremap_cache(), in acpi_os_ioremap()
  since they are no longer part of mapped system ram.
* Given that ACPI accessor/helper functions are compiled in without
  unaligned access support (ACPI_MISALIGNMENT_NOT_SUPPORTED),
  any unaligned access to ACPI tables can cause a fatal panic.

With this patch, acpi_os_ioremap() always honors memory attribute
information provided by the firmware (EFI) and retaining cacheability
allows the kernel safe access to ACPI tables.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: James Morse <james.morse@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reported-by and Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
---
 arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
 arch/arm64/kernel/acpi.c      | 11 +++--------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 0db62a4cbce2..68bc18cb2b85 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
 
@@ -129,7 +135,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 7b09487ff8fb..ed46dc188b22 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -18,6 +18,7 @@
 #include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <linux/cpumask.h>
+#include <linux/efi.h>
 #include <linux/efi-bgrt.h>
 #include <linux/init.h>
 #include <linux/irq.h>
@@ -29,13 +30,9 @@
 
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
+#include <asm/pgtable.h>
 #include <asm/smp_plat.h>
 
-#ifdef CONFIG_ACPI_APEI
-# include <linux/efi.h>
-# include <asm/pgtable.h>
-#endif
-
 int acpi_noirq = 1;		/* skip ACPI IRQ initialization */
 int acpi_disabled = 1;
 EXPORT_SYMBOL(acpi_disabled);
@@ -239,8 +236,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 +257,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_NC);
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }
-#endif
-- 
2.17.0

[PATCH v3.1 4/4] arm64: acpi: fix alignment fault in accessing ACPI

[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."

(kernel messages after panic kicked off kdump)
	   (snip...)
	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 are differently handled
  as they are "memblock-reserved", without NOMAP bit.
* So they are now excluded from device tree's "usable-memory-range"
  which kexec-tools determines based on a current view of /proc/iomem.
* When crash dump kernel boots up, it tries to accesses ACPI tables by
  mapping them with ioremap(), not ioremap_cache(), in acpi_os_ioremap()
  since they are no longer part of mapped system ram.
* Given that ACPI accessor/helper functions are compiled in without
  unaligned access support (ACPI_MISALIGNMENT_NOT_SUPPORTED),
  any unaligned access to ACPI tables can cause a fatal panic.

With this patch, acpi_os_ioremap() always honors memory attribute
information provided by the firmware (EFI) and retaining cacheability
allows the kernel safe access to ACPI tables.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: James Morse <james.morse@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reported-by and Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
---
 arch/arm64/include/asm/acpi.h | 23 ++++++++++++++++-------
 arch/arm64/kernel/acpi.c      | 11 +++--------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 0db62a4cbce2..68bc18cb2b85 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
 
@@ -129,7 +135,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 7b09487ff8fb..ed46dc188b22 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -18,6 +18,7 @@
 #include <linux/acpi.h>
 #include <linux/bootmem.h>
 #include <linux/cpumask.h>
+#include <linux/efi.h>
 #include <linux/efi-bgrt.h>
 #include <linux/init.h>
 #include <linux/irq.h>
@@ -29,13 +30,9 @@
 
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
+#include <asm/pgtable.h>
 #include <asm/smp_plat.h>
 
-#ifdef CONFIG_ACPI_APEI
-# include <linux/efi.h>
-# include <asm/pgtable.h>
-#endif
-
 int acpi_noirq = 1;		/* skip ACPI IRQ initialization */
 int acpi_disabled = 1;
 EXPORT_SYMBOL(acpi_disabled);
@@ -239,8 +236,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 +257,3 @@ pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_NC);
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }
-#endif
-- 
2.17.0


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

Re: [PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[James Morse]

Hi Ard,

On 10/07/18 00:42, AKASHI Takahiro wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> The BGRT code validates the contents of the table against the UEFI
> memory map, and so it expects it to be mapped when the code runs.
> 
> On ARM, this is currently not the case, since we tear down the early
> mapping after efi_init() completes, and only create the permanent
> mapping in arm_enable_runtime_services(), which executes as an early
> initcall, but still leaves a window where the UEFI memory map is not
> mapped.
> 
> So move the call to efi_memmap_unmap() from efi_init() to
> arm_enable_runtime_services().

I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.


> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index b5214c143fee..388a929baf95 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -259,7 +259,6 @@ void __init efi_init(void)
>  
>  	reserve_regions();
>  	efi_esrt_init();
> -	efi_memmap_unmap();
>  
>  	memblock_reserve(params.mmap & PAGE_MASK,
>  			 PAGE_ALIGN(params.mmap_size +
> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
> index 5889cbea60b8..59a8c0ec94d5 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
>  		return 0;
>  	}
>  
> +	efi_memmap_unmap();

This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
but this can only happen if the system table signature is wrong, (or we're out
of memory really early).

I think this is harmless as we end up passing NULL to early_memunmap() which
WARN()s and returns as its outside the fixmap range. Its just more noise on
systems with a corrupt efi system table.

Acked-by: James Morse <james.morse@arm.com>


Thanks,

James

[PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[James Morse]

Hi Ard,

On 10/07/18 00:42, AKASHI Takahiro wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> The BGRT code validates the contents of the table against the UEFI
> memory map, and so it expects it to be mapped when the code runs.
> 
> On ARM, this is currently not the case, since we tear down the early
> mapping after efi_init() completes, and only create the permanent
> mapping in arm_enable_runtime_services(), which executes as an early
> initcall, but still leaves a window where the UEFI memory map is not
> mapped.
> 
> So move the call to efi_memmap_unmap() from efi_init() to
> arm_enable_runtime_services().

I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.


> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index b5214c143fee..388a929baf95 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -259,7 +259,6 @@ void __init efi_init(void)
>  
>  	reserve_regions();
>  	efi_esrt_init();
> -	efi_memmap_unmap();
>  
>  	memblock_reserve(params.mmap & PAGE_MASK,
>  			 PAGE_ALIGN(params.mmap_size +
> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
> index 5889cbea60b8..59a8c0ec94d5 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
>  		return 0;
>  	}
>  
> +	efi_memmap_unmap();

This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
but this can only happen if the system table signature is wrong, (or we're out
of memory really early).

I think this is harmless as we end up passing NULL to early_memunmap() which
WARN()s and returns as its outside the fixmap range. Its just more noise on
systems with a corrupt efi system table.

Acked-by: James Morse <james.morse@arm.com>


Thanks,

James

Re: [PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[James Morse]

Hi Ard,

On 10/07/18 00:42, AKASHI Takahiro wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> The BGRT code validates the contents of the table against the UEFI
> memory map, and so it expects it to be mapped when the code runs.
> 
> On ARM, this is currently not the case, since we tear down the early
> mapping after efi_init() completes, and only create the permanent
> mapping in arm_enable_runtime_services(), which executes as an early
> initcall, but still leaves a window where the UEFI memory map is not
> mapped.
> 
> So move the call to efi_memmap_unmap() from efi_init() to
> arm_enable_runtime_services().

I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.


> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index b5214c143fee..388a929baf95 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -259,7 +259,6 @@ void __init efi_init(void)
>  
>  	reserve_regions();
>  	efi_esrt_init();
> -	efi_memmap_unmap();
>  
>  	memblock_reserve(params.mmap & PAGE_MASK,
>  			 PAGE_ALIGN(params.mmap_size +
> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
> index 5889cbea60b8..59a8c0ec94d5 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
>  		return 0;
>  	}
>  
> +	efi_memmap_unmap();

This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
but this can only happen if the system table signature is wrong, (or we're out
of memory really early).

I think this is harmless as we end up passing NULL to early_memunmap() which
WARN()s and returns as its outside the fixmap range. Its just more noise on
systems with a corrupt efi system table.

Acked-by: James Morse <james.morse@arm.com>


Thanks,

James

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

Re: [PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[Ard Biesheuvel]

On 10 July 2018 at 19:57, James Morse <james.morse@arm.com> wrote:
> Hi Ard,
>
> On 10/07/18 00:42, AKASHI Takahiro wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> The BGRT code validates the contents of the table against the UEFI
>> memory map, and so it expects it to be mapped when the code runs.
>>
>> On ARM, this is currently not the case, since we tear down the early
>> mapping after efi_init() completes, and only create the permanent
>> mapping in arm_enable_runtime_services(), which executes as an early
>> initcall, but still leaves a window where the UEFI memory map is not
>> mapped.
>>
>> So move the call to efi_memmap_unmap() from efi_init() to
>> arm_enable_runtime_services().
>
> I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
> call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.
>

I'm not sure I follow. The BGRT table only contains natively aligned
fields, so the alignment faults should not occur when accessing this
table after kexec. The issue addressed by this patch is that
efi_mem_type() bails when called while EFI_MEMMAP is cleared.

>
>> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
>> index b5214c143fee..388a929baf95 100644
>> --- a/drivers/firmware/efi/arm-init.c
>> +++ b/drivers/firmware/efi/arm-init.c
>> @@ -259,7 +259,6 @@ void __init efi_init(void)
>>
>>       reserve_regions();
>>       efi_esrt_init();
>> -     efi_memmap_unmap();
>>
>>       memblock_reserve(params.mmap & PAGE_MASK,
>>                        PAGE_ALIGN(params.mmap_size +
>> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
>> index 5889cbea60b8..59a8c0ec94d5 100644
>> --- a/drivers/firmware/efi/arm-runtime.c
>> +++ b/drivers/firmware/efi/arm-runtime.c
>> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
>>               return 0;
>>       }
>>
>> +     efi_memmap_unmap();
>
> This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
> but this can only happen if the system table signature is wrong, (or we're out
> of memory really early).
>

I guess we should check the EFI_MEMMAP attribute here as well then.

> I think this is harmless as we end up passing NULL to early_memunmap() which
> WARN()s and returns as its outside the fixmap range. Its just more noise on
> systems with a corrupt efi system table.
>
> Acked-by: James Morse <james.morse@arm.com>
>

Thanks James

[PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[Ard Biesheuvel]

On 10 July 2018 at 19:57, James Morse <james.morse@arm.com> wrote:
> Hi Ard,
>
> On 10/07/18 00:42, AKASHI Takahiro wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> The BGRT code validates the contents of the table against the UEFI
>> memory map, and so it expects it to be mapped when the code runs.
>>
>> On ARM, this is currently not the case, since we tear down the early
>> mapping after efi_init() completes, and only create the permanent
>> mapping in arm_enable_runtime_services(), which executes as an early
>> initcall, but still leaves a window where the UEFI memory map is not
>> mapped.
>>
>> So move the call to efi_memmap_unmap() from efi_init() to
>> arm_enable_runtime_services().
>
> I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
> call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.
>

I'm not sure I follow. The BGRT table only contains natively aligned
fields, so the alignment faults should not occur when accessing this
table after kexec. The issue addressed by this patch is that
efi_mem_type() bails when called while EFI_MEMMAP is cleared.

>
>> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
>> index b5214c143fee..388a929baf95 100644
>> --- a/drivers/firmware/efi/arm-init.c
>> +++ b/drivers/firmware/efi/arm-init.c
>> @@ -259,7 +259,6 @@ void __init efi_init(void)
>>
>>       reserve_regions();
>>       efi_esrt_init();
>> -     efi_memmap_unmap();
>>
>>       memblock_reserve(params.mmap & PAGE_MASK,
>>                        PAGE_ALIGN(params.mmap_size +
>> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
>> index 5889cbea60b8..59a8c0ec94d5 100644
>> --- a/drivers/firmware/efi/arm-runtime.c
>> +++ b/drivers/firmware/efi/arm-runtime.c
>> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
>>               return 0;
>>       }
>>
>> +     efi_memmap_unmap();
>
> This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
> but this can only happen if the system table signature is wrong, (or we're out
> of memory really early).
>

I guess we should check the EFI_MEMMAP attribute here as well then.

> I think this is harmless as we end up passing NULL to early_memunmap() which
> WARN()s and returns as its outside the fixmap range. Its just more noise on
> systems with a corrupt efi system table.
>
> Acked-by: James Morse <james.morse@arm.com>
>

Thanks James

Re: [PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[Ard Biesheuvel]

On 10 July 2018 at 19:57, James Morse <james.morse@arm.com> wrote:
> Hi Ard,
>
> On 10/07/18 00:42, AKASHI Takahiro wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> The BGRT code validates the contents of the table against the UEFI
>> memory map, and so it expects it to be mapped when the code runs.
>>
>> On ARM, this is currently not the case, since we tear down the early
>> mapping after efi_init() completes, and only create the permanent
>> mapping in arm_enable_runtime_services(), which executes as an early
>> initcall, but still leaves a window where the UEFI memory map is not
>> mapped.
>>
>> So move the call to efi_memmap_unmap() from efi_init() to
>> arm_enable_runtime_services().
>
> I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
> call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.
>

I'm not sure I follow. The BGRT table only contains natively aligned
fields, so the alignment faults should not occur when accessing this
table after kexec. The issue addressed by this patch is that
efi_mem_type() bails when called while EFI_MEMMAP is cleared.

>
>> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
>> index b5214c143fee..388a929baf95 100644
>> --- a/drivers/firmware/efi/arm-init.c
>> +++ b/drivers/firmware/efi/arm-init.c
>> @@ -259,7 +259,6 @@ void __init efi_init(void)
>>
>>       reserve_regions();
>>       efi_esrt_init();
>> -     efi_memmap_unmap();
>>
>>       memblock_reserve(params.mmap & PAGE_MASK,
>>                        PAGE_ALIGN(params.mmap_size +
>> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
>> index 5889cbea60b8..59a8c0ec94d5 100644
>> --- a/drivers/firmware/efi/arm-runtime.c
>> +++ b/drivers/firmware/efi/arm-runtime.c
>> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
>>               return 0;
>>       }
>>
>> +     efi_memmap_unmap();
>
> This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
> but this can only happen if the system table signature is wrong, (or we're out
> of memory really early).
>

I guess we should check the EFI_MEMMAP attribute here as well then.

> I think this is harmless as we end up passing NULL to early_memunmap() which
> WARN()s and returns as its outside the fixmap range. Its just more noise on
> systems with a corrupt efi system table.
>
> Acked-by: James Morse <james.morse@arm.com>
>

Thanks James

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

Re: [PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[Will Deacon]

On Tue, Jul 10, 2018 at 08:39:16PM +0200, Ard Biesheuvel wrote:
> On 10 July 2018 at 19:57, James Morse <james.morse@arm.com> wrote:
> > Hi Ard,
> >
> > On 10/07/18 00:42, AKASHI Takahiro wrote:
> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> The BGRT code validates the contents of the table against the UEFI
> >> memory map, and so it expects it to be mapped when the code runs.
> >>
> >> On ARM, this is currently not the case, since we tear down the early
> >> mapping after efi_init() completes, and only create the permanent
> >> mapping in arm_enable_runtime_services(), which executes as an early
> >> initcall, but still leaves a window where the UEFI memory map is not
> >> mapped.
> >>
> >> So move the call to efi_memmap_unmap() from efi_init() to
> >> arm_enable_runtime_services().
> >
> > I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
> > call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.
> >
> 
> I'm not sure I follow. The BGRT table only contains natively aligned
> fields, so the alignment faults should not occur when accessing this
> table after kexec. The issue addressed by this patch is that
> efi_mem_type() bails when called while EFI_MEMMAP is cleared.
> 
> >
> >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> >> index b5214c143fee..388a929baf95 100644
> >> --- a/drivers/firmware/efi/arm-init.c
> >> +++ b/drivers/firmware/efi/arm-init.c
> >> @@ -259,7 +259,6 @@ void __init efi_init(void)
> >>
> >>       reserve_regions();
> >>       efi_esrt_init();
> >> -     efi_memmap_unmap();
> >>
> >>       memblock_reserve(params.mmap & PAGE_MASK,
> >>                        PAGE_ALIGN(params.mmap_size +
> >> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
> >> index 5889cbea60b8..59a8c0ec94d5 100644
> >> --- a/drivers/firmware/efi/arm-runtime.c
> >> +++ b/drivers/firmware/efi/arm-runtime.c
> >> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
> >>               return 0;
> >>       }
> >>
> >> +     efi_memmap_unmap();
> >
> > This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
> > but this can only happen if the system table signature is wrong, (or we're out
> > of memory really early).
> >
> 
> I guess we should check the EFI_MEMMAP attribute here as well then.

Do you plan to spin a new version of this patch?

Will

[PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[Will Deacon]

On Tue, Jul 10, 2018 at 08:39:16PM +0200, Ard Biesheuvel wrote:
> On 10 July 2018 at 19:57, James Morse <james.morse@arm.com> wrote:
> > Hi Ard,
> >
> > On 10/07/18 00:42, AKASHI Takahiro wrote:
> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> The BGRT code validates the contents of the table against the UEFI
> >> memory map, and so it expects it to be mapped when the code runs.
> >>
> >> On ARM, this is currently not the case, since we tear down the early
> >> mapping after efi_init() completes, and only create the permanent
> >> mapping in arm_enable_runtime_services(), which executes as an early
> >> initcall, but still leaves a window where the UEFI memory map is not
> >> mapped.
> >>
> >> So move the call to efi_memmap_unmap() from efi_init() to
> >> arm_enable_runtime_services().
> >
> > I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
> > call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.
> >
> 
> I'm not sure I follow. The BGRT table only contains natively aligned
> fields, so the alignment faults should not occur when accessing this
> table after kexec. The issue addressed by this patch is that
> efi_mem_type() bails when called while EFI_MEMMAP is cleared.
> 
> >
> >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> >> index b5214c143fee..388a929baf95 100644
> >> --- a/drivers/firmware/efi/arm-init.c
> >> +++ b/drivers/firmware/efi/arm-init.c
> >> @@ -259,7 +259,6 @@ void __init efi_init(void)
> >>
> >>       reserve_regions();
> >>       efi_esrt_init();
> >> -     efi_memmap_unmap();
> >>
> >>       memblock_reserve(params.mmap & PAGE_MASK,
> >>                        PAGE_ALIGN(params.mmap_size +
> >> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
> >> index 5889cbea60b8..59a8c0ec94d5 100644
> >> --- a/drivers/firmware/efi/arm-runtime.c
> >> +++ b/drivers/firmware/efi/arm-runtime.c
> >> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
> >>               return 0;
> >>       }
> >>
> >> +     efi_memmap_unmap();
> >
> > This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
> > but this can only happen if the system table signature is wrong, (or we're out
> > of memory really early).
> >
> 
> I guess we should check the EFI_MEMMAP attribute here as well then.

Do you plan to spin a new version of this patch?

Will

Re: [PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[Will Deacon]

On Tue, Jul 10, 2018 at 08:39:16PM +0200, Ard Biesheuvel wrote:
> On 10 July 2018 at 19:57, James Morse <james.morse@arm.com> wrote:
> > Hi Ard,
> >
> > On 10/07/18 00:42, AKASHI Takahiro wrote:
> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> The BGRT code validates the contents of the table against the UEFI
> >> memory map, and so it expects it to be mapped when the code runs.
> >>
> >> On ARM, this is currently not the case, since we tear down the early
> >> mapping after efi_init() completes, and only create the permanent
> >> mapping in arm_enable_runtime_services(), which executes as an early
> >> initcall, but still leaves a window where the UEFI memory map is not
> >> mapped.
> >>
> >> So move the call to efi_memmap_unmap() from efi_init() to
> >> arm_enable_runtime_services().
> >
> > I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
> > call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.
> >
> 
> I'm not sure I follow. The BGRT table only contains natively aligned
> fields, so the alignment faults should not occur when accessing this
> table after kexec. The issue addressed by this patch is that
> efi_mem_type() bails when called while EFI_MEMMAP is cleared.
> 
> >
> >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> >> index b5214c143fee..388a929baf95 100644
> >> --- a/drivers/firmware/efi/arm-init.c
> >> +++ b/drivers/firmware/efi/arm-init.c
> >> @@ -259,7 +259,6 @@ void __init efi_init(void)
> >>
> >>       reserve_regions();
> >>       efi_esrt_init();
> >> -     efi_memmap_unmap();
> >>
> >>       memblock_reserve(params.mmap & PAGE_MASK,
> >>                        PAGE_ALIGN(params.mmap_size +
> >> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
> >> index 5889cbea60b8..59a8c0ec94d5 100644
> >> --- a/drivers/firmware/efi/arm-runtime.c
> >> +++ b/drivers/firmware/efi/arm-runtime.c
> >> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
> >>               return 0;
> >>       }
> >>
> >> +     efi_memmap_unmap();
> >
> > This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
> > but this can only happen if the system table signature is wrong, (or we're out
> > of memory really early).
> >
> 
> I guess we should check the EFI_MEMMAP attribute here as well then.

Do you plan to spin a new version of this patch?

Will

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

Re: [PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[Ard Biesheuvel]

On 12 July 2018 at 15:32, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 10, 2018 at 08:39:16PM +0200, Ard Biesheuvel wrote:
>> On 10 July 2018 at 19:57, James Morse <james.morse@arm.com> wrote:
>> > Hi Ard,
>> >
>> > On 10/07/18 00:42, AKASHI Takahiro wrote:
>> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >>
>> >> The BGRT code validates the contents of the table against the UEFI
>> >> memory map, and so it expects it to be mapped when the code runs.
>> >>
>> >> On ARM, this is currently not the case, since we tear down the early
>> >> mapping after efi_init() completes, and only create the permanent
>> >> mapping in arm_enable_runtime_services(), which executes as an early
>> >> initcall, but still leaves a window where the UEFI memory map is not
>> >> mapped.
>> >>
>> >> So move the call to efi_memmap_unmap() from efi_init() to
>> >> arm_enable_runtime_services().
>> >
>> > I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
>> > call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.
>> >
>>
>> I'm not sure I follow. The BGRT table only contains natively aligned
>> fields, so the alignment faults should not occur when accessing this
>> table after kexec. The issue addressed by this patch is that
>> efi_mem_type() bails when called while EFI_MEMMAP is cleared.
>>
>> >
>> >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
>> >> index b5214c143fee..388a929baf95 100644
>> >> --- a/drivers/firmware/efi/arm-init.c
>> >> +++ b/drivers/firmware/efi/arm-init.c
>> >> @@ -259,7 +259,6 @@ void __init efi_init(void)
>> >>
>> >>       reserve_regions();
>> >>       efi_esrt_init();
>> >> -     efi_memmap_unmap();
>> >>
>> >>       memblock_reserve(params.mmap & PAGE_MASK,
>> >>                        PAGE_ALIGN(params.mmap_size +
>> >> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
>> >> index 5889cbea60b8..59a8c0ec94d5 100644
>> >> --- a/drivers/firmware/efi/arm-runtime.c
>> >> +++ b/drivers/firmware/efi/arm-runtime.c
>> >> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
>> >>               return 0;
>> >>       }
>> >>
>> >> +     efi_memmap_unmap();
>> >
>> > This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
>> > but this can only happen if the system table signature is wrong, (or we're out
>> > of memory really early).
>> >
>>
>> I guess we should check the EFI_MEMMAP attribute here as well then.
>
> Do you plan to spin a new version of this patch?
>

Either that or fold in the hunk below.


--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -110,7 +110,7 @@ static int __init arm_enable_runtime_services(void)
 {
        u64 mapsize;

-       if (!efi_enabled(EFI_BOOT)) {
+       if (!efi_enabled(EFI_BOOT) || !efi_enabled(EFI_MEMMAP)) {
                pr_info("EFI services will not be available.\n");
                return 0;
        }

[PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[Ard Biesheuvel]

On 12 July 2018 at 15:32, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 10, 2018 at 08:39:16PM +0200, Ard Biesheuvel wrote:
>> On 10 July 2018 at 19:57, James Morse <james.morse@arm.com> wrote:
>> > Hi Ard,
>> >
>> > On 10/07/18 00:42, AKASHI Takahiro wrote:
>> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >>
>> >> The BGRT code validates the contents of the table against the UEFI
>> >> memory map, and so it expects it to be mapped when the code runs.
>> >>
>> >> On ARM, this is currently not the case, since we tear down the early
>> >> mapping after efi_init() completes, and only create the permanent
>> >> mapping in arm_enable_runtime_services(), which executes as an early
>> >> initcall, but still leaves a window where the UEFI memory map is not
>> >> mapped.
>> >>
>> >> So move the call to efi_memmap_unmap() from efi_init() to
>> >> arm_enable_runtime_services().
>> >
>> > I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
>> > call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.
>> >
>>
>> I'm not sure I follow. The BGRT table only contains natively aligned
>> fields, so the alignment faults should not occur when accessing this
>> table after kexec. The issue addressed by this patch is that
>> efi_mem_type() bails when called while EFI_MEMMAP is cleared.
>>
>> >
>> >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
>> >> index b5214c143fee..388a929baf95 100644
>> >> --- a/drivers/firmware/efi/arm-init.c
>> >> +++ b/drivers/firmware/efi/arm-init.c
>> >> @@ -259,7 +259,6 @@ void __init efi_init(void)
>> >>
>> >>       reserve_regions();
>> >>       efi_esrt_init();
>> >> -     efi_memmap_unmap();
>> >>
>> >>       memblock_reserve(params.mmap & PAGE_MASK,
>> >>                        PAGE_ALIGN(params.mmap_size +
>> >> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
>> >> index 5889cbea60b8..59a8c0ec94d5 100644
>> >> --- a/drivers/firmware/efi/arm-runtime.c
>> >> +++ b/drivers/firmware/efi/arm-runtime.c
>> >> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
>> >>               return 0;
>> >>       }
>> >>
>> >> +     efi_memmap_unmap();
>> >
>> > This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
>> > but this can only happen if the system table signature is wrong, (or we're out
>> > of memory really early).
>> >
>>
>> I guess we should check the EFI_MEMMAP attribute here as well then.
>
> Do you plan to spin a new version of this patch?
>

Either that or fold in the hunk below.


--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -110,7 +110,7 @@ static int __init arm_enable_runtime_services(void)
 {
        u64 mapsize;

-       if (!efi_enabled(EFI_BOOT)) {
+       if (!efi_enabled(EFI_BOOT) || !efi_enabled(EFI_MEMMAP)) {
                pr_info("EFI services will not be available.\n");
                return 0;
        }

Re: [PATCH v3.1 2/4] efi/arm: preserve early mapping of UEFI memory map longer for BGRT

[Ard Biesheuvel]

On 12 July 2018 at 15:32, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 10, 2018 at 08:39:16PM +0200, Ard Biesheuvel wrote:
>> On 10 July 2018 at 19:57, James Morse <james.morse@arm.com> wrote:
>> > Hi Ard,
>> >
>> > On 10/07/18 00:42, AKASHI Takahiro wrote:
>> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >>
>> >> The BGRT code validates the contents of the table against the UEFI
>> >> memory map, and so it expects it to be mapped when the code runs.
>> >>
>> >> On ARM, this is currently not the case, since we tear down the early
>> >> mapping after efi_init() completes, and only create the permanent
>> >> mapping in arm_enable_runtime_services(), which executes as an early
>> >> initcall, but still leaves a window where the UEFI memory map is not
>> >> mapped.
>> >>
>> >> So move the call to efi_memmap_unmap() from efi_init() to
>> >> arm_enable_runtime_services().
>> >
>> > I don't have a machine that generates a BGRT, but I can see that efi_mem_type()
>> > call in efi_bgrt_init() would cause the same problems we have with kexec and acpi.
>> >
>>
>> I'm not sure I follow. The BGRT table only contains natively aligned
>> fields, so the alignment faults should not occur when accessing this
>> table after kexec. The issue addressed by this patch is that
>> efi_mem_type() bails when called while EFI_MEMMAP is cleared.
>>
>> >
>> >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
>> >> index b5214c143fee..388a929baf95 100644
>> >> --- a/drivers/firmware/efi/arm-init.c
>> >> +++ b/drivers/firmware/efi/arm-init.c
>> >> @@ -259,7 +259,6 @@ void __init efi_init(void)
>> >>
>> >>       reserve_regions();
>> >>       efi_esrt_init();
>> >> -     efi_memmap_unmap();
>> >>
>> >>       memblock_reserve(params.mmap & PAGE_MASK,
>> >>                        PAGE_ALIGN(params.mmap_size +
>> >> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
>> >> index 5889cbea60b8..59a8c0ec94d5 100644
>> >> --- a/drivers/firmware/efi/arm-runtime.c
>> >> +++ b/drivers/firmware/efi/arm-runtime.c
>> >> @@ -115,6 +115,8 @@ static int __init arm_enable_runtime_services(void)
>> >>               return 0;
>> >>       }
>> >>
>> >> +     efi_memmap_unmap();
>> >
>> > This can get called twice if uefi_init() fails after setting the EFI_BOOT flag,
>> > but this can only happen if the system table signature is wrong, (or we're out
>> > of memory really early).
>> >
>>
>> I guess we should check the EFI_MEMMAP attribute here as well then.
>
> Do you plan to spin a new version of this patch?
>

Either that or fold in the hunk below.


--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -110,7 +110,7 @@ static int __init arm_enable_runtime_services(void)
 {
        u64 mapsize;

-       if (!efi_enabled(EFI_BOOT)) {
+       if (!efi_enabled(EFI_BOOT) || !efi_enabled(EFI_MEMMAP)) {
                pr_info("EFI services will not be available.\n");
                return 0;
        }

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

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Will Deacon]

Hi Akashi,

On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> This patch series is a set of bug fixes to address kexec/kdump
> failures which are sometimes observed on ACPI-only system and reported
> in LAK-ML before.

I tried picking this up, along with Ard's fixup, but I'm seeing a build
failure for allmodconfig:

arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'

I didn't investigate further. Please can you fix this?

Will

[PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Will Deacon]

Hi Akashi,

On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> This patch series is a set of bug fixes to address kexec/kdump
> failures which are sometimes observed on ACPI-only system and reported
> in LAK-ML before.

I tried picking this up, along with Ard's fixup, but I'm seeing a build
failure for allmodconfig:

arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'

I didn't investigate further. Please can you fix this?

Will

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Will Deacon]

Hi Akashi,

On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> This patch series is a set of bug fixes to address kexec/kdump
> failures which are sometimes observed on ACPI-only system and reported
> in LAK-ML before.

I tried picking this up, along with Ard's fixup, but I'm seeing a build
failure for allmodconfig:

arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'

I didn't investigate further. Please can you fix this?

Will

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

[PATCH v3.1 0/4] arm64: kexec, kdump: fix boot failures on acpi-only system

[Richard Ruigrok]

Hi Akashi,

On 7/9/2018 5:42 PM, AKASHI Takahiro wrote:
> This patch series is a set of bug fixes to address kexec/kdump
> failures which are sometimes observed on ACPI-only system and reported
> in LAK-ML before.
>
> In short, the phenomena are:
> 1. kexec'ed kernel can fail to boot because some ACPI table is corrupted
>    by a new kernel (or other data) being loaded into System RAM. Currently
>    kexec may possibly allocate space ignoring such "reserved" regions.
>    We will see no messages after "Bye!"
>
> 2. crash dump (kdump) kernel can fail to boot and get into panic due to
>    an alignment fault when accessing ACPI tables. This can happen because
>    those tables are not always properly aligned while they are mapped
>    non-cacheable (ioremap'ed) as they are not recognized as part of System
>    RAM under the current implementation.
>
> After discussing several possibilities to address those issues,
> the agreed approach, in my understanding, is
> * to add resource entries for every "reserved", i.e. memblock_reserve(),
>   regions to /proc/iomem.
>   (NOMAP regions, also marked as "reserved," remains at top-level for
>   backward compatibility. User-space can tell the difference between
>   reserved-system-ram and reserved-address-space.)
> * For case (1), user space (kexec-tools) should rule out such regions
>   in searching for free space for loaded data.
> * For case (2), the kernel should access ACPI tables by mapping
>   them with appropriate memory attributes described in UEFI memory map.
>   (This means that it doesn't require any changes in /proc/iomem, and
>   hence user space.)
>
> Please find past discussions about /proc/iomem in [1].
> --- more words from James ---
> Our attempts to fix this just in the kernel reached a dead end, because Kdump
> needs to include reserved-system-ram, whereas kexec has to avoid it. User-space
> needs to be able to tell reserved-system-ram and reserved-address-space apart.
> Hence we need to expose that information, and pick it up in user-space.
>
> Patched-kernel and unpatch-user-space will work the same way it does today, as
> the additional reserved regions are ignored by user-space.
>
> Unpatched-kernel and patched-user-space will also work the same way it does
> today as the additional reserved regions are missing.
> --->8---
>
> Patch#1 addresses kexec case, for which you are also required to update
> user space. See necessary patches in [2]. If you want to review Patch#1,
> please also take a look at and review [2].
>
> Patch#2, #3 and #4 address the kdump case above.
>
>
> Changes in v3.1 (2018, July 10, 2018)
> * add Ard's patch[4] to this patch set
>
> Changes in v3 (2018, July 9, 2018)
> * drop the v2's patch#3, preferring [4]
>
> Changes in v2 (2018, June 19, 2018)
> * re-organise v1's patch#2 and #3 into v2's #2, #3 and #4
>   not to break bisect
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/565980.html
> [2] https://git.linaro.org/people/takahiro.akashi/kexec-tools.git arm64/resv_mem
> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-April/573655.html
> [4] https://marc.info/?l=linux-efi&m=152930773507524&w=2
>
> AKASHI Takahiro (2):
>   efi/arm: map UEFI memory map even w/o runtime services enabled
>   arm64: acpi: fix alignment fault in accessing ACPI
>
> Ard Biesheuvel (1):
>   efi/arm: preserve early mapping of UEFI memory map longer for BGRT
>
> James Morse (1):
>   arm64: export memblock_reserve()d regions via /proc/iomem
>
>  arch/arm64/include/asm/acpi.h      | 23 ++++++++++++------
>  arch/arm64/kernel/acpi.c           | 11 +++------
>  arch/arm64/kernel/setup.c          | 38 ++++++++++++++++++++++++++++++
>  drivers/firmware/efi/arm-init.c    |  1 -
>  drivers/firmware/efi/arm-runtime.c | 16 +++++++------
>  5 files changed, 66 insertions(+), 23 deletions(-)
>
Please add my tested-by <rruigrok@codeaurora.org>

Tested this patch series on top of v4.17 on Centriq ARM64 system with the kexec-tool as specified : https://git.linaro.org/people/takahiro.akashi/kexec-tools.git? branch arm64/resv_mem
Prior to this patch set and Kexec tool update, we were seeing the ESRT region being over-written, it showed as a mangled region in a kexec'ed kernel.
- ex: [??? 0.000000] esrt: Unsupported ESRT version 5681331550117508106.
With this patchset and tool upadate, the ESRT region appears correct
[??? 0.000000] esrt: Reserving ESRT space from 0x000000000b6c8d18 to 0x000000000b6c8df0.

Thanks for fixing this!
Richard

-- 

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 v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[AKASHI Takahiro]

On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
> Hi Akashi,
> 
> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> > This patch series is a set of bug fixes to address kexec/kdump
> > failures which are sometimes observed on ACPI-only system and reported
> > in LAK-ML before.
> 
> I tried picking this up, along with Ard's fixup, but I'm seeing a build
> failure for allmodconfig:
> 
> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
> 
> I didn't investigate further. Please can you fix this?

Because CONFIG_ACPI is on and CONFIG_EFI is off.

This can happen in allmodconfig as CONFIG_EFI depends on
!CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.

Looking at __acpi_get_mem_attributes(), since there is no information
available on memory attributes, what we can do at best is
  * return PAGE_KERNEL (= cacheable) for mapped memory,
  * return DEVICE_nGnRnE (= non-cacheable) otherwise
(See a hunk to be applied on top of my patch#4.)

I think that, after applying, acpi_os_ioremap() would work almost
in the same way as the original before my patchset given that
MAP memblock attribute is used only under CONFIG_EFI for now.

Make sense?

-Takahiro AKASHI

> Will
---8<---
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index ed46dc188b22..cad3ed2666ef 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -238,6 +238,7 @@ void __init acpi_boot_table_init(void)
 
 pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 {
+#ifdef CONFIG_EFI
 	/*
 	 * According to "Table 8 Map: EFI memory types to AArch64 memory
 	 * types" of UEFI 2.5 section 2.3.6.1, each EFI memory type is
@@ -255,5 +256,9 @@ pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_WT);
 	if (attr & EFI_MEMORY_WC)
 		return __pgprot(PROT_NORMAL_NC);
+#else
+	if (memblock_is_map_memory(addr))
+		return PAGE_KERNEL;
+#endif
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }

[PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[AKASHI Takahiro]

On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
> Hi Akashi,
> 
> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> > This patch series is a set of bug fixes to address kexec/kdump
> > failures which are sometimes observed on ACPI-only system and reported
> > in LAK-ML before.
> 
> I tried picking this up, along with Ard's fixup, but I'm seeing a build
> failure for allmodconfig:
> 
> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
> 
> I didn't investigate further. Please can you fix this?

Because CONFIG_ACPI is on and CONFIG_EFI is off.

This can happen in allmodconfig as CONFIG_EFI depends on
!CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.

Looking at __acpi_get_mem_attributes(), since there is no information
available on memory attributes, what we can do at best is
  * return PAGE_KERNEL (= cacheable) for mapped memory,
  * return DEVICE_nGnRnE (= non-cacheable) otherwise
(See a hunk to be applied on top of my patch#4.)

I think that, after applying, acpi_os_ioremap() would work almost
in the same way as the original before my patchset given that
MAP memblock attribute is used only under CONFIG_EFI for now.

Make sense?

-Takahiro AKASHI

> Will
---8<---
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index ed46dc188b22..cad3ed2666ef 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -238,6 +238,7 @@ void __init acpi_boot_table_init(void)
 
 pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 {
+#ifdef CONFIG_EFI
 	/*
 	 * According to "Table 8 Map: EFI memory types to AArch64 memory
 	 * types" of UEFI 2.5 section 2.3.6.1, each EFI memory type is
@@ -255,5 +256,9 @@ pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_WT);
 	if (attr & EFI_MEMORY_WC)
 		return __pgprot(PROT_NORMAL_NC);
+#else
+	if (memblock_is_map_memory(addr))
+		return PAGE_KERNEL;
+#endif
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[AKASHI Takahiro]

On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
> Hi Akashi,
> 
> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> > This patch series is a set of bug fixes to address kexec/kdump
> > failures which are sometimes observed on ACPI-only system and reported
> > in LAK-ML before.
> 
> I tried picking this up, along with Ard's fixup, but I'm seeing a build
> failure for allmodconfig:
> 
> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
> 
> I didn't investigate further. Please can you fix this?

Because CONFIG_ACPI is on and CONFIG_EFI is off.

This can happen in allmodconfig as CONFIG_EFI depends on
!CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.

Looking at __acpi_get_mem_attributes(), since there is no information
available on memory attributes, what we can do at best is
  * return PAGE_KERNEL (= cacheable) for mapped memory,
  * return DEVICE_nGnRnE (= non-cacheable) otherwise
(See a hunk to be applied on top of my patch#4.)

I think that, after applying, acpi_os_ioremap() would work almost
in the same way as the original before my patchset given that
MAP memblock attribute is used only under CONFIG_EFI for now.

Make sense?

-Takahiro AKASHI

> Will
---8<---
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index ed46dc188b22..cad3ed2666ef 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -238,6 +238,7 @@ void __init acpi_boot_table_init(void)
 
 pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 {
+#ifdef CONFIG_EFI
 	/*
 	 * According to "Table 8 Map: EFI memory types to AArch64 memory
 	 * types" of UEFI 2.5 section 2.3.6.1, each EFI memory type is
@@ -255,5 +256,9 @@ pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
 		return __pgprot(PROT_NORMAL_WT);
 	if (attr & EFI_MEMORY_WC)
 		return __pgprot(PROT_NORMAL_NC);
+#else
+	if (memblock_is_map_memory(addr))
+		return PAGE_KERNEL;
+#endif
 	return __pgprot(PROT_DEVICE_nGnRnE);
 }

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

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Ard Biesheuvel]

On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
>> Hi Akashi,
>>
>> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
>> > This patch series is a set of bug fixes to address kexec/kdump
>> > failures which are sometimes observed on ACPI-only system and reported
>> > in LAK-ML before.
>>
>> I tried picking this up, along with Ard's fixup, but I'm seeing a build
>> failure for allmodconfig:
>>
>> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
>> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
>>
>> I didn't investigate further. Please can you fix this?
>
> Because CONFIG_ACPI is on and CONFIG_EFI is off.
>
> This can happen in allmodconfig as CONFIG_EFI depends on
> !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
>

Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
makes no sense at all. Things will surely break if you start using BE
memory accesses while parsing ACPI tables.

Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
on arm64, the only way to find the ACPI tables is through a UEFI
configuration table.

> Looking at __acpi_get_mem_attributes(), since there is no information
> available on memory attributes, what we can do at best is
>   * return PAGE_KERNEL (= cacheable) for mapped memory,
>   * return DEVICE_nGnRnE (= non-cacheable) otherwise
> (See a hunk to be applied on top of my patch#4.)
>
> I think that, after applying, acpi_os_ioremap() would work almost
> in the same way as the original before my patchset given that
> MAP memblock attribute is used only under CONFIG_EFI for now.
>
> Make sense?
>

Let's keep your code as is but fix the Kconfig dependencies instead.

[PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Ard Biesheuvel]

On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
>> Hi Akashi,
>>
>> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
>> > This patch series is a set of bug fixes to address kexec/kdump
>> > failures which are sometimes observed on ACPI-only system and reported
>> > in LAK-ML before.
>>
>> I tried picking this up, along with Ard's fixup, but I'm seeing a build
>> failure for allmodconfig:
>>
>> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
>> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
>>
>> I didn't investigate further. Please can you fix this?
>
> Because CONFIG_ACPI is on and CONFIG_EFI is off.
>
> This can happen in allmodconfig as CONFIG_EFI depends on
> !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
>

Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
makes no sense at all. Things will surely break if you start using BE
memory accesses while parsing ACPI tables.

Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
on arm64, the only way to find the ACPI tables is through a UEFI
configuration table.

> Looking at __acpi_get_mem_attributes(), since there is no information
> available on memory attributes, what we can do at best is
>   * return PAGE_KERNEL (= cacheable) for mapped memory,
>   * return DEVICE_nGnRnE (= non-cacheable) otherwise
> (See a hunk to be applied on top of my patch#4.)
>
> I think that, after applying, acpi_os_ioremap() would work almost
> in the same way as the original before my patchset given that
> MAP memblock attribute is used only under CONFIG_EFI for now.
>
> Make sense?
>

Let's keep your code as is but fix the Kconfig dependencies instead.

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Ard Biesheuvel]

On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
>> Hi Akashi,
>>
>> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
>> > This patch series is a set of bug fixes to address kexec/kdump
>> > failures which are sometimes observed on ACPI-only system and reported
>> > in LAK-ML before.
>>
>> I tried picking this up, along with Ard's fixup, but I'm seeing a build
>> failure for allmodconfig:
>>
>> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
>> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
>>
>> I didn't investigate further. Please can you fix this?
>
> Because CONFIG_ACPI is on and CONFIG_EFI is off.
>
> This can happen in allmodconfig as CONFIG_EFI depends on
> !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
>

Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
makes no sense at all. Things will surely break if you start using BE
memory accesses while parsing ACPI tables.

Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
on arm64, the only way to find the ACPI tables is through a UEFI
configuration table.

> Looking at __acpi_get_mem_attributes(), since there is no information
> available on memory attributes, what we can do at best is
>   * return PAGE_KERNEL (= cacheable) for mapped memory,
>   * return DEVICE_nGnRnE (= non-cacheable) otherwise
> (See a hunk to be applied on top of my patch#4.)
>
> I think that, after applying, acpi_os_ioremap() would work almost
> in the same way as the original before my patchset given that
> MAP memblock attribute is used only under CONFIG_EFI for now.
>
> Make sense?
>

Let's keep your code as is but fix the Kconfig dependencies instead.

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

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[AKASHI Takahiro]

Will,

On Fri, Jul 13, 2018 at 07:49:45AM +0200, Ard Biesheuvel wrote:
> On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
> >> Hi Akashi,
> >>
> >> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> >> > This patch series is a set of bug fixes to address kexec/kdump
> >> > failures which are sometimes observed on ACPI-only system and reported
> >> > in LAK-ML before.
> >>
> >> I tried picking this up, along with Ard's fixup, but I'm seeing a build
> >> failure for allmodconfig:
> >>
> >> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
> >> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
> >>
> >> I didn't investigate further. Please can you fix this?
> >
> > Because CONFIG_ACPI is on and CONFIG_EFI is off.
> >
> > This can happen in allmodconfig as CONFIG_EFI depends on
> > !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
> >
> 
> Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
> makes no sense at all. Things will surely break if you start using BE
> memory accesses while parsing ACPI tables.
> 
> Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
> on arm64, the only way to find the ACPI tables is through a UEFI
> configuration table.


Do you agree to this?

-Takahiro AKASHI

> > Looking at __acpi_get_mem_attributes(), since there is no information
> > available on memory attributes, what we can do at best is
> >   * return PAGE_KERNEL (= cacheable) for mapped memory,
> >   * return DEVICE_nGnRnE (= non-cacheable) otherwise
> > (See a hunk to be applied on top of my patch#4.)
> >
> > I think that, after applying, acpi_os_ioremap() would work almost
> > in the same way as the original before my patchset given that
> > MAP memblock attribute is used only under CONFIG_EFI for now.
> >
> > Make sense?
> >
> 
> Let's keep your code as is but fix the Kconfig dependencies instead.

[PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[AKASHI Takahiro]

Will,

On Fri, Jul 13, 2018 at 07:49:45AM +0200, Ard Biesheuvel wrote:
> On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
> >> Hi Akashi,
> >>
> >> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> >> > This patch series is a set of bug fixes to address kexec/kdump
> >> > failures which are sometimes observed on ACPI-only system and reported
> >> > in LAK-ML before.
> >>
> >> I tried picking this up, along with Ard's fixup, but I'm seeing a build
> >> failure for allmodconfig:
> >>
> >> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
> >> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
> >>
> >> I didn't investigate further. Please can you fix this?
> >
> > Because CONFIG_ACPI is on and CONFIG_EFI is off.
> >
> > This can happen in allmodconfig as CONFIG_EFI depends on
> > !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
> >
> 
> Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
> makes no sense at all. Things will surely break if you start using BE
> memory accesses while parsing ACPI tables.
> 
> Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
> on arm64, the only way to find the ACPI tables is through a UEFI
> configuration table.


Do you agree to this?

-Takahiro AKASHI

> > Looking at __acpi_get_mem_attributes(), since there is no information
> > available on memory attributes, what we can do at best is
> >   * return PAGE_KERNEL (= cacheable) for mapped memory,
> >   * return DEVICE_nGnRnE (= non-cacheable) otherwise
> > (See a hunk to be applied on top of my patch#4.)
> >
> > I think that, after applying, acpi_os_ioremap() would work almost
> > in the same way as the original before my patchset given that
> > MAP memblock attribute is used only under CONFIG_EFI for now.
> >
> > Make sense?
> >
> 
> Let's keep your code as is but fix the Kconfig dependencies instead.

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[AKASHI Takahiro]

Will,

On Fri, Jul 13, 2018 at 07:49:45AM +0200, Ard Biesheuvel wrote:
> On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
> >> Hi Akashi,
> >>
> >> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> >> > This patch series is a set of bug fixes to address kexec/kdump
> >> > failures which are sometimes observed on ACPI-only system and reported
> >> > in LAK-ML before.
> >>
> >> I tried picking this up, along with Ard's fixup, but I'm seeing a build
> >> failure for allmodconfig:
> >>
> >> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
> >> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
> >>
> >> I didn't investigate further. Please can you fix this?
> >
> > Because CONFIG_ACPI is on and CONFIG_EFI is off.
> >
> > This can happen in allmodconfig as CONFIG_EFI depends on
> > !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
> >
> 
> Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
> makes no sense at all. Things will surely break if you start using BE
> memory accesses while parsing ACPI tables.
> 
> Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
> on arm64, the only way to find the ACPI tables is through a UEFI
> configuration table.


Do you agree to this?

-Takahiro AKASHI

> > Looking at __acpi_get_mem_attributes(), since there is no information
> > available on memory attributes, what we can do at best is
> >   * return PAGE_KERNEL (= cacheable) for mapped memory,
> >   * return DEVICE_nGnRnE (= non-cacheable) otherwise
> > (See a hunk to be applied on top of my patch#4.)
> >
> > I think that, after applying, acpi_os_ioremap() would work almost
> > in the same way as the original before my patchset given that
> > MAP memblock attribute is used only under CONFIG_EFI for now.
> >
> > Make sense?
> >
> 
> Let's keep your code as is but fix the Kconfig dependencies instead.

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

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Will Deacon]

On Tue, Jul 17, 2018 at 02:12:23PM +0900, AKASHI Takahiro wrote:
> On Fri, Jul 13, 2018 at 07:49:45AM +0200, Ard Biesheuvel wrote:
> > On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
> > >> Hi Akashi,
> > >>
> > >> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> > >> > This patch series is a set of bug fixes to address kexec/kdump
> > >> > failures which are sometimes observed on ACPI-only system and reported
> > >> > in LAK-ML before.
> > >>
> > >> I tried picking this up, along with Ard's fixup, but I'm seeing a build
> > >> failure for allmodconfig:
> > >>
> > >> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
> > >> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
> > >>
> > >> I didn't investigate further. Please can you fix this?
> > >
> > > Because CONFIG_ACPI is on and CONFIG_EFI is off.
> > >
> > > This can happen in allmodconfig as CONFIG_EFI depends on
> > > !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
> > >
> > 
> > Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
> > makes no sense at all. Things will surely break if you start using BE
> > memory accesses while parsing ACPI tables.
> > 
> > Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
> > on arm64, the only way to find the ACPI tables is through a UEFI
> > configuration table.
> 
> 
> Do you agree to this?

Yes; please post a new series which resolves these dependencies and includes
Ard's fixup from before.

Thanks,

Will

[PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Will Deacon]

On Tue, Jul 17, 2018 at 02:12:23PM +0900, AKASHI Takahiro wrote:
> On Fri, Jul 13, 2018 at 07:49:45AM +0200, Ard Biesheuvel wrote:
> > On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
> > >> Hi Akashi,
> > >>
> > >> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> > >> > This patch series is a set of bug fixes to address kexec/kdump
> > >> > failures which are sometimes observed on ACPI-only system and reported
> > >> > in LAK-ML before.
> > >>
> > >> I tried picking this up, along with Ard's fixup, but I'm seeing a build
> > >> failure for allmodconfig:
> > >>
> > >> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
> > >> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
> > >>
> > >> I didn't investigate further. Please can you fix this?
> > >
> > > Because CONFIG_ACPI is on and CONFIG_EFI is off.
> > >
> > > This can happen in allmodconfig as CONFIG_EFI depends on
> > > !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
> > >
> > 
> > Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
> > makes no sense at all. Things will surely break if you start using BE
> > memory accesses while parsing ACPI tables.
> > 
> > Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
> > on arm64, the only way to find the ACPI tables is through a UEFI
> > configuration table.
> 
> 
> Do you agree to this?

Yes; please post a new series which resolves these dependencies and includes
Ard's fixup from before.

Thanks,

Will

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Will Deacon]

On Tue, Jul 17, 2018 at 02:12:23PM +0900, AKASHI Takahiro wrote:
> On Fri, Jul 13, 2018 at 07:49:45AM +0200, Ard Biesheuvel wrote:
> > On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
> > >> Hi Akashi,
> > >>
> > >> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
> > >> > This patch series is a set of bug fixes to address kexec/kdump
> > >> > failures which are sometimes observed on ACPI-only system and reported
> > >> > in LAK-ML before.
> > >>
> > >> I tried picking this up, along with Ard's fixup, but I'm seeing a build
> > >> failure for allmodconfig:
> > >>
> > >> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
> > >> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
> > >>
> > >> I didn't investigate further. Please can you fix this?
> > >
> > > Because CONFIG_ACPI is on and CONFIG_EFI is off.
> > >
> > > This can happen in allmodconfig as CONFIG_EFI depends on
> > > !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
> > >
> > 
> > Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
> > makes no sense at all. Things will surely break if you start using BE
> > memory accesses while parsing ACPI tables.
> > 
> > Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
> > on arm64, the only way to find the ACPI tables is through a UEFI
> > configuration table.
> 
> 
> Do you agree to this?

Yes; please post a new series which resolves these dependencies and includes
Ard's fixup from before.

Thanks,

Will

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

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Bhupesh Sharma]

Hi Will,

On Mon, Jul 23, 2018 at 7:05 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 17, 2018 at 02:12:23PM +0900, AKASHI Takahiro wrote:
>> On Fri, Jul 13, 2018 at 07:49:45AM +0200, Ard Biesheuvel wrote:
>> > On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>> > > On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
>> > >> Hi Akashi,
>> > >>
>> > >> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
>> > >> > This patch series is a set of bug fixes to address kexec/kdump
>> > >> > failures which are sometimes observed on ACPI-only system and reported
>> > >> > in LAK-ML before.
>> > >>
>> > >> I tried picking this up, along with Ard's fixup, but I'm seeing a build
>> > >> failure for allmodconfig:
>> > >>
>> > >> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
>> > >> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
>> > >>
>> > >> I didn't investigate further. Please can you fix this?
>> > >
>> > > Because CONFIG_ACPI is on and CONFIG_EFI is off.
>> > >
>> > > This can happen in allmodconfig as CONFIG_EFI depends on
>> > > !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
>> > >
>> >
>> > Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
>> > makes no sense at all. Things will surely break if you start using BE
>> > memory accesses while parsing ACPI tables.
>> >
>> > Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
>> > on arm64, the only way to find the ACPI tables is through a UEFI
>> > configuration table.
>>
>>
>> Do you agree to this?
>
> Yes; please post a new series which resolves these dependencies and includes
> Ard's fixup from before.

I see that Akashi has already posted a v4 here with the suggested fixes:
https://lkml.org/lkml/2018/7/22/321

Thanks,
Bhupesh

[PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Bhupesh Sharma]

Hi Will,

On Mon, Jul 23, 2018 at 7:05 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 17, 2018 at 02:12:23PM +0900, AKASHI Takahiro wrote:
>> On Fri, Jul 13, 2018 at 07:49:45AM +0200, Ard Biesheuvel wrote:
>> > On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>> > > On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
>> > >> Hi Akashi,
>> > >>
>> > >> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
>> > >> > This patch series is a set of bug fixes to address kexec/kdump
>> > >> > failures which are sometimes observed on ACPI-only system and reported
>> > >> > in LAK-ML before.
>> > >>
>> > >> I tried picking this up, along with Ard's fixup, but I'm seeing a build
>> > >> failure for allmodconfig:
>> > >>
>> > >> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
>> > >> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
>> > >>
>> > >> I didn't investigate further. Please can you fix this?
>> > >
>> > > Because CONFIG_ACPI is on and CONFIG_EFI is off.
>> > >
>> > > This can happen in allmodconfig as CONFIG_EFI depends on
>> > > !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
>> > >
>> >
>> > Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
>> > makes no sense at all. Things will surely break if you start using BE
>> > memory accesses while parsing ACPI tables.
>> >
>> > Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
>> > on arm64, the only way to find the ACPI tables is through a UEFI
>> > configuration table.
>>
>>
>> Do you agree to this?
>
> Yes; please post a new series which resolves these dependencies and includes
> Ard's fixup from before.

I see that Akashi has already posted a v4 here with the suggested fixes:
https://lkml.org/lkml/2018/7/22/321

Thanks,
Bhupesh

Re: [PATCH v3.1 0/4] arm64: kexec,kdump: fix boot failures on acpi-only system

[Bhupesh Sharma]

Hi Will,

On Mon, Jul 23, 2018 at 7:05 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 17, 2018 at 02:12:23PM +0900, AKASHI Takahiro wrote:
>> On Fri, Jul 13, 2018 at 07:49:45AM +0200, Ard Biesheuvel wrote:
>> > On 13 July 2018 at 02:34, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>> > > On Thu, Jul 12, 2018 at 05:49:19PM +0100, Will Deacon wrote:
>> > >> Hi Akashi,
>> > >>
>> > >> On Tue, Jul 10, 2018 at 08:42:25AM +0900, AKASHI Takahiro wrote:
>> > >> > This patch series is a set of bug fixes to address kexec/kdump
>> > >> > failures which are sometimes observed on ACPI-only system and reported
>> > >> > in LAK-ML before.
>> > >>
>> > >> I tried picking this up, along with Ard's fixup, but I'm seeing a build
>> > >> failure for allmodconfig:
>> > >>
>> > >> arch/arm64/kernel/acpi.o: In function `__acpi_get_mem_attribute':
>> > >> acpi.c:(.text+0x60): undefined reference to `efi_mem_attributes'
>> > >>
>> > >> I didn't investigate further. Please can you fix this?
>> > >
>> > > Because CONFIG_ACPI is on and CONFIG_EFI is off.
>> > >
>> > > This can happen in allmodconfig as CONFIG_EFI depends on
>> > > !CONFIG_CPU_BIG_ENDIAN, which is actually on in this case.
>> > >
>> >
>> > Allowing both CONFIG_ACPI and CONFIG_CPU_BIG_ENDIAN to be configured
>> > makes no sense at all. Things will surely break if you start using BE
>> > memory accesses while parsing ACPI tables.
>> >
>> > Allowing CONFIG_ACPI without CONFIG_EFI makes no sense either, since
>> > on arm64, the only way to find the ACPI tables is through a UEFI
>> > configuration table.
>>
>>
>> Do you agree to this?
>
> Yes; please post a new series which resolves these dependencies and includes
> Ard's fixup from before.

I see that Akashi has already posted a v4 here with the suggested fixes:
https://lkml.org/lkml/2018/7/22/321

Thanks,
Bhupesh

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