Re: [Qemu-devel] [PATCH for-2.0] cpu: do not use QOM casts in ENV_GET_CPU

[Laurent Desnogues]

On Wed, Mar 26, 2014 at 2:55 PM, Paolo Bonzini <address@hidden> wrote:
> Il 26/03/2014 14:42, Paolo Bonzini ha scritto:
>
>> QOM casts are only typesafe inasmuch as we know that the argument is
>> a QOM object.  If it is not, the accesses to fields in Object can
>> access invalid memory and thus cause a segfault.
>>
>> Using a QOM cast in ENV_GET_CPU is useless and harmful.  Useless,
>> because the cast is applied to the result of container_of, which is
>> type safe.  So the QOM cast is nothing but typesafety theater.
>> Harmful, because ENV_GET_CPU *is* used in hot paths especially
>> now that, in 2.0, the movement of fields from CPU_COMMON to
>> CPUState was completed.
>>
>> Reported-by: Laurent Desnogues <address@hidden>
>> Cc: Andreas Faerber <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>
>
> Andreas pointed out on IRC that this is just the tip of the iceberg, due to
> code like this:
>
> static void vmsa_ttbcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>                              uint64_t value)
> {
>     ARMCPU *cpu = arm_env_get_cpu(env);
>
>     if (arm_feature(env, ARM_FEATURE_LPAE)) {
>         /* With LPAE the TTBCR could result in a change of ASID
>          * via the TTBCR.A1 bit, so do a TLB flush.
>          */
>         tlb_flush(CPU(cpu), 1);
>     }
>     vmsa_ttbcr_raw_write(env, ri, value);
> }
>
> (from target-arm/helper.c).  Given this, and the limited time to fix the
> problem in 2.0 where it got magnified, I think we should reconsider whether
> releases should ahve QOM cast debugging enabled.

I agree, but I guess your patch is still useful.  Here are results
with/out your patch with/out --disable-qom-cast-debug

main               17.8
main+disable       17.3
main+patch         17.0
main+patch+disable 16.8

Mainline is at 9c5793c5036c7608ff2c87846c6d11f904c08b58.

For QEMU 1.7, I get 16.4s.


Laurent