Re: [PATCH v2 06/14] target/arm: use gdb_get_reg helpers

[Alex Bennée]

Alan Hayward <address@hidden> writes:

>> On 1 Dec 2019, at 20:05, Philippe Mathieu-Daudé <address@hidden> wrote:
>> 
>> On 11/30/19 9:45 AM, Alex Bennée wrote:
>>> This is cleaner than poking memory directly and will make later
>>> clean-ups easier.
>>> Signed-off-by: Alex Bennée <address@hidden>
>>> ---
>>> v2
>>>   - make sure we pass hi/lo correctly as quads are stored in LE order
>>> ---
>>>  target/arm/helper.c | 18 +++++++-----------
>>>  1 file changed, 7 insertions(+), 11 deletions(-)
>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>>> index 0bf8f53d4b8..0ac950d6c71 100644
>>> --- a/target/arm/helper.c
>>> +++ b/target/arm/helper.c
>>> @@ -105,21 +105,17 @@ static int aarch64_fpu_gdb_get_reg(CPUARMState *env, 
>>> uint8_t *buf, int reg)
>>>  {
>>>      switch (reg) {
>>>      case 0 ... 31:
>>> -        /* 128 bit FP register */
>>> -        {
>>> -            uint64_t *q = aa64_vfp_qreg(env, reg);
>>> -            stq_le_p(buf, q[0]);
>>> -            stq_le_p(buf + 8, q[1]);
>>> -            return 16;
>>> -        }
>>> +    {
>>> +        /* 128 bit FP register - quads are in LE order */
>> 
>> Oh, this was always wrong on BE :(
>
> Am I right in thinking this patch correctly matches the SVE BE changes from 
> June?
>
> Specifically, this patch:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2019-June/657826.html

Not quite. This is just taking into account the way we store the data
internally in cpu.h. The gdb_get_reg128 helper will then ensure stuff is
in target endian format which is what gdbstub defines.

There aren't any actual kernel to userspace transfers going on here.

>
>
> Alan.
>
>> 
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>> 
>>> +        uint64_t *q = aa64_vfp_qreg(env, reg);
>>> +        return gdb_get_reg128(buf, q[1], q[0]);
>>> +    }
>>>      case 32:
>>>          /* FPSR */
>>> -        stl_p(buf, vfp_get_fpsr(env));
>>> -        return 4;
>>> +        return gdb_get_reg32(buf, vfp_get_fpsr(env));
>>>      case 33:
>>>          /* FPCR */
>>> -        stl_p(buf, vfp_get_fpcr(env));
>>> -        return 4;
>>> +        return gdb_get_reg32(buf,vfp_get_fpcr(env));
>>>      default:
>>>          return 0;
>>>      }


-- 
Alex Bennée