| Message ID | 20220726092248.128336-9-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | dump: Add arch section and s390x PV dump | expand |
On 7/26/22 11:22, Janosch Frank 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. > > At the same time we move the writing of the section to the end of the > dump process. This allows the upcoming architecture section code to > add data after all of the common dump data has been written. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > dump/dump.c | 112 ++++++++++++++++++++++++++++++++---------- > include/sysemu/dump.h | 4 ++ > 2 files changed, 90 insertions(+), 26 deletions(-) > > diff --git a/dump/dump.c b/dump/dump.c > index 8a7ce95610..a6bb7bfa21 100644 > --- a/dump/dump.c > +++ b/dump/dump.c > @@ -342,30 +342,69 @@ static void write_elf_phdr_note(DumpState *s, Error **errp) > } > } > > -static void write_elf_section(DumpState *s, int type, Error **errp) > +static size_t prepare_elf_section_hdr_zero(DumpState *s) You don't use the return value and it is well known, the length of a section header entry. > { > - Elf32_Shdr shdr32; > - Elf64_Shdr shdr64; > - int shdr_size; > - void *shdr; > - int ret; > + if (dump_is_64bit(s)) { > + Elf64_Shdr *shdr64 = s->elf_section_hdrs; > > - 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; > + 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 = s->elf_section_hdrs; > + > + 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 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 +596,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 file descriptor */ > 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 +624,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); > } > @@ -647,6 +688,19 @@ static void dump_iterate(DumpState *s, Error **errp) > } > } > > +static void dump_end(DumpState *s, Error **errp) > +{ This function doesn't do anything yet, does it? Not sure if I like splitting the patches like that, but squashing them will blow up the patch size. Maybe you should just mention in the patch description which functionality remains dormant and under what circumstances it will become active. > + 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(); > @@ -657,6 +711,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 7f9a848573..686555f908 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 */
On 7/26/22 11:22, Janosch Frank 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. > > At the same time we move the writing of the section to the end of the > dump process. This allows the upcoming architecture section code to > add data after all of the common dump data has been written. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > dump/dump.c | 112 ++++++++++++++++++++++++++++++++---------- > include/sysemu/dump.h | 4 ++ > 2 files changed, 90 insertions(+), 26 deletions(-) > > diff --git a/dump/dump.c b/dump/dump.c > index 8a7ce95610..a6bb7bfa21 100644 > --- a/dump/dump.c > +++ b/dump/dump.c > @@ -342,30 +342,69 @@ static void write_elf_phdr_note(DumpState *s, Error **errp) > } > } > [...] > + > +static void prepare_elf_section_hdrs(DumpState *s) > +{ > + 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); > + > + /* Write special section first */ > + if (s->phdr_num == PN_XNUM) { Should be >= right? > + prepare_elf_section_hdr_zero(s); > + } > +} > + [...]
On 7/29/22 19:16, Janis Schoetterl-Glausch wrote: > On 7/26/22 11:22, Janosch Frank 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. >> >> At the same time we move the writing of the section to the end of the >> dump process. This allows the upcoming architecture section code to >> add data after all of the common dump data has been written. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> dump/dump.c | 112 ++++++++++++++++++++++++++++++++---------- >> include/sysemu/dump.h | 4 ++ >> 2 files changed, 90 insertions(+), 26 deletions(-) >> [...] >> + /* Write special section first */ >> + if (s->phdr_num == PN_XNUM) { > > Should be >= right? Yes, just fixed it. > >> + prepare_elf_section_hdr_zero(s); >> + } >> +} >> + > [...]
diff --git a/dump/dump.c b/dump/dump.c index 8a7ce95610..a6bb7bfa21 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -342,30 +342,69 @@ static void write_elf_phdr_note(DumpState *s, Error **errp) } } -static void write_elf_section(DumpState *s, int type, Error **errp) +static size_t prepare_elf_section_hdr_zero(DumpState *s) { - Elf32_Shdr shdr32; - Elf64_Shdr shdr64; - int shdr_size; - void *shdr; - int ret; + if (dump_is_64bit(s)) { + Elf64_Shdr *shdr64 = s->elf_section_hdrs; - 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; + 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 = s->elf_section_hdrs; + + 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) +{ + 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); + + /* Write special section first */ + if (s->phdr_num == PN_XNUM) { + prepare_elf_section_hdr_zero(s); + } +} + +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 headers"); + } +} + +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 +596,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 file descriptor */ 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 +624,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); } @@ -647,6 +688,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(); @@ -657,6 +711,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 7f9a848573..686555f908 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. At the same time we move the writing of the section to the end of the dump process. This allows the upcoming architecture section code to add data after all of the common dump data has been written. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- dump/dump.c | 112 ++++++++++++++++++++++++++++++++---------- include/sysemu/dump.h | 4 ++ 2 files changed, 90 insertions(+), 26 deletions(-)