On 04/11/2020 17:25, Eduardo Habkost wrote:
> The "iu-version" property is declared as uint64_t but points to a
> target_ulong struct field. On the sparc 32-bit target, This
> makes every write of iu-version corrupt the 4 bytes after
> sparc_def_t.iu_version (where the fpu_version field is located).
>
> Change the type of the iu_version struct field to uint64_t,
> and just use DEFINE_PROP_UINT64.
>
> The only place where env.def.iu_version field is read is the
> env->version = env->def.iu_version;
> assignment at sparc_cpu_realizefn(). This means behavior will be
> kept exactly the same (except for the memory corruption bug fix).
>
> It would be nice to explicitly validate iu_version against
> target_ulong limits, but that would be a new feature (since the
> first version of this code, iu-version was parsed as 64-bit value
> value). It can be done later, once we have an appropriate API to
> define limits for integer properties.
>
> Fixes: de05005bf785 ("sparc: convert cpu features to qdev properties")
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Artyom Tarasenko <atar4qemu@gmail.com>
> Cc: qemu-devel@nongnu.org
> ---
> target/sparc/cpu.h | 2 +-
> target/sparc/cpu.c | 5 ++---
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index b9369398f2..8ed0f4fef3 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -252,7 +252,7 @@ typedef struct trap_state {
>
> struct sparc_def_t {
> const char *name;
> - target_ulong iu_version;
> + uint64_t iu_version;
> uint32_t fpu_version;
> uint32_t mmu_version;
> uint32_t mmu_bm;
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index ec59a13eb8..5a9397f19a 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -576,7 +576,7 @@ void sparc_cpu_list(void)
> unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(sparc_defs); i++) {
> - qemu_printf("Sparc %16s IU " TARGET_FMT_lx
> + qemu_printf("Sparc %16s IU %" PRIx64
> " FPU %08x MMU %08x NWINS %d ",
> sparc_defs[i].name,
> sparc_defs[i].iu_version,
> @@ -838,8 +838,7 @@ static Property sparc_cpu_properties[] = {
> DEFINE_PROP_BIT("hypv", SPARCCPU, env.def.features, 11, false),
> DEFINE_PROP_BIT("cmt", SPARCCPU, env.def.features, 12, false),
> DEFINE_PROP_BIT("gl", SPARCCPU, env.def.features, 13, false),
> - DEFINE_PROP_UNSIGNED("iu-version", SPARCCPU, env.def.iu_version, 0,
> - prop_info_uint64, target_ulong),
> + DEFINE_PROP_UINT64("iu-version", SPARCCPU, env.def.iu_version, 0),
> DEFINE_PROP_UINT32("fpu-version", SPARCCPU, env.def.fpu_version, 0),
> DEFINE_PROP_UINT32("mmu-version", SPARCCPU, env.def.mmu_version, 0),
> DEFINE_PROP("nwindows", SPARCCPU, env.def.nwindows,
Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.