Re: [PATCH v2 19/21] gdbstub: move register helpers into standalone include

2023-01-06 Thread Richard Henderson 

On 1/5/23 08:43, Alex Bennée wrote:

These inline helpers are all used by target specific code so move them
out of the general header so we don't needlessly pollute the rest of
the API with target specific stuff.

Note we have to include cpu.h in semihosting as it was relying on a
side effect before.

Signed-off-by: Alex Bennée
---


With Max's xtensa note fixed,
Reviewed-by: Richard Henderson 


r~

Re: [PATCH v2 19/21] gdbstub: move register helpers into standalone include

2023-01-05 Thread Max Filippov 
On Thu, Jan 5, 2023 at 8:51 AM Alex Bennée  wrote:
>
> These inline helpers are all used by target specific code so move them
> out of the general header so we don't needlessly pollute the rest of
> the API with target specific stuff.
>
> Note we have to include cpu.h in semihosting as it was relying on a
> side effect before.
>
> Signed-off-by: Alex Bennée 
> ---
>  target/xtensa/core-dc232b.c|   2 +-
>  target/xtensa/core-dc233c.c|   2 +-
>  target/xtensa/core-de212.c |   2 +-
>  target/xtensa/core-de233_fpu.c |   2 +-
>  target/xtensa/core-dsp3400.c   |   2 +-
>  target/xtensa/core-fsf.c   |   2 +-
>  target/xtensa/core-lx106.c |   2 +-
>  target/xtensa/core-sample_controller.c |   2 +-
>  target/xtensa/core-test_kc705_be.c |   2 +-
>  target/xtensa/core-test_mmuhifi_c3.c   |   2 +-
>  target/xtensa/gdbstub.c|   2 +-
>  target/xtensa/helper.c |   2 +-

Please update the target/xtensa/import_core.sh as well.

-- 
Thanks.
-- Max

Re: [PATCH v2 19/21] gdbstub: move register helpers into standalone include

2023-01-05 Thread Philippe Mathieu-Daudé 

On 5/1/23 17:43, Alex Bennée wrote:

These inline helpers are all used by target specific code so move them
out of the general header so we don't needlessly pollute the rest of
the API with target specific stuff.

Note we have to include cpu.h in semihosting as it was relying on a
side effect before.

Signed-off-by: Alex Bennée 
---
  include/exec/gdbstub.h |  86 -
  include/gdbstub/helpers.h  | 103 +
  semihosting/syscalls.c |   1 +
  target/alpha/gdbstub.c |   2 +-
  target/arm/gdbstub.c   |   1 +
  target/arm/gdbstub64.c |   2 +-
  target/arm/helper-a64.c|   2 +-
  target/arm/m_helper.c  |   2 +-
  target/avr/gdbstub.c   |   2 +-
  target/cris/gdbstub.c  |   2 +-
  target/hexagon/gdbstub.c   |   2 +-
  target/hppa/gdbstub.c  |   2 +-
  target/i386/gdbstub.c  |   2 +-
  target/i386/whpx/whpx-all.c|   2 +-
  target/loongarch/gdbstub.c |   1 +
  target/m68k/gdbstub.c  |   2 +-
  target/m68k/helper.c   |   1 +
  target/m68k/m68k-semi.c|   1 +
  target/microblaze/gdbstub.c|   2 +-
  target/mips/gdbstub.c  |   2 +-
  target/mips/tcg/sysemu/mips-semi.c |   1 +
  target/nios2/cpu.c |   2 +-
  target/nios2/nios2-semi.c  |   1 +
  target/openrisc/gdbstub.c  |   2 +-
  target/openrisc/interrupt.c|   2 +-
  target/openrisc/mmu.c  |   2 +-
  target/ppc/cpu_init.c  |   2 +-
  target/ppc/gdbstub.c   |   1 +
  target/riscv/gdbstub.c |   1 +
  target/rx/gdbstub.c|   2 +-
  target/s390x/gdbstub.c |   1 +
  target/s390x/helper.c  |   2 +-
  target/sh4/gdbstub.c   |   2 +-
  target/sparc/gdbstub.c |   2 +-
  target/tricore/gdbstub.c   |   2 +-
  target/xtensa/core-dc232b.c|   2 +-
  target/xtensa/core-dc233c.c|   2 +-
  target/xtensa/core-de212.c |   2 +-
  target/xtensa/core-de233_fpu.c |   2 +-
  target/xtensa/core-dsp3400.c   |   2 +-
  target/xtensa/core-fsf.c   |   2 +-
  target/xtensa/core-lx106.c |   2 +-
  target/xtensa/core-sample_controller.c |   2 +-
  target/xtensa/core-test_kc705_be.c |   2 +-
  target/xtensa/core-test_mmuhifi_c3.c   |   2 +-
  target/xtensa/gdbstub.c|   2 +-
  target/xtensa/helper.c |   2 +-
  47 files changed, 148 insertions(+), 121 deletions(-)
  create mode 100644 include/gdbstub/helpers.h


