* [PATCH] elf2dmp: Don't abandon when Prcb is set to 0 @ 2023-06-11 3:34 Akihiko Odaki 2023-06-12 10:42 ` Viktor Prutyanov 0 siblings, 1 reply; 6+ messages in thread From: Akihiko Odaki @ 2023-06-11 3:34 UTC (permalink / raw) Cc: qemu-devel, Viktor Prutyanov, Akihiko Odaki Prcb may be set to 0 for some CPUs if the dump was taken before they start. The dump may still contain valuable information for started CPUs so don't abandon conversion in such a case. Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> --- contrib/elf2dmp/main.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c index d77b8f98f7..91c58e4424 100644 --- a/contrib/elf2dmp/main.c +++ b/contrib/elf2dmp/main.c @@ -312,6 +312,11 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg, return 1; } + if (!Prcb) { + eprintf("Context for CPU #%d is missing\n", i); + continue; + } + if (va_space_rw(vs, Prcb + kdbg->OffsetPrcbContext, &Context, sizeof(Context), 0)) { eprintf("Failed to read CPU #%d ContextFrame location\n", i); -- 2.40.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] elf2dmp: Don't abandon when Prcb is set to 0 2023-06-11 3:34 [PATCH] elf2dmp: Don't abandon when Prcb is set to 0 Akihiko Odaki @ 2023-06-12 10:42 ` Viktor Prutyanov 2023-06-13 0:39 ` Akihiko Odaki 0 siblings, 1 reply; 6+ messages in thread From: Viktor Prutyanov @ 2023-06-12 10:42 UTC (permalink / raw) To: Akihiko Odaki; +Cc: qemu-devel, Viktor Prutyanov > Prcb may be set to 0 for some CPUs if the dump was taken before they > start. The dump may still contain valuable information for started CPUs > so don't abandon conversion in such a case. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > --- > contrib/elf2dmp/main.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c > index d77b8f98f7..91c58e4424 100644 > --- a/contrib/elf2dmp/main.c > +++ b/contrib/elf2dmp/main.c > @@ -312,6 +312,11 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg, > return 1; > } > > + if (!Prcb) { > + eprintf("Context for CPU #%d is missing\n", i); > + continue; > + } > + > if (va_space_rw(vs, Prcb + kdbg->OffsetPrcbContext, > &Context, sizeof(Context), 0)) { > eprintf("Failed to read CPU #%d ContextFrame location\n", i); > > -- > 2.40.1 Hi Akihiko, How this fix can be tested? NumberProcessors field is still set to qemu_elf.state_nr, how does WinDbg react to this? Viktor ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] elf2dmp: Don't abandon when Prcb is set to 0 2023-06-12 10:42 ` Viktor Prutyanov @ 2023-06-13 0:39 ` Akihiko Odaki 2023-06-16 12:05 ` Viktor Prutyanov 0 siblings, 1 reply; 6+ messages in thread From: Akihiko Odaki @ 2023-06-13 0:39 UTC (permalink / raw) To: Viktor Prutyanov; +Cc: qemu-devel, Viktor Prutyanov On 2023/06/12 12:42, Viktor Prutyanov wrote: >> Prcb may be set to 0 for some CPUs if the dump was taken before they >> start. The dump may still contain valuable information for started CPUs >> so don't abandon conversion in such a case. >> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >> --- >> contrib/elf2dmp/main.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c >> index d77b8f98f7..91c58e4424 100644 >> --- a/contrib/elf2dmp/main.c >> +++ b/contrib/elf2dmp/main.c >> @@ -312,6 +312,11 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg, >> return 1; >> } >> >> + if (!Prcb) { >> + eprintf("Context for CPU #%d is missing\n", i); >> + continue; >> + } >> + >> if (va_space_rw(vs, Prcb + kdbg->OffsetPrcbContext, >> &Context, sizeof(Context), 0)) { >> eprintf("Failed to read CPU #%d ContextFrame location\n", i); >> >> -- >> 2.40.1 > > Hi Akihiko, > > How this fix can be tested? It is a bit difficult to test it as you need to interrupt the very early stage of boot. I applied the following change to TCG so that it stops immediately after the first processor configures Prcb. diff --git a/target/i386/tcg/sysemu/misc_helper.c b/target/i386/tcg/sysemu/misc_helper.c index e1528b7f80..f68eba9cac 100644 --- a/target/i386/tcg/sysemu/misc_helper.c +++ b/target/i386/tcg/sysemu/misc_helper.c @@ -25,6 +25,9 @@ #include "exec/address-spaces.h" #include "exec/exec-all.h" #include "tcg/helper-tcg.h" +#include "exec/gdbstub.h" +#include "hw/core/cpu.h" +#include "sysemu/runstate.h" void helper_outb(CPUX86State *env, uint32_t port, uint32_t data) { @@ -217,7 +220,10 @@ void helper_wrmsr(CPUX86State *env) env->segs[R_FS].base = val; break; case MSR_GSBASE: + printf("%s: %" PRIx64 "\n", __func__, val); env->segs[R_GS].base = val; + gdb_set_stop_cpu(current_cpu); + vm_stop(RUN_STATE_PAUSED); break; case MSR_KERNELGSBASE: env->kernelgsbase = val; > > NumberProcessors field is still set to qemu_elf.state_nr, how does WinDbg react to this? If Prcb for processor 1 is missing, WinDbg outputs: KiProcessorBlock[1] is null. You can still debug the started processors with no issue. Regards, Akihiko Odaki > > Viktor ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] elf2dmp: Don't abandon when Prcb is set to 0 2023-06-13 0:39 ` Akihiko Odaki @ 2023-06-16 12:05 ` Viktor Prutyanov 2023-07-30 19:52 ` Viktor Prutyanov 0 siblings, 1 reply; 6+ messages in thread From: Viktor Prutyanov @ 2023-06-16 12:05 UTC (permalink / raw) To: Akihiko Odaki; +Cc: qemu-devel, Viktor Prutyanov > On 2023/06/12 12:42, Viktor Prutyanov wrote: > >>> Prcb may be set to 0 for some CPUs if the dump was taken before they >>> start. The dump may still contain valuable information for started CPUs >>> so don't abandon conversion in such a case. >>> >>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>> --- >>> contrib/elf2dmp/main.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c >>> index d77b8f98f7..91c58e4424 100644 >>> --- a/contrib/elf2dmp/main.c >>> +++ b/contrib/elf2dmp/main.c >>> @@ -312,6 +312,11 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg, >>> return 1; >>> } >>> >>> + if (!Prcb) { >>> + eprintf("Context for CPU #%d is missing\n", i); >>> + continue; >>> + } >>> + >>> if (va_space_rw(vs, Prcb + kdbg->OffsetPrcbContext, >>> &Context, sizeof(Context), 0)) { >>> eprintf("Failed to read CPU #%d ContextFrame location\n", i); >>> >>> -- >>> 2.40.1 >> >> Hi Akihiko, >> >> How this fix can be tested? > > It is a bit difficult to test it as you need to interrupt the very early > stage of boot. I applied the following change to TCG so that it stops > immediately after the first processor configures Prcb. > > diff --git a/target/i386/tcg/sysemu/misc_helper.c > b/target/i386/tcg/sysemu/misc_helper.c > index e1528b7f80..f68eba9cac 100644 > --- a/target/i386/tcg/sysemu/misc_helper.c > +++ b/target/i386/tcg/sysemu/misc_helper.c > @@ -25,6 +25,9 @@ > #include "exec/address-spaces.h" > #include "exec/exec-all.h" > #include "tcg/helper-tcg.h" > +#include "exec/gdbstub.h" > +#include "hw/core/cpu.h" > +#include "sysemu/runstate.h" > > void helper_outb(CPUX86State *env, uint32_t port, uint32_t data) > { > @@ -217,7 +220,10 @@ void helper_wrmsr(CPUX86State *env) > env->segs[R_FS].base = val; > break; > case MSR_GSBASE: > + printf("%s: %" PRIx64 "\n", __func__, val); > env->segs[R_GS].base = val; > + gdb_set_stop_cpu(current_cpu); > + vm_stop(RUN_STATE_PAUSED); > break; > case MSR_KERNELGSBASE: > env->kernelgsbase = val; > >> NumberProcessors field is still set to qemu_elf.state_nr, how does WinDbg react to this? > > If Prcb for processor 1 is missing, WinDbg outputs: KiProcessorBlock[1] > is null. > You can still debug the started processors with no issue. > > Regards, > Akihiko Odaki > >> Viktor Reviewed-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] elf2dmp: Don't abandon when Prcb is set to 0 2023-06-16 12:05 ` Viktor Prutyanov @ 2023-07-30 19:52 ` Viktor Prutyanov 2023-07-31 10:21 ` Peter Maydell 0 siblings, 1 reply; 6+ messages in thread From: Viktor Prutyanov @ 2023-07-30 19:52 UTC (permalink / raw) To: peter.maydell; +Cc: Akihiko Odaki, qemu-devel, viktor >> On 2023/06/12 12:42, Viktor Prutyanov wrote: >> >>>> Prcb may be set to 0 for some CPUs if the dump was taken before they >>>> start. The dump may still contain valuable information for started CPUs >>>> so don't abandon conversion in such a case. >>>> >>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>>> --- >>>> contrib/elf2dmp/main.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c >>>> index d77b8f98f7..91c58e4424 100644 >>>> --- a/contrib/elf2dmp/main.c >>>> +++ b/contrib/elf2dmp/main.c >>>> @@ -312,6 +312,11 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg, >>>> return 1; >>>> } >>>> >>>> + if (!Prcb) { >>>> + eprintf("Context for CPU #%d is missing\n", i); >>>> + continue; >>>> + } >>>> + >>>> if (va_space_rw(vs, Prcb + kdbg->OffsetPrcbContext, >>>> &Context, sizeof(Context), 0)) { >>>> eprintf("Failed to read CPU #%d ContextFrame location\n", i); >>>> >>>> -- >>>> 2.40.1 >>> >>> Hi Akihiko, >>> >>> How this fix can be tested? >> >> It is a bit difficult to test it as you need to interrupt the very early >> stage of boot. I applied the following change to TCG so that it stops >> immediately after the first processor configures Prcb. >> >> diff --git a/target/i386/tcg/sysemu/misc_helper.c >> b/target/i386/tcg/sysemu/misc_helper.c >> index e1528b7f80..f68eba9cac 100644 >> --- a/target/i386/tcg/sysemu/misc_helper.c >> +++ b/target/i386/tcg/sysemu/misc_helper.c >> @@ -25,6 +25,9 @@ >> #include "exec/address-spaces.h" >> #include "exec/exec-all.h" >> #include "tcg/helper-tcg.h" >> +#include "exec/gdbstub.h" >> +#include "hw/core/cpu.h" >> +#include "sysemu/runstate.h" >> >> void helper_outb(CPUX86State *env, uint32_t port, uint32_t data) >> { >> @@ -217,7 +220,10 @@ void helper_wrmsr(CPUX86State *env) >> env->segs[R_FS].base = val; >> break; >> case MSR_GSBASE: >> + printf("%s: %" PRIx64 "\n", __func__, val); >> env->segs[R_GS].base = val; >> + gdb_set_stop_cpu(current_cpu); >> + vm_stop(RUN_STATE_PAUSED); >> break; >> case MSR_KERNELGSBASE: >> env->kernelgsbase = val; >> >>> NumberProcessors field is still set to qemu_elf.state_nr, how does WinDbg react to this? >> >> If Prcb for processor 1 is missing, WinDbg outputs: KiProcessorBlock[1] >> is null. >> You can still debug the started processors with no issue. >> >> Regards, >> Akihiko Odaki >> >>> Viktor > > Reviewed-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu> Hi Peter, Could you please put Akihiko's patch into your branch? Thank you, Viktor Prutyanov ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] elf2dmp: Don't abandon when Prcb is set to 0 2023-07-30 19:52 ` Viktor Prutyanov @ 2023-07-31 10:21 ` Peter Maydell 0 siblings, 0 replies; 6+ messages in thread From: Peter Maydell @ 2023-07-31 10:21 UTC (permalink / raw) To: Viktor Prutyanov; +Cc: Akihiko Odaki, qemu-devel, viktor On Sun, 30 Jul 2023 at 20:52, Viktor Prutyanov <viktor.prutyanov@phystech.edu> wrote: > > > >> On 2023/06/12 12:42, Viktor Prutyanov wrote: > >> > >>>> Prcb may be set to 0 for some CPUs if the dump was taken before they > >>>> start. The dump may still contain valuable information for started CPUs > >>>> so don't abandon conversion in such a case. > >>>> > >>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > >>>> --- > >>>> contrib/elf2dmp/main.c | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c > >>>> index d77b8f98f7..91c58e4424 100644 > >>>> --- a/contrib/elf2dmp/main.c > >>>> +++ b/contrib/elf2dmp/main.c > >>>> @@ -312,6 +312,11 @@ static int fill_context(KDDEBUGGER_DATA64 *kdbg, > >>>> return 1; > >>>> } > >>>> > >>>> + if (!Prcb) { > >>>> + eprintf("Context for CPU #%d is missing\n", i); > >>>> + continue; > >>>> + } > >>>> + > >>>> if (va_space_rw(vs, Prcb + kdbg->OffsetPrcbContext, > >>>> &Context, sizeof(Context), 0)) { > >>>> eprintf("Failed to read CPU #%d ContextFrame location\n", i); > >>>> > >>>> -- > >>>> 2.40.1 > >>> > >>> Hi Akihiko, > >>> > >>> How this fix can be tested? > >> > >> It is a bit difficult to test it as you need to interrupt the very early > >> stage of boot. I applied the following change to TCG so that it stops > >> immediately after the first processor configures Prcb. > >> > >> diff --git a/target/i386/tcg/sysemu/misc_helper.c > >> b/target/i386/tcg/sysemu/misc_helper.c > >> index e1528b7f80..f68eba9cac 100644 > >> --- a/target/i386/tcg/sysemu/misc_helper.c > >> +++ b/target/i386/tcg/sysemu/misc_helper.c > >> @@ -25,6 +25,9 @@ > >> #include "exec/address-spaces.h" > >> #include "exec/exec-all.h" > >> #include "tcg/helper-tcg.h" > >> +#include "exec/gdbstub.h" > >> +#include "hw/core/cpu.h" > >> +#include "sysemu/runstate.h" > >> > >> void helper_outb(CPUX86State *env, uint32_t port, uint32_t data) > >> { > >> @@ -217,7 +220,10 @@ void helper_wrmsr(CPUX86State *env) > >> env->segs[R_FS].base = val; > >> break; > >> case MSR_GSBASE: > >> + printf("%s: %" PRIx64 "\n", __func__, val); > >> env->segs[R_GS].base = val; > >> + gdb_set_stop_cpu(current_cpu); > >> + vm_stop(RUN_STATE_PAUSED); > >> break; > >> case MSR_KERNELGSBASE: > >> env->kernelgsbase = val; > >> > >>> NumberProcessors field is still set to qemu_elf.state_nr, how does WinDbg react to this? > >> > >> If Prcb for processor 1 is missing, WinDbg outputs: KiProcessorBlock[1] > >> is null. > >> You can still debug the started processors with no issue. > >> > >> Regards, > >> Akihiko Odaki > >> > >>> Viktor > > > > Reviewed-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu> > > Hi Peter, > > Could you please put Akihiko's patch into your branch? Sure. I've applied this to target-arm.next and should be doing a pullreq for it later today or tomorrow. -- PMM ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-31 10:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-11 3:34 [PATCH] elf2dmp: Don't abandon when Prcb is set to 0 Akihiko Odaki 2023-06-12 10:42 ` Viktor Prutyanov 2023-06-13 0:39 ` Akihiko Odaki 2023-06-16 12:05 ` Viktor Prutyanov 2023-07-30 19:52 ` Viktor Prutyanov 2023-07-31 10:21 ` Peter Maydell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).