Re: [PATCH RFC 3/6] arm64: allocate memory for other segments after kernel

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Geoff,

On 04/21/2015 07:06 PM, AKASHI Takahiro wrote:
> Prayush,
> Cc: Geoff
>
> Please add me to Cc if you intend to modify my part of kexec-tools since I don't
> subscribe kexec ML, and keep in mind that my repo on git.linaro.org is not for
> public review.
>
> On 04/21/2015 01:19 PM, Pratyush Anand wrote:
>>
>>
>> On Monday 20 April 2015 08:51 AM, Baoquan He wrote:
>>> On 04/16/15 at 10:17pm, Pratyush Anand wrote:
>>>> In case of KEXEC_ON_CRASH, load other segments after the addresses where
>>>> kernel segments finish.
>>>>
>>>> Signed-off-by: Pratyush Anand <panand@redhat.com>
>>>> ---
>>>>   kexec/arch/arm64/crashdump-arm64.c | 3 ++-
>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/kexec/arch/arm64/crashdump-arm64.c b/kexec/arch/arm64/crashdump-arm64.c
>>>> index 41266f294589..75f4e4d269ca 100644
>>>> --- a/kexec/arch/arm64/crashdump-arm64.c
>>>> +++ b/kexec/arch/arm64/crashdump-arm64.c
>>>> @@ -312,5 +312,6 @@ void set_crash_entry(struct mem_ehdr *ehdr, struct kexec_info *info)
>>>>   off_t locate_dtb_in_crashmem(struct kexec_info *info, off_t dtb_size)
>>>>   {
>>>>       return locate_hole(info, dtb_size, 128UL * 1024,
>>>> -        crash_reserved_mem.start, crash_reserved_mem.end, 1);
>>>> +        crash_reserved_mem.start + arm64_mem.text_offset +
>>>> +        arm64_mem.image_size, crash_reserved_mem.end, 1);
>>>
>>> So you have decided to hard code the sequence of segment. It
>>> seems not good. Why don't do it by calling add_buffer_phys_virt() or
>>> implement a similar function if you don't like add_buffer_phys_virt.
>>
>> I agree that code is a bit tightly coupled here. I think, Takahiro  can comment better.
>
> The *hard-coded* segments order is attributed to Geoff's original code.
>
>> arm64 does use add_buffer_phys_virt to add all the buffers into segment list. arm64_load_other_segments function adds
>> buffers for all segments other than kernel and elfcorehdr. This function calls add_buffer_phys_virt to load different
>> segments like dtb, initrd and purgatory. Only limitation we are putting that we find free area for all these segments
>> after the area where kernel finishes. Currently load sequence is like dtb, initrd and then purgatory. But I believe,
>> even if we use other load sequence in arm64_load_other_segments, it should work.
>
> I think that add_buffer_phys_virt() should be used instead of locate_hole() to remove
> this enforced ordering because locate_hole() assumes that segments in struct info
> be sorted properly.
> I'm trying the following hack: (pls ignore #if's)

If you agree, please think of taking my patch (the last commit) in v0.12 in my repo.
(I tested the code in case of kexec as well as kdump with 4MB initrd.)

-Takahiro AKASHI

> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 2c5238c..642c4a9 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -583,6 +583,7 @@ int arm64_load_other_segments(struct kexec_info *info,
>          struct mem_ehdr ehdr;
>          unsigned long dtb_max;
>          unsigned long dtb_base;
> +       unsigned long hole_min, hole_max;
>          char *initrd_buf = NULL;
>          uint64_t purgatory_sink;
>          unsigned long purgatory_base;
> @@ -639,17 +640,28 @@ int arm64_load_other_segments(struct kexec_info *info,
>           * to the DTB size for any DTB growth.
>           */
>
> -       dtb_max = dtb_2.size + 2 * 1024;
> +       if (info->kexec_flags & KEXEC_ON_CRASH) {
> +#if 1
> +               hole_min = crash_reserved_mem.start;
> +#else
> +               hole_min = crash_reserved_mem.start + arm64_mem.text_offset
> +                               + arm64_mem.image_size;
> +#endif
> +               hole_max = crash_reserved_mem.end;
> +       } else {
> +               hole_min = arm64_mem.memstart + arm64_mem.text_offset
> +                               + arm64_mem.image_size;
> +               hole_max = ULONG_MAX;
> +       }
>
> -       if (info->kexec_flags & KEXEC_ON_CRASH)
> -               dtb_base = locate_dtb_in_crashmem(info, dtb_max);
> -       else
> -               dtb_base = locate_hole(info, dtb_max, 128UL * 1024,
> -                       arm64_mem.memstart + arm64_mem.text_offset
> -                               + arm64_mem.image_size,
> -                       _ALIGN_UP(arm64_mem.memstart + arm64_mem.text_offset,
> -                               512UL * 1024 * 1024),
> -                       1);
> +       dtb_max = dtb_2.size + 2 * 1024;
> +#if 1
> +       dtb_base = add_buffer_phys_virt(info, dtb_2.buf, dtb_2.size, dtb_max,
> +                       128UL * 1024, hole_min, hole_max, 1, 0);
> +#else
> +       dtb_base = locate_hole(info, dtb_max, 128UL * 1024,
> +                       hole_min, hole_max, 1);
> +#endif
>          dbgprintf("dtb:    base %lx, size %lxh (%ld)\n", dtb_base, dtb_2.size,
>                  dtb_2.size);
> @@ -670,8 +686,14 @@ int arm64_load_other_segments(struct kexec_info *info,
>                          /* Put the initrd after the DTB with an alignment of
>                           * page size. */
>
> +#if 1
> +                       initrd_base = add_buffer_phys_virt(info, initrd_buf,
> +                               initrd_size, initrd_size, 0,
> +                               hole_min, hole_max, 1, 0);
> +#else
>                          initrd_base = locate_hole(info, initrd_size, 0,
> -                               dtb_base + dtb_max, -1, 1);
> +                               dtb_base + dtb_max, hole_max, 1);
> +#endif
>
>                          dbgprintf("initrd: base %lx, size %lxh (%ld)\n",
>                                  initrd_base, initrd_size, initrd_size);
> @@ -695,12 +717,14 @@ int arm64_load_other_segments(struct kexec_info *info,
>                  return -EINVAL;
>          }
>
> +#if 0
>          add_segment_phys_virt(info, dtb_2.buf, dtb_2.size, dtb_base,
>                  dtb_2.size, 0);
>
>          if (arm64_opts.initrd)
>                  add_segment_phys_virt(info, initrd_buf, initrd_size,
>                                  initrd_base, initrd_size, 0);
> +#endif
>
>          if (arm64_opts.lite)
>                  info->entry = (void *)kernel_entry;
> @@ -714,8 +738,13 @@ int arm64_load_other_segments(struct kexec_info *info,
>                          return -EBADF;
>                  }
>
> +#if 1
> +               elf_rel_build_load(info, &info->rhdr, purgatory, purgatory_size,
> +                       hole_min, hole_max, 1, 0);
> +#else
>                  elf_rel_build_load(info, &info->rhdr, purgatory, purgatory_size,
> -                       purgatory_base, ULONG_MAX, 1, 0);
> +                       purgatory_base, hole_max, 1, 0);
> +#endif
>
>                  info->entry = (void *)elf_rel_get_addr(&info->rhdr,
>                          "purgatory_start");
>
> Thanks,
> -Takahiro AKASHI
>
>>
>> Not sure, if understood the question and replied well.
>>
>> ~Pratyush
>>
>>
>>
>>>
>>>>   }
>>>> --
>>>> 2.1.0
>>>>
>>>>
>>>> _______________________________________________
>>>> kexec mailing list
>>>> kexec@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/kexec
>>>
>>> _______________________________________________
>>> kexec mailing list
>>> kexec@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/kexec
>>>

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