Reviewed-by: Philippe Mathieu-Daudé

RE: [PATCH v2 19/21] gdbstub: move register helpers into standalone include

2023-01-05 Thread Taylor Simpson 


> -Original Message-
> From: Alex Bennée 
> Sent: Thursday, January 5, 2023 10:43 AM
> To: qemu-devel@nongnu.org; alex.ben...@gmail.com
> Cc: David Hildenbrand ; Sunil Muthuswamy
> ; Aurelien Jarno ;
> Michael Rolnik ; Aleksandar Rikalo
> ; Greg Kurz ; Ilya
> Leoshkevich ; Thomas Huth ;
> qemu-...@nongnu.org; Laurent Vivier ; Max Filippov
> ; Yanan Wang ; Marek
> Vasut ; Stafford Horne ; Peter
> Maydell ; Daniel Henrique Barboza
> ; Palmer Dabbelt ; Alex
> Bennée ; Taylor Simpson
> ; Marcel Apfelbaum
> ; Alexandre Iooss ;
> Chris Wulff ; Richard Henderson
> ; Eduardo Habkost
> ; Song Gao ; Mark Cave-
> Ayland ; Cédric Le Goater ;
> Artyom Tarasenko ; Paolo Bonzini
> ; qemu-ri...@nongnu.org; qemu-
> s3...@nongnu.org; Alistair Francis ; Edgar E.
> Iglesias ; Bastian Koppelmann
> ; Jiaxun Yang
> ; Philippe Mathieu-Daudé ;
> Bin Meng ; Mahmoud Mandour
> ; David Gibson ;
> Yoshinori Sato ; Xiaojuan Yang
> ; qemu-...@nongnu.org
> Subject: [PATCH v2 19/21] gdbstub: move register helpers into standalone
> include
> 
> These inline helpers are all used by target specific code so move them out of
> the general header so we don't needlessly pollute the rest of the API with
> target specific stuff.
> 
> Note we have to include cpu.h in semihosting as it was relying on a side
> effect before.
> 
> Signed-off-by: Alex Bennée 
>
> diff --git a/target/hexagon/gdbstub.c b/target/hexagon/gdbstub.c
> index d152d01bfe..46083da620 100644
> --- a/target/hexagon/gdbstub.c
> +++ b/target/hexagon/gdbstub.c
> @@ -16,7 +16,7 @@
>   */
> 
>  #include "qemu/osdep.h"
> -#include "exec/gdbstub.h"
> +#include "gdbstub/helpers.h"
>  #include "cpu.h"
>  #include "internal.h"

Reviewed-by: Taylor Simpson

[PATCH v2 19/21] gdbstub: move register helpers into standalone include

2023-01-05 Thread Alex Bennée 
These inline helpers are all used by target specific code so move them
out of the general header so we don't needlessly pollute the rest of
the API with target specific stuff.

Note we have to include cpu.h in semihosting as it was relying on a
side effect before.

