Re: [Qemu-devel] [PULL 1/2] target-mips: replace cpu_save/cpu_load with VMStateDescription

[Leon Alrae <leon.alrae@imgtec.com>]

On 22/07/2015 13:08, Peter Maydell wrote:
> On 11 March 2015 at 16:15, Leon Alrae <leon.alrae@imgtec.com> wrote:
>> Create VMStateDescription for MIPS CPU. The new structure contains exactly the
>> same fields as before, therefore leaving existing version_id.
>>
>> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> 
> Hi. I've just noticed that this commit has a bug, which provokes
> a warning on FreeBSD:
> 
> target-mips/machine.c:171:20: warning: incompatible pointer types
> passing 'uint_fast8_t *' (aka 'unsigned int *') to parameter of
> type 'const uint8_t *' (ake 'const unsigned char *')
> [-Wincompatible-pointer-types]
>   qemu_put_8s(f, &v->ASID);
>                  ^~~~~~~~
> 
> This is because in the old code:
> 
> 
>> -    /* Save TLB */
>> -    qemu_put_be32s(f, &env->tlb->nb_tlb);
>> -    qemu_put_be32s(f, &env->tlb->tlb_in_use);
>> -    for(i = 0; i < MIPS_TLB_MAX; i++) {
>> -        uint16_t flags = ((env->tlb->mmu.r4k.tlb[i].EHINV << 15) |
>> -                          (env->tlb->mmu.r4k.tlb[i].RI1 << 14) |
>> -                          (env->tlb->mmu.r4k.tlb[i].RI0 << 13) |
>> -                          (env->tlb->mmu.r4k.tlb[i].XI1 << 12) |
>> -                          (env->tlb->mmu.r4k.tlb[i].XI0 << 11) |
>> -                          (env->tlb->mmu.r4k.tlb[i].G << 10) |
>> -                          (env->tlb->mmu.r4k.tlb[i].C0 << 7) |
>> -                          (env->tlb->mmu.r4k.tlb[i].C1 << 4) |
>> -                          (env->tlb->mmu.r4k.tlb[i].V0 << 3) |
>> -                          (env->tlb->mmu.r4k.tlb[i].V1 << 2) |
>> -                          (env->tlb->mmu.r4k.tlb[i].D0 << 1) |
>> -                          (env->tlb->mmu.r4k.tlb[i].D1 << 0));
>> -        uint8_t asid;
>> -
>> -        qemu_put_betls(f, &env->tlb->mmu.r4k.tlb[i].VPN);
>> -        qemu_put_be32s(f, &env->tlb->mmu.r4k.tlb[i].PageMask);
>> -        asid = env->tlb->mmu.r4k.tlb[i].ASID;
>> -        qemu_put_8s(f, &asid);
> 
> we copied the asid to a local variable of the right size
> to pass the address to qemu_put_8s()...
> 
>> +    uint16_t flags = ((v->EHINV << 15) |
>> +                      (v->RI1 << 14) |
>> +                      (v->RI0 << 13) |
>> +                      (v->XI1 << 12) |
>> +                      (v->XI0 << 11) |
>> +                      (v->G << 10) |
>> +                      (v->C0 << 7) |
>> +                      (v->C1 << 4) |
>> +                      (v->V0 << 3) |
>> +                      (v->V1 << 2) |
>> +                      (v->D0 << 1) |
>> +                      (v->D1 << 0));
>> +
>> +    qemu_put_betls(f, &v->VPN);
>> +    qemu_put_be32s(f, &v->PageMask);
>> +    qemu_put_8s(f, &v->ASID);
> 
> ...but in the new code we just directly use the
> address in the struct, which won't work because it has
> a (potentially) incompatible type.

I've just posted the patch correcting this.

Thanks,
Leon