[Pdbg] [PATCH 11/14] libpdbg: Remove enable_attn target command

[Joel Stanley]

On Mon, 14 Mar 2022 at 04:18, Nicholas Piggin <npiggin at gmail.com> wrote:
>
> enable_attn is only used by gdbserver and it can be implemented with
> existing target register access. Move it to gdbserver.

The downside of this is it moves chip-specific code into the proxy.
Would it make sense to keep all hardware specific code in libpdbg?



>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
>  libpdbg/hwunit.h |  1 -
>  libpdbg/p8chip.c | 31 -------------------------------
>  src/pdbgproxy.c  | 38 +++++++++++++++++++++++++++++++++++---
>  3 files changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h
> index 4662aec..2ce41db 100644
> --- a/libpdbg/hwunit.h
> +++ b/libpdbg/hwunit.h
> @@ -155,7 +155,6 @@ struct thread {
>         int (*ram_setup)(struct thread *);
>         int (*ram_instruction)(struct thread *, uint64_t opcode, uint64_t *scratch);
>         int (*ram_destroy)(struct thread *);
> -       int (*enable_attn)(struct thread *);
>
>         int (*getmem)(struct thread *, uint64_t, uint64_t *);
>         int (*getregs)(struct thread *, struct thread_regs *regs);
> diff --git a/libpdbg/p8chip.c b/libpdbg/p8chip.c
> index 92e18cc..5b2a90a 100644
> --- a/libpdbg/p8chip.c
> +++ b/libpdbg/p8chip.c
> @@ -611,36 +611,6 @@ static int p8_get_hid0(struct pdbg_target *chip, uint64_t *value)
>         return 0;
>  }
>
> -static int p8_put_hid0(struct pdbg_target *chip, uint64_t value)
> -{
> -       CHECK_ERR(pib_write(chip, HID0_REG, value));
> -       return 0;
> -}
> -
> -static int p8_enable_attn(struct thread *thread)
> -{
> -       struct pdbg_target *core;
> -       uint64_t hid0;
> -
> -       core = pdbg_target_require_parent("core", &thread->target);
> -
> -       /* Need to enable the attn instruction in HID0 */
> -       if (p8_get_hid0(core, &hid0)) {
> -               PR_ERROR("Unable to get HID0\n");
> -               return 1;
> -       }
> -       PR_INFO("HID0 was 0x%"PRIx64 " \n", hid0);
> -
> -       hid0 |= EN_ATTN;
> -
> -       PR_INFO("writing 0x%"PRIx64 " to HID0\n", hid0);
> -       if (p8_put_hid0(core, hid0)) {
> -               PR_ERROR("Unable to set HID0\n");
> -               return 1;
> -       }
> -       return 0;
> -}
> -
>  static struct thread p8_thread = {
>         .target = {
>                 .name = "POWER8 Thread",
> @@ -657,7 +627,6 @@ static struct thread p8_thread = {
>         .ram_setup = p8_ram_setup,
>         .ram_instruction = p8_ram_instruction,
>         .ram_destroy = p8_ram_destroy,
> -       .enable_attn = p8_enable_attn,
>         .getmem = ram_getmem,
>         .getregs = ram_getregs,
>         .getgpr = ram_getgpr,
> diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c
> index ce52e9d..75e14b7 100644
> --- a/src/pdbgproxy.c
> +++ b/src/pdbgproxy.c
> @@ -24,6 +24,7 @@
>  #include "optcmd.h"
>  #include "debug.h"
>  #include "path.h"
> +#include "sprs.h"
>
>  #ifndef DISABLE_GDBSERVER
>
> @@ -101,6 +102,35 @@ static void detach(uint64_t *stack, void *priv)
>         send_response(fd, OK);
>  }
>
> +#define POWER8_HID_ENABLE_ATTN                 PPC_BIT(31)
> +
> +static int set_attn(bool enable)
> +{
> +       uint64_t hid;
> +
> +       if (thread_getspr(thread_target, SPR_HID, &hid))
> +               return -1;
> +
> +       if (pdbg_target_compatible(thread_target, "ibm,power8-thread")) {
> +               if (enable) {
> +                       if (hid & POWER8_HID_ENABLE_ATTN)
> +                               return 0;
> +                       hid |= POWER8_HID_ENABLE_ATTN;
> +               } else {
> +                       if (!(hid & POWER8_HID_ENABLE_ATTN))
> +                               return 0;
> +                       hid &= ~POWER8_HID_ENABLE_ATTN;
> +               }
> +       } else {
> +               return -1;
> +       }
> +
> +       if (thread_putspr(thread_target, SPR_HID, hid))
> +               return -1;
> +
> +       return 0;
> +}
> +
>  /* 32 registers represented as 16 char hex numbers with null-termination */
>  #define REG_DATA_SIZE (32*16+1)
>  static void get_gprs(uint64_t *stack, void *priv)
> @@ -254,7 +284,6 @@ static void put_mem(uint64_t *stack, void *priv)
>         uint8_t *data;
>         uint8_t attn_opcode[] = {0x00, 0x00, 0x02, 0x00};
>         int err = 0;
> -       struct thread *thread = target_to_thread(thread_target);
>
>         if (littleendian) {
>                 attn_opcode[1] = 0x02;
> @@ -285,10 +314,13 @@ static void put_mem(uint64_t *stack, void *priv)
>                 data = attn_opcode;
>
>                 /* Need to enable the attn instruction in HID0 */
> -               if (thread->enable_attn(thread))
> +               if (set_attn(true)) {
> +                       err = 2;
>                         goto out;
> -       } else
> +               }
> +       } else {
>                 stack[2] = __builtin_bswap64(stack[2]) >> 32;
> +       }
>
>         PR_INFO("put_mem 0x%016" PRIx64 " = 0x%016" PRIx64 "\n", addr, stack[2]);
>
> --
> 2.23.0
>
> --
> Pdbg mailing list
> Pdbg at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/pdbg