| Message ID | 20240320015025.372056-2-bgray@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] target/ppc: Restore [H]DEXCR to 64-bits | expand |
On Wed Mar 20, 2024 at 11:50 AM AEST, Benjamin Gray wrote: > The GDB server protocol assigns an arbitrary numbering of the SPRs. > We track this correspondence on each SPR with gdb_id, using it to > resolve any SPR requests GDB makes. > > Early on we generate an XML representation of the SPRs to give GDB, > including this numbering. However the XML is cached globally, and we > skip setting the SPR gdb_id values on subsequent threads if we detect > it is cached. This causes QEMU to fail to resolve SPR requests against > secondary CPUs because it cannot find the matching gdb_id value on that > thread's SPRs. > > This is a minimal fix to first assign the gdb_id values, then return > early if the XML is cached. Otherwise we generate the XML using the > now already initialised gdb_id values. Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > > Fixes: 1b53948ff8f7 ("target/ppc: Use GDBFeature for dynamic XML") > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> > --- > target/ppc/gdbstub.c | 31 ++++++++++++++++++++----------- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c > index 3f1e61bdb7..3b28d4e21c 100644 > --- a/target/ppc/gdbstub.c > +++ b/target/ppc/gdbstub.c > @@ -305,14 +305,6 @@ static void gdb_gen_spr_feature(CPUState *cs) > unsigned int num_regs = 0; > int i; > > - if (pcc->gdb_spr.xml) { > - return; > - } > - > - gdb_feature_builder_init(&builder, &pcc->gdb_spr, > - "org.qemu.power.spr", "power-spr.xml", > - cs->gdb_num_regs); > - > for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { > ppc_spr_t *spr = &env->spr_cb[i]; > > @@ -320,9 +312,6 @@ static void gdb_gen_spr_feature(CPUState *cs) > continue; > } > > - gdb_feature_builder_append_reg(&builder, g_ascii_strdown(spr->name, -1), > - TARGET_LONG_BITS, num_regs, > - "int", "spr"); > /* > * GDB identifies registers based on the order they are > * presented in the XML. These ids will not match QEMU's > @@ -335,6 +324,26 @@ static void gdb_gen_spr_feature(CPUState *cs) > num_regs++; > } > > + if (pcc->gdb_spr.xml) { > + return; > + } > + > + gdb_feature_builder_init(&builder, &pcc->gdb_spr, > + "org.qemu.power.spr", "power-spr.xml", > + cs->gdb_num_regs); > + > + for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { > + ppc_spr_t *spr = &env->spr_cb[i]; > + > + if (!spr->name) { > + continue; > + } > + > + gdb_feature_builder_append_reg(&builder, g_ascii_strdown(spr->name, -1), > + TARGET_LONG_BITS, spr->gdb_id, > + "int", "spr"); > + } > + > gdb_feature_builder_end(&builder); > } > #endif
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c index 3f1e61bdb7..3b28d4e21c 100644 --- a/target/ppc/gdbstub.c +++ b/target/ppc/gdbstub.c @@ -305,14 +305,6 @@ static void gdb_gen_spr_feature(CPUState *cs) unsigned int num_regs = 0; int i; - if (pcc->gdb_spr.xml) { - return; - } - - gdb_feature_builder_init(&builder, &pcc->gdb_spr, - "org.qemu.power.spr", "power-spr.xml", - cs->gdb_num_regs); - for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { ppc_spr_t *spr = &env->spr_cb[i]; @@ -320,9 +312,6 @@ static void gdb_gen_spr_feature(CPUState *cs) continue; } - gdb_feature_builder_append_reg(&builder, g_ascii_strdown(spr->name, -1), - TARGET_LONG_BITS, num_regs, - "int", "spr"); /* * GDB identifies registers based on the order they are * presented in the XML. These ids will not match QEMU's @@ -335,6 +324,26 @@ static void gdb_gen_spr_feature(CPUState *cs) num_regs++; } + if (pcc->gdb_spr.xml) { + return; + } + + gdb_feature_builder_init(&builder, &pcc->gdb_spr, + "org.qemu.power.spr", "power-spr.xml", + cs->gdb_num_regs); + + for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) { + ppc_spr_t *spr = &env->spr_cb[i]; + + if (!spr->name) { + continue; + } + + gdb_feature_builder_append_reg(&builder, g_ascii_strdown(spr->name, -1), + TARGET_LONG_BITS, spr->gdb_id, + "int", "spr"); + } + gdb_feature_builder_end(&builder); } #endif
The GDB server protocol assigns an arbitrary numbering of the SPRs. We track this correspondence on each SPR with gdb_id, using it to resolve any SPR requests GDB makes. Early on we generate an XML representation of the SPRs to give GDB, including this numbering. However the XML is cached globally, and we skip setting the SPR gdb_id values on subsequent threads if we detect it is cached. This causes QEMU to fail to resolve SPR requests against secondary CPUs because it cannot find the matching gdb_id value on that thread's SPRs. This is a minimal fix to first assign the gdb_id values, then return early if the XML is cached. Otherwise we generate the XML using the now already initialised gdb_id values. Fixes: 1b53948ff8f7 ("target/ppc: Use GDBFeature for dynamic XML") Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> --- target/ppc/gdbstub.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)