diff mbox series

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

Message ID 20220420065013.222816-18-npiggin@gmail.com
State New
Headers show
Series gdbserver multi-threaded debugging and POWER9/10 support | expand

Commit Message

Nicholas Piggin April 20, 2022, 6:49 a.m. UTC
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@gmail.com>
---
 libpdbg/hwunit.h |  1 -
 libpdbg/p8chip.c | 31 -------------------------------
 src/pdbgproxy.c  | 35 +++++++++++++++++++++++++++++++++--
 3 files changed, 33 insertions(+), 34 deletions(-)

Comments

Joel Stanley May 3, 2022, 7:05 a.m. UTC | #1
On Wed, 20 Apr 2022 at 06:51, Nicholas Piggin <npiggin@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@gmail.com>

Reviewed-by: Joel Stanley <joel@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@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/pdbg
diff mbox series

Patch

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)) {