[Pdbg] [PATCH v4 02/13] libpdbg: Drop target argument from mem_read/mem_write
Alistair Popple
alistair at popple.id.au
Mon Jul 22 16:10:57 AEST 2019
Hi Amitay,
Does anything in the rest of this series depend on patches 1 and 2? From a
design standpoint they conflict a bit with what we're trying to do with the
backend descriptions so if possible I'd like to revisit the approach here.
However I would like to merge the rest of the functionality in this series so
if you don't mind I'd like to drop these two patches and treat them
separately. Thanks.
- Alistair
On Wednesday, 17 July 2019 2:08:41 PM AEST Amitay Isaacs wrote:
> The api in libpdbg requires a target only if operating on a specific
> instance of that class. For memory there is only single instance (even
> though there is no memory class in libpdbg yet).
>
> Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> ---
> libpdbg/libpdbg.h | 4 ++--
> libpdbg/target.c | 4 ++--
> src/mem.c | 4 ++--
> src/pdbgproxy.c | 23 +++++------------------
> src/thread.c | 22 +++++++---------------
> 5 files changed, 18 insertions(+), 39 deletions(-)
>
> diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> index 711c817..63b7a0f 100644
> --- a/libpdbg/libpdbg.h
> +++ b/libpdbg/libpdbg.h
> @@ -232,8 +232,8 @@ int __adu_getmem(struct pdbg_target *target, uint64_t
> addr, uint8_t *ouput, int __adu_putmem(struct pdbg_target *target, uint64_t
> addr, uint8_t *input, uint64_t size, bool ci);
>
> -int mem_read(struct pdbg_target *target, uint64_t addr, uint8_t *output,
> uint64_t size, uint8_t block_size, bool ci); -int mem_write(struct
> pdbg_target *target, uint64_t addr, uint8_t *input, uint64_t size, uint8_t
> block_size, bool ci); +int mem_read(uint64_t addr, uint8_t *output,
> uint64_t size, uint8_t block_size, bool ci); +int mem_write(uint64_t addr,
> uint8_t *input, uint64_t size, uint8_t block_size, bool ci);
>
> int opb_read(struct pdbg_target *target, uint32_t addr, uint32_t *data);
> int opb_write(struct pdbg_target *target, uint32_t addr, uint32_t data);
> diff --git a/libpdbg/target.c b/libpdbg/target.c
> index 757c0db..4cf5491 100644
> --- a/libpdbg/target.c
> +++ b/libpdbg/target.c
> @@ -217,7 +217,7 @@ int fsi_write(struct pdbg_target *fsi_dt, uint32_t addr,
> uint32_t data) return fsi->write(fsi, addr64, data);
> }
>
> -int mem_read(struct pdbg_target *target, uint64_t addr, uint8_t *output,
> uint64_t size, uint8_t block_size, bool ci) +int mem_read(uint64_t addr,
> uint8_t *output, uint64_t size, uint8_t block_size, bool ci) {
> int rc = -1;
> struct pdbg_target *sbefifo_target, *adu_target;
> @@ -245,7 +245,7 @@ int mem_read(struct pdbg_target *target, uint64_t addr,
> uint8_t *output, uint64_ return rc;
> }
>
> -int mem_write(struct pdbg_target *target, uint64_t addr, uint8_t *input,
> uint64_t size, uint8_t block_size, bool ci) +int mem_write(uint64_t addr,
> uint8_t *input, uint64_t size, uint8_t block_size, bool ci) {
> int rc = -1;
> struct pdbg_target *sbefifo_target, *adu_target;
> diff --git a/src/mem.c b/src/mem.c
> index 8b62de2..dfca4d7 100644
> --- a/src/mem.c
> +++ b/src/mem.c
> @@ -96,7 +96,7 @@ static int _getmem(uint64_t addr, uint64_t size, uint8_t
> block_size, bool ci, bo
>
> pdbg_set_progress_tick(progress_tick);
> progress_init();
> - rc = mem_read(NULL, addr, buf, size, block_size, ci);
> + rc = mem_read(addr, buf, size, block_size, ci);
> progress_end();
>
> if (rc) {
> @@ -163,7 +163,7 @@ static int _putmem(uint64_t addr, uint8_t block_size,
> bool ci)
>
> pdbg_set_progress_tick(progress_tick);
> progress_init();
> - rc = mem_write(NULL, addr, buf, buflen, block_size, ci);
> + rc = mem_write(addr, buf, buflen, block_size, ci);
> progress_end();
>
> if (rc) {
> diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c
> index 5bba03f..dfa3075 100644
> --- a/src/pdbgproxy.c
> +++ b/src/pdbgproxy.c
> @@ -42,7 +42,6 @@
> #define TEST_SKIBOOT_ADDR 0x40000000
>
> static struct pdbg_target *thread_target = NULL;
> -static struct pdbg_target *adu_target;
> static struct timeval timeout;
> static int poll_interval = 100;
> static int fd = -1;
> @@ -223,7 +222,7 @@ static void get_mem(uint64_t *stack, void *priv)
>
> linear_map = get_real_addr(addr);
> if (linear_map != -1UL) {
> - if (mem_read(adu_target, linear_map, (uint8_t *) data, len, 0,
false)) {
> + if (mem_read(linear_map, (uint8_t *) data, len, 0, false)) {
> PR_ERROR("Unable to read memory\n");
> err = 1;
> }
> @@ -293,7 +292,7 @@ static void put_mem(uint64_t *stack, void *priv)
>
> PR_INFO("put_mem 0x%016" PRIx64 " = 0x%016" PRIx64 "\n", addr, stack[2]);
>
> - if (mem_write(adu_target, addr, data, len, 0, false)) {
> + if (mem_write(addr, data, len, 0, false)) {
> PR_ERROR("Unable to write memory\n");
> err = 3;
> }
> @@ -420,7 +419,7 @@ command_cb callbacks[LAST_CMD + 1] = {
> disconnect,
> NULL};
>
> -int gdbserver_start(struct pdbg_target *thread, struct pdbg_target *adu,
> uint16_t port) +int gdbserver_start(struct pdbg_target *thread, uint16_t
> port)
> {
> int sock, i;
> struct sockaddr_in name;
> @@ -428,7 +427,6 @@ int gdbserver_start(struct pdbg_target *thread, struct
> pdbg_target *adu, uint16_
>
> parser_init(callbacks);
> thread_target = thread;
> - adu_target = adu;
>
> sock = socket(PF_INET, SOCK_STREAM, 0);
> if (sock < 0) {
> @@ -495,7 +493,7 @@ int gdbserver_start(struct pdbg_target *thread, struct
> pdbg_target *adu, uint16_
>
> static int gdbserver(uint16_t port)
> {
> - struct pdbg_target *target, *adu, *thread = NULL;
> + struct pdbg_target *target, *thread = NULL;
> uint64_t msr;
> int rc;
>
> @@ -531,18 +529,7 @@ static int gdbserver(uint16_t port)
> }
> littleendian = 0x01 & msr;
>
> - /* Select ADU target */
> - pdbg_for_each_class_target("adu", adu) {
> - if (pdbg_target_probe(adu) == PDBG_TARGET_ENABLED)
> - break;
> - }
> -
> - if (!adu) {
> - fprintf(stderr, "No ADU found\n");
> - return 0;
> - }
> -
> - gdbserver_start(thread, adu, port);
> + gdbserver_start(thread, port);
> return 0;
> }
> #else
> diff --git a/src/thread.c b/src/thread.c
> index 7fd53a8..20b5eeb 100644
> --- a/src/thread.c
> +++ b/src/thread.c
> @@ -33,9 +33,9 @@ static bool is_real_address(struct thread_regs *regs,
> uint64_t addr) return false;
> }
>
> -static int load8(struct pdbg_target *target, uint64_t addr, uint64_t
> *value) +static int load8(uint64_t addr, uint64_t *value)
> {
> - if (mem_read(target, addr, (uint8_t *)value, 8, 0, false)) {
> + if (mem_read(addr, (uint8_t *)value, 8, 0, false)) {
> pdbg_log(PDBG_ERROR, "Unable to read memory address=%016" PRIx64 ".
\n",
> addr); return 0;
> }
> @@ -52,7 +52,7 @@ uint64_t flip_endian(uint64_t v)
> #endif
> }
>
> -static int dump_stack(struct thread_regs *regs, struct pdbg_target *adu)
> +static int dump_stack(struct thread_regs *regs)
> {
> uint64_t next_sp = regs->gprs[1];
> uint64_t pc;
> @@ -78,9 +78,9 @@ static int dump_stack(struct thread_regs *regs, struct
> pdbg_target *adu) if (!is_real_address(regs, sp))
> break;
>
> - if (!load8(adu, sp, &tmp))
> + if (!load8(sp, &tmp))
> return 1;
> - if (!load8(adu, sp + 16, &pc))
> + if (!load8(sp + 16, &pc))
> return 1;
>
> if (!tmp) {
> @@ -337,16 +337,8 @@ static int thread_regs_print(struct reg_flags flags)
> if (thread_getregs(thread, ®s))
> continue;
>
> - if (flags.do_backtrace) {
> - struct pdbg_target *adu;
> -
> - pdbg_for_each_class_target("adu", adu) {
> - if (pdbg_target_probe(adu) == PDBG_TARGET_ENABLED) {
> - dump_stack(®s, adu);
> - break;
> - }
> - }
> - }
> + if (flags.do_backtrace)
> + dump_stack(®s);
>
> count++;
> }
More information about the Pdbg
mailing list