Re: [PATCH v17 00/15] arm64: MMU enabled kexec relocation
> FWIW, I'm fine with this series. I think the only concern raised by > James and not addressed is the possibility of the failure of the memory > allocation for copying the page tables during kexec. If anyone will > complain in the future, we can look into adding a fallback mechanism to > do the relocation with the MMU off. > > Acked-by: Catalin Marinas Thank you. Pasha ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 00/15] arm64: MMU enabled kexec relocation
On Wed, Sep 29, 2021 at 8:49 AM Will Deacon wrote: > > On Thu, Sep 16, 2021 at 07:13:10PM -0400, Pasha Tatashin wrote: > > Changelog: > > v17: > > - Merged with 5.15-rc1 as requested by Catalin Marinas > > - Added Tested-by: Pingfan Liu > > This looks pretty good to me. I've left some minor comments on a few of the > patches, but other than those I'd like to try getting these queued for > 5.16. Thanks Will, I will send the updated series tomorrow. Pasha ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 15/15] arm64: trans_pgd: remove trans_pgd_map_page()
On Wed, Sep 29, 2021 at 12:43 PM Catalin Marinas
wrote:
>
> On Thu, Sep 16, 2021 at 07:13:25PM -0400, Pasha Tatashin wrote:
> > The intend of trans_pgd_map_page() was to map contiguous range of VA
> > memory to the memory that is getting relocated during kexec. However,
> > since we are now using linear map instead of contiguous range this
> > function is not needed
> >
> > Suggested-by: Pingfan Liu
> > Signed-off-by: Pasha Tatashin
> > ---
> > arch/arm64/include/asm/trans_pgd.h | 5 +--
> > arch/arm64/mm/trans_pgd.c | 57 --
> > 2 files changed, 1 insertion(+), 61 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/trans_pgd.h
> > b/arch/arm64/include/asm/trans_pgd.h
> > index 7b04d32b102c..033d400a4ea4 100644
> > --- a/arch/arm64/include/asm/trans_pgd.h
> > +++ b/arch/arm64/include/asm/trans_pgd.h
> > @@ -15,7 +15,7 @@
> > /*
> > * trans_alloc_page
> > * - Allocator that should return exactly one zeroed page, if this
> > - * allocator fails, trans_pgd_create_copy() and trans_pgd_map_page()
> > + * allocator fails, trans_pgd_create_copy() and trans_pgd_idmap_page()
> > * return -ENOMEM error.
> > *
> > * trans_alloc_arg
> > @@ -30,9 +30,6 @@ struct trans_pgd_info {
> > int trans_pgd_create_copy(struct trans_pgd_info *info, pgd_t **trans_pgd,
> > unsigned long start, unsigned long end);
> >
> > -int trans_pgd_map_page(struct trans_pgd_info *info, pgd_t *trans_pgd,
> > -void *page, unsigned long dst_addr, pgprot_t pgprot);
>
> So this function never got used in mainline after commit 7018d467ff2d
> ("arm64: trans_pgd: hibernate: idmap the single page that holds the copy
> page routines"). I guess it's because we merged part of v10 of this
> series and in v12 you dropped the contiguous VA range in favour of a
> copy of the linear map.
This is exactly right. This function was meant for the contiguous-va
relocation version of this series.
Pasha
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 10/15] arm64: kexec: use ld script for relocation function
Sorry, missed two comments: > > /* Flush the reloc_code in preparation for its execution. */ > > dcache_clean_inval_poc((unsigned long)reloc_code, > > -(unsigned long)reloc_code + > > -arm64_relocate_new_kernel_size); > > +(unsigned long)reloc_code + reloc_size); > > Extra whitespace. Yeap, extra whitespace after '+', will fix it :) > > > icache_inval_pou((uintptr_t)reloc_code, > > - (uintptr_t)reloc_code + > > - arm64_relocate_new_kernel_size); > > + (uintptr_t)reloc_code + reloc_size); > > kexec_list_flush(kimage); > > kexec_image_info(kimage); > > > > diff --git a/arch/arm64/kernel/relocate_kernel.S > > b/arch/arm64/kernel/relocate_kernel.S > > index b4fb97312a80..9d2400855ee4 100644 > > --- a/arch/arm64/kernel/relocate_kernel.S > > +++ b/arch/arm64/kernel/relocate_kernel.S > > @@ -15,6 +15,7 @@ > > #include > > #include > > > > +.pushsection".kexec_relocate.text", "ax" > > Just use .section if you're putting the entire file in there? Good point, I will change it to .section. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 10/15] arm64: kexec: use ld script for relocation function
On Wed, Sep 29, 2021 at 8:45 AM Will Deacon wrote: > > On Thu, Sep 16, 2021 at 07:13:20PM -0400, Pasha Tatashin wrote: > > Currently, relocation code declares start and end variables > > which are used to compute its size. > > > > The better way to do this is to use ld script incited, and put relocation > > function in its own section. > > "incited"? I don't understand ... I will correct it: s/incited// > > +#ifdef CONFIG_KEXEC_CORE > > +/* kexec relocation code should fit into one KEXEC_CONTROL_PAGE_SIZE */ > > +ASSERT(__relocate_new_kernel_end - (__relocate_new_kernel_start & ~(SZ_4K > > - 1)) > > + <= SZ_4K, "kexec relocation code is too big or misaligned") > > +ASSERT(KEXEC_CONTROL_PAGE_SIZE >= SZ_4K, "KEXEC_CONTROL_PAGE_SIZE is > > brokern") > > Typo: "brokern", Will correct it. Thanks, Pasha ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 08/15] arm64: kexec: configure EL2 vectors for kexec
> > +/* Allocates pages for kexec page table */
> > +static void *kexec_page_alloc(void *arg)
> > +{
> > + struct kimage *kimage = (struct kimage *)arg;
> > + struct page *page = kimage_alloc_control_pages(kimage, 0);
> > +
> > + if (!page)
> > + return NULL;
> > +
> > + memset(page_address(page), 0, PAGE_SIZE);
>
> Hmm, I think we might be missing barriers here to ensure that the zeroes
> are visible to the page-table walker before we plumb the page into the
> page-table.
>
> Usually, that's taken care of by the smp_wmb() in __pXX_alloc() but I
> can't see that here. Is it hiding?
Based on the comment in __pte_alloc() that smp_wmb() is needed in
order to synchronize pte setup with other cpus prior to making it
visible to them. This is not needed here. First, by the time these
page tables are used the other cpus are offlined (kexec reboot code is
single threaded). Second, we never insert any entry into a page table
that is actively used by any cpu.
Pasha
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 05/15] arm64: kexec: skip relocation code for inplace kexec
Hi Will, > > + cpu_install_idmap(); > > + restart = (void > > *)__pa_symbol(function_nocfi(__cpu_soft_restart)); > > + restart(is_hyp_nvhe(), kimage->start, kimage->arch.dtb_mem, > > + 0, 0); > > Why can't you call: > > cpu_soft_restart(kimage->start, kimage->arch.dtb_mem, 0, 0); > > here instead of open-coding it? This is part of a cleanup to remove cpu_soft_restart() wrapper and the header file that contains it. The wrapper is simple enough and has only one call site. It makes more sense to do what is needed directly from machine_kexec(). Pasha ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 00/15] arm64: MMU enabled kexec relocation
On Thu, Sep 16, 2021 at 07:13:10PM -0400, Pasha Tatashin wrote: > Pasha Tatashin (15): > arm64: kernel: add helper for booted at EL2 and not VHE > arm64: trans_pgd: hibernate: Add trans_pgd_copy_el2_vectors > arm64: hibernate: abstract ttrb0 setup function > arm64: kexec: flush image and lists during kexec load time > arm64: kexec: skip relocation code for inplace kexec > arm64: kexec: Use dcache ops macros instead of open-coding > arm64: kexec: pass kimage as the only argument to relocation function > arm64: kexec: configure EL2 vectors for kexec > arm64: kexec: relocate in EL1 mode > arm64: kexec: use ld script for relocation function > arm64: kexec: install a copy of the linear-map > arm64: kexec: keep MMU enabled during kexec relocation > arm64: kexec: remove the pre-kexec PoC maintenance > arm64: kexec: remove cpu-reset.h > arm64: trans_pgd: remove trans_pgd_map_page() FWIW, I'm fine with this series. I think the only concern raised by James and not addressed is the possibility of the failure of the memory allocation for copying the page tables during kexec. If anyone will complain in the future, we can look into adding a fallback mechanism to do the relocation with the MMU off. Acked-by: Catalin Marinas ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 3/4] memblock: add MEMBLOCK_DRIVER_MANAGED to mimic IORESOURCE_SYSRAM_DRIVER_MANAGED
On 29.09.21 18:39, Mike Rapoport wrote: Hi, On Mon, Sep 27, 2021 at 05:05:17PM +0200, David Hildenbrand wrote: Let's add a flag that corresponds to IORESOURCE_SYSRAM_DRIVER_MANAGED. Similar to MEMBLOCK_HOTPLUG, most infrastructure has to treat such memory like ordinary MEMBLOCK_NONE memory -- for example, when selecting memory regions to add to the vmcore for dumping in the crashkernel via for_each_mem_range(). Can you please elaborate on the difference in semantics of MEMBLOCK_HOTPLUG and MEMBLOCK_DRIVER_MANAGED? Unless I'm missing something they both mark memory that can be unplugged anytime and so it should not be used in certain cases. Why is there a need for a new flag? In the cover letter I have "Alternative B: Reuse MEMBLOCK_HOTPLUG. MEMBLOCK_HOTPLUG serves a different purpose, though.", but looking into the details it won't work as is. MEMBLOCK_HOTPLUG is used to mark memory early during boot that can later get hotunplugged again and should be placed into ZONE_MOVABLE if the "movable_node" kernel parameter is set. The confusing part is that we talk about "hotpluggable" but really mean "hotunpluggable": the reason is that HW flags DIMM slots that can later be hotplugged as "hotpluggable" even though there is already something hotplugged. For example, ranges in the ACPI SRAT that are marked as ACPI_SRAT_MEM_HOT_PLUGGABLE will be marked MEMBLOCK_HOTPLUG early during boot (drivers/acpi/numa/srat.c:acpi_numa_memory_affinity_init()). Later, we use that information to size ZONE_MOVABLE (mm/page_alloc.c:find_zone_movable_pfns_for_nodes()). This will make sure that these "hotpluggable" DIMMs can later get hotunplugged. Also, see should_skip_region() how this relates to the "movable_node" kernel parameter: /* skip hotpluggable memory regions if needed */ if (movable_node_is_enabled() && memblock_is_hotpluggable(m) && (flags & MEMBLOCK_HOTPLUG)) return true; Long story short: MEMBLOCK_HOTPLUG has different semantics and is a special case for "movable_node". -- Thanks, David / dhildenb ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 15/15] arm64: trans_pgd: remove trans_pgd_map_page()
On Thu, Sep 16, 2021 at 07:13:25PM -0400, Pasha Tatashin wrote:
> The intend of trans_pgd_map_page() was to map contiguous range of VA
> memory to the memory that is getting relocated during kexec. However,
> since we are now using linear map instead of contiguous range this
> function is not needed
>
> Suggested-by: Pingfan Liu
> Signed-off-by: Pasha Tatashin
> ---
> arch/arm64/include/asm/trans_pgd.h | 5 +--
> arch/arm64/mm/trans_pgd.c | 57 --
> 2 files changed, 1 insertion(+), 61 deletions(-)
>
> diff --git a/arch/arm64/include/asm/trans_pgd.h
> b/arch/arm64/include/asm/trans_pgd.h
> index 7b04d32b102c..033d400a4ea4 100644
> --- a/arch/arm64/include/asm/trans_pgd.h
> +++ b/arch/arm64/include/asm/trans_pgd.h
> @@ -15,7 +15,7 @@
> /*
> * trans_alloc_page
> * - Allocator that should return exactly one zeroed page, if this
> - * allocator fails, trans_pgd_create_copy() and trans_pgd_map_page()
> + * allocator fails, trans_pgd_create_copy() and trans_pgd_idmap_page()
> * return -ENOMEM error.
> *
> * trans_alloc_arg
> @@ -30,9 +30,6 @@ struct trans_pgd_info {
> int trans_pgd_create_copy(struct trans_pgd_info *info, pgd_t **trans_pgd,
> unsigned long start, unsigned long end);
>
> -int trans_pgd_map_page(struct trans_pgd_info *info, pgd_t *trans_pgd,
> -void *page, unsigned long dst_addr, pgprot_t pgprot);
So this function never got used in mainline after commit 7018d467ff2d
("arm64: trans_pgd: hibernate: idmap the single page that holds the copy
page routines"). I guess it's because we merged part of v10 of this
series and in v12 you dropped the contiguous VA range in favour of a
copy of the linear map.
--
Catalin
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 3/4] memblock: add MEMBLOCK_DRIVER_MANAGED to mimic IORESOURCE_SYSRAM_DRIVER_MANAGED
Hi,
On Mon, Sep 27, 2021 at 05:05:17PM +0200, David Hildenbrand wrote:
> Let's add a flag that corresponds to IORESOURCE_SYSRAM_DRIVER_MANAGED.
> Similar to MEMBLOCK_HOTPLUG, most infrastructure has to treat such memory
> like ordinary MEMBLOCK_NONE memory -- for example, when selecting memory
> regions to add to the vmcore for dumping in the crashkernel via
> for_each_mem_range().
Can you please elaborate on the difference in semantics of MEMBLOCK_HOTPLUG
and MEMBLOCK_DRIVER_MANAGED?
Unless I'm missing something they both mark memory that can be unplugged
anytime and so it should not be used in certain cases. Why is there a need
for a new flag?
> However, especially kexec_file is not supposed to select such memblocks via
> for_each_free_mem_range() / for_each_free_mem_range_reverse() to place
> kexec images, similar to how we handle IORESOURCE_SYSRAM_DRIVER_MANAGED
> without CONFIG_ARCH_KEEP_MEMBLOCK.
>
> Let's document why kexec_walk_memblock() won't try placing images on
> areas marked MEMBLOCK_DRIVER_MANAGED -- similar to
> IORESOURCE_SYSRAM_DRIVER_MANAGED handling in locate_mem_hole_callback()
> via kexec_walk_resources().
>
> We'll make sure that memory hotplug code sets the flag where applicable
> (IORESOURCE_SYSRAM_DRIVER_MANAGED) next. This prepares architectures
> that need CONFIG_ARCH_KEEP_MEMBLOCK, such as arm64, for virtio-mem
> support.
>
> Signed-off-by: David Hildenbrand
> ---
> include/linux/memblock.h | 16 ++--
> kernel/kexec_file.c | 5 +
> mm/memblock.c| 4
> 3 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index b49a58f621bc..7d8d656d5082 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -33,12 +33,17 @@ extern unsigned long long max_possible_pfn;
> * @MEMBLOCK_NOMAP: don't add to kernel direct mapping and treat as
> * reserved in the memory map; refer to memblock_mark_nomap() description
> * for further details
> + * @MEMBLOCK_DRIVER_MANAGED: memory region that is always detected via a
> driver,
> + * corresponding to IORESOURCE_SYSRAM_DRIVER_MANAGED in the kernel resource
> + * tree. Especially kexec should never use this memory for placing images and
> + * shouldn't expose this memory to the second kernel.
> */
> enum memblock_flags {
> MEMBLOCK_NONE = 0x0, /* No special request */
> MEMBLOCK_HOTPLUG= 0x1, /* hotpluggable region */
> MEMBLOCK_MIRROR = 0x2, /* mirrored region */
> MEMBLOCK_NOMAP = 0x4, /* don't add to kernel direct mapping */
> + MEMBLOCK_DRIVER_MANAGED = 0x8, /* always detected via a driver */
> };
>
> /**
> @@ -209,7 +214,8 @@ static inline void __next_physmem_range(u64 *idx, struct
> memblock_type *type,
> */
> #define for_each_mem_range(i, p_start, p_end) \
> __for_each_mem_range(i, , NULL, NUMA_NO_NODE, \
> - MEMBLOCK_HOTPLUG, p_start, p_end, NULL)
> + MEMBLOCK_HOTPLUG | MEMBLOCK_DRIVER_MANAGED, \
> + p_start, p_end, NULL)
>
> /**
> * for_each_mem_range_rev - reverse iterate through memblock areas from
> @@ -220,7 +226,8 @@ static inline void __next_physmem_range(u64 *idx, struct
> memblock_type *type,
> */
> #define for_each_mem_range_rev(i, p_start, p_end)\
> __for_each_mem_range_rev(i, , NULL, NUMA_NO_NODE, \
> - MEMBLOCK_HOTPLUG, p_start, p_end, NULL)
> + MEMBLOCK_HOTPLUG | MEMBLOCK_DRIVER_MANAGED,\
> + p_start, p_end, NULL)
>
> /**
> * for_each_reserved_mem_range - iterate over all reserved memblock areas
> @@ -250,6 +257,11 @@ static inline bool memblock_is_nomap(struct
> memblock_region *m)
> return m->flags & MEMBLOCK_NOMAP;
> }
>
> +static inline bool memblock_is_driver_managed(struct memblock_region *m)
> +{
> + return m->flags & MEMBLOCK_DRIVER_MANAGED;
> +}
> +
> int memblock_search_pfn_nid(unsigned long pfn, unsigned long *start_pfn,
> unsigned long *end_pfn);
> void __next_mem_pfn_range(int *idx, int nid, unsigned long *out_start_pfn,
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 33400ff051a8..8347fc158d2b 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -556,6 +556,11 @@ static int kexec_walk_memblock(struct kexec_buf *kbuf,
> if (kbuf->image->type == KEXEC_TYPE_CRASH)
> return func(_res, kbuf);
>
> + /*
> + * Using MEMBLOCK_NONE will properly skip MEMBLOCK_DRIVER_MANAGED. See
> + * IORESOURCE_SYSRAM_DRIVER_MANAGED handling in
> + * locate_mem_hole_callback().
> + */
> if (kbuf->top_down) {
> for_each_free_mem_range_reverse(i, NUMA_NO_NODE, MEMBLOCK_NONE,
> , , NULL) {
> diff --git a/mm/memblock.c b/mm/memblock.c
>
Re: [PATCH v1 2/4] memblock: allow to specify flags with memblock_add_node()
On 29.09.21 18:25, Mike Rapoport wrote: On Mon, Sep 27, 2021 at 05:05:16PM +0200, David Hildenbrand wrote: We want to specify flags when hotplugging memory. Let's prepare to pass flags to memblock_add_node() by adjusting all existing users. Note that when hotplugging memory the system is already up and running and we don't want to add the memory first and apply flags later: it should happen within one memblock call. Why is it important that the system is up and why it should happen in a single call? I don't mind adding flags parameter to memblock_add_node() but this changelog does not really explain the reasons to do it. "After memblock_add_node(), we could race with anybody performing a search for MEMBLOCK_NONE, like kexec_file -- and that only happens once the system is already up and running. So we want both steps to happen atomically." I can add that to the patch description. (I think it still won't be completely atomic because memblock isn't properly implementing locking yet, but that's a different story) -- Thanks, David / dhildenb ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 2/4] memblock: allow to specify flags with memblock_add_node()
On Mon, Sep 27, 2021 at 05:05:16PM +0200, David Hildenbrand wrote:
> We want to specify flags when hotplugging memory. Let's prepare to pass
> flags to memblock_add_node() by adjusting all existing users.
>
> Note that when hotplugging memory the system is already up and running
> and we don't want to add the memory first and apply flags later: it
> should happen within one memblock call.
Why is it important that the system is up and why it should happen in a
single call?
I don't mind adding flags parameter to memblock_add_node() but this
changelog does not really explain the reasons to do it.
> Signed-off-by: David Hildenbrand
> ---
> arch/arc/mm/init.c | 4 ++--
> arch/ia64/mm/contig.c| 2 +-
> arch/ia64/mm/init.c | 2 +-
> arch/m68k/mm/mcfmmu.c| 3 ++-
> arch/m68k/mm/motorola.c | 6 --
> arch/mips/loongson64/init.c | 4 +++-
> arch/mips/sgi-ip27/ip27-memory.c | 3 ++-
> arch/s390/kernel/setup.c | 3 ++-
> include/linux/memblock.h | 3 ++-
> include/linux/mm.h | 2 +-
> mm/memblock.c| 9 +
> mm/memory_hotplug.c | 2 +-
> 12 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
> index 699ecf119641..110eb69e9bee 100644
> --- a/arch/arc/mm/init.c
> +++ b/arch/arc/mm/init.c
> @@ -59,13 +59,13 @@ void __init early_init_dt_add_memory_arch(u64 base, u64
> size)
>
> low_mem_sz = size;
> in_use = 1;
> - memblock_add_node(base, size, 0);
> + memblock_add_node(base, size, 0, MEMBLOCK_NONE);
> } else {
> #ifdef CONFIG_HIGHMEM
> high_mem_start = base;
> high_mem_sz = size;
> in_use = 1;
> - memblock_add_node(base, size, 1);
> + memblock_add_node(base, size, 1, MEMBLOCK_NONE);
> memblock_reserve(base, size);
> #endif
> }
> diff --git a/arch/ia64/mm/contig.c b/arch/ia64/mm/contig.c
> index 42e025cfbd08..24901d809301 100644
> --- a/arch/ia64/mm/contig.c
> +++ b/arch/ia64/mm/contig.c
> @@ -153,7 +153,7 @@ find_memory (void)
> efi_memmap_walk(find_max_min_low_pfn, NULL);
> max_pfn = max_low_pfn;
>
> - memblock_add_node(0, PFN_PHYS(max_low_pfn), 0);
> + memblock_add_node(0, PFN_PHYS(max_low_pfn), 0, MEMBLOCK_NONE);
>
> find_initrd();
>
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 5c6da8d83c1a..5d165607bf35 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -378,7 +378,7 @@ int __init register_active_ranges(u64 start, u64 len, int
> nid)
> #endif
>
> if (start < end)
> - memblock_add_node(__pa(start), end - start, nid);
> + memblock_add_node(__pa(start), end - start, nid, MEMBLOCK_NONE);
> return 0;
> }
>
> diff --git a/arch/m68k/mm/mcfmmu.c b/arch/m68k/mm/mcfmmu.c
> index eac9dde65193..6f1f25125294 100644
> --- a/arch/m68k/mm/mcfmmu.c
> +++ b/arch/m68k/mm/mcfmmu.c
> @@ -174,7 +174,8 @@ void __init cf_bootmem_alloc(void)
> m68k_memory[0].addr = _rambase;
> m68k_memory[0].size = _ramend - _rambase;
>
> - memblock_add_node(m68k_memory[0].addr, m68k_memory[0].size, 0);
> + memblock_add_node(m68k_memory[0].addr, m68k_memory[0].size, 0,
> + MEMBLOCK_NONE);
>
> /* compute total pages in system */
> num_pages = PFN_DOWN(_ramend - _rambase);
> diff --git a/arch/m68k/mm/motorola.c b/arch/m68k/mm/motorola.c
> index 3a653f0a4188..e80c5d7e6728 100644
> --- a/arch/m68k/mm/motorola.c
> +++ b/arch/m68k/mm/motorola.c
> @@ -410,7 +410,8 @@ void __init paging_init(void)
>
> min_addr = m68k_memory[0].addr;
> max_addr = min_addr + m68k_memory[0].size;
> - memblock_add_node(m68k_memory[0].addr, m68k_memory[0].size, 0);
> + memblock_add_node(m68k_memory[0].addr, m68k_memory[0].size, 0,
> + MEMBLOCK_NONE);
> for (i = 1; i < m68k_num_memory;) {
> if (m68k_memory[i].addr < min_addr) {
> printk("Ignoring memory chunk at 0x%lx:0x%lx before the
> first chunk\n",
> @@ -421,7 +422,8 @@ void __init paging_init(void)
> (m68k_num_memory - i) * sizeof(struct
> m68k_mem_info));
> continue;
> }
> - memblock_add_node(m68k_memory[i].addr, m68k_memory[i].size, i);
> + memblock_add_node(m68k_memory[i].addr, m68k_memory[i].size, i,
> + MEMBLOCK_NONE);
> addr = m68k_memory[i].addr + m68k_memory[i].size;
> if (addr > max_addr)
> max_addr = addr;
> diff --git a/arch/mips/loongson64/init.c b/arch/mips/loongson64/init.c
> index 76e0a9636a0e..4ac5ba80bbf6 100644
> --- a/arch/mips/loongson64/init.c
> +++ b/arch/mips/loongson64/init.c
> @@ -77,7 +77,9 @@ void __init szmem(unsigned int node)
>
Re: [PATCH 2/2] arm64: kexec_file: use more system keyrings to verify kernel image signature
On Mon, Sep 27, 2021 at 08:50:04AM +0800, Coiby Xu wrote:
> From: Coiby Xu
>
> This allows to verify arm64 kernel image signature using not only
> .builtin_trusted_keys but also .secondary_trusted_keys and .platform keyring.
>
> Signed-off-by: Coiby Xu
> ---
> arch/arm64/kernel/kexec_image.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> index 9ec34690e255..2357ee2f229a 100644
> --- a/arch/arm64/kernel/kexec_image.c
> +++ b/arch/arm64/kernel/kexec_image.c
> @@ -14,7 +14,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -133,8 +132,7 @@ static void *image_load(struct kimage *image,
> #ifdef CONFIG_KEXEC_IMAGE_VERIFY_SIG
> static int image_verify_sig(const char *kernel, unsigned long kernel_len)
> {
> - return verify_pefile_signature(kernel, kernel_len, NULL,
> -VERIFYING_KEXEC_PE_SIGNATURE);
> + return arch_kexec_kernel_verify_pe_sig(kernel, kernel_len);
I'm fine with this in principle, but it looks like the first patch is the
important one.
So for the arm64 bit:
Acked-by: Will Deacon
Will
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 2/8] x86/xen: simplify xen_oldmem_pfn_is_ram()
On 29.09.21 16:22, Boris Ostrovsky wrote:
On 9/29/21 5:03 AM, David Hildenbrand wrote:
On 29.09.21 10:45, David Hildenbrand wrote:
Can we go one step further and do
@@ -20,24 +20,11 @@ static int xen_oldmem_pfn_is_ram(unsigned long pfn)
struct xen_hvm_get_mem_type a = {
.domid = DOMID_SELF,
.pfn = pfn,
+ .mem_type = HVMMEM_ram_rw,
};
- int ram;
- if (HYPERVISOR_hvm_op(HVMOP_get_mem_type, ))
- return -ENXIO;
-
- switch (a.mem_type) {
- case HVMMEM_mmio_dm:
- ram = 0;
- break;
- case HVMMEM_ram_rw:
- case HVMMEM_ram_ro:
- default:
- ram = 1;
- break;
- }
-
- return ram;
+ HYPERVISOR_hvm_op(HVMOP_get_mem_type, );
+ return a.mem_type != HVMMEM_mmio_dm;
I was actually thinking of asking you to add another patch with pr_warn_once()
here (and print error code as well). This call failing is indication of
something going quite wrong and it would be good to know about this.
Will include a patch in v2, thanks!
--
Thanks,
David / dhildenb
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 2/8] x86/xen: simplify xen_oldmem_pfn_is_ram()
On 9/29/21 5:03 AM, David Hildenbrand wrote:
> On 29.09.21 10:45, David Hildenbrand wrote:
>>>
>> Can we go one step further and do
>>
>>
>> @@ -20,24 +20,11 @@ static int xen_oldmem_pfn_is_ram(unsigned long pfn)
>> struct xen_hvm_get_mem_type a = {
>> .domid = DOMID_SELF,
>> .pfn = pfn,
>> + .mem_type = HVMMEM_ram_rw,
>> };
>> - int ram;
>> - if (HYPERVISOR_hvm_op(HVMOP_get_mem_type, ))
>> - return -ENXIO;
>> -
>> - switch (a.mem_type) {
>> - case HVMMEM_mmio_dm:
>> - ram = 0;
>> - break;
>> - case HVMMEM_ram_rw:
>> - case HVMMEM_ram_ro:
>> - default:
>> - ram = 1;
>> - break;
>> - }
>> -
>> - return ram;
>> + HYPERVISOR_hvm_op(HVMOP_get_mem_type, );
>> + return a.mem_type != HVMMEM_mmio_dm;
I was actually thinking of asking you to add another patch with pr_warn_once()
here (and print error code as well). This call failing is indication of
something going quite wrong and it would be good to know about this.
>> }
>> #endif
>>
>>
>> Assuming that if HYPERVISOR_hvm_op() fails that
>> .mem_type is not set to HVMMEM_mmio_dm.
I don't think we can assume that argument described as OUT in the ABI will not
be clobbered in case of error
>>
>
> Okay we can't, due to "__must_check" ...
so this is a good thing ;-)
-boris
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 00/15] arm64: MMU enabled kexec relocation
On Thu, Sep 16, 2021 at 07:13:10PM -0400, Pasha Tatashin wrote: > Changelog: > v17: > - Merged with 5.15-rc1 as requested by Catalin Marinas > - Added Tested-by: Pingfan Liu This looks pretty good to me. I've left some minor comments on a few of the patches, but other than those I'd like to try getting these queued for 5.16. Will ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 10/15] arm64: kexec: use ld script for relocation function
On Thu, Sep 16, 2021 at 07:13:20PM -0400, Pasha Tatashin wrote:
> Currently, relocation code declares start and end variables
> which are used to compute its size.
>
> The better way to do this is to use ld script incited, and put relocation
> function in its own section.
"incited"? I don't understand ...
>
> Signed-off-by: Pasha Tatashin
> ---
> arch/arm64/include/asm/sections.h | 1 +
> arch/arm64/kernel/machine_kexec.c | 16 ++--
> arch/arm64/kernel/relocate_kernel.S | 15 ++-
> arch/arm64/kernel/vmlinux.lds.S | 19 +++
> 4 files changed, 28 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm64/include/asm/sections.h
> b/arch/arm64/include/asm/sections.h
> index e4ad9db53af1..152cb35bf9df 100644
> --- a/arch/arm64/include/asm/sections.h
> +++ b/arch/arm64/include/asm/sections.h
> @@ -21,5 +21,6 @@ extern char __exittext_begin[], __exittext_end[];
> extern char __irqentry_text_start[], __irqentry_text_end[];
> extern char __mmuoff_data_start[], __mmuoff_data_end[];
> extern char __entry_tramp_text_start[], __entry_tramp_text_end[];
> +extern char __relocate_new_kernel_start[], __relocate_new_kernel_end[];
>
> #endif /* __ASM_SECTIONS_H */
> diff --git a/arch/arm64/kernel/machine_kexec.c
> b/arch/arm64/kernel/machine_kexec.c
> index cf5d6f22a041..83da6045cd45 100644
> --- a/arch/arm64/kernel/machine_kexec.c
> +++ b/arch/arm64/kernel/machine_kexec.c
> @@ -21,14 +21,11 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "cpu-reset.h"
>
> -/* Global variables for the arm64_relocate_new_kernel routine. */
> -extern const unsigned char arm64_relocate_new_kernel[];
> -extern const unsigned long arm64_relocate_new_kernel_size;
> -
> /**
> * kexec_image_info - For debugging output.
> */
> @@ -163,6 +160,7 @@ static void *kexec_page_alloc(void *arg)
> int machine_kexec_post_load(struct kimage *kimage)
> {
> void *reloc_code = page_to_virt(kimage->control_code_page);
> + long reloc_size;
> struct trans_pgd_info info = {
> .trans_alloc_page = kexec_page_alloc,
> .trans_alloc_arg= kimage,
> @@ -183,17 +181,15 @@ int machine_kexec_post_load(struct kimage *kimage)
> return rc;
> }
>
> - memcpy(reloc_code, arm64_relocate_new_kernel,
> -arm64_relocate_new_kernel_size);
> + reloc_size = __relocate_new_kernel_end - __relocate_new_kernel_start;
> + memcpy(reloc_code, __relocate_new_kernel_start, reloc_size);
> kimage->arch.kern_reloc = __pa(reloc_code);
>
> /* Flush the reloc_code in preparation for its execution. */
> dcache_clean_inval_poc((unsigned long)reloc_code,
> -(unsigned long)reloc_code +
> -arm64_relocate_new_kernel_size);
> +(unsigned long)reloc_code + reloc_size);
Extra whitespace.
> icache_inval_pou((uintptr_t)reloc_code,
> - (uintptr_t)reloc_code +
> - arm64_relocate_new_kernel_size);
> + (uintptr_t)reloc_code + reloc_size);
> kexec_list_flush(kimage);
> kexec_image_info(kimage);
>
> diff --git a/arch/arm64/kernel/relocate_kernel.S
> b/arch/arm64/kernel/relocate_kernel.S
> index b4fb97312a80..9d2400855ee4 100644
> --- a/arch/arm64/kernel/relocate_kernel.S
> +++ b/arch/arm64/kernel/relocate_kernel.S
> @@ -15,6 +15,7 @@
> #include
> #include
>
> +.pushsection".kexec_relocate.text", "ax"
Just use .section if you're putting the entire file in there?
> /*
> * arm64_relocate_new_kernel - Put a 2nd stage image in place and boot it.
> *
> @@ -77,16 +78,4 @@ SYM_CODE_START(arm64_relocate_new_kernel)
> mov x3, xzr
> br x4 /* Jumps from el1 */
> SYM_CODE_END(arm64_relocate_new_kernel)
> -
> -.align 3 /* To keep the 64-bit values below naturally aligned. */
> -
> -.Lcopy_end:
> -.org KEXEC_CONTROL_PAGE_SIZE
> -
> -/*
> - * arm64_relocate_new_kernel_size - Number of bytes to copy to the
> - * control_code_page.
> - */
> -.globl arm64_relocate_new_kernel_size
> -arm64_relocate_new_kernel_size:
> - .quad .Lcopy_end - arm64_relocate_new_kernel
> +.popsection
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index f6b1a88245db..ab457b609e69 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -63,6 +63,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -100,6 +101,16 @@ jiffies = jiffies_64;
> #define HIBERNATE_TEXT
> #endif
>
> +#ifdef CONFIG_KEXEC_CORE
> +#define KEXEC_TEXT \
> + . = ALIGN(SZ_4K); \
> + __relocate_new_kernel_start = .;\
> + *(.kexec_relocate.text) \
> + __relocate_new_kernel_end = .;
> +#else
Re: [PATCH v17 08/15] arm64: kexec: configure EL2 vectors for kexec
On Thu, Sep 16, 2021 at 07:13:18PM -0400, Pasha Tatashin wrote:
> If we have a EL2 mode without VHE, the EL2 vectors are needed in order
> to switch to EL2 and jump to new world with hypervisor privileges.
>
> In preparation to MMU enabled relocation, configure our EL2 table now.
>
> Kexec uses #HVC_SOFT_RESTART to branch to the new world, so extend
> el1_sync vector that is provided by trans_pgd_copy_el2_vectors() to
> support this case.
>
> Signed-off-by: Pasha Tatashin
> ---
> arch/arm64/Kconfig| 2 +-
> arch/arm64/include/asm/kexec.h| 1 +
> arch/arm64/kernel/asm-offsets.c | 1 +
> arch/arm64/kernel/machine_kexec.c | 31 +++
> arch/arm64/mm/trans_pgd-asm.S | 9 -
> 5 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 5c7ae4c3954b..552a057b40af 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1135,7 +1135,7 @@ config CRASH_DUMP
>
> config TRANS_TABLE
> def_bool y
> - depends on HIBERNATION
> + depends on HIBERNATION || KEXEC_CORE
>
> config XEN_DOM0
> def_bool y
> diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
> index 00dbcc71aeb2..753a1c398898 100644
> --- a/arch/arm64/include/asm/kexec.h
> +++ b/arch/arm64/include/asm/kexec.h
> @@ -96,6 +96,7 @@ struct kimage_arch {
> void *dtb;
> phys_addr_t dtb_mem;
> phys_addr_t kern_reloc;
> + phys_addr_t el2_vectors;
> };
>
> #ifdef CONFIG_KEXEC_FILE
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index 1d3319c7518e..6a2b8b1a4872 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -174,6 +174,7 @@ int main(void)
> #endif
> #ifdef CONFIG_KEXEC_CORE
>DEFINE(KIMAGE_ARCH_DTB_MEM,offsetof(struct kimage,
> arch.dtb_mem));
> + DEFINE(KIMAGE_ARCH_EL2_VECTORS,offsetof(struct kimage,
> arch.el2_vectors));
>DEFINE(KIMAGE_HEAD,offsetof(struct kimage, head));
>DEFINE(KIMAGE_START, offsetof(struct kimage, start));
>BLANK();
> diff --git a/arch/arm64/kernel/machine_kexec.c
> b/arch/arm64/kernel/machine_kexec.c
> index e210b19592c6..59a4b4172b68 100644
> --- a/arch/arm64/kernel/machine_kexec.c
> +++ b/arch/arm64/kernel/machine_kexec.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
>
> #include "cpu-reset.h"
>
> @@ -43,7 +44,9 @@ static void _kexec_image_info(const char *func, int line,
> pr_debug("start: %lx\n", kimage->start);
> pr_debug("head:%lx\n", kimage->head);
> pr_debug("nr_segments: %lu\n", kimage->nr_segments);
> + pr_debug("dtb_mem: %pa\n", >arch.dtb_mem);
> pr_debug("kern_reloc: %pa\n", >arch.kern_reloc);
> + pr_debug("el2_vectors: %pa\n", >arch.el2_vectors);
>
> for (i = 0; i < kimage->nr_segments; i++) {
> pr_debug(" segment[%lu]: %016lx - %016lx, 0x%lx bytes, %lu
> pages\n",
> @@ -143,9 +146,27 @@ static void kexec_segment_flush(const struct kimage
> *kimage)
> }
> }
>
> +/* Allocates pages for kexec page table */
> +static void *kexec_page_alloc(void *arg)
> +{
> + struct kimage *kimage = (struct kimage *)arg;
> + struct page *page = kimage_alloc_control_pages(kimage, 0);
> +
> + if (!page)
> + return NULL;
> +
> + memset(page_address(page), 0, PAGE_SIZE);
Hmm, I think we might be missing barriers here to ensure that the zeroes
are visible to the page-table walker before we plumb the page into the
page-table.
Usually, that's taken care of by the smp_wmb() in __pXX_alloc() but I
can't see that here. Is it hiding?
Will
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v17 05/15] arm64: kexec: skip relocation code for inplace kexec
On Thu, Sep 16, 2021 at 07:13:15PM -0400, Pasha Tatashin wrote:
> In case of kdump or when segments are already in place the relocation
> is not needed, therefore the setup of relocation function and call to
> it can be skipped.
>
> Signed-off-by: Pasha Tatashin
> Suggested-by: James Morse
> ---
> arch/arm64/kernel/machine_kexec.c | 34 ++---
> arch/arm64/kernel/relocate_kernel.S | 3 ---
> 2 files changed, 21 insertions(+), 16 deletions(-)
[...]
> @@ -188,19 +190,25 @@ void machine_kexec(struct kimage *kimage)
> local_daif_mask();
>
> /*
> - * cpu_soft_restart will shutdown the MMU, disable data caches, then
> - * transfer control to the kern_reloc which contains a copy of
> - * the arm64_relocate_new_kernel routine. arm64_relocate_new_kernel
> - * uses physical addressing to relocate the new image to its final
> - * position and transfers control to the image entry point when the
> - * relocation is complete.
> + * Both restart and cpu_soft_restart will shutdown the MMU, disable data
> + * caches. However, restart will start new kernel or purgatory directly,
> + * cpu_soft_restart will transfer control to arm64_relocate_new_kernel
>* In kexec case, kimage->start points to purgatory assuming that
>* kernel entry and dtb address are embedded in purgatory by
>* userspace (kexec-tools).
>* In kexec_file case, the kernel starts directly without purgatory.
>*/
> - cpu_soft_restart(kimage->arch.kern_reloc, kimage->head, kimage->start,
> - kimage->arch.dtb_mem);
> + if (kimage->head & IND_DONE) {
> + typeof(__cpu_soft_restart) *restart;
> +
> + cpu_install_idmap();
> + restart = (void
> *)__pa_symbol(function_nocfi(__cpu_soft_restart));
> + restart(is_hyp_nvhe(), kimage->start, kimage->arch.dtb_mem,
> + 0, 0);
Why can't you call:
cpu_soft_restart(kimage->start, kimage->arch.dtb_mem, 0, 0);
here instead of open-coding it?
Will
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 2/8] x86/xen: simplify xen_oldmem_pfn_is_ram()
On 29.09.21 10:45, David Hildenbrand wrote:
How about
return a.mem_type != HVMMEM_mmio_dm;
Ha, how could I have missed that :)
Result should be promoted to int and this has added benefit of not requiring
changes in patch 4.
Can we go one step further and do
@@ -20,24 +20,11 @@ static int xen_oldmem_pfn_is_ram(unsigned long pfn)
struct xen_hvm_get_mem_type a = {
.domid = DOMID_SELF,
.pfn = pfn,
+ .mem_type = HVMMEM_ram_rw,
};
- int ram;
- if (HYPERVISOR_hvm_op(HVMOP_get_mem_type, ))
- return -ENXIO;
-
- switch (a.mem_type) {
- case HVMMEM_mmio_dm:
- ram = 0;
- break;
- case HVMMEM_ram_rw:
- case HVMMEM_ram_ro:
- default:
- ram = 1;
- break;
- }
-
- return ram;
+ HYPERVISOR_hvm_op(HVMOP_get_mem_type, );
+ return a.mem_type != HVMMEM_mmio_dm;
}
#endif
Assuming that if HYPERVISOR_hvm_op() fails that
.mem_type is not set to HVMMEM_mmio_dm.
Okay we can't, due to "__must_check" ...
--
Thanks,
David / dhildenb
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 2/8] x86/xen: simplify xen_oldmem_pfn_is_ram()
How about
return a.mem_type != HVMMEM_mmio_dm;
Ha, how could I have missed that :)
Result should be promoted to int and this has added benefit of not requiring
changes in patch 4.
Can we go one step further and do
@@ -20,24 +20,11 @@ static int xen_oldmem_pfn_is_ram(unsigned long pfn)
struct xen_hvm_get_mem_type a = {
.domid = DOMID_SELF,
.pfn = pfn,
+ .mem_type = HVMMEM_ram_rw,
};
- int ram;
- if (HYPERVISOR_hvm_op(HVMOP_get_mem_type, ))
- return -ENXIO;
-
- switch (a.mem_type) {
- case HVMMEM_mmio_dm:
- ram = 0;
- break;
- case HVMMEM_ram_rw:
- case HVMMEM_ram_ro:
- default:
- ram = 1;
- break;
- }
-
- return ram;
+ HYPERVISOR_hvm_op(HVMOP_get_mem_type, );
+ return a.mem_type != HVMMEM_mmio_dm;
}
#endif
Assuming that if HYPERVISOR_hvm_op() fails that
.mem_type is not set to HVMMEM_mmio_dm.
--
Thanks,
David / dhildenb
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 8/8] virtio-mem: kdump mode to sanitize /proc/vmcore access
[...]
+
+static bool virtio_mem_vmcore_pfn_is_ram(struct vmcore_cb *cb,
+unsigned long pfn)
+{
+ struct virtio_mem *vm = container_of(cb, struct virtio_mem,
+vmcore_cb);
+ uint64_t addr = PFN_PHYS(pfn);
+ bool is_ram;
+ int rc;
+
+ if (!virtio_mem_contains_range(vm, addr, addr + PAGE_SIZE))
Some more testing revealed that this has to be
if (!virtio_mem_contains_range(vm, addr, PAGE_SIZE))
--
Thanks,
David / dhildenb
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH] kexec-tools:Add some necessary free() calls
free should be called before the function exit abnormally.
Signed-off-by: Kai Song
---
kexec/kexec-uImage.c | 10 --
util_lib/elf_info.c | 4 +++-
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/kexec/kexec-uImage.c b/kexec/kexec-uImage.c
index 4be..016be10 100644
--- a/kexec/kexec-uImage.c
+++ b/kexec/kexec-uImage.c
@@ -157,12 +157,15 @@ static int uImage_gz_load(const char *buf, off_t len,
skip = 10;
/* check GZ magic */
- if (buf[0] != 0x1f || buf[1] != 0x8b)
+ if (buf[0] != 0x1f || buf[1] != 0x8b) {
+ free(uncomp_buf);
return -1;
+ }
flags = buf[3];
if (buf[2] != Z_DEFLATED || (flags & RESERVED) != 0) {
puts ("Error: Bad gzipped data\n");
+ free(uncomp_buf);
return -1;
}
@@ -187,8 +190,10 @@ static int uImage_gz_load(const char *buf, off_t len,
/* - activates parsing gz headers */
ret = inflateInit2(, -MAX_WBITS);
- if (ret != Z_OK)
+ if (ret != Z_OK) {
+ free(uncomp_buf);
return -1;
+ }
strm.next_out = uncomp_buf;
strm.avail_out = mem_alloc;
@@ -214,6 +219,7 @@ static int uImage_gz_load(const char *buf, off_t len,
strm.next_out = uncomp_buf + mem_alloc - inc_buf;
strm.avail_out = inc_buf;
} else {
+ free(uncomp_buf);
printf("Error during decompression %d\n", ret);
return -1;
}
diff --git a/util_lib/elf_info.c b/util_lib/elf_info.c
index 676926c..51d8b92 100644
--- a/util_lib/elf_info.c
+++ b/util_lib/elf_info.c
@@ -605,8 +605,10 @@ static int scan_notes(int fd, loff_t start, loff_t lsize)
scan_vmcoreinfo(n_desc, n_descsz);
}
- if ((note + sizeof(Elf_Nhdr)) == last)
+ if ((note + sizeof(Elf_Nhdr)) == last) {
+ free(buf);
return -1;
+ }
free(buf);
--
2.27.0
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH] kexec:Add some necessary fclose() calls
fclose should be called before function exits
Signed-off-by: Kai Song
---
kexec/arch/ppc/fixup_dtb.c | 2 ++
kexec/arch/ppc64/crashdump-ppc64.c | 1 +
kexec/arch/ppc64/kexec-ppc64.c | 4 +++-
kexec/symbols.c| 2 ++
4 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/kexec/arch/ppc/fixup_dtb.c b/kexec/arch/ppc/fixup_dtb.c
index 4524c8c..92a0bfd 100644
--- a/kexec/arch/ppc/fixup_dtb.c
+++ b/kexec/arch/ppc/fixup_dtb.c
@@ -382,6 +382,8 @@ static void save_fixed_up_dtb(char *blob_buf, off_t
blob_size)
} else {
dbgprintf("Unable to write debug.dtb\n");
}
+
+ fclose(fp);
} else {
dbgprintf("Unable to dump flat device tree to debug.dtb\n");
}
diff --git a/kexec/arch/ppc64/crashdump-ppc64.c
b/kexec/arch/ppc64/crashdump-ppc64.c
index addd769..91f9521 100644
--- a/kexec/arch/ppc64/crashdump-ppc64.c
+++ b/kexec/arch/ppc64/crashdump-ppc64.c
@@ -161,6 +161,7 @@ static int get_dyn_reconf_crash_memory_ranges(void)
fprintf(stderr,
"Error: Number of crash memory ranges"
" excedeed the max limit\n");
+ fclose(file);
return -1;
}
diff --git a/kexec/arch/ppc64/kexec-ppc64.c b/kexec/arch/ppc64/kexec-ppc64.c
index 4e70b13..5b17740 100644
--- a/kexec/arch/ppc64/kexec-ppc64.c
+++ b/kexec/arch/ppc64/kexec-ppc64.c
@@ -200,8 +200,10 @@ static int get_dyn_reconf_base_ranges(void)
fclose(file);
return -1;
}
- if (nr_memory_ranges >= max_memory_ranges)
+ if (nr_memory_ranges >= max_memory_ranges) {
+ fclose(file);
return -1;
+ }
/*
* If the property is ibm,dynamic-memory-v2, the first 4 bytes
diff --git a/kexec/symbols.c b/kexec/symbols.c
index e88f7f3..04377ca 100644
--- a/kexec/symbols.c
+++ b/kexec/symbols.c
@@ -24,11 +24,13 @@ unsigned long long get_kernel_sym(const char *symbol)
if (strcmp(sym, symbol) == 0) {
dbgprintf("kernel symbol %s vaddr = %16llx\n",
symbol, vaddr);
+ fclose(fp);
return vaddr;
}
}
dbgprintf("Cannot get kernel %s symbol address\n", symbol);
+ fclose(fp);
return 0;
}
--
2.27.0
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
