Re: [Qemu-devel] [PATCH v1] s390x/cpumodel: wire up cpu type + id for TCG

[Aurelien Jarno]

On 2017-06-02 12:52, David Hildenbrand wrote:
> 
> >> +
> >> +#ifndef CONFIG_USER_ONLY
> >> +void HELPER(stidp)(CPUS390XState *env, uint64_t addr)
> >> +{
> >> +    S390CPU *cpu = s390_env_get_cpu(env);
> >> +    uint64_t cpuid = s390_cpuid_from_cpu_model(cpu->model);
> >> +
> >> +    if (addr & 0x7) {
> >> +        program_interrupt(env, PGM_SPECIFICATION, ILEN_LATER_INC);
> >> +        return;
> >> +    }
> >> +
> >> +    /* basic mode, write the cpu address into the first 4 bit of the ID */
> >> +    cpuid |= ((uint64_t)env->cpu_num & 0xf) << 54;
> >> +    cpu_stq_data(env, addr, cpuid);
> >> +}
> >> +#endif
> > 
> > I don't really see the point of using an helper instead of just updating
> > the existing code. From what I understand the cpuid does not change at
> > runtime, so the s390_cpuid_from_cpu_model function can also be called 
> > from translate.c.
> > 
> > Aurelien
> > 
> 
> From what I can see, conditional exceptions are more complicated to
> implement without helpers (involves generating compares, jumps and so

In that case you don't need to do any compare an jump. It's a standard
load/store alignement check, you can just specify the MO_ALIGN flag to
the tcg_gen_qemu_st_i64 function.


> on). As this function is not expected to be executed on hot paths, I
> think moving it into a helper is the right thing to do.

This is not only about performance, but also avoiding code that is
spread in many files. Well theoretically increasing the number of
entries in the helper hash table has a performance impact, but i don't
think it is measurable.

Aurelien        

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net