| Message ID | 20220713130322.25517-4-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dump: Add arch section and s390x PV dump | expand |
Hi On Wed, Jul 13, 2022 at 5:07 PM Janosch Frank <frankja@linux.ibm.com> wrote: > > By splitting the writing of the section headers and (future) section > data we prepare for the addition of a string table section and > architecture sections. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > dump/dump.c | 116 ++++++++++++++++++++++++++++++++---------- > include/sysemu/dump.h | 4 ++ > 2 files changed, 94 insertions(+), 26 deletions(-) > > diff --git a/dump/dump.c b/dump/dump.c > index 16d7474258..467d934bc1 100644 > --- a/dump/dump.c > +++ b/dump/dump.c > @@ -342,30 +342,73 @@ static void write_elf_phdr_note(DumpState *s, Error **errp) > } > } > > -static void write_elf_section(DumpState *s, int type, Error **errp) > +static size_t write_elf_section_hdr_zero(DumpState *s, void *buff) Since the function no longer write, I'd suggest to rename it with prepare_ prefix > { > - Elf32_Shdr shdr32; > - Elf64_Shdr shdr64; > - int shdr_size; > - void *shdr; > - int ret; > + if (dump_is_64bit(s)) { > + Elf64_Shdr *shdr64 = buff; > > - if (type == 0) { > - shdr_size = sizeof(Elf32_Shdr); > - memset(&shdr32, 0, shdr_size); > - shdr32.sh_info = cpu_to_dump32(s, s->phdr_num); > - shdr = &shdr32; > + memset(buff, 0, sizeof(Elf64_Shdr)); You can drop this > + shdr64->sh_info = cpu_to_dump32(s, s->phdr_num); > } else { > - shdr_size = sizeof(Elf64_Shdr); > - memset(&shdr64, 0, shdr_size); > - shdr64.sh_info = cpu_to_dump32(s, s->phdr_num); > - shdr = &shdr64; > + Elf32_Shdr *shdr32 = buff; > + > + memset(buff, 0, sizeof(Elf32_Shdr)); and this > + shdr32->sh_info = cpu_to_dump32(s, s->phdr_num); > } > > - ret = fd_write_vmcore(shdr, shdr_size, s); > + return dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr); > +} > + > +static void prepare_elf_section_hdrs(DumpState *s) > +{ > + uint8_t *buff_hdr; > + size_t len, sizeof_shdr; > + > + /* > + * Section ordering: > + * - HDR zero (if needed) > + */ > + sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr); > + len = sizeof_shdr * s->shdr_num; > + s->elf_section_hdrs = g_malloc0(len); since you alloc0 here > + buff_hdr = s->elf_section_hdrs; > + > + /* Write special section first */ > + if (s->phdr_num == PN_XNUM) { > + write_elf_section_hdr_zero(s, buff_hdr); Eventually, drop buff_hdr, and pass only "s" as argument + Indentation is off > + } > +} > + > +static void prepare_elf_sections(DumpState *s, Error **errp) > +{ > + if (!s->shdr_num) { > + return; > + } > + > + prepare_elf_section_hdrs(s); > +} > + > +static void write_elf_section_headers(DumpState *s, Error **errp) > +{ > + size_t sizeof_shdr; > + int ret; > + > + sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr); > + > + ret = fd_write_vmcore(s->elf_section_hdrs, s->shdr_num * sizeof_shdr, s); > if (ret < 0) { > - error_setg_errno(errp, -ret, > - "dump: failed to write section header table"); > + error_setg_errno(errp, -ret, "dump: failed to write section data"); nit: data->header > + } > +} > + > +static void write_elf_sections(DumpState *s, Error **errp) > +{ > + int ret; > + > + /* Write section zero */ > + ret = fd_write_vmcore(s->elf_section_data, s->elf_section_data_size, s); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "dump: failed to write section data"); > } > } > > @@ -557,12 +600,22 @@ static void dump_begin(DumpState *s, Error **errp) > /* Write elf header to buffer */ > prepare_elf_header(s); > > + prepare_elf_sections(s, errp); > + if (*errp) { > + return; > + } > + > /* Start to write stuff into files*/ > write_elf_header(s, errp); > if (*errp) { > return; > } > > + write_elf_section_headers(s, errp); Why do you reorder the sections? Could you explain in the commit message why? Is this is format compliant? and update the comment above? thanks > + if (*errp) { > + return; > + } > + > /* write PT_NOTE to vmcore */ > write_elf_phdr_note(s, errp); > if (*errp) { > @@ -575,14 +628,6 @@ static void dump_begin(DumpState *s, Error **errp) > return; > } > > - /* write section to vmcore */ > - if (s->shdr_num) { > - write_elf_section(s, 1, errp); > - if (*errp) { > - return; > - } > - } > - > /* write notes to vmcore */ > write_elf_notes(s, errp); > } > @@ -610,6 +655,19 @@ static void dump_iterate(DumpState *s, Error **errp) > } > } > > +static void dump_end(DumpState *s, Error **errp) > +{ > + ERRP_GUARD(); > + > + if (!s->elf_section_data_size) { > + return; > + } > + s->elf_section_data = g_malloc0(s->elf_section_data_size); > + > + /* write sections to vmcore */ > + write_elf_sections(s, errp); > +} > + > static void create_vmcore(DumpState *s, Error **errp) > { > ERRP_GUARD(); > @@ -620,6 +678,12 @@ static void create_vmcore(DumpState *s, Error **errp) > } > > dump_iterate(s, errp); > + if (*errp) { > + return; > + } > + > + /* Write section data after memory has been dumped */ > + dump_end(s, errp); > } > > static int write_start_flat_header(int fd) > diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h > index 736f681d01..bd49532232 100644 > --- a/include/sysemu/dump.h > +++ b/include/sysemu/dump.h > @@ -172,6 +172,10 @@ typedef struct DumpState { > int64_t length; /* Length of the dump we want to dump */ > > void *elf_header; > + void *elf_section_hdrs; > + uint64_t elf_section_data_size; > + void *elf_section_data; > + > uint8_t *note_buf; /* buffer for notes */ > size_t note_buf_offset; /* the writing place in note_buf */ > uint32_t nr_cpus; /* number of guest's cpu */ > -- > 2.34.1 >
On 7/13/22 17:31, Marc-André Lureau wrote: > Hi > > On Wed, Jul 13, 2022 at 5:07 PM Janosch Frank <frankja@linux.ibm.com> wrote: >> >> By splitting the writing of the section headers and (future) section >> data we prepare for the addition of a string table section and >> architecture sections. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- [...] >> @@ -557,12 +600,22 @@ static void dump_begin(DumpState *s, Error **errp) >> /* Write elf header to buffer */ >> prepare_elf_header(s); >> >> + prepare_elf_sections(s, errp); >> + if (*errp) { >> + return; >> + } >> + >> /* Start to write stuff into files*/ >> write_elf_header(s, errp); >> if (*errp) { >> return; >> } >> >> + write_elf_section_headers(s, errp); > > Why do you reorder the sections? Could you explain in the commit > message why? Is this is format compliant? and update the comment > above? thanks Having the section data at the end of the file is unfortunately a s390 PV requirement since we can only grab the encrypted page tweaks and counts *after* all of the memory has been encrypted. The sections are the most obvious way to add such data to the file since they are basically unused right now and we're able to write a string table at the very end after everyone registered their strings. All of this is ELF compliant AFAIK, that's why elf specifies offsets of the headers and the data. From what I see only the main elf header needs to start at offset 0.
diff --git a/dump/dump.c b/dump/dump.c index 16d7474258..467d934bc1 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -342,30 +342,73 @@ static void write_elf_phdr_note(DumpState *s, Error **errp) } } -static void write_elf_section(DumpState *s, int type, Error **errp) +static size_t write_elf_section_hdr_zero(DumpState *s, void *buff) { - Elf32_Shdr shdr32; - Elf64_Shdr shdr64; - int shdr_size; - void *shdr; - int ret; + if (dump_is_64bit(s)) { + Elf64_Shdr *shdr64 = buff; - if (type == 0) { - shdr_size = sizeof(Elf32_Shdr); - memset(&shdr32, 0, shdr_size); - shdr32.sh_info = cpu_to_dump32(s, s->phdr_num); - shdr = &shdr32; + memset(buff, 0, sizeof(Elf64_Shdr)); + shdr64->sh_info = cpu_to_dump32(s, s->phdr_num); } else { - shdr_size = sizeof(Elf64_Shdr); - memset(&shdr64, 0, shdr_size); - shdr64.sh_info = cpu_to_dump32(s, s->phdr_num); - shdr = &shdr64; + Elf32_Shdr *shdr32 = buff; + + memset(buff, 0, sizeof(Elf32_Shdr)); + shdr32->sh_info = cpu_to_dump32(s, s->phdr_num); } - ret = fd_write_vmcore(shdr, shdr_size, s); + return dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr); +} + +static void prepare_elf_section_hdrs(DumpState *s) +{ + uint8_t *buff_hdr; + size_t len, sizeof_shdr; + + /* + * Section ordering: + * - HDR zero (if needed) + */ + sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr); + len = sizeof_shdr * s->shdr_num; + s->elf_section_hdrs = g_malloc0(len); + buff_hdr = s->elf_section_hdrs; + + /* Write special section first */ + if (s->phdr_num == PN_XNUM) { + write_elf_section_hdr_zero(s, buff_hdr); + } +} + +static void prepare_elf_sections(DumpState *s, Error **errp) +{ + if (!s->shdr_num) { + return; + } + + prepare_elf_section_hdrs(s); +} + +static void write_elf_section_headers(DumpState *s, Error **errp) +{ + size_t sizeof_shdr; + int ret; + + sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr); + + ret = fd_write_vmcore(s->elf_section_hdrs, s->shdr_num * sizeof_shdr, s); if (ret < 0) { - error_setg_errno(errp, -ret, - "dump: failed to write section header table"); + error_setg_errno(errp, -ret, "dump: failed to write section data"); + } +} + +static void write_elf_sections(DumpState *s, Error **errp) +{ + int ret; + + /* Write section zero */ + ret = fd_write_vmcore(s->elf_section_data, s->elf_section_data_size, s); + if (ret < 0) { + error_setg_errno(errp, -ret, "dump: failed to write section data"); } } @@ -557,12 +600,22 @@ static void dump_begin(DumpState *s, Error **errp) /* Write elf header to buffer */ prepare_elf_header(s); + prepare_elf_sections(s, errp); + if (*errp) { + return; + } + /* Start to write stuff into files*/ write_elf_header(s, errp); if (*errp) { return; } + write_elf_section_headers(s, errp); + if (*errp) { + return; + } + /* write PT_NOTE to vmcore */ write_elf_phdr_note(s, errp); if (*errp) { @@ -575,14 +628,6 @@ static void dump_begin(DumpState *s, Error **errp) return; } - /* write section to vmcore */ - if (s->shdr_num) { - write_elf_section(s, 1, errp); - if (*errp) { - return; - } - } - /* write notes to vmcore */ write_elf_notes(s, errp); } @@ -610,6 +655,19 @@ static void dump_iterate(DumpState *s, Error **errp) } } +static void dump_end(DumpState *s, Error **errp) +{ + ERRP_GUARD(); + + if (!s->elf_section_data_size) { + return; + } + s->elf_section_data = g_malloc0(s->elf_section_data_size); + + /* write sections to vmcore */ + write_elf_sections(s, errp); +} + static void create_vmcore(DumpState *s, Error **errp) { ERRP_GUARD(); @@ -620,6 +678,12 @@ static void create_vmcore(DumpState *s, Error **errp) } dump_iterate(s, errp); + if (*errp) { + return; + } + + /* Write section data after memory has been dumped */ + dump_end(s, errp); } static int write_start_flat_header(int fd) diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h index 736f681d01..bd49532232 100644 --- a/include/sysemu/dump.h +++ b/include/sysemu/dump.h @@ -172,6 +172,10 @@ typedef struct DumpState { int64_t length; /* Length of the dump we want to dump */ void *elf_header; + void *elf_section_hdrs; + uint64_t elf_section_data_size; + void *elf_section_data; + uint8_t *note_buf; /* buffer for notes */ size_t note_buf_offset; /* the writing place in note_buf */ uint32_t nr_cpus; /* number of guest's cpu */
By splitting the writing of the section headers and (future) section data we prepare for the addition of a string table section and architecture sections. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- dump/dump.c | 116 ++++++++++++++++++++++++++++++++---------- include/sysemu/dump.h | 4 ++ 2 files changed, 94 insertions(+), 26 deletions(-)