[Pdbg] [PATCH v2 17/39] libpdbg: Remove enable_attn target command

[Joel Stanley]

On Wed, 20 Apr 2022 at 06:51, Nicholas Piggin <npiggin at gmail.com> wrote:
>
> enable_attn is only used by gdbserver and it can be implemented with
> existing target register access. Implementing it for P9, P10, and
> sbefifo backends results in duplication and P9/P10 cases in the sbefifo
> backend.
>
> Arguably it should be a middle-end function in the p*chip.c files which
> is backend-agnostic. For now move it to gdbserver and we can look at
> cleaning it up later.
>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>

Reviewed-by: Joel Stanley <joel at jms.id.au>

> ---
>  libpdbg/hwunit.h |  1 -
>  libpdbg/p8chip.c | 31 -------------------------------
>  src/pdbgproxy.c  | 35 +++++++++++++++++++++++++++++++++--
>  3 files changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h
> index 4662aec4..2ce41db8 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 92e18ccd..5b2a90a9 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 0cc33fbf..7267fd8a 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)
> @@ -255,7 +285,6 @@ static void put_mem(uint64_t *stack, void *priv)
>         uint8_t attn_opcode[] = {0x00, 0x00, 0x02, 0x00};
>         uint8_t gdb_break_opcode[] = {0x7d, 0x82, 0x10, 0x08};
>         int err = 0;
> -       struct thread *thread = target_to_thread(thread_target);
>
>         if (littleendian) {
>                 attn_opcode[1] = 0x02;
> @@ -285,8 +314,10 @@ static void put_mem(uint64_t *stack, void *priv)
>                 memcpy(data, attn_opcode, 4);
>
>                 /* Need to enable the attn instruction in HID0 */
> -               if (thread->enable_attn(thread))
> +               if (set_attn(true)) {
> +                       err = 2;
>                         goto out;
> +               }
>         }
>
>         if (mem_write(adu_target, addr, data, len, 0, false)) {
> --
> 2.35.1
>
> --
> Pdbg mailing list
> Pdbg at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/pdbg