Re: [PATCH v4 24/24] target/m68k: Implement FPIAR

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 3/3/25 21:40, Richard Henderson wrote:
> On 3/3/25 10:39, Philippe Mathieu-Daudé wrote:
>> Hi Richard,
>>
>> On 24/2/25 18:14, Richard Henderson wrote:
>>> So far, this is only read-as-written.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2497
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>   target/m68k/cpu.h       |  1 +
>>>   target/m68k/cpu.c       | 23 ++++++++++++++++++++++-
>>>   target/m68k/helper.c    | 14 ++++++++------
>>>   target/m68k/translate.c |  3 ++-
>>>   4 files changed, 33 insertions(+), 8 deletions(-)
>>
>>
>>> diff --git a/target/m68k/helper.c b/target/m68k/helper.c
>>> index 6e3bb96762..bc787cbf05 100644
>>> --- a/target/m68k/helper.c
>>> +++ b/target/m68k/helper.c
>>> @@ -45,8 +45,8 @@ static int cf_fpu_gdb_get_reg(CPUState *cs, 
>>> GByteArray *mem_buf, int n)
>>>           return gdb_get_reg32(mem_buf, env->fpcr);
>>>       case 9: /* fpstatus */
>>>           return gdb_get_reg32(mem_buf, env->fpsr);
>>> -    case 10: /* fpiar, not implemented */
>>> -        return gdb_get_reg32(mem_buf, 0);
>>> +    case 10: /* fpiar */
>>> +        return gdb_get_reg32(mem_buf, env->fpiar);
>>>       }
>>>       return 0;
>>>   }
>>> @@ -69,7 +69,8 @@ static int cf_fpu_gdb_set_reg(CPUState *cs, uint8_t 
>>> *mem_buf, int n)
>>>       case 9: /* fpstatus */
>>>           env->fpsr = ldl_be_p(mem_buf);
>>>           return 4;
>>> -    case 10: /* fpiar, not implemented */
>>> +    case 10: /* fpiar */
>>> +        env->fpiar = ldl_p(mem_buf);
>>
>> Should we consider target endianness?
> 
> I am.  Are you suggesting that the TARGET_BIG_ENDIAN shorthand be 
> eliminated entirely, even from target-specific code?

As we figured earlier, while ldl_p() is expanded as ldl_be_p() for
m68k, it is easier to review using the expanded form, as done
previously in this file in commit 3a76d302047:

commit 3a76d302047ce4518cedef7a9feca1238a00f97b
Author: Philippe Mathieu-Daudé <philmd@linaro.org>
Date:   Fri Oct 4 13:30:34 2024 -0300

     target/m68k: Use explicit big-endian LD/ST API

     The M68K architecture uses big endianness. Directly use
     the big-endian LD/ST API.

So using ldl_be_p():
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Regards,

Phil.