| Message ID | 20250318045125.759259-10-pierrick.bouvier@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | single-binary: start make hw/arm/ common (boot.c) | expand |
On 18/3/25 05:51, Pierrick Bouvier wrote: > This will affect zregs field for aarch32. > This field is used for MVE and SVE implementations. MVE implementation > is clipping index value to 0 or 1 for zregs[*].d[], > so we should not touch the rest of data in this case anyway. We should describe why it is safe for migration. I.e. vmstate_za depends on za_needed() -> SME, not included in 32-bit cpus, etc. Should we update target/arm/machine.c in this same patch, or a preliminary one? > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > target/arm/cpu.h | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index 27a0d4550f2..00f78d64bd8 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -169,11 +169,7 @@ typedef struct ARMGenericTimer { > * Align the data for use with TCG host vector operations. > */ > > -#ifdef TARGET_AARCH64 > -# define ARM_MAX_VQ 16 > -#else > -# define ARM_MAX_VQ 1 > -#endif > +#define ARM_MAX_VQ 16 > > typedef struct ARMVectorReg { > uint64_t d[2 * ARM_MAX_VQ] QEMU_ALIGNED(16);
On 3/18/25 11:50, Philippe Mathieu-Daudé wrote: > On 18/3/25 05:51, Pierrick Bouvier wrote: >> This will affect zregs field for aarch32. >> This field is used for MVE and SVE implementations. MVE implementation >> is clipping index value to 0 or 1 for zregs[*].d[], >> so we should not touch the rest of data in this case anyway. > > We should describe why it is safe for migration. > > I.e. vmstate_za depends on za_needed() -> SME, not included in 32-bit > cpus, etc. > > Should we update target/arm/machine.c in this same patch, or a > preliminary one? > vmstate_za definition and inclusion in vmstate_arm_cpu is under #ifdef TARGET_AARCH64. In this case (TARGET_AARCH64), ARM_MAX_VQ was already defined as 16, so there should not be any change. Other values depending on ARM_MAX_VQ, for migration, are as well under TARGET_AARCH64 ifdefs (vmstate_zreg_hi_reg, vmstate_preg_reg, vmstate_vreg). And for vmstate_vfp, which is present for aarch32 as well, the size of data under each register is specifically set to 2. VMSTATE_UINT64_SUB_ARRAY(env.vfp.zregs[0].d, ARMCPU, 0, 2) So even if storage has more space, it should not impact any usage of it. Even though this change is trivial, I didn't do it blindly to "make it compile" and I checked the various usages of ARM_MAX_VQ and zregs, and I didn't see anything that seems to be a problem. >> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> target/arm/cpu.h | 6 +----- >> 1 file changed, 1 insertion(+), 5 deletions(-) >> >> diff --git a/target/arm/cpu.h b/target/arm/cpu.h >> index 27a0d4550f2..00f78d64bd8 100644 >> --- a/target/arm/cpu.h >> +++ b/target/arm/cpu.h >> @@ -169,11 +169,7 @@ typedef struct ARMGenericTimer { >> * Align the data for use with TCG host vector operations. >> */ >> >> -#ifdef TARGET_AARCH64 >> -# define ARM_MAX_VQ 16 >> -#else >> -# define ARM_MAX_VQ 1 >> -#endif >> +#define ARM_MAX_VQ 16 >> >> typedef struct ARMVectorReg { >> uint64_t d[2 * ARM_MAX_VQ] QEMU_ALIGNED(16); >
On 3/17/25 21:51, Pierrick Bouvier wrote: > This will affect zregs field for aarch32. > This field is used for MVE and SVE implementations. MVE implementation > is clipping index value to 0 or 1 for zregs[*].d[], > so we should not touch the rest of data in this case anyway. > > Signed-off-by: Pierrick Bouvier<pierrick.bouvier@linaro.org> > --- > target/arm/cpu.h | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 27a0d4550f2..00f78d64bd8 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -169,11 +169,7 @@ typedef struct ARMGenericTimer { * Align the data for use with TCG host vector operations. */ -#ifdef TARGET_AARCH64 -# define ARM_MAX_VQ 16 -#else -# define ARM_MAX_VQ 1 -#endif +#define ARM_MAX_VQ 16 typedef struct ARMVectorReg { uint64_t d[2 * ARM_MAX_VQ] QEMU_ALIGNED(16);
This will affect zregs field for aarch32. This field is used for MVE and SVE implementations. MVE implementation is clipping index value to 0 or 1 for zregs[*].d[], so we should not touch the rest of data in this case anyway. Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- target/arm/cpu.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)