[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 1/2] target-mips: replace cpu_save/cpu_load with
From: |
Leon Alrae |
Subject: |
Re: [Qemu-devel] [PULL 1/2] target-mips: replace cpu_save/cpu_load with VMStateDescription |
Date: |
Wed, 22 Jul 2015 15:03:13 +0100 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 22/07/2015 13:08, Peter Maydell wrote:
> On 11 March 2015 at 16:15, Leon Alrae <address@hidden> 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 <address@hidden>
>
> 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