On Sat, 30 Aug 2025 at 18:16, Richard Henderson
<[email protected]> wrote:
>
> The ARM now defines 36 bits in SPSR_ELx in aarch64 mode, so
> it's time to bite the bullet and extend PSTATE to match.
>
> Most changes are straightforward, adjusting printf formats,
> changing local variable types. More complex is migration,
> where to maintain backward compatibility a new pstate64
> record is introduced, and only when one of the extensions
> that sets bits 32-35 are active.
>
> The fate of gdbstub is left undecided for the moment.
>
> Reviewed-by: Pierrick Bouvier <[email protected]>
> Signed-off-by: Richard Henderson <[email protected]>
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index 8dbeca2867..9b00c14b4a 100644
> --- a/target/arm/machine.c
> +++ b/target/arm/machine.c
> @@ -836,6 +836,61 @@ static const VMStateInfo vmstate_cpsr = {
> .put = put_cpsr,
> };
>
> +static int get_pstate64_1(QEMUFile *f, void *opaque, size_t size,
> + const VMStateField *field)
Why the _1 suffix ?
> +{
> + ARMCPU *cpu = opaque;
> + CPUARMState *env = &cpu->env;
> + uint64_t val = qemu_get_be64(f);
> +
> + env->aarch64 = ((val & PSTATE_nRW) == 0);
> + pstate_write(env, val);
We should either enforce that the value has nRW == 0
(failing migration by returning nonzero if it is not) or
else handle the case where nRW != 0 via cpsr_write().
I note that there is actually a defined bit above 32
in the SPSR_ELx format for exceptions taken from
AArch32 to AArch64: PPEND, used with FEAT_SEBEP. That
suggests we should probably at least consider handling
64-bit AArch32 "CPSR" values, though FEAT_SEBEP in
particular may be out of scope for us.
Incidentally I think we are not correctly migrating
PSTATE.SS when in AArch32 -- we will migrate the CPSR
via cpsr_read() / cpsr_write(), but that view doesn't have
the PSTATE.SS bit in it. Possibly these things could
be addressed at the same time, so we have a subsection
for 64-bit pstate/cpsr, and its save/load uses
cpsr_read_for_spsr_elx() (and a corresponding _write_
that we don't have yet) when AArch32, and the .needed
function is "if top 32 bits not all zero, or PSTATE_SS bit
is set".
> + return 0;
> +}
> +
> +static int put_pstate64_1(QEMUFile *f, void *opaque, size_t size,
> + const VMStateField *field, JSONWriter *vmdesc)
> +{
> + ARMCPU *cpu = opaque;
> + CPUARMState *env = &cpu->env;
> + uint64_t val = pstate_read(env);
> +
> + qemu_put_be64(f, val);
> + return 0;
> +}
> +
> +static const VMStateInfo vmstate_pstate64_1 = {
> + .name = "pstate64",
> + .get = get_pstate64_1,
> + .put = put_pstate64_1,
> +};
> +
> +static bool pstate64_needed(void *opaque)
> +{
> + ARMCPU *cpu = opaque;
> + CPUARMState *env = &cpu->env;
> +
> + return is_a64(env) && pstate_read(env) > UINT32_MAX;
> +}
> +
> +static const VMStateDescription vmstate_pstate64 = {
> + .name = "cpu/pstate64",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = pstate64_needed,
> + .fields = (const VMStateField[]) {
> + {
> + .name = "pstate64",
> + .version_id = 0,
> + .size = sizeof(uint64_t),
> + .info = &vmstate_pstate64_1,
> + .flags = VMS_SINGLE,
> + .offset = 0,
> + },
> + VMSTATE_END_OF_LIST()
> + },
> +};
Probably worth also adding a comment to get_cpsr() along
the lines of "If any bits are set in the upper 32 bits of
PSTATE then the cpu/pstate64 subsection will override this
pstate_write() later with the full 64-bit PSTATE."
> +
> static int get_power(QEMUFile *f, void *opaque, size_t size,
> const VMStateField *field)
> {
> @@ -1119,6 +1174,7 @@ const VMStateDescription vmstate_arm_cpu = {
> &vmstate_serror,
> &vmstate_irq_line_state,
> &vmstate_wfxt_timer,
> + &vmstate_pstate64,
> NULL
> }
> };
> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
> index 71c6c44ee8..f61adf1f80 100644
> --- a/target/arm/tcg/helper-a64.c
> +++ b/target/arm/tcg/helper-a64.c
> @@ -639,7 +639,7 @@ void HELPER(exception_return)(CPUARMState *env, uint64_t
> new_pc)
> ARMCPU *cpu = env_archcpu(env);
> int cur_el = arm_current_el(env);
> unsigned int spsr_idx = aarch64_banked_spsr_index(cur_el);
> - uint32_t spsr = env->banked_spsr[spsr_idx];
> + uint64_t spsr = env->banked_spsr[spsr_idx];
> int new_el;
> bool return_to_aa64 = (spsr & PSTATE_nRW) == 0;
thanks
-- PMM
