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
> 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
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
> 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>
>> 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
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
© 2016 - 2024 Red Hat, Inc.