Re: [PATCH v4 10/17] dump: Swap segment and section header locations

[Janis Schoetterl-Glausch]

You swapped the headers in patch 8, you just fixing up the elf header in this 
patch, right?
Also I don't understand the reason for swapping the headers.
And the comment diagram in dump_begin still reflects the old ordering.

On 7/26/22 11:22, Janosch Frank wrote:
> For the upcoming string table and arch section support we need to
> modify the elf layout a bit. Instead of the segments, i.e. the guest's
> memory contents, beeing the last area the section data will live at
> the end of the file. This will allow us to write the section data
> after all guest memory has been dumped which is important for the s390
> PV dump support.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  dump/dump.c           | 21 ++++++++++++---------
>  include/sysemu/dump.h |  1 +
>  2 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/dump/dump.c b/dump/dump.c
> index a6bb7bfa21..3cf846d0a0 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -588,6 +588,9 @@ static void dump_begin(DumpState *s, Error **errp)
>       *   --------------
>       *   |  memory     |
>       *   --------------
> +     *   |  sectn data |
> +     *   --------------
> +
>       *
>       * we only know where the memory is saved after we write elf note into
>       * vmcore.
> @@ -1852,18 +1855,18 @@ static void dump_init(DumpState *s, int fd, bool 
> has_format,
>          }
>      }
>  
> +    tmp = (s->phdr_num == PN_XNUM) ? s->sh_info : s->phdr_num;

You don't need this, do you? s->phdr_num is the correct value, it's the value
in the elf header that gets adjusted.

>      if (dump_is_64bit(s)) {
> -        s->phdr_offset = sizeof(Elf64_Ehdr);
> -        s->shdr_offset = s->phdr_offset + sizeof(Elf64_Phdr) * s->phdr_num;
> -        s->note_offset = s->shdr_offset + sizeof(Elf64_Shdr) * s->shdr_num;
> -        s->memory_offset = s->note_offset + s->note_size;
> +        s->shdr_offset = sizeof(Elf64_Ehdr);
> +        s->phdr_offset = s->shdr_offset + sizeof(Elf64_Shdr) * s->shdr_num;
> +        s->note_offset = s->phdr_offset + sizeof(Elf64_Phdr) * tmp;
>      } else {
> -
> -        s->phdr_offset = sizeof(Elf32_Ehdr);
> -        s->shdr_offset = s->phdr_offset + sizeof(Elf32_Phdr) * s->phdr_num;
> -        s->note_offset = s->shdr_offset + sizeof(Elf32_Shdr) * s->shdr_num;
> -        s->memory_offset = s->note_offset + s->note_size;
> +        s->shdr_offset = sizeof(Elf32_Ehdr);
> +        s->phdr_offset = s->shdr_offset + sizeof(Elf32_Shdr) * s->shdr_num;
> +        s->note_offset = s->phdr_offset + sizeof(Elf32_Phdr) * tmp;
>      }
> +    s->memory_offset = s->note_offset + s->note_size;
> +    s->section_offset = s->memory_offset + s->total_size;
>  
>      return;
>  
> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
> index 3937afe0f9..696e6c67d6 100644
> --- a/include/sysemu/dump.h
> +++ b/include/sysemu/dump.h
> @@ -172,6 +172,7 @@ typedef struct DumpState {
>      hwaddr shdr_offset;
>      hwaddr phdr_offset;
>      hwaddr note_offset;
> +    hwaddr section_offset;
>  
>      void *elf_header;
>      void *elf_section_hdrs;