[PATCH 6/7] contrib/elf2dmp: Continue even contexts are lacking

Akihiko Odaki posted 7 patches 8 months, 3 weeks ago
Maintainers: Viktor Prutyanov <viktor.prutyanov@phystech.edu>, Akihiko Odaki <akihiko.odaki@daynix.com>
There is a newer version of this series
[PATCH 6/7] contrib/elf2dmp: Continue even contexts are lacking
Posted by Akihiko Odaki 8 months, 3 weeks ago
Contexts of some CPUs may be lacking or corrupted due to premature boot,
but the output may still contain valuable information of other CPUs and
memory.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 contrib/elf2dmp/main.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 432f8629f321..33066310b9fa 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -332,7 +332,7 @@ static int fill_header(WinDumpHeader64 *hdr, struct pa_space *ps,
     return 0;
 }
 
-static int fill_context(KDDEBUGGER_DATA64 *kdbg,
+static void fill_context(KDDEBUGGER_DATA64 *kdbg,
         struct va_space *vs, QEMU_Elf *qe)
 {
     int i;
@@ -346,7 +346,7 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg,
         if (va_space_rw(vs, kdbg->KiProcessorBlock + sizeof(Prcb) * i,
                     &Prcb, sizeof(Prcb), 0)) {
             eprintf("Failed to read CPU #%d PRCB location\n", i);
-            return 1;
+            continue;
         }
 
         if (!Prcb) {
@@ -357,7 +357,7 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg,
         if (va_space_rw(vs, Prcb + kdbg->OffsetPrcbContext,
                     &Context, sizeof(Context), 0)) {
             eprintf("Failed to read CPU #%d ContextFrame location\n", i);
-            return 1;
+            continue;
         }
 
         printf("Filling context for CPU #%d...\n", i);
@@ -365,11 +365,9 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg,
 
         if (va_space_rw(vs, Context, &ctx, sizeof(ctx), 1)) {
             eprintf("Failed to fill CPU #%d context\n", i);
-            return 1;
+            continue;
         }
     }
-
-    return 0;
 }
 
 static int pe_get_data_dir_entry(uint64_t base, void *start_addr, int idx,
@@ -624,10 +622,7 @@ int main(int argc, char *argv[])
         goto out_kdbg;
     }
 
-    if (fill_context(kdbg, &vs, &qemu_elf)) {
-        err = 1;
-        goto out_kdbg;
-    }
+    fill_context(kdbg, &vs, &qemu_elf);
 
     if (write_dump(&ps, &header, argv[2])) {
         eprintf("Failed to save dump\n");

-- 
2.44.0
Re: [PATCH 6/7] contrib/elf2dmp: Continue even contexts are lacking
Posted by Peter Maydell 8 months, 3 weeks ago
On Sun, 3 Mar 2024 at 10:52, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> Contexts of some CPUs may be lacking or corrupted due to premature boot,
> but the output may still contain valuable information of other CPUs and
> memory.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  contrib/elf2dmp/main.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)

I think it would be worth having a brief comment explaining why
we don't make these memory read failures fatal errors, to
avoid somebody in future putting the error path back in again.

Otherwise

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM