| Message ID | 20191130084602.10818-7-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gdbstub refactor and SVE support | expand |
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 <alex.bennee@linaro.org> > > --- > 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 :( Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + 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; > } >
On 11/30/19 8: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 <alex.bennee@linaro.org> > > --- > 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(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
> On 1 Dec 2019, at 20:05, Philippe Mathieu-Daudé <philmd@redhat.com> 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 <alex.bennee@linaro.org> >> --- >> 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 Alan. > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> + 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; >> }
Alan Hayward <Alan.Hayward@arm.com> writes: >> On 1 Dec 2019, at 20:05, Philippe Mathieu-Daudé <philmd@redhat.com> 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 <alex.bennee@linaro.org> >>> --- >>> 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é <philmd@redhat.com> >> >>> + 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; >>> }
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 */ + 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; }
This is cleaner than poking memory directly and will make later clean-ups easier. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- 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(-)