Signed-off-by: Alex Bennée 
---
 include/exec/gdbstub.h |  86 -
 include/gdbstub/helpers.h  | 103 +
 semihosting/syscalls.c |   1 +
 target/alpha/gdbstub.c |   2 +-
 target/arm/gdbstub.c   |   1 +
 target/arm/gdbstub64.c |   2 +-
 target/arm/helper-a64.c|   2 +-
 target/arm/m_helper.c  |   2 +-
 target/avr/gdbstub.c   |   2 +-
 target/cris/gdbstub.c  |   2 +-
 target/hexagon/gdbstub.c   |   2 +-
 target/hppa/gdbstub.c  |   2 +-
 target/i386/gdbstub.c  |   2 +-
 target/i386/whpx/whpx-all.c|   2 +-
 target/loongarch/gdbstub.c |   1 +
 target/m68k/gdbstub.c  |   2 +-
 target/m68k/helper.c   |   1 +
 target/m68k/m68k-semi.c|   1 +
 target/microblaze/gdbstub.c|   2 +-
 target/mips/gdbstub.c  |   2 +-
 target/mips/tcg/sysemu/mips-semi.c |   1 +
 target/nios2/cpu.c |   2 +-
 target/nios2/nios2-semi.c  |   1 +
 target/openrisc/gdbstub.c  |   2 +-
 target/openrisc/interrupt.c|   2 +-
 target/openrisc/mmu.c  |   2 +-
 target/ppc/cpu_init.c  |   2 +-
 target/ppc/gdbstub.c   |   1 +
 target/riscv/gdbstub.c |   1 +
 target/rx/gdbstub.c|   2 +-
 target/s390x/gdbstub.c |   1 +
 target/s390x/helper.c  |   2 +-
 target/sh4/gdbstub.c   |   2 +-
 target/sparc/gdbstub.c |   2 +-
 target/tricore/gdbstub.c   |   2 +-
 target/xtensa/core-dc232b.c|   2 +-
 target/xtensa/core-dc233c.c|   2 +-
 target/xtensa/core-de212.c |   2 +-
 target/xtensa/core-de233_fpu.c |   2 +-
 target/xtensa/core-dsp3400.c   |   2 +-
 target/xtensa/core-fsf.c   |   2 +-
 target/xtensa/core-lx106.c |   2 +-
 target/xtensa/core-sample_controller.c |   2 +-
 target/xtensa/core-test_kc705_be.c |   2 +-
 target/xtensa/core-test_mmuhifi_c3.c   |   2 +-
 target/xtensa/gdbstub.c|   2 +-
 target/xtensa/helper.c |   2 +-
 47 files changed, 148 insertions(+), 121 deletions(-)
 create mode 100644 include/gdbstub/helpers.h

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 8fff5450ed..bb8a3928dd 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -110,92 +110,6 @@ void gdb_register_coprocessor(CPUState *cpu,
   gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
   int num_regs, const char *xml, int g_pos);
 
-#ifdef NEED_CPU_H
-#include "cpu.h"
-
-/*
- * The GDB remote protocol transfers values in target byte order. As
- * the gdbstub may be batching up several register values we always
- * append to the array.
- */
-
-static inline int gdb_get_reg8(GByteArray *buf, uint8_t val)
-{
-g_byte_array_append(buf, &val, 1);
-return 1;
-}
-
-static inline int gdb_get_reg16(GByteArray *buf, uint16_t val)
-{
-uint16_t to_word = tswap16(val);
-g_byte_array_append(buf, (uint8_t *) &to_word, 2);
-return 2;
-}
-
-static inline int gdb_get_reg32(GByteArray *buf, uint32_t val)
-{
-uint32_t to_long = tswap32(val);
-g_byte_array_append(buf, (uint8_t *) &to_long, 4);
-return 4;
-}
-
-static inline int gdb_get_reg64(GByteArray *buf, uint64_t val)
-{
-uint64_t to_quad = tswap64(val);
-g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-return 8;
-}
-
-static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi,
- uint64_t val_lo)
-{
-uint64_t to_quad;
-#if TARGET_BIG_ENDIAN
-to_quad = tswap64(val_hi);
-g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-to_quad = tswap64(val_lo);
-g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-#else
-to_quad = tswap64(val_lo);
-g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-to_quad = tswap64(val_hi);
-g_byte_array_append(buf, (uint8_t *) &to_quad, 8);
-#endif
-return 16;
-}
-
-static inline int gdb_get_zeroes(GByteArray *array, size_t len)
-{
-guint oldlen = array->len;
-g_byte_array_set_size(array, oldlen + len);
-memset(array->data + oldlen, 0, len);
-
-return len;
-}
-
-/**
- * gdb_get_reg_ptr: get pointer to start of last element
- * @len: length of element
- *
- * This is a helper function to extract the pointer to the last
- * element for additional processing. Some front-ends do additional
- * d