Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
gdb-xml/aarch64-core.xml | 5 +++++
target/arm/cpu.h | 1 +
target/arm/gdbstub64.c | 6 ++++++
3 files changed, 12 insertions(+)
diff --git a/gdb-xml/aarch64-core.xml b/gdb-xml/aarch64-core.xml
index b8046510b9a085d30463d37b3ecc8d435f5fb7a4..19ad743dc5607b4021fb795bfb9b8e9cf0adef68 100644
--- a/gdb-xml/aarch64-core.xml
+++ b/gdb-xml/aarch64-core.xml
@@ -91,4 +91,9 @@
</flags>
<reg name="cpsr" bitsize="32" type="cpsr_flags"/>
+ <flags id="current_el_flags" size="8">
+ <!-- Exception Level. -->
+ <field name="EL" start="2" end="3"/>
+ </flags>
+ <reg name="CurrentEL" bitsize="64" type="current_el"/>
</feature>
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index dc9b6dce4c92297b2636d0d7c0dce580f1806d5b..c3070cd9863381fac40f5640e0a7a84dfa1c6e06 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1473,6 +1473,7 @@ void pmu_init(ARMCPU *cpu);
* AArch32 mode SPSRs are basically CPSR-format.
*/
#define PSTATE_SP (1U)
+#define PSTATE_EL (3U << 2)
#define PSTATE_M (0xFU)
#define PSTATE_nRW (1U << 4)
#define PSTATE_F (1U << 6)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 08e28585396816ab90d6d8e460ff8171892fe8da..16b564e1a970cb5e854a705619f71ffc61545a73 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -48,6 +48,9 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
return gdb_get_reg64(mem_buf, env->pc);
case 33:
return gdb_get_reg32(mem_buf, pstate_read(env));
+ case 34:
+ /* CurrentEL */
+ return gdb_get_reg64(mem_buf, env->pstate & PSTATE_EL);
}
/* Unknown register. */
return 0;
@@ -77,6 +80,9 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
/* CPSR */
pstate_write(env, tmp);
return 4;
+ case 34:
+ /* CurrentEL */
+ return 0;
}
/* Unknown register. */
return 0;
--
2.47.2On Fri, 8 Aug 2025 at 12:30, Manos Pitsidianakis <manos.pitsidianakis@linaro.org> wrote: > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> > --- > gdb-xml/aarch64-core.xml | 5 +++++ > target/arm/cpu.h | 1 + > target/arm/gdbstub64.c | 6 ++++++ > 3 files changed, 12 insertions(+) > > diff --git a/gdb-xml/aarch64-core.xml b/gdb-xml/aarch64-core.xml > index b8046510b9a085d30463d37b3ecc8d435f5fb7a4..19ad743dc5607b4021fb795bfb9b8e9cf0adef68 100644 > --- a/gdb-xml/aarch64-core.xml > +++ b/gdb-xml/aarch64-core.xml > @@ -91,4 +91,9 @@ > </flags> > <reg name="cpsr" bitsize="32" type="cpsr_flags"/> > > + <flags id="current_el_flags" size="8"> > + <!-- Exception Level. --> > + <field name="EL" start="2" end="3"/> > + </flags> > + <reg name="CurrentEL" bitsize="64" type="current_el"/> > </feature> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index dc9b6dce4c92297b2636d0d7c0dce580f1806d5b..c3070cd9863381fac40f5640e0a7a84dfa1c6e06 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -1473,6 +1473,7 @@ void pmu_init(ARMCPU *cpu); > * AArch32 mode SPSRs are basically CPSR-format. > */ > #define PSTATE_SP (1U) > +#define PSTATE_EL (3U << 2) > #define PSTATE_M (0xFU) > #define PSTATE_nRW (1U << 4) > #define PSTATE_F (1U << 6) > diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c > index 08e28585396816ab90d6d8e460ff8171892fe8da..16b564e1a970cb5e854a705619f71ffc61545a73 100644 > --- a/target/arm/gdbstub64.c > +++ b/target/arm/gdbstub64.c > @@ -48,6 +48,9 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) > return gdb_get_reg64(mem_buf, env->pc); > case 33: > return gdb_get_reg32(mem_buf, pstate_read(env)); > + case 34: > + /* CurrentEL */ > + return gdb_get_reg64(mem_buf, env->pstate & PSTATE_EL); > } The debugger already has this information in the 'cpsr' register, so it could implement convenience views of the subfields itself if it liked. If we're going to do this I would prefer it to be because we've gained some consensus with e.g. the gdb maintainers that this is the "preferred" way to expose the CPU state. The XML config stuff lets us do it in our own way if we want to, but I think there is value in consistency across stubs here. thanks -- PMM
On Fri, Aug 8, 2025 at 3:21 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 8 Aug 2025 at 12:30, Manos Pitsidianakis > <manos.pitsidianakis@linaro.org> wrote: > > > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> > > --- > > gdb-xml/aarch64-core.xml | 5 +++++ > > target/arm/cpu.h | 1 + > > target/arm/gdbstub64.c | 6 ++++++ > > 3 files changed, 12 insertions(+) > > > > diff --git a/gdb-xml/aarch64-core.xml b/gdb-xml/aarch64-core.xml > > index b8046510b9a085d30463d37b3ecc8d435f5fb7a4..19ad743dc5607b4021fb795bfb9b8e9cf0adef68 100644 > > --- a/gdb-xml/aarch64-core.xml > > +++ b/gdb-xml/aarch64-core.xml > > @@ -91,4 +91,9 @@ > > </flags> > > <reg name="cpsr" bitsize="32" type="cpsr_flags"/> > > > > + <flags id="current_el_flags" size="8"> > > + <!-- Exception Level. --> > > + <field name="EL" start="2" end="3"/> > > + </flags> > > + <reg name="CurrentEL" bitsize="64" type="current_el"/> > > </feature> > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > > index dc9b6dce4c92297b2636d0d7c0dce580f1806d5b..c3070cd9863381fac40f5640e0a7a84dfa1c6e06 100644 > > --- a/target/arm/cpu.h > > +++ b/target/arm/cpu.h > > @@ -1473,6 +1473,7 @@ void pmu_init(ARMCPU *cpu); > > * AArch32 mode SPSRs are basically CPSR-format. > > */ > > #define PSTATE_SP (1U) > > +#define PSTATE_EL (3U << 2) > > #define PSTATE_M (0xFU) > > #define PSTATE_nRW (1U << 4) > > #define PSTATE_F (1U << 6) > > diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c > > index 08e28585396816ab90d6d8e460ff8171892fe8da..16b564e1a970cb5e854a705619f71ffc61545a73 100644 > > --- a/target/arm/gdbstub64.c > > +++ b/target/arm/gdbstub64.c > > @@ -48,6 +48,9 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) > > return gdb_get_reg64(mem_buf, env->pc); > > case 33: > > return gdb_get_reg32(mem_buf, pstate_read(env)); > > + case 34: > > + /* CurrentEL */ > > + return gdb_get_reg64(mem_buf, env->pstate & PSTATE_EL); > > } > > The debugger already has this information in the 'cpsr' > register, so it could implement convenience views of > the subfields itself if it liked. Yep, but consider: it is a register, architecturally, so it's nice to include it for consistency. It's redundant only because gdb has cpsr which is not a register. So this is about more about being technically correct than correcting an actual problem. > > If we're going to do this I would prefer it to be because > we've gained some consensus with e.g. the gdb maintainers > that this is the "preferred" way to expose the CPU state. > The XML config stuff lets us do it in our own way if we > want to, but I think there is value in consistency across > stubs here. I plan to upstream the XML patches to gdb as well, separately. But since we use custom target xml it's not like we depend on gdb to change it. > > thanks > -- PMM
On 8/8/25 5:26 AM, Manos Pitsidianakis wrote: > On Fri, Aug 8, 2025 at 3:21 PM Peter Maydell <peter.maydell@linaro.org> wrote: >> >> On Fri, 8 Aug 2025 at 12:30, Manos Pitsidianakis >> <manos.pitsidianakis@linaro.org> wrote: >>> >>> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> >>> --- >>> gdb-xml/aarch64-core.xml | 5 +++++ >>> target/arm/cpu.h | 1 + >>> target/arm/gdbstub64.c | 6 ++++++ >>> 3 files changed, 12 insertions(+) >>> >>> diff --git a/gdb-xml/aarch64-core.xml b/gdb-xml/aarch64-core.xml >>> index b8046510b9a085d30463d37b3ecc8d435f5fb7a4..19ad743dc5607b4021fb795bfb9b8e9cf0adef68 100644 >>> --- a/gdb-xml/aarch64-core.xml >>> +++ b/gdb-xml/aarch64-core.xml >>> @@ -91,4 +91,9 @@ >>> </flags> >>> <reg name="cpsr" bitsize="32" type="cpsr_flags"/> >>> >>> + <flags id="current_el_flags" size="8"> >>> + <!-- Exception Level. --> >>> + <field name="EL" start="2" end="3"/> >>> + </flags> >>> + <reg name="CurrentEL" bitsize="64" type="current_el"/> >>> </feature> >>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h >>> index dc9b6dce4c92297b2636d0d7c0dce580f1806d5b..c3070cd9863381fac40f5640e0a7a84dfa1c6e06 100644 >>> --- a/target/arm/cpu.h >>> +++ b/target/arm/cpu.h >>> @@ -1473,6 +1473,7 @@ void pmu_init(ARMCPU *cpu); >>> * AArch32 mode SPSRs are basically CPSR-format. >>> */ >>> #define PSTATE_SP (1U) >>> +#define PSTATE_EL (3U << 2) >>> #define PSTATE_M (0xFU) >>> #define PSTATE_nRW (1U << 4) >>> #define PSTATE_F (1U << 6) >>> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c >>> index 08e28585396816ab90d6d8e460ff8171892fe8da..16b564e1a970cb5e854a705619f71ffc61545a73 100644 >>> --- a/target/arm/gdbstub64.c >>> +++ b/target/arm/gdbstub64.c >>> @@ -48,6 +48,9 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) >>> return gdb_get_reg64(mem_buf, env->pc); >>> case 33: >>> return gdb_get_reg32(mem_buf, pstate_read(env)); >>> + case 34: >>> + /* CurrentEL */ >>> + return gdb_get_reg64(mem_buf, env->pstate & PSTATE_EL); >>> } >> >> The debugger already has this information in the 'cpsr' >> register, so it could implement convenience views of >> the subfields itself if it liked. > > Yep, but consider: it is a register, architecturally, so it's nice to > include it for consistency. It's redundant only because gdb has cpsr > which is not a register. So this is about more about being technically > correct than correcting an actual problem. > I agree with Manos on this. As mentioned on a previous thread, cpsr is not even supposed to exist for aarch64. So adding architecturally defined registers, even if data is redundant with cpsr, should not be a problem. I'm sure gdb folks can understand this too. > >> >> If we're going to do this I would prefer it to be because >> we've gained some consensus with e.g. the gdb maintainers >> that this is the "preferred" way to expose the CPU state. >> The XML config stuff lets us do it in our own way if we >> want to, but I think there is value in consistency across >> stubs here. > > I plan to upstream the XML patches to gdb as well, separately. But > since we use custom target xml it's not like we depend on gdb to > change it. > >> >> thanks >> -- PMM
On Fri, 8 Aug 2025 at 17:11, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote: > > On 8/8/25 5:26 AM, Manos Pitsidianakis wrote: > > On Fri, Aug 8, 2025 at 3:21 PM Peter Maydell <peter.maydell@linaro.org> wrote: > >> > >> On Fri, 8 Aug 2025 at 12:30, Manos Pitsidianakis > >> <manos.pitsidianakis@linaro.org> wrote: > >> The debugger already has this information in the 'cpsr' > >> register, so it could implement convenience views of > >> the subfields itself if it liked. > > > > Yep, but consider: it is a register, architecturally, so it's nice to > > include it for consistency. It's redundant only because gdb has cpsr > > which is not a register. So this is about more about being technically > > correct than correcting an actual problem. > > > > I agree with Manos on this. > As mentioned on a previous thread, cpsr is not even supposed to exist > for aarch64. So adding architecturally defined registers, even if data > is redundant with cpsr, should not be a problem. > I'm sure gdb folks can understand this too. I'm not saying this is the wrong way to represent this. I'm just saying we're not the only gdbstub in the world, and it would be nice to have a wider discussion than just QEMU folks so we are consistent about how we represent PSTATE (including what we want to do about the new bits that appear in the high 32 bits of an SPSR), before we commit to any particular direction. -- PMM
On 8/8/25 9:14 AM, Peter Maydell wrote: > On Fri, 8 Aug 2025 at 17:11, Pierrick Bouvier > <pierrick.bouvier@linaro.org> wrote: >> >> On 8/8/25 5:26 AM, Manos Pitsidianakis wrote: >>> On Fri, Aug 8, 2025 at 3:21 PM Peter Maydell <peter.maydell@linaro.org> wrote: >>>> >>>> On Fri, 8 Aug 2025 at 12:30, Manos Pitsidianakis >>>> <manos.pitsidianakis@linaro.org> wrote: >>>> The debugger already has this information in the 'cpsr' >>>> register, so it could implement convenience views of >>>> the subfields itself if it liked. >>> >>> Yep, but consider: it is a register, architecturally, so it's nice to >>> include it for consistency. It's redundant only because gdb has cpsr >>> which is not a register. So this is about more about being technically >>> correct than correcting an actual problem. >>> >> >> I agree with Manos on this. >> As mentioned on a previous thread, cpsr is not even supposed to exist >> for aarch64. So adding architecturally defined registers, even if data >> is redundant with cpsr, should not be a problem. >> I'm sure gdb folks can understand this too. > > I'm not saying this is the wrong way to represent this. > I'm just saying we're not the only gdbstub in the world, > and it would be nice to have a wider discussion than just > QEMU folks so we are consistent about how we represent > PSTATE (including what we want to do about the new > bits that appear in the high 32 bits of an SPSR), before > we commit to any particular direction. > Considering we have our own set of gdb xml, is that really important to agree on pstate layout before we simply make those registers visible on our side? The new registers added in this series on gdb/kgdb side at the moment, so we don't really break anything. I agree it would be good to see this on gdb side, but my point is that we are not necessarily stuck and we can make this visible without waiting two releases. As well, it would be a good motivation to add this on gdb showing QEMU already exposes this. > -- PMM
© 2016 - 2026 Red Hat, Inc.