* [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).