Tyler, Jeffrey, On Fri, Mar 02, 2018 at 08:27:11AM -0500, Tyler Baicar wrote: > On 3/2/2018 12:53 AM, AKASHI Takahiro wrote: > >Tyler, Jeffrey, > > > >[Note: This issue takes place in kexec, not kdump. So to be precise, > >it is not the same phenomenon as what I addressed in [1],[2]: > > [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-February/557254.html > > [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-January/553098.html > >] > > > >On Thu, Mar 01, 2018 at 12:56:38PM -0500, Tyler Baicar wrote: > >>Hello, > >> > >>On 2/28/2018 9:50 PM, AKASHI Takahiro wrote: > >>>Hi, > >>> > >>>On Wed, Feb 28, 2018 at 08:39:42AM -0700, Jeffrey Hugo wrote: > >>>>On 2/27/2018 11:19 PM, AKASHI Takahiro wrote: > >>>>>Tyler, > >>>>> > >>>>># I missed catching your patch as its subject doesn't contain arm64. > >>>>> > >>>>>On Fri, Feb 23, 2018 at 12:42:31PM -0700, Tyler Baicar wrote: > >>>>>>Currently on arm64 ESRT memory does not appear to be properly blocked off. > >>>>>>Upon successful initialization, ESRT prints out the memory region that it > >>>>>>exists in like: > >>>>>> > >>>>>>esrt: Reserving ESRT space from 0x000000000a4c1c18 to 0x000000000a4c1cf0. > >>>>>> > >>>>>>But then by dumping /proc/iomem this region appears as part of System RAM > >>>>>>rather than being reserved: > >>>>>> > >>>>>>08f10000-0deeffff : System RAM > >>>>>> > >>>>>>This causes issues when trying to kexec if the kernel is relocatable. When > >>>>>>kexec tries to execute, this memory can be selected to relocate the kernel to > >>>>>>which then overwrites all the ESRT information. Then when the kexec'd kernel > >>>>>>tries to initialize ESRT, it doesn't recognize the ESRT version number and > >>>>>>just returns from efi_esrt_init(). > >>>>>I'm not sure what is the root cause of your problem. > >>>>>Do you have good confidence that the kernel (2nd kernel image in this case?) > >>>>>really overwrite ESRT region? > >>>>According to my debug, yes. > >>>>Using JTAG, I was able to determine that the ESRT memory region was getting > >>>>overwritten by the secondary kernel in > >>>>kernel/arch/arm64/kernel/relocate_kernel.S - specifically the "copy_page" > >>>>line of arm64_relocate_new_kernel() > >>>> > >>>>>To my best knowledge, kexec is carefully designed not to do such a thing > >>>>>as it allocates a temporary buffer for kernel image and copies it to the > >>>>>final destination at the very end of the 1st kernel. > >>>>> > >>>>>My guess is that kexec, or rather kexec-tools, tries to load the kernel image > >>>>>at 0x8f80000 (or 0x9080000?, not sure) in your case. It may or may not be > >>>>>overlapped with ESRT. > >>>>>(Try "-d" option when executing kexec command for confirmation.) > >>>>With -d, I see > >>>> > >>>>get_memory_ranges_iomem_cb: 0000000009611000 - 000000000e5fffff : System RAM > >>>> > >>>>That overlaps the ESRT reservation - > >>>>[ 0.000000] esrt: Reserving ESRT space from 0x000000000b708718 to > >>>>0x000000000b7087f0 > >>>> > >>>>>Are you using initrd with kexec? > >>>>Yes > >>>To make the things clear, can you show me, if possible, the followings: > >>I have attached all of these: > >Many thanks. > >According to the data, ESRT was overwritten by initrd, not the kernel image. > >It doesn't matter to you though :) > > > >The solution would be, as Ard suggested, that more information be > >added to /proc/iomem. > >I'm going to fix the issue as quickly as possible. > Great, thank you!! Please add us to the fix and we will gladly test it out. I have created a workaround patch, attached below, as a kind of PoC. Can you give it a go, please? You need another patch for kexec-tools, too. See https:/git.linaro.org/people/takahiro.akashi/kexecl-tools.git arm64/resv_mem With this patch, extra entries for firmware-reserved memory resources, which means any regions that are already reserved before arm64_memblock_init(), or specifically efi/acpi tables in this case, are added to /proc/iomem. $ cat /proc/iomem (on my qemu+edk2 execution) ... 40000000-5871ffff : System RAM 40080000-40f1ffff : Kernel code 41040000-411e9fff : Kernel data 54400000-583fffff : Crash kernel 58590000-585effff : reserved 58700000-5871ffff : reserved 58720000-58b5ffff : reserved 58b60000-5be3ffff : System RAM 58b61000-58b61fff : reserved 59a7b118-59a7b667 : reserved 5be40000-5becffff : reserved 5bed0000-5bedffff : System RAM 5bee0000-5bffffff : reserved 5c000000-5fffffff : System RAM 5ec00000-5edfffff : reserved 8000000000-ffffffffff : PCI Bus 0000:00 8000000000-8000003fff : 0000:00:01.0 8000000000-8000003fff : virtio-pci-modern While all the entries are currently marked as just "reserved," we'd better give them more specific names for general/extensive use. (Then it will require modifying respective fw/drivers.) Kexec-tools will allocate spaces for kernel, initrd and dtb so that they will not be overlapped with "reserved" memory. As I haven't run extensive tests, please let me know if you find any problems. Thanks, -Takahiro AKASHI > > Thanks, > Tyler > > -- > Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. > Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project. > ===8<=== >From 57d93b89d16b967c913f3949601a5559ddf4aa57 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro <takahiro.akashi@xxxxxxxxxx> Date: Fri, 2 Mar 2018 16:39:18 +0900 Subject: [PATCH] arm64: kexec: set asdie firmware-reserved memory regions Signed-off-by: AKASHI Takahiro <takahiro.akashi@xxxxxxxxxx> --- arch/arm64/kernel/setup.c | 24 ++++++++++++++++++++---- arch/arm64/mm/init.c | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 30ad2f085d1f..997f07e86243 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -87,6 +87,9 @@ static struct resource mem_res[] = { #define kernel_code mem_res[0] #define kernel_data mem_res[1] +/* TODO: Firmware-reserved memory resources */ +extern struct memblock_type fw_mem; + /* * The recorded values of x0 .. x3 upon kernel entry. */ @@ -206,7 +209,20 @@ static void __init request_standard_resources(void) { struct memblock_region *region; struct resource *res; + int i; + + /* add firmware-reserved memory first */ + for (i = 1; i < fw_mem.cnt; i++) { + res = alloc_bootmem_low(sizeof(*res)); + res->name = "reserved"; + res->flags = IORESOURCE_MEM; + res->start = fw_mem.regions[i].base; + res->end = fw_mem.regions[i].base + fw_mem.regions[i].size - 1; + request_resource(&iomem_resource, res); + } + + /* add standard resources */ kernel_code.start = __pa_symbol(_text); kernel_code.end = __pa_symbol(__init_begin - 1); kernel_data.start = __pa_symbol(_sdata); @@ -224,19 +240,19 @@ static void __init request_standard_resources(void) res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region)); res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1; - request_resource(&iomem_resource, res); + insert_resource(&iomem_resource, res); if (kernel_code.start >= res->start && kernel_code.end <= res->end) - request_resource(res, &kernel_code); + insert_resource(res, &kernel_code); if (kernel_data.start >= res->start && kernel_data.end <= res->end) - request_resource(res, &kernel_data); + insert_resource(res, &kernel_data); #ifdef CONFIG_KEXEC_CORE /* Userspace will find "Crash kernel" region in /proc/iomem. */ if (crashk_res.end && crashk_res.start >= res->start && crashk_res.end <= res->end) - request_resource(res, &crashk_res); + insert_resource(res, &crashk_res); #endif } } diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 9f3c47acf8ff..b6f86a7bbfb7 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -62,6 +62,14 @@ s64 memstart_addr __ro_after_init = -1; phys_addr_t arm64_dma_phys_limit __ro_after_init; +static struct memblock_region fw_mem_regions[INIT_MEMBLOCK_REGIONS]; +struct memblock_type fw_mem = { + .regions = fw_mem_regions, + .cnt = 1, /* empty dummy entry */ + .max = INIT_MEMBLOCK_REGIONS, + .name = "firmware-reserved memory", +}; + #ifdef CONFIG_BLK_DEV_INITRD static int __init early_initrd(char *p) { @@ -362,6 +370,19 @@ static void __init fdt_enforce_memory_region(void) void __init arm64_memblock_init(void) { const s64 linear_region_size = -(s64)PAGE_OFFSET; + struct memblock_region *region; + + /* + * Export firmware-reserved memory regions + * TODO: via more generic interface + */ + for_each_memblock(reserved, region) { + if (WARN_ON(fw_mem.cnt >= fw_mem.max)) + break; + fw_mem.regions[fw_mem.cnt].base = region->base; + fw_mem.regions[fw_mem.cnt].size = region->size; + fw_mem.cnt++; + } /* Handle linux,usable-memory-range property */ fdt_enforce_memory_region(); -- 2.16.2 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
