This is a 64-bit register on AArch64, even if the high 44 bits
are RES0.  Because this is defined as ARM_CP_STATE_BOTH, we are
asserting that the cpreg field is 64-bits.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1400
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

During my perigrinations of reorganizing cpregs, I've been thinking
of ways to detect these sorts of errors -- preferably at build time,
but failing that at startup.  I think all raw usage of offsetof has
got to be replaced with something like

#define cpreg_fieldoffset(field) \
    .fieldoffset = offsetof(CPUARMState, field), \
    .fieldsize = sizeof(((CPUARMState *)0)->field),

I'm not going to touch this until Fabiano's --disable-tcg cleanup lands.

r~

---
 target/arm/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index bf2bce046d..1feb63b4d7 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -479,7 +479,7 @@ typedef struct CPUArchState {
         };
         uint64_t c14_cntfrq; /* Counter Frequency register */
         uint64_t c14_cntkctl; /* Timer Control register */
-        uint32_t cnthctl_el2; /* Counter/Timer Hyp Control register */
+        uint64_t cnthctl_el2; /* Counter/Timer Hyp Control register */
         uint64_t cntvoff_el2; /* Counter Virtual Offset register */
         ARMGenericTimer c14_timer[NUM_GTIMERS];
         uint32_t c15_cpar; /* XScale Coprocessor Access Register */
-- 
2.34.1

On Sun, 15 Jan 2023 at 17:16, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This is a 64-bit register on AArch64, even if the high 44 bits
> are RES0.  Because this is defined as ARM_CP_STATE_BOTH, we are
> asserting that the cpreg field is 64-bits.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1400
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> During my perigrinations of reorganizing cpregs, I've been thinking
> of ways to detect these sorts of errors -- preferably at build time,
> but failing that at startup.  I think all raw usage of offsetof has
> got to be replaced with something like
>
> #define cpreg_fieldoffset(field) \
>     .fieldoffset = offsetof(CPUARMState, field), \
>     .fieldsize = sizeof(((CPUARMState *)0)->field),

The other cpreg sanity check I've vaguely wondered about in
the past is whether we can somehow check that all the fields
that we expose to the guest as a register are migrated. This
definitely isn't true currently because for a 32-bit CPU with
TrustZone we messed up migration of the Secure banked state.
Incorrect use of alias flags could also result in that kind
of bug. I guess we'd need to have some way to tell the check
about fields that really are manually migrated, though.

Anyway, for this patch, applied to target-arm.next.

thanks
-- PMM