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

[Peter Maydell]

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.

thanks
-- PMM