Re: [PATCH 1/4] s390x/tcg: Fix CPU address returned by STIDP

[David Hildenbrand]

On 01.06.23 18:25, Ilya Leoshkevich wrote:
> In qemu-user-s390x, /proc/cpuinfo contains:
>
>      cpu number      : 0
>      identification  : 000000
>
>      cpu number      : 1
>      identification  : 400000
>
> The highest nibble is supposed to contain the CPU address, but it's off
> by 2 bits. Fix the shift value and provide a symbolic constant for it.

Would be helpful to show the output with this fix.

> Fixes: 076d4d39b65f ("s390x/cpumodel: wire up cpu type + id for TCG")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/s390x/cpu_models.c |  4 ++--
>   target/s390x/cpu_models.h | 10 +++++++++-
>   2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 457b5cb10c4..ae8880e81dc 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -607,8 +607,8 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
>   #if !defined(CONFIG_USER_ONLY)
>       cpu->env.cpuid = s390_cpuid_from_cpu_model(cpu->model);
>       if (tcg_enabled()) {
> -        /* basic mode, write the cpu address into the first 4 bit of the ID */
> -        cpu->env.cpuid = deposit64(cpu->env.cpuid, 54, 4, cpu->env.core_id);
> +        cpu->env.cpuid = deposit64(cpu->env.cpuid, CPU_PHYS_ADDR_SHIFT,
> +                                   CPU_PHYS_ADDR_BITS, cpu->env.core_id);
>       }
>   #endif
>   }
> diff --git a/target/s390x/cpu_models.h b/target/s390x/cpu_models.h
> index fb1adc8b210..cc7305ec213 100644
> --- a/target/s390x/cpu_models.h
> +++ b/target/s390x/cpu_models.h
> @@ -96,10 +96,18 @@ static inline bool s390_known_cpu_type(uint16_t type)
>   {
>       return s390_get_gen_for_cpu_type(type) != 0;
>   }
> +#define CPU_ID_SHIFT 32
> +#define CPU_ID_BITS 24
> +/*
> + * When cpu_id_format is 0 (basic mode), the leftmost 4 bits of cpu_id contain
> + * the rightmost 4 bits of the physical CPU address.
> + */
> +#define CPU_PHYS_ADDR_BITS 4
> +#define CPU_PHYS_ADDR_SHIFT (CPU_ID_SHIFT + CPU_ID_BITS - CPU_PHYS_ADDR_BITS)
>   static inline uint64_t s390_cpuid_from_cpu_model(const S390CPUModel *model)
>   {
>       return ((uint64_t)model->cpu_ver << 56) |
> -           ((uint64_t)model->cpu_id << 32) |
> +           ((uint64_t)model->cpu_id << CPU_ID_SHIFT) |
>              ((uint64_t)model->def->type << 16) |
>              (model->def->gen == 7 ? 0 : (uint64_t)model->cpu_id_format << 15);
>   }

Thanks!

Reviewed-by: David Hildenbrand <david@redhat.com>

--
Thanks,

David / dhildenb