Re: [Qemu-devel] [PATCH 2/5] m68k: implement server and client side

[Laurent Vivier]

Le 18/02/2017 à 23:37, Peter Maydell a écrit :
> On 7 February 2017 at 18:33, Laurent Vivier <address@hidden> wrote:
>> This also adds the basic test file and the configuration update.
>>
>> This implementation can only test instructions with values in register and
>> no memory access.
>>
>> Signed-off-by: Laurent Vivier <address@hidden>
> 
> Hi; I got round to setting up my machine with an m68k cross
> compiler so I can at least compile-test the other target
> architectures, and I noticed this code generates compiler
> warnings:
> 
>> +/* reginfo_dump: print state to a stream, returns nonzero on success */
>> +void reginfo_dump(struct reginfo *ri, int is_master)
>> +{
>> +    int i;
>> +    if (is_master) {
>> +        fprintf(stderr, "  pc            \e[1;101;37m0x%08x\e[0m\n",
>> +                ri->pc);
>> +    }
>> +    fprintf(stderr, "\tPC: %08x\n", ri->gregs[R_PC]);
>> +    fprintf(stderr, "\tPS: %04x\n", ri->gregs[R_PS]);
>> +
>> +    for (i = 0; i < 8; i++) {
>> +        fprintf(stderr, "\tD%d: %8x\tA%d: %8x\n", i, ri->gregs[i],
>> +                i, ri->gregs[i + 8]);
>> +    }
>> +
>> +
>> +    for (i = 0; i < 8; i++) {
>> +        fprintf(stderr, "\tFP%d: %08x %08x %08x\n", i,
>> +                ri->fpregs.f_fpregs[i * 3], ri->fpregs.f_fpregs[i * 3 + 1],
>> +                ri->fpregs.f_fpregs[i * 3 + 2]);
> 
> /home/pm215/risu/risu_reginfo_m68k.c:95:37: warning: format ‘%x’
> expects argument of type ‘unsigned int’, but argument 4 has type ‘int
> *’ [-Wformat=]
>          fprintf(stderr, "\tFP%d: %08x %08x %08x\n", i,
>                                      ^
> 
> and similarly for the other 3 f_fpregs[] arguments here
> and in the fprintf calls in reginfo_dump_mismatch().
> 
> Looking at the m68k sys/ucontext.h its definition of
> struct fpregset is
> #ifdef __mcoldfire__
>   int f_fpregs[8][2];
> #else
>   int f_fpregs[8][3];
> #endif
> 
> so it's a 2d array, not a 1d array.

In fact, in etch-m68k, there are two definitions of fpregset:

/usr/include/sys/ucontext.h

typedef struct fpregset
{
  int f_fpregs[8][3];
  int f_pcr;
  int f_psr;
  int f_fpiaddr;
} fpregset_t;

/usr/include/asm/ucontext.h

typedef struct fpregset {
        int f_fpcntl[3];
        int f_fpregs[8*3];
} fpregset_t;

This is the one used by the kernel:

arch/m68k/include/asm/ucontext.h

typedef struct fpregset {
        int f_fpcntl[3];
        int f_fpregs[8*3];
} fpregset_t;

In the past, as the one from sys/ucontext.h was not compatible with the
one from the kernel, I have updated my system to use the one from the
kernel.

But in debian unstable, we have now:

typedef struct fpregset
{
  int f_pcr;
  int f_psr;
  int f_fpiaddr;
#ifdef __mcoldfire__
  int f_fpregs[8][2];
#else
  int f_fpregs[8][3];
#endif
} fpregset_t;

And this is compatible with the kernel one.

So I'm going to update the RISU code to use the 2d array.

Thanks,
Laurent