diff mbox series

[8/8] pdbg: Use new command parsing

Message ID 20180620053409.14538-9-alistair@popple.id.au
State Accepted
Headers show
Series Rework option parsing | expand

Commit Message

Alistair Popple June 20, 2018, 5:34 a.m. UTC
This switches all commands except the htm command to use the new command/option
parsing code. For the moment we leave the usage help and generic targeting flag
processing alone, but future changes should allow this to also use the common
parsing code.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 Makefile.am  |   1 +
 src/cfam.c   |  55 ++++++--------------
 src/cfam.h   |  18 -------
 src/main.c   | 125 ++++++++++++++++++++++++++++-----------------
 src/mem.c    | 124 ++++++++------------------------------------
 src/mem.h    |  20 --------
 src/reg.c    | 164 +++++++++++++++--------------------------------------------
 src/reg.h    |  20 --------
 src/ring.c   |  32 ++----------
 src/ring.h   |  17 -------
 src/scom.c   |  54 +++++---------------
 src/scom.h   |  18 -------
 src/thread.c |  87 ++++++++++++++++++++++---------
 src/thread.h |  23 ---------
 14 files changed, 237 insertions(+), 521 deletions(-)
 delete mode 100644 src/cfam.h
 delete mode 100644 src/mem.h
 delete mode 100644 src/reg.h
 delete mode 100644 src/ring.h
 delete mode 100644 src/scom.h
 delete mode 100644 src/thread.h

Comments

Rashmica Gupta June 28, 2018, 1:55 a.m. UTC | #1
On 20/06/18 15:34, Alistair Popple wrote:
> This switches all commands except the htm command to use the new command/option
> parsing code. For the moment we leave the usage help and generic targeting flag
> processing alone, but future changes should allow this to also use the common
> parsing code.
>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
>  Makefile.am  |   1 +
>  src/cfam.c   |  55 ++++++--------------
>  src/cfam.h   |  18 -------
>  src/main.c   | 125 ++++++++++++++++++++++++++++-----------------
>  src/mem.c    | 124 ++++++++------------------------------------
>  src/mem.h    |  20 --------
>  src/reg.c    | 164 +++++++++++++++--------------------------------------------
>  src/reg.h    |  20 --------
>  src/ring.c   |  32 ++----------
>  src/ring.h   |  17 -------
>  src/scom.c   |  54 +++++---------------
>  src/scom.h   |  18 -------
>  src/thread.c |  87 ++++++++++++++++++++++---------
>  src/thread.h |  23 ---------
>  14 files changed, 237 insertions(+), 521 deletions(-)
>  delete mode 100644 src/cfam.h
>  delete mode 100644 src/mem.h
>  delete mode 100644 src/reg.h
>  delete mode 100644 src/ring.h
>  delete mode 100644 src/scom.h
>  delete mode 100644 src/thread.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 23f2080..4e0dce2 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -39,6 +39,7 @@ pdbg_SOURCES = \
>  	src/ring.c \
>  	src/htm.c \
>  	src/progress.c \
> +	src/parsers.c \
>  	src/optcmd.c \
>  	src/options_@ARCH@.c
>  
> diff --git a/src/cfam.c b/src/cfam.c
> index 269123e..6dab388 100644
> --- a/src/cfam.c
> +++ b/src/cfam.c
> @@ -20,8 +20,9 @@
>  #include <inttypes.h>
>  
>  #include "main.h"
> +#include "optcmd.h"
>  
> -static int getcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)
> +static int _getcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)
>  {
>  	uint32_t value;
>  
> @@ -33,7 +34,15 @@ static int getcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, u
>  	return 1;
>  }
>  
> -static int putcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)
> +static int getcfam(uint32_t addr)
> +{
> +	uint64_t addr64 = addr;
> +
> +	return for_each_target("fsi", _getcfam, &addr64, NULL);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getcfam, getcfam, (ADDRESS32));
> +
> +static int _putcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)
>  {
>  	if (fsi_write(target, *addr, *data))
>  		return 0;
> @@ -41,44 +50,10 @@ static int putcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, u
>  	return 1;
>  }
>  
> -int handle_cfams(int optind, int argc, char *argv[])
> +static int putcfam(uint32_t addr, uint32_t data)
>  {
> -	uint64_t addr;
> -	char *endptr;
> -
> -	if (optind + 1 >= argc) {
> -		printf("%s: command '%s' requires an address\n", argv[0], argv[optind]);
> -		return -1;
> -	}
> -
> -	errno = 0;
> -	addr = strtoull(argv[optind + 1], &endptr, 0);
> -	if (errno || *endptr != '\0') {
> -		printf("%s: command '%s' couldn't parse address '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -		return -1;
> -	}
> +	uint64_t addr64 = addr, data64 = data;
>  
> -	if (strcmp(argv[optind], "putcfam") == 0) {
> -		uint64_t data;
> -
> -		if (optind + 2 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> -
> -		errno = 0;
> -		data = strtoull(argv[optind + 2], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> -
> -		return for_each_target("fsi", putcfam, &addr, &data);
> -	}
> -
> -	return for_each_target("fsi", getcfam, &addr, NULL);
> +	return for_each_target("fsi", _putcfam, &addr64, &data64);
>  }
> -
> -
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putcfam, putcfam, (ADDRESS32, DATA32));
> diff --git a/src/cfam.h b/src/cfam.h
> deleted file mode 100644
> index 997ed3d..0000000
> --- a/src/cfam.h
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - * 	http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -#include <inttypes.h>
> -
> -int handle_cfams(int optind, int argc, char *argv[]);
> diff --git a/src/main.c b/src/main.c
> index cf9a3b2..1cfaca7 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -34,14 +34,9 @@
>  #include <target.h>
>  
>  #include "main.h"
> -#include "cfam.h"
> -#include "scom.h"
> -#include "reg.h"
> -#include "ring.h"
> -#include "mem.h"
> -#include "thread.h"
>  #include "htm.h"
>  #include "options.h"
> +#include "optcmd.h"
>  
>  #define PR_ERROR(x, args...) \
>  	pdbg_log(PDBG_ERROR, x, ##args)
> @@ -77,44 +72,64 @@ static int **processorsel[MAX_PROCESSORS];
>  static int *chipsel[MAX_PROCESSORS][MAX_CHIPS];
>  static int threadsel[MAX_PROCESSORS][MAX_CHIPS][MAX_THREADS];
>  
> -static int handle_probe(int optind, int argc, char *argv[]);
> -static int handle_release(int optind, int argc, char *argv[]);
> +static int probe(void);
> +static int release(void);
> +
> +/* TODO: We are repeating ourselves here. A little bit more macro magic could
> + * easily fix this but I was hesitant to introduce too much magic all at
> + * once. */
> +extern struct optcmd_cmd
> +	optcmd_getscom, optcmd_putscom,	optcmd_getcfam, optcmd_putcfam,
> +	optcmd_getgpr, optcmd_putgpr, optcmd_getspr, optcmd_putspr,
> +	optcmd_getnia, optcmd_putnia, optcmd_getmsr, optcmd_putmsr,
> +	optcmd_getring, optcmd_start, optcmd_stop, optcmd_step,
> +	optcmd_threadstatus, optcmd_sreset, optcmd_regs, optcmd_probe,
> +	optcmd_getmem, optcmd_putmem;
> +
> +static struct optcmd_cmd *cmds[] = {
> +	&optcmd_getscom, &optcmd_putscom, &optcmd_getcfam, &optcmd_putcfam,
> +	&optcmd_getgpr, &optcmd_putgpr, &optcmd_getspr, &optcmd_putspr,
> +	&optcmd_getnia, &optcmd_putnia, &optcmd_getmsr, &optcmd_putmsr,
> +	&optcmd_getring, &optcmd_start, &optcmd_stop, &optcmd_step,
> +	&optcmd_threadstatus, &optcmd_sreset, &optcmd_regs, &optcmd_probe,
> +	&optcmd_getmem, &optcmd_putmem,
> +};

I think you're missing optcmd_release from these declarations?


>  
> +/* Purely for printing usage text. We could integrate printing argument and flag
> + * help into optcmd if desired. */
>  struct action {
>  	const char *name;
>  	const char *args;
>  	const char *desc;
> -	int (*fn)(int, int, char **);
>  };
>  
>  static struct action actions[] = {
> -	{ "getgpr",  "<gpr>", "Read General Purpose Register (GPR)", &handle_gpr },
> -	{ "putgpr",  "<gpr> <value>", "Write General Purpose Register (GPR)", &handle_gpr },
> -	{ "getnia",  "", "Get Next Instruction Address (NIA)", &handle_nia },
> -	{ "putnia",  "<value>", "Write Next Instrution Address (NIA)", &handle_nia },
> -	{ "getspr",  "<spr>", "Get Special Purpose Register (SPR)", &handle_spr },
> -	{ "putspr",  "<spr> <value>", "Write Special Purpose Register (SPR)", &handle_spr },
> -	{ "getmsr",  "", "Get Machine State Register (MSR)", &handle_msr },
> -	{ "putmsr",  "<value>", "Write Machine State Register (MSR)", &handle_msr },
> -	{ "getring", "<addr> <len>", "Read a ring. Length must be correct", &handle_getring },
> -	{ "start",   "", "Start thread", &thread_start },
> -	{ "step",    "<count>", "Set a thread <count> instructions", &thread_step },
> -	{ "stop",    "", "Stop thread", &thread_stop },
> -	{ "htm", "core|nest start|stop|status|reset|dump|trace|analyse", "Hardware Trace Macro", &run_htm },
> -	{ "release", "", "Should be called after pdbg work is finished, to release special wakeups and other resources.", &handle_release},
> -	{ "probe", "", "", &handle_probe },
> -	{ "getcfam", "<address>", "Read system cfam", &handle_cfams },
> -	{ "putcfam", "<address> <value> [<mask>]", "Write system cfam", &handle_cfams },
> -	{ "getscom", "<address>", "Read system scom", &handle_scoms },
> -	{ "putscom", "<address> <value> [<mask>]", "Write system scom", &handle_scoms },
> -	{ "getmem",  "<address> <count>", "Read system memory", &handle_mem },
> -	{ "putmem",  "<address>", "Write to system memory", &handle_mem },
> -	{ "threadstatus", "", "Print the status of a thread", &thread_status_print },
> -	{ "sreset",  "", "Reset", &thread_sreset },
> -	{ "regs",  "", "State", &thread_state },
> +	{ "getgpr",  "<gpr>", "Read General Purpose Register (GPR)" },
> +	{ "putgpr",  "<gpr> <value>", "Write General Purpose Register (GPR)" },
> +	{ "getnia",  "", "Get Next Instruction Address (NIA)" },
> +	{ "putnia",  "<value>", "Write Next Instrution Address (NIA)" },
> +	{ "getspr",  "<spr>", "Get Special Purpose Register (SPR)" },
> +	{ "putspr",  "<spr> <value>", "Write Special Purpose Register (SPR)" },
> +	{ "getmsr",  "", "Get Machine State Register (MSR)" },
> +	{ "putmsr",  "<value>", "Write Machine State Register (MSR)" },
> +	{ "getring", "<addr> <len>", "Read a ring. Length must be correct" },
> +	{ "start",   "", "Start thread" },
> +	{ "step",    "<count>", "Set a thread <count> instructions" },
> +	{ "stop",    "", "Stop thread" },
> +	{ "htm", "core|nest start|stop|status|reset|dump|trace|analyse", "Hardware Trace Macro" },
> +	{ "release", "", "Should be called after pdbg work is finished" },
> +	{ "probe", "", "" },
> +	{ "getcfam", "<address>", "Read system cfam" },
> +	{ "putcfam", "<address> <value> [<mask>]", "Write system cfam" },
> +	{ "getscom", "<address>", "Read system scom" },
> +	{ "putscom", "<address> <value> [<mask>]", "Write system scom" },
> +	{ "getmem",  "<address> <count>", "Read system memory" },
> +	{ "putmem",  "<address>", "Write to system memory" },
> +	{ "threadstatus", "", "Print the status of a thread" },
> +	{ "sreset",  "", "Reset" },
> +	{ "regs",  "", "State" },
>  };
>  
> -
>  static void print_usage(char *pname)
>  {
>  	int i;
> @@ -600,7 +615,7 @@ static void release_target(struct pdbg_target *target)
>  	pdbg_target_release(target);
>  }
>  
> -static void do_release(void)
> +static int release(void)
>  {
>  	struct pdbg_target_class *target_class;
>  
> @@ -610,7 +625,10 @@ static void do_release(void)
>  		pdbg_for_each_class_target(target_class->name, target)
>  			release_target(target);
>  	}
> +
> +	return 0;
>  }
> +OPTCMD_DEFINE_CMD(release, release);
>  
>  void print_target(struct pdbg_target *target, int level)
>  {
> @@ -652,7 +670,7 @@ void print_target(struct pdbg_target *target, int level)
>  	}
>  }
>  
> -static int handle_probe(int optind, int argc, char *argv[])
> +static int probe(void)
>  {
>  	struct pdbg_target *target;
>  
> @@ -664,25 +682,21 @@ static int handle_probe(int optind, int argc, char *argv[])
>  
>  	return 1;
>  }
> +OPTCMD_DEFINE_CMD(probe, probe);
>  
>  /*
>   * Release handler.
>   */
>  static void atexit_release(void)
>  {
> -	do_release();
> -}
> -
> -static int handle_release(int optind, int argc, char *argv[])
> -{
> -	do_release();
> -
> -	return 1;
> +	release();
>  }
>  
>  int main(int argc, char *argv[])
>  {
>  	int i, rc = 0;
> +	void **args, **flags;
> +	optcmd_cmd_t *cmd;
>  
>  	backend = default_backend();
>  	device_node = default_target(backend);
> @@ -714,13 +728,28 @@ int main(int argc, char *argv[])
>  
>  	atexit(atexit_release);
>  
> -	for (i = 0; i < ARRAY_SIZE(actions); i++) {
> -		if (strcmp(argv[optind], actions[i].name) == 0) {
> -			rc = actions[i].fn(optind, argc, argv);
> -			goto found_action;
> +	for (i = 0; i < ARRAY_SIZE(cmds); i++) {
> +		if (!strcmp(argv[optind], cmds[i]->cmd)) {
> +			/* Found our command */
> +			cmd = optcmd_parse(cmds[i], (const char **) &argv[optind + 1],
> +					   argc - (optind + 1), &args, &flags);
> +			if (cmd) {
> +				rc = cmd(args, flags);
> +				goto found_action;
> +			} else {
> +				/* Error parsing arguments so exit return directly */
> +				return 1;
> +			}
>  		}
>  	}
>  
> +	/* Process subcommands. Currently only 'htm'.
> +	 * TODO: Move htm command parsing to optcmd once htm clean-up is complete */
> +	if (!strcmp(argv[optind], "htm")) {
> +		run_htm(optind, argc, argv);
> +		goto found_action;
> +	}
> +
>  	PR_ERROR("Unsupported command: %s\n", argv[optind]);
>  	return 1;
>  
> diff --git a/src/mem.c b/src/mem.c
> index e0327d1..29fd21e 100644
> --- a/src/mem.c
> +++ b/src/mem.c
> @@ -20,21 +20,34 @@
>  #include <string.h>
>  #include <unistd.h>
>  #include <assert.h>
> +#include <stdbool.h>
>  
>  #include <libpdbg.h>
>  
>  #include "main.h"
>  #include "progress.h"
> +#include "optcmd.h"
> +#include "parsers.h"
>  
>  #define PR_ERROR(x, args...) \
>  	pdbg_log(PDBG_ERROR, x, ##args)
>  
>  #define PUTMEM_BUF_SIZE 1024
> -static int getmem(uint64_t addr, uint64_t size, bool ci)
> +
> +struct mem_flags {
> +	bool ci;
> +};
> +
> +#define MEM_CI_FLAG ("--ci", ci, parse_flag_noarg, false)
> +
> +static int getmem(uint64_t addr, uint64_t size, struct mem_flags flags)
>  {
>  	struct pdbg_target *target;
>  	uint8_t *buf;
>  	int rc = 0;
> +
> +	printf("getmem ci %d\n", flags.ci);
> +
>  	buf = malloc(size);
>  	assert(buf);
>  	pdbg_for_each_class_target("adu", target) {
> @@ -43,7 +56,7 @@ static int getmem(uint64_t addr, uint64_t size, bool ci)
>  
>  		pdbg_set_progress_tick(progress_tick);
>  		progress_init();
> -		if (!__adu_getmem(target, addr, buf, size, ci)) {
> +		if (!__adu_getmem(target, addr, buf, size, flags.ci)) {
>  			if (write(STDOUT_FILENO, buf, size) < 0)
>  				PR_ERROR("Unable to write stdout.\n");
>  			else
> @@ -58,7 +71,10 @@ static int getmem(uint64_t addr, uint64_t size, bool ci)
>  	return rc;
>  
>  }
> -static int putmem(uint64_t addr, bool ci)
> +OPTCMD_DEFINE_CMD_WITH_FLAGS(getmem, getmem, (ADDRESS, DATA),
> +			     mem_flags, (MEM_CI_FLAG));
> +
> +static int putmem(uint64_t addr, struct mem_flags flags)
>  {
>  	uint8_t *buf;
>  	int read_size, rc = 0;
> @@ -76,7 +92,7 @@ static int putmem(uint64_t addr, bool ci)
>  	progress_init();
>  	do {
>  		read_size = read(STDIN_FILENO, buf, PUTMEM_BUF_SIZE);
> -		if (__adu_putmem(adu_target, addr, buf, read_size, ci)) {
> +		if (__adu_putmem(adu_target, addr, buf, read_size, flags.ci)) {
>  			rc = 0;
>  			printf("Unable to write memory.\n");
>  			break;
> @@ -89,101 +105,5 @@ static int putmem(uint64_t addr, bool ci)
>  	free(buf);
>  	return rc;
>  }
> -
> -static bool is_real_address(struct thread_regs *regs, uint64_t addr)
> -{
> -	return true;
> -	if ((addr & 0xf000000000000000ULL) == 0xc000000000000000ULL)
> -		return true;
> -	return false;
> -}
> -
> -static int load8(struct pdbg_target *target, uint64_t addr, uint64_t *value)
> -{
> -	if (adu_getmem(target, addr, (uint8_t *)value, 8)) {
> -		PR_ERROR("Unable to read memory address=%016" PRIx64 ".\n", addr);
> -		return 0;
> -	}
> -
> -	return 1;
> -}
> -
> -int dump_stack(struct thread_regs *regs)
> -{
> -	struct pdbg_target *target;
> -	uint64_t sp = regs->gprs[1];
> -	uint64_t pc;
> -
> -	pdbg_for_each_class_target("adu", target) {
> -		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> -			continue;
> -		break;
> -	}
> -
> -	printf("STACK:\n");
> -	if (!target)
> -		PR_ERROR("Unable to read memory (no ADU found)\n");
> -
> -	if (sp && is_real_address(regs, sp)) {
> -		if (!load8(target, sp, &sp))
> -			return 1;
> -		while (sp && is_real_address(regs, sp)) {
> -			if (!load8(target, sp + 16, &pc))
> -				return 1;
> -
> -			printf(" 0x%016" PRIx64 " 0x%16" PRIx64 "\n", sp, pc);
> -
> -			if (!load8(target, sp, &sp))
> -				return 1;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
> -int handle_mem(int optind, int argc, char *argv[])
> -{
> -	uint64_t addr;
> -	char *endptr;
> -	bool ci = false;
> -
> -	if (optind + 1 >= argc) {
> -		printf("%s: command '%s' requires an address\n", argv[0], argv[optind]);
> -		return -1;
> -	}
> -
> -	errno = 0;
> -
> -	if (strcmp(argv[optind +1], "-ci") == 0) {
> -		/* Set cache-inhibited flag */
> -		ci = true;
> -	}
> -
> -	addr = strtoull(argv[optind + 1 + ci], &endptr, 0);
> -	if (errno || *endptr != '\0') {
> -		printf("%s: command '%s' couldn't parse address '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1 + ci]);
> -		return -1;
> -	}
> -
> -	if (strcmp(argv[optind], "getmem") == 0) {
> -		uint64_t size;
> -
> -		if (optind + 2 + ci >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> -
> -		errno = 0;
> -		size = strtoull(argv[optind + 2 + ci], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1 + ci]);
> -			return -1;
> -		}
> -
> -		return getmem(addr, size, ci);
> -	}
> -
> -	return putmem(addr, ci);
> -}
> +OPTCMD_DEFINE_CMD_WITH_FLAGS(putmem, putmem, (ADDRESS),
> +			     mem_flags, (MEM_CI_FLAG));
> diff --git a/src/mem.h b/src/mem.h
> deleted file mode 100644
> index 42bdc04..0000000
> --- a/src/mem.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - * 	http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -#include <inttypes.h>
> -#include <libpdbg.h>
> -
> -int dump_stack(struct thread_regs *regs);
> -int handle_mem(int optind, int argc, char *argv[]);
> diff --git a/src/reg.c b/src/reg.c
> index 21bec13..aa77a8a 100644
> --- a/src/reg.c
> +++ b/src/reg.c
> @@ -22,6 +22,7 @@
>  #include <libpdbg.h>
>  
>  #include "main.h"
> +#include "optcmd.h"
>  
>  #define REG_MEM -3
>  #define REG_MSR -2
> @@ -91,143 +92,58 @@ static int getprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg,
>  	return !rc;
>  }
>  
> -int handle_gpr(int optind, int argc, char *argv[])
> +static int getgpr(int gpr)
>  {
> -	char *endptr;
> -	uint64_t gpr;
> -
> -	if (optind + 1 >= argc) {
> -		printf("%s: command '%s' requires a GPR\n", argv[0], argv[optind]);
> -		return -1;
> -	}
> -
> -	errno = 0;
> -	gpr = strtoull(argv[optind + 1], &endptr, 0);
> -	if (errno || *endptr != '\0') {
> -		printf("%s: command '%s' couldn't parse GPR '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -		return -1;
> -	}
> -
> -	if (gpr > 31) {
> -		printf("A GPR must be between zero and 31 inclusive\n");
> -		return -1;
> -	}
> -
> -	if (strcmp(argv[optind], "putgpr") == 0) {
> -		uint64_t data;
> -
> -		if (optind + 2 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> -
> -		errno = 0;
> -		data = strtoull(argv[optind + 2], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> -
> -		return for_each_target("thread", putprocreg, &gpr, &data);
> -	}
> -
> -	return for_each_target("thread", getprocreg, &gpr, NULL);
> +	uint64_t reg = gpr;
> +	return for_each_target("thread", getprocreg, &reg, NULL);
>  }
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getgpr, getgpr, (GPR));
>  
> -int handle_nia(int optind, int argc, char *argv[])
> +static int putgpr(int gpr, uint64_t data)
>  {
> -	uint64_t reg = REG_NIA;
> -	char *endptr;
> -
> -	if (strcmp(argv[optind], "putnia") == 0) {
> -		uint64_t data;
> -
> -		if (optind + 1 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> -
> -		errno = 0;
> -		data = strtoull(argv[optind + 1], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> -
> -		return for_each_target("thread", putprocreg, &reg, &data);
> -	}
> +	uint64_t reg = gpr;
> +	return for_each_target("thread", putprocreg, &reg, &data);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putgpr, putgpr, (GPR, DATA));
>  
> +static int getnia(void)
> +{
> +	uint64_t reg = REG_NIA;
>  	return for_each_target("thread", getprocreg, &reg, NULL);
>  }
> +OPTCMD_DEFINE_CMD(getnia, getnia);
>  
> -int handle_spr(int optind, int argc, char *argv[])
> +static int putnia(uint64_t nia)
>  {
> -	char *endptr;
> -	uint64_t spr;
> -
> -	if (optind + 1 >= argc) {
> -		printf("%s: command '%s' requires a GPR\n", argv[0], argv[optind]);
> -		return -1;
> -	}
> -
> -	errno = 0;
> -	spr = strtoull(argv[optind + 1], &endptr, 0);
> -	if (errno || *endptr != '\0') {
> -		printf("%s: command '%s' couldn't parse GPR '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -		return -1;
> -	}
> -
> -	spr += REG_R31;
> -
> -	if (strcmp(argv[optind], "putspr") == 0) {
> -		uint64_t data;
> -
> -		if (optind + 2 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> -
> -		errno = 0;
> -		data = strtoull(argv[optind + 2], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> -
> -		return for_each_target("thread", putprocreg, &spr, &data);
> -	}
> -
> -	return for_each_target("thread", getprocreg, &spr, NULL);
> +	uint64_t reg = REG_NIA;
> +	return for_each_target("thread", getprocreg, &reg, &nia);
>  }
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putnia, putnia, (DATA));
>  
> -int handle_msr(int optind, int argc, char *argv[])
> +static int getspr(int spr)
>  {
> -	uint64_t msr = REG_MSR;
> -	char *endptr;
> -
> -	if (strcmp(argv[optind], "putmsr") == 0) {
> -		uint64_t data;
> -
> -		if (optind + 1 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> +	uint64_t reg = spr + REG_R31;
> +	return for_each_target("thread", getprocreg, &reg, NULL);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getspr, getspr, (SPR));
>  
> -		errno = 0;
> -		data = strtoull(argv[optind + 1], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> +static int putspr(int spr, uint64_t data)
> +{
> +	uint64_t reg = spr + REG_R31;
> +	return for_each_target("thread", putprocreg, &reg, &data);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putspr, putspr, (SPR, DATA));
>  
> -		return for_each_target("thread", putprocreg, &msr, &data);
> -	}
> +static int getmsr(void)
> +{
> +	uint64_t reg = REG_MSR;
> +	return for_each_target("thread", getprocreg, &reg, NULL);
> +}
> +OPTCMD_DEFINE_CMD(getmsr, getmsr);
>  
> -	return for_each_target("thread", getprocreg, &msr, NULL);
> +static int putmsr(uint64_t data)
> +{
> +	uint64_t reg = REG_MSR;
> +	return for_each_target("thread", putprocreg, &reg, &data);
>  }
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putmsr, putmsr, (DATA));
> diff --git a/src/reg.h b/src/reg.h
> deleted file mode 100644
> index ad41d9d..0000000
> --- a/src/reg.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - * 	http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -
> -int handle_gpr(int optind, int argc, char *argv[]);
> -int handle_nia(int optind, int argc, char *argv[]);
> -int handle_spr(int optind, int argc, char *argv[]);
> -int handle_msr(int optind, int argc, char *argv[]);
> diff --git a/src/ring.c b/src/ring.c
> index b0c9376..58df4d1 100644
> --- a/src/ring.c
> +++ b/src/ring.c
> @@ -18,11 +18,12 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <assert.h>
>  
> -#include <target.h>
> -#include <operations.h>
> +#include <libpdbg.h>
>  
>  #include "main.h"
> +#include "optcmd.h"
>  
>  static int pdbg_getring(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *len)
>  {
> @@ -51,31 +52,8 @@ static int pdbg_getring(struct pdbg_target *target, uint32_t index, uint64_t *ad
>  	return 1;
>  }
>  
> -int handle_getring(int optind, int argc, char *argv[])
> +static int _getring(uint64_t ring_addr, uint64_t ring_len)
>  {
> -	uint64_t ring_addr, ring_len;
> -	char *endptr;
> -
> -	if (optind + 2 >= argc) {
> -		printf("%s: command '%s' requires two arguments (address and length)\n",
> -		       argv[0], argv[optind]);
> -		return -1;
> -	}
> -
> -	errno = 0;
> -	ring_addr = strtoull(argv[optind + 1], &endptr, 0);
> -	if (errno || *endptr != '\0') {
> -		printf("%s: command '%s' couldn't parse ring address '%s'\n",
> -		       argv[0], argv[optind], argv[optind + 1]);
> -		return -1;
> -	}
> -
> -	ring_len = strtoull(argv[optind + 2], &endptr, 0);
> -	if (errno || *endptr != '\0') {
> -		printf("%s: command '%s' couldn't parse ring length '%s'\n",
> -		       argv[0], argv[optind], argv[optind + 2]);
> -		return -1;
> -	}
> -
>  	return for_each_target("chiplet", pdbg_getring, &ring_addr, &ring_len);
>  }
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getring, _getring, (ADDRESS, DATA));
> diff --git a/src/ring.h b/src/ring.h
> deleted file mode 100644
> index a72c875..0000000
> --- a/src/ring.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/* Copyright 2018 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - *      http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -
> -int handle_getring(int optind, int argc, char *argv[]);
> diff --git a/src/scom.c b/src/scom.c
> index 4c59e2a..2372e91 100644
> --- a/src/scom.c
> +++ b/src/scom.c
> @@ -22,8 +22,9 @@
>  #include <libpdbg.h>
>  
>  #include "main.h"
> +#include "optcmd.h"
>  
> -static int getscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)
> +static int _getscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)
>  {
>  	uint64_t value;
>  
> @@ -35,7 +36,13 @@ static int getscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, u
>  	return 1;
>  }
>  
> -static int putscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)
> + int getscom(uint64_t addr)
> +{
> +	return for_each_target("pib", _getscom, &addr, NULL);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getscom, getscom, (ADDRESS));
> +
> +static int _putscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)
>  {
>  	if (pib_write(target, *addr, *data))
>  		return 0;
> @@ -43,44 +50,9 @@ static int putscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, u
>  	return 1;
>  }
>  
> -
> -int handle_scoms(int optind, int argc, char *argv[])
> + int putscom(uint64_t addr, uint64_t data, uint64_t mask)
>  {
> -	uint64_t addr;
> -	char *endptr;
> -
> -	if (optind + 1 >= argc) {
> -		printf("%s: command '%s' requires an address\n", argv[0], argv[optind]);
> -		return -1;
> -	}
> -
> -	errno = 0;
> -	addr = strtoull(argv[optind + 1], &endptr, 0);
> -	if (errno || *endptr != '\0') {
> -		printf("%s: command '%s' couldn't parse address '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -		return -1;
> -	}
> -
> -	if (strcmp(argv[optind], "putscom") == 0) {
> -		uint64_t data;
> -
> -		if (optind + 2 >= argc) {
> -			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
> -			return -1;
> -		}
> -
> -		errno = 0;
> -		data = strtoull(argv[optind + 2], &endptr, 0);
> -		if (errno || *endptr != '\0') {
> -			printf("%s: command '%s' couldn't parse data '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -			return -1;
> -		}
> -
> -		return for_each_target("pib", putscom, &addr, &data);
> -	}
> -
> -	return for_each_target("pib", getscom, &addr, NULL);
> +	/* TODO: Restore the <mask> functionality */
> +	return for_each_target("pib", _putscom, &addr, &data);
>  }
> -
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putscom, putscom, (ADDRESS, DATA, DEFAULT_DATA("0xffffffffffffffff")));
> diff --git a/src/scom.h b/src/scom.h
> deleted file mode 100644
> index d4325b5..0000000
> --- a/src/scom.h
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - * 	http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -#include <inttypes.h>
> -
> -int handle_scoms(int optind, int argc, char *argv[]);
> diff --git a/src/thread.c b/src/thread.c
> index e8b54cb..4b95636 100644
> --- a/src/thread.c
> +++ b/src/thread.c
> @@ -21,7 +21,7 @@
>  #include <libpdbg.h>
>  
>  #include "main.h"
> -#include "mem.h"
> +#include "optcmd.h"
>  
>  static int print_thread_status(struct pdbg_target *target, uint32_t index, uint64_t *arg, uint64_t *unused1)
>  {
> @@ -81,6 +81,57 @@ static int print_core_thread_status(struct pdbg_target *core_target, uint32_t in
>  	return rc;
>  }
>  
> +static bool is_real_address(struct thread_regs *regs, uint64_t addr)
> +{
> +	return true;
> +	if ((addr & 0xf000000000000000ULL) == 0xc000000000000000ULL)
> +		return true;
> +	return false;
> +}
> +
> +static int load8(struct pdbg_target *target, uint64_t addr, uint64_t *value)
> +{
> +	if (adu_getmem(target, addr, (uint8_t *)value, 8)) {
> +		pdbg_log(PDBG_ERROR, "Unable to read memory address=%016" PRIx64 ".\n", addr);
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static int dump_stack(struct thread_regs *regs)
> +{
> +	struct pdbg_target *target;
> +	uint64_t sp = regs->gprs[1];
> +	uint64_t pc;
> +
> +	pdbg_for_each_class_target("adu", target) {
> +		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> +			continue;
> +		break;
> +	}
> +
> +	printf("STACK:\n");
> +	if (!target)
> +		pdbg_log(PDBG_ERROR, "Unable to read memory (no ADU found)\n");
> +
> +	if (sp && is_real_address(regs, sp)) {
> +		if (!load8(target, sp, &sp))
> +			return 1;
> +		while (sp && is_real_address(regs, sp)) {
> +			if (!load8(target, sp + 16, &pc))
> +				return 1;
> +
> +			printf(" 0x%016" PRIx64 " 0x%16" PRIx64 "\n", sp, pc);
> +
> +			if (!load8(target, sp, &sp))
> +				return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int get_thread_max_index(struct pdbg_target *target, uint32_t index, uint64_t *maxindex, uint64_t *unused)
>  {
>  	if (index > *maxindex)
> @@ -140,48 +191,37 @@ static int state_thread(struct pdbg_target *thread_target, uint32_t index, uint6
>  	return 1;
>  }
>  
> -int thread_start(int optind, int argc, char *argv[])
> +static int thread_start(void)
>  {
>  	return for_each_target("thread", start_thread, NULL, NULL);
>  }
> +OPTCMD_DEFINE_CMD(start, thread_start);
>  
> -int thread_step(int optind, int argc, char *argv[])
> +static int thread_step(uint64_t count)
>  {
> -	uint64_t count;
> -	char *endptr;
> -
> -	if (optind + 1 >= argc) {
> -		printf("%s: command '%s' requires a count\n", argv[0], argv[optind]);
> -		return -1;
> -	}
> -
> -	errno = 0;
> -	count = strtoull(argv[optind + 1], &endptr, 0);
> -	if (errno || *endptr != '\0') {
> -		printf("%s: command '%s' couldn't parse count '%s'\n",
> -				argv[0], argv[optind], argv[optind + 1]);
> -		return -1;
> -	}
> -
>  	return for_each_target("thread", step_thread, &count, NULL);
>  }
> +OPTCMD_DEFINE_CMD_WITH_ARGS(step, thread_step, (DATA));
>  
> -int thread_stop(int optind, int argc, char *argv[])
> +static int thread_stop(void)
>  {
>  	return for_each_target("thread", stop_thread, NULL, NULL);
>  }
> +OPTCMD_DEFINE_CMD(stop, thread_stop);
>  
> -int thread_status_print(int optind, int argc, char *argv[])
> +static int thread_status_print(void)
>  {
>  	return for_each_target("pib", print_proc_thread_status, NULL, NULL);
>  }
> +OPTCMD_DEFINE_CMD(threadstatus, thread_status_print);
>  
> -int thread_sreset(int optind, int argc, char *argv[])
> +static int thread_sreset(void)
>  {
>  	return for_each_target("thread", sreset_thread, NULL, NULL);
>  }
> +OPTCMD_DEFINE_CMD(sreset, thread_sreset);
>  
> -int thread_state(int optind, int argc, char *argv[])
> +static int thread_state(void)
>  {
>  	int err;
>  
> @@ -191,3 +231,4 @@ int thread_state(int optind, int argc, char *argv[])
>  
>  	return err;
>  }
> +OPTCMD_DEFINE_CMD(regs, thread_state);
> diff --git a/src/thread.h b/src/thread.h
> deleted file mode 100644
> index 0c1caa2..0000000
> --- a/src/thread.h
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - * 	http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -#include <inttypes.h>
> -
> -int thread_start(int optind, int argc, char *argv[]);
> -int thread_step(int optind, int argc, char *argv[]);
> -int thread_stop(int optind, int argc, char *argv[]);
> -int thread_status_print(int optind, int argc, char *argv[]);
> -int thread_sreset(int optind, int argc, char *argv[]);
> -int thread_state(int optind, int argc, char *argv[]);
Rashmica Gupta June 29, 2018, 6:36 a.m. UTC | #2
On Wed, Jun 20, 2018, 3:35 PM Alistair Popple <alistair@popple.id.au> wrote:

> This switches all commands except the htm command to use the new
> command/option
> parsing code. For the moment we leave the usage help and generic targeting
> flag
> processing alone, but future changes should allow this to also use the
> common
> parsing code.
>
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
>  Makefile.am  |   1 +
>  src/cfam.c   |  55 ++++++--------------
>  src/cfam.h   |  18 -------
>  src/main.c   | 125 ++++++++++++++++++++++++++++-----------------
>  src/mem.c    | 124 ++++++++------------------------------------
>  src/mem.h    |  20 --------
>  src/reg.c    | 164
> +++++++++++++++--------------------------------------------
>  src/reg.h    |  20 --------
>  src/ring.c   |  32 ++----------
>  src/ring.h   |  17 -------
>  src/scom.c   |  54 +++++---------------
>  src/scom.h   |  18 -------
>  src/thread.c |  87 ++++++++++++++++++++++---------
>  src/thread.h |  23 ---------
>  14 files changed, 237 insertions(+), 521 deletions(-)
>  delete mode 100644 src/cfam.h
>  delete mode 100644 src/mem.h
>  delete mode 100644 src/reg.h
>  delete mode 100644 src/ring.h
>  delete mode 100644 src/scom.h
>  delete mode 100644 src/thread.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 23f2080..4e0dce2 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -39,6 +39,7 @@ pdbg_SOURCES = \
>         src/ring.c \
>         src/htm.c \
>         src/progress.c \
> +       src/parsers.c \
>         src/optcmd.c \
>         src/options_@ARCH@.c
>
> diff --git a/src/cfam.c b/src/cfam.c
> index 269123e..6dab388 100644
> --- a/src/cfam.c
> +++ b/src/cfam.c
> @@ -20,8 +20,9 @@
>  #include <inttypes.h>
>
>  #include "main.h"
> +#include "optcmd.h"
>
> -static int getcfam(struct pdbg_target *target, uint32_t index, uint64_t
> *addr, uint64_t *unused)
> +static int _getcfam(struct pdbg_target *target, uint32_t index, uint64_t
> *addr, uint64_t *unused)
>  {
>         uint32_t value;
>
> @@ -33,7 +34,15 @@ static int getcfam(struct pdbg_target *target, uint32_t
> index, uint64_t *addr, u
>         return 1;
>  }
>
> -static int putcfam(struct pdbg_target *target, uint32_t index, uint64_t
> *addr, uint64_t *data)
> +static int getcfam(uint32_t addr)
> +{
> +       uint64_t addr64 = addr;
> +
> +       return for_each_target("fsi", _getcfam, &addr64, NULL);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getcfam, getcfam, (ADDRESS32));
> +
> +static int _putcfam(struct pdbg_target *target, uint32_t index, uint64_t
> *addr, uint64_t *data)
>  {
>         if (fsi_write(target, *addr, *data))
>                 return 0;
> @@ -41,44 +50,10 @@ static int putcfam(struct pdbg_target *target,
> uint32_t index, uint64_t *addr, u
>         return 1;
>  }
>
> -int handle_cfams(int optind, int argc, char *argv[])
> +static int putcfam(uint32_t addr, uint32_t data)
>  {
> -       uint64_t addr;
> -       char *endptr;
> -
> -       if (optind + 1 >= argc) {
> -               printf("%s: command '%s' requires an address\n", argv[0],
> argv[optind]);
> -               return -1;
> -       }
> -
> -       errno = 0;
> -       addr = strtoull(argv[optind + 1], &endptr, 0);
> -       if (errno || *endptr != '\0') {
> -               printf("%s: command '%s' couldn't parse address '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -               return -1;
> -       }
> +       uint64_t addr64 = addr, data64 = data;
>
> -       if (strcmp(argv[optind], "putcfam") == 0) {
> -               uint64_t data;
> -
> -               if (optind + 2 >= argc) {
> -                       printf("%s: command '%s' requires data\n",
> argv[0], argv[optind]);
> -                       return -1;
> -               }
> -
> -               errno = 0;
> -               data = strtoull(argv[optind + 2], &endptr, 0);
> -               if (errno || *endptr != '\0') {
> -                       printf("%s: command '%s' couldn't parse data
> '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -                       return -1;
> -               }
> -
> -               return for_each_target("fsi", putcfam, &addr, &data);
> -       }
> -
> -       return for_each_target("fsi", getcfam, &addr, NULL);
> +       return for_each_target("fsi", _putcfam, &addr64, &data64);
>  }
> -
> -
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putcfam, putcfam, (ADDRESS32, DATA32));
> diff --git a/src/cfam.h b/src/cfam.h
> deleted file mode 100644
> index 997ed3d..0000000
> --- a/src/cfam.h
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - *     http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -#include <inttypes.h>
> -
> -int handle_cfams(int optind, int argc, char *argv[]);
> diff --git a/src/main.c b/src/main.c
> index cf9a3b2..1cfaca7 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -34,14 +34,9 @@
>  #include <target.h>
>
>  #include "main.h"
> -#include "cfam.h"
> -#include "scom.h"
> -#include "reg.h"
> -#include "ring.h"
> -#include "mem.h"
> -#include "thread.h"
>  #include "htm.h"
>  #include "options.h"
> +#include "optcmd.h"
>
>  #define PR_ERROR(x, args...) \
>         pdbg_log(PDBG_ERROR, x, ##args)
> @@ -77,44 +72,64 @@ static int **processorsel[MAX_PROCESSORS];
>  static int *chipsel[MAX_PROCESSORS][MAX_CHIPS];
>  static int threadsel[MAX_PROCESSORS][MAX_CHIPS][MAX_THREADS];
>
> -static int handle_probe(int optind, int argc, char *argv[]);
> -static int handle_release(int optind, int argc, char *argv[]);
> +static int probe(void);
> +static int release(void);
> +
> +/* TODO: We are repeating ourselves here. A little bit more macro magic
> could
> + * easily fix this but I was hesitant to introduce too much magic all at
> + * once. */
> +extern struct optcmd_cmd
> +       optcmd_getscom, optcmd_putscom, optcmd_getcfam, optcmd_putcfam,
> +       optcmd_getgpr, optcmd_putgpr, optcmd_getspr, optcmd_putspr,
> +       optcmd_getnia, optcmd_putnia, optcmd_getmsr, optcmd_putmsr,
> +       optcmd_getring, optcmd_start, optcmd_stop, optcmd_step,
> +       optcmd_threadstatus, optcmd_sreset, optcmd_regs, optcmd_probe,
> +       optcmd_getmem, optcmd_putmem;
> +
> +static struct optcmd_cmd *cmds[] = {
> +       &optcmd_getscom, &optcmd_putscom, &optcmd_getcfam, &optcmd_putcfam,
> +       &optcmd_getgpr, &optcmd_putgpr, &optcmd_getspr, &optcmd_putspr,
> +       &optcmd_getnia, &optcmd_putnia, &optcmd_getmsr, &optcmd_putmsr,
> +       &optcmd_getring, &optcmd_start, &optcmd_stop, &optcmd_step,
> +       &optcmd_threadstatus, &optcmd_sreset, &optcmd_regs, &optcmd_probe,
> +       &optcmd_getmem, &optcmd_putmem,
> +};
>
> +/* Purely for printing usage text. We could integrate printing argument
> and flag
> + * help into optcmd if desired. */
>  struct action {
>         const char *name;
>         const char *args;
>         const char *desc;
> -       int (*fn)(int, int, char **);
>  };
>
>  static struct action actions[] = {
> -       { "getgpr",  "<gpr>", "Read General Purpose Register (GPR)",
> &handle_gpr },
> -       { "putgpr",  "<gpr> <value>", "Write General Purpose Register
> (GPR)", &handle_gpr },
> -       { "getnia",  "", "Get Next Instruction Address (NIA)", &handle_nia
> },
> -       { "putnia",  "<value>", "Write Next Instrution Address (NIA)",
> &handle_nia },
> -       { "getspr",  "<spr>", "Get Special Purpose Register (SPR)",
> &handle_spr },
> -       { "putspr",  "<spr> <value>", "Write Special Purpose Register
> (SPR)", &handle_spr },
> -       { "getmsr",  "", "Get Machine State Register (MSR)", &handle_msr },
> -       { "putmsr",  "<value>", "Write Machine State Register (MSR)",
> &handle_msr },
> -       { "getring", "<addr> <len>", "Read a ring. Length must be
> correct", &handle_getring },
> -       { "start",   "", "Start thread", &thread_start },
> -       { "step",    "<count>", "Set a thread <count> instructions",
> &thread_step },
> -       { "stop",    "", "Stop thread", &thread_stop },
> -       { "htm", "core|nest start|stop|status|reset|dump|trace|analyse",
> "Hardware Trace Macro", &run_htm },
> -       { "release", "", "Should be called after pdbg work is finished, to
> release special wakeups and other resources.", &handle_release},
> -       { "probe", "", "", &handle_probe },
> -       { "getcfam", "<address>", "Read system cfam", &handle_cfams },
> -       { "putcfam", "<address> <value> [<mask>]", "Write system cfam",
> &handle_cfams },
> -       { "getscom", "<address>", "Read system scom", &handle_scoms },
> -       { "putscom", "<address> <value> [<mask>]", "Write system scom",
> &handle_scoms },
> -       { "getmem",  "<address> <count>", "Read system memory",
> &handle_mem },
> -       { "putmem",  "<address>", "Write to system memory", &handle_mem },
> -       { "threadstatus", "", "Print the status of a thread",
> &thread_status_print },
> -       { "sreset",  "", "Reset", &thread_sreset },
> -       { "regs",  "", "State", &thread_state },
> +       { "getgpr",  "<gpr>", "Read General Purpose Register (GPR)" },
> +       { "putgpr",  "<gpr> <value>", "Write General Purpose Register
> (GPR)" },
> +       { "getnia",  "", "Get Next Instruction Address (NIA)" },
> +       { "putnia",  "<value>", "Write Next Instrution Address (NIA)" },
> +       { "getspr",  "<spr>", "Get Special Purpose Register (SPR)" },
> +       { "putspr",  "<spr> <value>", "Write Special Purpose Register
> (SPR)" },
> +       { "getmsr",  "", "Get Machine State Register (MSR)" },
> +       { "putmsr",  "<value>", "Write Machine State Register (MSR)" },
> +       { "getring", "<addr> <len>", "Read a ring. Length must be correct"
> },
> +       { "start",   "", "Start thread" },
> +       { "step",    "<count>", "Set a thread <count> instructions" },
> +       { "stop",    "", "Stop thread" },
> +       { "htm", "core|nest start|stop|status|reset|dump|trace|analyse",
> "Hardware Trace Macro" },
> +       { "release", "", "Should be called after pdbg work is finished" },
> +       { "probe", "", "" },
> +       { "getcfam", "<address>", "Read system cfam" },
> +       { "putcfam", "<address> <value> [<mask>]", "Write system cfam" },
> +       { "getscom", "<address>", "Read system scom" },
> +       { "putscom", "<address> <value> [<mask>]", "Write system scom" },
> +       { "getmem",  "<address> <count>", "Read system memory" },
> +       { "putmem",  "<address>", "Write to system memory" },
> +       { "threadstatus", "", "Print the status of a thread" },
> +       { "sreset",  "", "Reset" },
> +       { "regs",  "", "State" },
>  };
>
> -
>  static void print_usage(char *pname)
>  {
>         int i;
> @@ -600,7 +615,7 @@ static void release_target(struct pdbg_target *target)
>         pdbg_target_release(target);
>  }
>
> -static void do_release(void)
> +static int release(void)
>  {
>         struct pdbg_target_class *target_class;
>
> @@ -610,7 +625,10 @@ static void do_release(void)
>                 pdbg_for_each_class_target(target_class->name, target)
>                         release_target(target);
>         }
> +
> +       return 0;
>  }
> +OPTCMD_DEFINE_CMD(release, release);
>
>  void print_target(struct pdbg_target *target, int level)
>  {
> @@ -652,7 +670,7 @@ void print_target(struct pdbg_target *target, int
> level)
>         }
>  }
>
> -static int handle_probe(int optind, int argc, char *argv[])
> +static int probe(void)
>  {
>         struct pdbg_target *target;
>
> @@ -664,25 +682,21 @@ static int handle_probe(int optind, int argc, char
> *argv[])
>
>         return 1;
>  }
> +OPTCMD_DEFINE_CMD(probe, probe);
>
>  /*
>   * Release handler.
>   */
>  static void atexit_release(void)
>  {
> -       do_release();
> -}
> -
> -static int handle_release(int optind, int argc, char *argv[])
> -{
> -       do_release();
> -
> -       return 1;
> +       release();
>  }
>
>  int main(int argc, char *argv[])
>  {
>         int i, rc = 0;
> +       void **args, **flags;
> +       optcmd_cmd_t *cmd;
>
>         backend = default_backend();
>         device_node = default_target(backend);
> @@ -714,13 +728,28 @@ int main(int argc, char *argv[])
>
>         atexit(atexit_release);
>
> -       for (i = 0; i < ARRAY_SIZE(actions); i++) {
> -               if (strcmp(argv[optind], actions[i].name) == 0) {
> -                       rc = actions[i].fn(optind, argc, argv);
> -                       goto found_action;
> +       for (i = 0; i < ARRAY_SIZE(cmds); i++) {
> +               if (!strcmp(argv[optind], cmds[i]->cmd)) {
> +                       /* Found our command */
> +                       cmd = optcmd_parse(cmds[i], (const char **)
> &argv[optind + 1],
> +                                          argc - (optind + 1), &args,
> &flags);
> +                       if (cmd) {
> +                               rc = cmd(args, flags);
> +                               goto found_action;
> +                       } else {
> +                               /* Error parsing arguments so exit return
> directly */
> +                               return 1;
> +                       }
>                 }
>         }
>
> +       /* Process subcommands. Currently only 'htm'.
> +        * TODO: Move htm command parsing to optcmd once htm clean-up is
> complete */
> +       if (!strcmp(argv[optind], "htm")) {
> +               run_htm(optind, argc, argv);
> +               goto found_action;
> +       }
> +
>         PR_ERROR("Unsupported command: %s\n", argv[optind]);
>         return 1;
>
> diff --git a/src/mem.c b/src/mem.c
> index e0327d1..29fd21e 100644
> --- a/src/mem.c
> +++ b/src/mem.c
> @@ -20,21 +20,34 @@
>  #include <string.h>
>  #include <unistd.h>
>  #include <assert.h>
> +#include <stdbool.h>
>
>  #include <libpdbg.h>
>
>  #include "main.h"
>  #include "progress.h"
> +#include "optcmd.h"
> +#include "parsers.h"
>
>  #define PR_ERROR(x, args...) \
>         pdbg_log(PDBG_ERROR, x, ##args)
>
>  #define PUTMEM_BUF_SIZE 1024
> -static int getmem(uint64_t addr, uint64_t size, bool ci)
> +
> +struct mem_flags {
> +       bool ci;
> +};
> +
> +#define MEM_CI_FLAG ("--ci", ci, parse_flag_noarg, false)
> +
> +static int getmem(uint64_t addr, uint64_t size, struct mem_flags flags)
>  {
>         struct pdbg_target *target;
>         uint8_t *buf;
>         int rc = 0;
> +
> +       printf("getmem ci %d\n", flags.ci);
> +
>         buf = malloc(size);
>         assert(buf);
>         pdbg_for_each_class_target("adu", target) {
> @@ -43,7 +56,7 @@ static int getmem(uint64_t addr, uint64_t size, bool ci)
>
>                 pdbg_set_progress_tick(progress_tick);
>                 progress_init();
> -               if (!__adu_getmem(target, addr, buf, size, ci)) {
> +               if (!__adu_getmem(target, addr, buf, size, flags.ci)) {
>                         if (write(STDOUT_FILENO, buf, size) < 0)
>                                 PR_ERROR("Unable to write stdout.\n");
>                         else
> @@ -58,7 +71,10 @@ static int getmem(uint64_t addr, uint64_t size, bool ci)
>         return rc;
>
>  }
> -static int putmem(uint64_t addr, bool ci)
> +OPTCMD_DEFINE_CMD_WITH_FLAGS(getmem, getmem, (ADDRESS, DATA),
> +                            mem_flags, (MEM_CI_FLAG));
> +
> +static int putmem(uint64_t addr, struct mem_flags flags)
>  {
>         uint8_t *buf;
>         int read_size, rc = 0;
> @@ -76,7 +92,7 @@ static int putmem(uint64_t addr, bool ci)
>         progress_init();
>         do {
>                 read_size = read(STDIN_FILENO, buf, PUTMEM_BUF_SIZE);
> -               if (__adu_putmem(adu_target, addr, buf, read_size, ci)) {
> +               if (__adu_putmem(adu_target, addr, buf, read_size,
> flags.ci)) {
>                         rc = 0;
>                         printf("Unable to write memory.\n");
>                         break;
> @@ -89,101 +105,5 @@ static int putmem(uint64_t addr, bool ci)
>         free(buf);
>         return rc;
>  }
> -
> -static bool is_real_address(struct thread_regs *regs, uint64_t addr)
> -{
> -       return true;
> -       if ((addr & 0xf000000000000000ULL) == 0xc000000000000000ULL)
> -               return true;
> -       return false;
> -}
> -
> -static int load8(struct pdbg_target *target, uint64_t addr, uint64_t
> *value)
> -{
> -       if (adu_getmem(target, addr, (uint8_t *)value, 8)) {
> -               PR_ERROR("Unable to read memory address=%016" PRIx64
> ".\n", addr);
> -               return 0;
> -       }
> -
> -       return 1;
> -}
> -
> -int dump_stack(struct thread_regs *regs)
> -{
> -       struct pdbg_target *target;
> -       uint64_t sp = regs->gprs[1];
> -       uint64_t pc;
> -
> -       pdbg_for_each_class_target("adu", target) {
> -               if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> -                       continue;
> -               break;
> -       }
> -
> -       printf("STACK:\n");
> -       if (!target)
> -               PR_ERROR("Unable to read memory (no ADU found)\n");
> -
> -       if (sp && is_real_address(regs, sp)) {
> -               if (!load8(target, sp, &sp))
> -                       return 1;
> -               while (sp && is_real_address(regs, sp)) {
> -                       if (!load8(target, sp + 16, &pc))
> -                               return 1;
> -
> -                       printf(" 0x%016" PRIx64 " 0x%16" PRIx64 "\n", sp,
> pc);
> -
> -                       if (!load8(target, sp, &sp))
> -                               return 1;
> -               }
> -       }
> -
> -       return 0;
> -}
> -
> -int handle_mem(int optind, int argc, char *argv[])
> -{
> -       uint64_t addr;
> -       char *endptr;
> -       bool ci = false;
> -
> -       if (optind + 1 >= argc) {
> -               printf("%s: command '%s' requires an address\n", argv[0],
> argv[optind]);
> -               return -1;
> -       }
> -
> -       errno = 0;
> -
> -       if (strcmp(argv[optind +1], "-ci") == 0) {
> -               /* Set cache-inhibited flag */
> -               ci = true;
> -       }
> -
> -       addr = strtoull(argv[optind + 1 + ci], &endptr, 0);
> -       if (errno || *endptr != '\0') {
> -               printf("%s: command '%s' couldn't parse address '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1 +
> ci]);
> -               return -1;
> -       }
> -
> -       if (strcmp(argv[optind], "getmem") == 0) {
> -               uint64_t size;
> -
> -               if (optind + 2 + ci >= argc) {
> -                       printf("%s: command '%s' requires data\n",
> argv[0], argv[optind]);
> -                       return -1;
> -               }
> -
> -               errno = 0;
> -               size = strtoull(argv[optind + 2 + ci], &endptr, 0);
> -               if (errno || *endptr != '\0') {
> -                       printf("%s: command '%s' couldn't parse data
> '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1 +
> ci]);
> -                       return -1;
> -               }
> -
> -               return getmem(addr, size, ci);
> -       }
> -
> -       return putmem(addr, ci);
> -}
> +OPTCMD_DEFINE_CMD_WITH_FLAGS(putmem, putmem, (ADDRESS),
> +                            mem_flags, (MEM_CI_FLAG));
> diff --git a/src/mem.h b/src/mem.h
> deleted file mode 100644
> index 42bdc04..0000000
> --- a/src/mem.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - *     http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -#include <inttypes.h>
> -#include <libpdbg.h>
> -
> -int dump_stack(struct thread_regs *regs);
> -int handle_mem(int optind, int argc, char *argv[]);
> diff --git a/src/reg.c b/src/reg.c
> index 21bec13..aa77a8a 100644
> --- a/src/reg.c
> +++ b/src/reg.c
> @@ -22,6 +22,7 @@
>  #include <libpdbg.h>
>
>  #include "main.h"
> +#include "optcmd.h"
>
>  #define REG_MEM -3
>  #define REG_MSR -2
> @@ -91,143 +92,58 @@ static int getprocreg(struct pdbg_target *target,
> uint32_t index, uint64_t *reg,
>         return !rc;
>  }
>
> -int handle_gpr(int optind, int argc, char *argv[])
> +static int getgpr(int gpr)
>  {
> -       char *endptr;
> -       uint64_t gpr;
> -
> -       if (optind + 1 >= argc) {
> -               printf("%s: command '%s' requires a GPR\n", argv[0],
> argv[optind]);
> -               return -1;
> -       }
> -
> -       errno = 0;
> -       gpr = strtoull(argv[optind + 1], &endptr, 0);
> -       if (errno || *endptr != '\0') {
> -               printf("%s: command '%s' couldn't parse GPR '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -               return -1;
> -       }
> -
> -       if (gpr > 31) {
> -               printf("A GPR must be between zero and 31 inclusive\n");
> -               return -1;
> -       }
> -
> -       if (strcmp(argv[optind], "putgpr") == 0) {
> -               uint64_t data;
> -
> -               if (optind + 2 >= argc) {
> -                       printf("%s: command '%s' requires data\n",
> argv[0], argv[optind]);
> -                       return -1;
> -               }
> -
> -               errno = 0;
> -               data = strtoull(argv[optind + 2], &endptr, 0);
> -               if (errno || *endptr != '\0') {
> -                       printf("%s: command '%s' couldn't parse data
> '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -                       return -1;
> -               }
> -
> -               return for_each_target("thread", putprocreg, &gpr, &data);
> -       }
> -
> -       return for_each_target("thread", getprocreg, &gpr, NULL);
> +       uint64_t reg = gpr;
> +       return for_each_target("thread", getprocreg, &reg, NULL);
>  }
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getgpr, getgpr, (GPR));
>
> -int handle_nia(int optind, int argc, char *argv[])
> +static int putgpr(int gpr, uint64_t data)
>  {
> -       uint64_t reg = REG_NIA;
> -       char *endptr;
> -
> -       if (strcmp(argv[optind], "putnia") == 0) {
> -               uint64_t data;
> -
> -               if (optind + 1 >= argc) {
> -                       printf("%s: command '%s' requires data\n",
> argv[0], argv[optind]);
> -                       return -1;
> -               }
> -
> -               errno = 0;
> -               data = strtoull(argv[optind + 1], &endptr, 0);
> -               if (errno || *endptr != '\0') {
> -                       printf("%s: command '%s' couldn't parse data
> '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -                       return -1;
> -               }
> -
> -               return for_each_target("thread", putprocreg, &reg, &data);
> -       }
> +       uint64_t reg = gpr;
> +       return for_each_target("thread", putprocreg, &reg, &data);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putgpr, putgpr, (GPR, DATA));
>
> +static int getnia(void)
> +{
> +       uint64_t reg = REG_NIA;
>         return for_each_target("thread", getprocreg, &reg, NULL);
>  }
> +OPTCMD_DEFINE_CMD(getnia, getnia);
>
> -int handle_spr(int optind, int argc, char *argv[])
> +static int putnia(uint64_t nia)
>  {
> -       char *endptr;
> -       uint64_t spr;
> -
> -       if (optind + 1 >= argc) {
> -               printf("%s: command '%s' requires a GPR\n", argv[0],
> argv[optind]);
> -               return -1;
> -       }
> -
> -       errno = 0;
> -       spr = strtoull(argv[optind + 1], &endptr, 0);
> -       if (errno || *endptr != '\0') {
> -               printf("%s: command '%s' couldn't parse GPR '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -               return -1;
> -       }
> -
> -       spr += REG_R31;
> -
> -       if (strcmp(argv[optind], "putspr") == 0) {
> -               uint64_t data;
> -
> -               if (optind + 2 >= argc) {
> -                       printf("%s: command '%s' requires data\n",
> argv[0], argv[optind]);
> -                       return -1;
> -               }
> -
> -               errno = 0;
> -               data = strtoull(argv[optind + 2], &endptr, 0);
> -               if (errno || *endptr != '\0') {
> -                       printf("%s: command '%s' couldn't parse data
> '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -                       return -1;
> -               }
> -
> -               return for_each_target("thread", putprocreg, &spr, &data);
> -       }
> -
> -       return for_each_target("thread", getprocreg, &spr, NULL);
> +       uint64_t reg = REG_NIA;
> +       return for_each_target("thread", getprocreg, &reg, &nia);
>  }
>

I think this should be putprocreg. There is at least one more that I found
that seems to be using the wrong <put/get>procreg


+OPTCMD_DEFINE_CMD_WITH_ARGS(putnia, putnia, (DATA));
>
> -int handle_msr(int optind, int argc, char *argv[])
> +static int getspr(int spr)
>  {
> -       uint64_t msr = REG_MSR;
> -       char *endptr;
> -
> -       if (strcmp(argv[optind], "putmsr") == 0) {
> -               uint64_t data;
> -
> -               if (optind + 1 >= argc) {
> -                       printf("%s: command '%s' requires data\n",
> argv[0], argv[optind]);
> -                       return -1;
> -               }
> +       uint64_t reg = spr + REG_R31;
> +       return for_each_target("thread", getprocreg, &reg, NULL);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getspr, getspr, (SPR));
>
> -               errno = 0;
> -               data = strtoull(argv[optind + 1], &endptr, 0);
> -               if (errno || *endptr != '\0') {
> -                       printf("%s: command '%s' couldn't parse data
> '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -                       return -1;
> -               }
> +static int putspr(int spr, uint64_t data)
> +{
> +       uint64_t reg = spr + REG_R31;
> +       return for_each_target("thread", putprocreg, &reg, &data);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putspr, putspr, (SPR, DATA));
>
> -               return for_each_target("thread", putprocreg, &msr, &data);
> -       }
> +static int getmsr(void)
> +{
> +       uint64_t reg = REG_MSR;
> +       return for_each_target("thread", getprocreg, &reg, NULL);
> +}
> +OPTCMD_DEFINE_CMD(getmsr, getmsr);
>
> -       return for_each_target("thread", getprocreg, &msr, NULL);
> +static int putmsr(uint64_t data)
> +{
> +       uint64_t reg = REG_MSR;
> +       return for_each_target("thread", putprocreg, &reg, &data);
>  }
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putmsr, putmsr, (DATA));
> diff --git a/src/reg.h b/src/reg.h
> deleted file mode 100644
> index ad41d9d..0000000
> --- a/src/reg.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - *     http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -
> -int handle_gpr(int optind, int argc, char *argv[]);
> -int handle_nia(int optind, int argc, char *argv[]);
> -int handle_spr(int optind, int argc, char *argv[]);
> -int handle_msr(int optind, int argc, char *argv[]);
> diff --git a/src/ring.c b/src/ring.c
> index b0c9376..58df4d1 100644
> --- a/src/ring.c
> +++ b/src/ring.c
> @@ -18,11 +18,12 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <assert.h>
>
> -#include <target.h>
> -#include <operations.h>
> +#include <libpdbg.h>
>
>  #include "main.h"
> +#include "optcmd.h"
>
>  static int pdbg_getring(struct pdbg_target *target, uint32_t index,
> uint64_t *addr, uint64_t *len)
>  {
> @@ -51,31 +52,8 @@ static int pdbg_getring(struct pdbg_target *target,
> uint32_t index, uint64_t *ad
>         return 1;
>  }
>
> -int handle_getring(int optind, int argc, char *argv[])
> +static int _getring(uint64_t ring_addr, uint64_t ring_len)
>  {
> -       uint64_t ring_addr, ring_len;
> -       char *endptr;
> -
> -       if (optind + 2 >= argc) {
> -               printf("%s: command '%s' requires two arguments (address
> and length)\n",
> -                      argv[0], argv[optind]);
> -               return -1;
> -       }
> -
> -       errno = 0;
> -       ring_addr = strtoull(argv[optind + 1], &endptr, 0);
> -       if (errno || *endptr != '\0') {
> -               printf("%s: command '%s' couldn't parse ring address
> '%s'\n",
> -                      argv[0], argv[optind], argv[optind + 1]);
> -               return -1;
> -       }
> -
> -       ring_len = strtoull(argv[optind + 2], &endptr, 0);
> -       if (errno || *endptr != '\0') {
> -               printf("%s: command '%s' couldn't parse ring length
> '%s'\n",
> -                      argv[0], argv[optind], argv[optind + 2]);
> -               return -1;
> -       }
> -
>         return for_each_target("chiplet", pdbg_getring, &ring_addr,
> &ring_len);
>  }
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getring, _getring, (ADDRESS, DATA));
> diff --git a/src/ring.h b/src/ring.h
> deleted file mode 100644
> index a72c875..0000000
> --- a/src/ring.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/* Copyright 2018 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - *      http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -
> -int handle_getring(int optind, int argc, char *argv[]);
> diff --git a/src/scom.c b/src/scom.c
> index 4c59e2a..2372e91 100644
> --- a/src/scom.c
> +++ b/src/scom.c
> @@ -22,8 +22,9 @@
>  #include <libpdbg.h>
>
>  #include "main.h"
> +#include "optcmd.h"
>
> -static int getscom(struct pdbg_target *target, uint32_t index, uint64_t
> *addr, uint64_t *unused)
> +static int _getscom(struct pdbg_target *target, uint32_t index, uint64_t
> *addr, uint64_t *unused)
>  {
>         uint64_t value;
>
> @@ -35,7 +36,13 @@ static int getscom(struct pdbg_target *target, uint32_t
> index, uint64_t *addr, u
>         return 1;
>  }
>
> -static int putscom(struct pdbg_target *target, uint32_t index, uint64_t
> *addr, uint64_t *data)
> + int getscom(uint64_t addr)
> +{
> +       return for_each_target("pib", _getscom, &addr, NULL);
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(getscom, getscom, (ADDRESS));
> +
> +static int _putscom(struct pdbg_target *target, uint32_t index, uint64_t
> *addr, uint64_t *data)
>  {
>         if (pib_write(target, *addr, *data))
>                 return 0;
> @@ -43,44 +50,9 @@ static int putscom(struct pdbg_target *target, uint32_t
> index, uint64_t *addr, u
>         return 1;
>  }
>
> -
> -int handle_scoms(int optind, int argc, char *argv[])
> + int putscom(uint64_t addr, uint64_t data, uint64_t mask)
>  {
> -       uint64_t addr;
> -       char *endptr;
> -
> -       if (optind + 1 >= argc) {
> -               printf("%s: command '%s' requires an address\n", argv[0],
> argv[optind]);
> -               return -1;
> -       }
> -
> -       errno = 0;
> -       addr = strtoull(argv[optind + 1], &endptr, 0);
> -       if (errno || *endptr != '\0') {
> -               printf("%s: command '%s' couldn't parse address '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -               return -1;
> -       }
> -
> -       if (strcmp(argv[optind], "putscom") == 0) {
> -               uint64_t data;
> -
> -               if (optind + 2 >= argc) {
> -                       printf("%s: command '%s' requires data\n",
> argv[0], argv[optind]);
> -                       return -1;
> -               }
> -
> -               errno = 0;
> -               data = strtoull(argv[optind + 2], &endptr, 0);
> -               if (errno || *endptr != '\0') {
> -                       printf("%s: command '%s' couldn't parse data
> '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -                       return -1;
> -               }
> -
> -               return for_each_target("pib", putscom, &addr, &data);
> -       }
> -
> -       return for_each_target("pib", getscom, &addr, NULL);
> +       /* TODO: Restore the <mask> functionality */
> +       return for_each_target("pib", _putscom, &addr, &data);
>  }
> -
> +OPTCMD_DEFINE_CMD_WITH_ARGS(putscom, putscom, (ADDRESS, DATA,
> DEFAULT_DATA("0xffffffffffffffff")));
> diff --git a/src/scom.h b/src/scom.h
> deleted file mode 100644
> index d4325b5..0000000
> --- a/src/scom.h
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - *     http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -#include <inttypes.h>
> -
> -int handle_scoms(int optind, int argc, char *argv[]);
> diff --git a/src/thread.c b/src/thread.c
> index e8b54cb..4b95636 100644
> --- a/src/thread.c
> +++ b/src/thread.c
> @@ -21,7 +21,7 @@
>  #include <libpdbg.h>
>
>  #include "main.h"
> -#include "mem.h"
> +#include "optcmd.h"
>
>  static int print_thread_status(struct pdbg_target *target, uint32_t
> index, uint64_t *arg, uint64_t *unused1)
>  {
> @@ -81,6 +81,57 @@ static int print_core_thread_status(struct pdbg_target
> *core_target, uint32_t in
>         return rc;
>  }
>
> +static bool is_real_address(struct thread_regs *regs, uint64_t addr)
> +{
> +       return true;
> +       if ((addr & 0xf000000000000000ULL) == 0xc000000000000000ULL)
> +               return true;
> +       return false;
> +}
> +
> +static int load8(struct pdbg_target *target, uint64_t addr, uint64_t
> *value)
> +{
> +       if (adu_getmem(target, addr, (uint8_t *)value, 8)) {
> +               pdbg_log(PDBG_ERROR, "Unable to read memory address=%016"
> PRIx64 ".\n", addr);
> +               return 0;
> +       }
> +
> +       return 1;
> +}
> +
> +static int dump_stack(struct thread_regs *regs)
> +{
> +       struct pdbg_target *target;
> +       uint64_t sp = regs->gprs[1];
> +       uint64_t pc;
> +
> +       pdbg_for_each_class_target("adu", target) {
> +               if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> +                       continue;
> +               break;
> +       }
> +
> +       printf("STACK:\n");
> +       if (!target)
> +               pdbg_log(PDBG_ERROR, "Unable to read memory (no ADU
> found)\n");
> +
> +       if (sp && is_real_address(regs, sp)) {
> +               if (!load8(target, sp, &sp))
> +                       return 1;
> +               while (sp && is_real_address(regs, sp)) {
> +                       if (!load8(target, sp + 16, &pc))
> +                               return 1;
> +
> +                       printf(" 0x%016" PRIx64 " 0x%16" PRIx64 "\n", sp,
> pc);
> +
> +                       if (!load8(target, sp, &sp))
> +                               return 1;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  static int get_thread_max_index(struct pdbg_target *target, uint32_t
> index, uint64_t *maxindex, uint64_t *unused)
>  {
>         if (index > *maxindex)
> @@ -140,48 +191,37 @@ static int state_thread(struct pdbg_target
> *thread_target, uint32_t index, uint6
>         return 1;
>  }
>
> -int thread_start(int optind, int argc, char *argv[])
> +static int thread_start(void)
>  {
>         return for_each_target("thread", start_thread, NULL, NULL);
>  }
> +OPTCMD_DEFINE_CMD(start, thread_start);
>
> -int thread_step(int optind, int argc, char *argv[])
> +static int thread_step(uint64_t count)
>  {
> -       uint64_t count;
> -       char *endptr;
> -
> -       if (optind + 1 >= argc) {
> -               printf("%s: command '%s' requires a count\n", argv[0],
> argv[optind]);
> -               return -1;
> -       }
> -
> -       errno = 0;
> -       count = strtoull(argv[optind + 1], &endptr, 0);
> -       if (errno || *endptr != '\0') {
> -               printf("%s: command '%s' couldn't parse count '%s'\n",
> -                               argv[0], argv[optind], argv[optind + 1]);
> -               return -1;
> -       }
> -
>         return for_each_target("thread", step_thread, &count, NULL);
>  }
> +OPTCMD_DEFINE_CMD_WITH_ARGS(step, thread_step, (DATA));
>
> -int thread_stop(int optind, int argc, char *argv[])
> +static int thread_stop(void)
>  {
>         return for_each_target("thread", stop_thread, NULL, NULL);
>  }
> +OPTCMD_DEFINE_CMD(stop, thread_stop);
>
> -int thread_status_print(int optind, int argc, char *argv[])
> +static int thread_status_print(void)
>  {
>         return for_each_target("pib", print_proc_thread_status, NULL,
> NULL);
>  }
> +OPTCMD_DEFINE_CMD(threadstatus, thread_status_print);
>
> -int thread_sreset(int optind, int argc, char *argv[])
> +static int thread_sreset(void)
>  {
>         return for_each_target("thread", sreset_thread, NULL, NULL);
>  }
> +OPTCMD_DEFINE_CMD(sreset, thread_sreset);
>
> -int thread_state(int optind, int argc, char *argv[])
> +static int thread_state(void)
>  {
>         int err;
>
> @@ -191,3 +231,4 @@ int thread_state(int optind, int argc, char *argv[])
>
>         return err;
>  }
> +OPTCMD_DEFINE_CMD(regs, thread_state);
> diff --git a/src/thread.h b/src/thread.h
> deleted file mode 100644
> index 0c1caa2..0000000
> --- a/src/thread.h
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/* Copyright 2017 IBM Corp.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at
> - *
> - *     http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> - * implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -#include <inttypes.h>
> -
> -int thread_start(int optind, int argc, char *argv[]);
> -int thread_step(int optind, int argc, char *argv[]);
> -int thread_stop(int optind, int argc, char *argv[]);
> -int thread_status_print(int optind, int argc, char *argv[]);
> -int thread_sreset(int optind, int argc, char *argv[]);
> -int thread_state(int optind, int argc, char *argv[]);
> --
> 2.11.0
>
> --
> Pdbg mailing list
> Pdbg@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/pdbg
>
<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Jun 20, 2018, 3:35 PM Alistair Popple &lt;<a href="mailto:alistair@popple.id.au">alistair@popple.id.au</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This switches all commands except the htm command to use the new command/option<br>
parsing code. For the moment we leave the usage help and generic targeting flag<br>
processing alone, but future changes should allow this to also use the common<br>
parsing code.<br>
<br>
Signed-off-by: Alistair Popple &lt;<a href="mailto:alistair@popple.id.au" target="_blank" rel="noreferrer">alistair@popple.id.au</a>&gt;<br>
---<br>
 Makefile.am  |   1 +<br>
 src/cfam.c   |  55 ++++++--------------<br>
 src/cfam.h   |  18 -------<br>
 src/main.c   | 125 ++++++++++++++++++++++++++++-----------------<br>
 src/mem.c    | 124 ++++++++------------------------------------<br>
 src/mem.h    |  20 --------<br>
 src/reg.c    | 164 +++++++++++++++--------------------------------------------<br>
 src/reg.h    |  20 --------<br>
 src/ring.c   |  32 ++----------<br>
 src/ring.h   |  17 -------<br>
 src/scom.c   |  54 +++++---------------<br>
 src/scom.h   |  18 -------<br>
 src/thread.c |  87 ++++++++++++++++++++++---------<br>
 src/thread.h |  23 ---------<br>
 14 files changed, 237 insertions(+), 521 deletions(-)<br>
 delete mode 100644 src/cfam.h<br>
 delete mode 100644 src/mem.h<br>
 delete mode 100644 src/reg.h<br>
 delete mode 100644 src/ring.h<br>
 delete mode 100644 src/scom.h<br>
 delete mode 100644 src/thread.h<br>
<br>
diff --git a/Makefile.am b/Makefile.am<br>
index 23f2080..4e0dce2 100644<br>
--- a/Makefile.am<br>
+++ b/Makefile.am<br>
@@ -39,6 +39,7 @@ pdbg_SOURCES = \<br>
        src/ring.c \<br>
        src/htm.c \<br>
        src/progress.c \<br>
+       src/parsers.c \<br>
        src/optcmd.c \<br>
        src/options_@ARCH@.c<br>
<br>
diff --git a/src/cfam.c b/src/cfam.c<br>
index 269123e..6dab388 100644<br>
--- a/src/cfam.c<br>
+++ b/src/cfam.c<br>
@@ -20,8 +20,9 @@<br>
 #include &lt;inttypes.h&gt;<br>
<br>
 #include &quot;main.h&quot;<br>
+#include &quot;optcmd.h&quot;<br>
<br>
-static int getcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)<br>
+static int _getcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)<br>
 {<br>
        uint32_t value;<br>
<br>
@@ -33,7 +34,15 @@ static int getcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, u<br>
        return 1;<br>
 }<br>
<br>
-static int putcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)<br>
+static int getcfam(uint32_t addr)<br>
+{<br>
+       uint64_t addr64 = addr;<br>
+<br>
+       return for_each_target(&quot;fsi&quot;, _getcfam, &amp;addr64, NULL);<br>
+}<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(getcfam, getcfam, (ADDRESS32));<br>
+<br>
+static int _putcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)<br>
 {<br>
        if (fsi_write(target, *addr, *data))<br>
                return 0;<br>
@@ -41,44 +50,10 @@ static int putcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, u<br>
        return 1;<br>
 }<br>
<br>
-int handle_cfams(int optind, int argc, char *argv[])<br>
+static int putcfam(uint32_t addr, uint32_t data)<br>
 {<br>
-       uint64_t addr;<br>
-       char *endptr;<br>
-<br>
-       if (optind + 1 &gt;= argc) {<br>
-               printf(&quot;%s: command &#39;%s&#39; requires an address\n&quot;, argv[0], argv[optind]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       errno = 0;<br>
-       addr = strtoull(argv[optind + 1], &amp;endptr, 0);<br>
-       if (errno || *endptr != &#39;\0&#39;) {<br>
-               printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse address &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-               return -1;<br>
-       }<br>
+       uint64_t addr64 = addr, data64 = data;<br>
<br>
-       if (strcmp(argv[optind], &quot;putcfam&quot;) == 0) {<br>
-               uint64_t data;<br>
-<br>
-               if (optind + 2 &gt;= argc) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; requires data\n&quot;, argv[0], argv[optind]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               errno = 0;<br>
-               data = strtoull(argv[optind + 2], &amp;endptr, 0);<br>
-               if (errno || *endptr != &#39;\0&#39;) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse data &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               return for_each_target(&quot;fsi&quot;, putcfam, &amp;addr, &amp;data);<br>
-       }<br>
-<br>
-       return for_each_target(&quot;fsi&quot;, getcfam, &amp;addr, NULL);<br>
+       return for_each_target(&quot;fsi&quot;, _putcfam, &amp;addr64, &amp;data64);<br>
 }<br>
-<br>
-<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(putcfam, putcfam, (ADDRESS32, DATA32));<br>
diff --git a/src/cfam.h b/src/cfam.h<br>
deleted file mode 100644<br>
index 997ed3d..0000000<br>
--- a/src/cfam.h<br>
+++ /dev/null<br>
@@ -1,18 +0,0 @@<br>
-/* Copyright 2017 IBM Corp.<br>
- *<br>
- * Licensed under the Apache License, Version 2.0 (the &quot;License&quot;);<br>
- * you may not use this file except in compliance with the License.<br>
- * You may obtain a copy of the License at<br>
- *<br>
- *     <a href="http://www.apache.org/licenses/LICENSE-2.0" rel="noreferrer noreferrer" target="_blank">http://www.apache.org/licenses/LICENSE-2.0</a><br>
- *<br>
- * Unless required by applicable law or agreed to in writing, software<br>
- * distributed under the License is distributed on an &quot;AS IS&quot; BASIS,<br>
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or<br>
- * implied.<br>
- * See the License for the specific language governing permissions and<br>
- * limitations under the License.<br>
- */<br>
-#include &lt;inttypes.h&gt;<br>
-<br>
-int handle_cfams(int optind, int argc, char *argv[]);<br>
diff --git a/src/main.c b/src/main.c<br>
index cf9a3b2..1cfaca7 100644<br>
--- a/src/main.c<br>
+++ b/src/main.c<br>
@@ -34,14 +34,9 @@<br>
 #include &lt;target.h&gt;<br>
<br>
 #include &quot;main.h&quot;<br>
-#include &quot;cfam.h&quot;<br>
-#include &quot;scom.h&quot;<br>
-#include &quot;reg.h&quot;<br>
-#include &quot;ring.h&quot;<br>
-#include &quot;mem.h&quot;<br>
-#include &quot;thread.h&quot;<br>
 #include &quot;htm.h&quot;<br>
 #include &quot;options.h&quot;<br>
+#include &quot;optcmd.h&quot;<br>
<br>
 #define PR_ERROR(x, args...) \<br>
        pdbg_log(PDBG_ERROR, x, ##args)<br>
@@ -77,44 +72,64 @@ static int **processorsel[MAX_PROCESSORS];<br>
 static int *chipsel[MAX_PROCESSORS][MAX_CHIPS];<br>
 static int threadsel[MAX_PROCESSORS][MAX_CHIPS][MAX_THREADS];<br>
<br>
-static int handle_probe(int optind, int argc, char *argv[]);<br>
-static int handle_release(int optind, int argc, char *argv[]);<br>
+static int probe(void);<br>
+static int release(void);<br>
+<br>
+/* TODO: We are repeating ourselves here. A little bit more macro magic could<br>
+ * easily fix this but I was hesitant to introduce too much magic all at<br>
+ * once. */<br>
+extern struct optcmd_cmd<br>
+       optcmd_getscom, optcmd_putscom, optcmd_getcfam, optcmd_putcfam,<br>
+       optcmd_getgpr, optcmd_putgpr, optcmd_getspr, optcmd_putspr,<br>
+       optcmd_getnia, optcmd_putnia, optcmd_getmsr, optcmd_putmsr,<br>
+       optcmd_getring, optcmd_start, optcmd_stop, optcmd_step,<br>
+       optcmd_threadstatus, optcmd_sreset, optcmd_regs, optcmd_probe,<br>
+       optcmd_getmem, optcmd_putmem;<br>
+<br>
+static struct optcmd_cmd *cmds[] = {<br>
+       &amp;optcmd_getscom, &amp;optcmd_putscom, &amp;optcmd_getcfam, &amp;optcmd_putcfam,<br>
+       &amp;optcmd_getgpr, &amp;optcmd_putgpr, &amp;optcmd_getspr, &amp;optcmd_putspr,<br>
+       &amp;optcmd_getnia, &amp;optcmd_putnia, &amp;optcmd_getmsr, &amp;optcmd_putmsr,<br>
+       &amp;optcmd_getring, &amp;optcmd_start, &amp;optcmd_stop, &amp;optcmd_step,<br>
+       &amp;optcmd_threadstatus, &amp;optcmd_sreset, &amp;optcmd_regs, &amp;optcmd_probe,<br>
+       &amp;optcmd_getmem, &amp;optcmd_putmem,<br>
+};<br>
<br>
+/* Purely for printing usage text. We could integrate printing argument and flag<br>
+ * help into optcmd if desired. */<br>
 struct action {<br>
        const char *name;<br>
        const char *args;<br>
        const char *desc;<br>
-       int (*fn)(int, int, char **);<br>
 };<br>
<br>
 static struct action actions[] = {<br>
-       { &quot;getgpr&quot;,  &quot;&lt;gpr&gt;&quot;, &quot;Read General Purpose Register (GPR)&quot;, &amp;handle_gpr },<br>
-       { &quot;putgpr&quot;,  &quot;&lt;gpr&gt; &lt;value&gt;&quot;, &quot;Write General Purpose Register (GPR)&quot;, &amp;handle_gpr },<br>
-       { &quot;getnia&quot;,  &quot;&quot;, &quot;Get Next Instruction Address (NIA)&quot;, &amp;handle_nia },<br>
-       { &quot;putnia&quot;,  &quot;&lt;value&gt;&quot;, &quot;Write Next Instrution Address (NIA)&quot;, &amp;handle_nia },<br>
-       { &quot;getspr&quot;,  &quot;&lt;spr&gt;&quot;, &quot;Get Special Purpose Register (SPR)&quot;, &amp;handle_spr },<br>
-       { &quot;putspr&quot;,  &quot;&lt;spr&gt; &lt;value&gt;&quot;, &quot;Write Special Purpose Register (SPR)&quot;, &amp;handle_spr },<br>
-       { &quot;getmsr&quot;,  &quot;&quot;, &quot;Get Machine State Register (MSR)&quot;, &amp;handle_msr },<br>
-       { &quot;putmsr&quot;,  &quot;&lt;value&gt;&quot;, &quot;Write Machine State Register (MSR)&quot;, &amp;handle_msr },<br>
-       { &quot;getring&quot;, &quot;&lt;addr&gt; &lt;len&gt;&quot;, &quot;Read a ring. Length must be correct&quot;, &amp;handle_getring },<br>
-       { &quot;start&quot;,   &quot;&quot;, &quot;Start thread&quot;, &amp;thread_start },<br>
-       { &quot;step&quot;,    &quot;&lt;count&gt;&quot;, &quot;Set a thread &lt;count&gt; instructions&quot;, &amp;thread_step },<br>
-       { &quot;stop&quot;,    &quot;&quot;, &quot;Stop thread&quot;, &amp;thread_stop },<br>
-       { &quot;htm&quot;, &quot;core|nest start|stop|status|reset|dump|trace|analyse&quot;, &quot;Hardware Trace Macro&quot;, &amp;run_htm },<br>
-       { &quot;release&quot;, &quot;&quot;, &quot;Should be called after pdbg work is finished, to release special wakeups and other resources.&quot;, &amp;handle_release},<br>
-       { &quot;probe&quot;, &quot;&quot;, &quot;&quot;, &amp;handle_probe },<br>
-       { &quot;getcfam&quot;, &quot;&lt;address&gt;&quot;, &quot;Read system cfam&quot;, &amp;handle_cfams },<br>
-       { &quot;putcfam&quot;, &quot;&lt;address&gt; &lt;value&gt; [&lt;mask&gt;]&quot;, &quot;Write system cfam&quot;, &amp;handle_cfams },<br>
-       { &quot;getscom&quot;, &quot;&lt;address&gt;&quot;, &quot;Read system scom&quot;, &amp;handle_scoms },<br>
-       { &quot;putscom&quot;, &quot;&lt;address&gt; &lt;value&gt; [&lt;mask&gt;]&quot;, &quot;Write system scom&quot;, &amp;handle_scoms },<br>
-       { &quot;getmem&quot;,  &quot;&lt;address&gt; &lt;count&gt;&quot;, &quot;Read system memory&quot;, &amp;handle_mem },<br>
-       { &quot;putmem&quot;,  &quot;&lt;address&gt;&quot;, &quot;Write to system memory&quot;, &amp;handle_mem },<br>
-       { &quot;threadstatus&quot;, &quot;&quot;, &quot;Print the status of a thread&quot;, &amp;thread_status_print },<br>
-       { &quot;sreset&quot;,  &quot;&quot;, &quot;Reset&quot;, &amp;thread_sreset },<br>
-       { &quot;regs&quot;,  &quot;&quot;, &quot;State&quot;, &amp;thread_state },<br>
+       { &quot;getgpr&quot;,  &quot;&lt;gpr&gt;&quot;, &quot;Read General Purpose Register (GPR)&quot; },<br>
+       { &quot;putgpr&quot;,  &quot;&lt;gpr&gt; &lt;value&gt;&quot;, &quot;Write General Purpose Register (GPR)&quot; },<br>
+       { &quot;getnia&quot;,  &quot;&quot;, &quot;Get Next Instruction Address (NIA)&quot; },<br>
+       { &quot;putnia&quot;,  &quot;&lt;value&gt;&quot;, &quot;Write Next Instrution Address (NIA)&quot; },<br>
+       { &quot;getspr&quot;,  &quot;&lt;spr&gt;&quot;, &quot;Get Special Purpose Register (SPR)&quot; },<br>
+       { &quot;putspr&quot;,  &quot;&lt;spr&gt; &lt;value&gt;&quot;, &quot;Write Special Purpose Register (SPR)&quot; },<br>
+       { &quot;getmsr&quot;,  &quot;&quot;, &quot;Get Machine State Register (MSR)&quot; },<br>
+       { &quot;putmsr&quot;,  &quot;&lt;value&gt;&quot;, &quot;Write Machine State Register (MSR)&quot; },<br>
+       { &quot;getring&quot;, &quot;&lt;addr&gt; &lt;len&gt;&quot;, &quot;Read a ring. Length must be correct&quot; },<br>
+       { &quot;start&quot;,   &quot;&quot;, &quot;Start thread&quot; },<br>
+       { &quot;step&quot;,    &quot;&lt;count&gt;&quot;, &quot;Set a thread &lt;count&gt; instructions&quot; },<br>
+       { &quot;stop&quot;,    &quot;&quot;, &quot;Stop thread&quot; },<br>
+       { &quot;htm&quot;, &quot;core|nest start|stop|status|reset|dump|trace|analyse&quot;, &quot;Hardware Trace Macro&quot; },<br>
+       { &quot;release&quot;, &quot;&quot;, &quot;Should be called after pdbg work is finished&quot; },<br>
+       { &quot;probe&quot;, &quot;&quot;, &quot;&quot; },<br>
+       { &quot;getcfam&quot;, &quot;&lt;address&gt;&quot;, &quot;Read system cfam&quot; },<br>
+       { &quot;putcfam&quot;, &quot;&lt;address&gt; &lt;value&gt; [&lt;mask&gt;]&quot;, &quot;Write system cfam&quot; },<br>
+       { &quot;getscom&quot;, &quot;&lt;address&gt;&quot;, &quot;Read system scom&quot; },<br>
+       { &quot;putscom&quot;, &quot;&lt;address&gt; &lt;value&gt; [&lt;mask&gt;]&quot;, &quot;Write system scom&quot; },<br>
+       { &quot;getmem&quot;,  &quot;&lt;address&gt; &lt;count&gt;&quot;, &quot;Read system memory&quot; },<br>
+       { &quot;putmem&quot;,  &quot;&lt;address&gt;&quot;, &quot;Write to system memory&quot; },<br>
+       { &quot;threadstatus&quot;, &quot;&quot;, &quot;Print the status of a thread&quot; },<br>
+       { &quot;sreset&quot;,  &quot;&quot;, &quot;Reset&quot; },<br>
+       { &quot;regs&quot;,  &quot;&quot;, &quot;State&quot; },<br>
 };<br>
<br>
-<br>
 static void print_usage(char *pname)<br>
 {<br>
        int i;<br>
@@ -600,7 +615,7 @@ static void release_target(struct pdbg_target *target)<br>
        pdbg_target_release(target);<br>
 }<br>
<br>
-static void do_release(void)<br>
+static int release(void)<br>
 {<br>
        struct pdbg_target_class *target_class;<br>
<br>
@@ -610,7 +625,10 @@ static void do_release(void)<br>
                pdbg_for_each_class_target(target_class-&gt;name, target)<br>
                        release_target(target);<br>
        }<br>
+<br>
+       return 0;<br>
 }<br>
+OPTCMD_DEFINE_CMD(release, release);<br>
<br>
 void print_target(struct pdbg_target *target, int level)<br>
 {<br>
@@ -652,7 +670,7 @@ void print_target(struct pdbg_target *target, int level)<br>
        }<br>
 }<br>
<br>
-static int handle_probe(int optind, int argc, char *argv[])<br>
+static int probe(void)<br>
 {<br>
        struct pdbg_target *target;<br>
<br>
@@ -664,25 +682,21 @@ static int handle_probe(int optind, int argc, char *argv[])<br>
<br>
        return 1;<br>
 }<br>
+OPTCMD_DEFINE_CMD(probe, probe);<br>
<br>
 /*<br>
  * Release handler.<br>
  */<br>
 static void atexit_release(void)<br>
 {<br>
-       do_release();<br>
-}<br>
-<br>
-static int handle_release(int optind, int argc, char *argv[])<br>
-{<br>
-       do_release();<br>
-<br>
-       return 1;<br>
+       release();<br>
 }<br>
<br>
 int main(int argc, char *argv[])<br>
 {<br>
        int i, rc = 0;<br>
+       void **args, **flags;<br>
+       optcmd_cmd_t *cmd;<br>
<br>
        backend = default_backend();<br>
        device_node = default_target(backend);<br>
@@ -714,13 +728,28 @@ int main(int argc, char *argv[])<br>
<br>
        atexit(atexit_release);<br>
<br>
-       for (i = 0; i &lt; ARRAY_SIZE(actions); i++) {<br>
-               if (strcmp(argv[optind], actions[i].name) == 0) {<br>
-                       rc = actions[i].fn(optind, argc, argv);<br>
-                       goto found_action;<br>
+       for (i = 0; i &lt; ARRAY_SIZE(cmds); i++) {<br>
+               if (!strcmp(argv[optind], cmds[i]-&gt;cmd)) {<br>
+                       /* Found our command */<br>
+                       cmd = optcmd_parse(cmds[i], (const char **) &amp;argv[optind + 1],<br>
+                                          argc - (optind + 1), &amp;args, &amp;flags);<br>
+                       if (cmd) {<br>
+                               rc = cmd(args, flags);<br>
+                               goto found_action;<br>
+                       } else {<br>
+                               /* Error parsing arguments so exit return directly */<br>
+                               return 1;<br>
+                       }<br>
                }<br>
        }<br>
<br>
+       /* Process subcommands. Currently only &#39;htm&#39;.<br>
+        * TODO: Move htm command parsing to optcmd once htm clean-up is complete */<br>
+       if (!strcmp(argv[optind], &quot;htm&quot;)) {<br>
+               run_htm(optind, argc, argv);<br>
+               goto found_action;<br>
+       }<br>
+<br>
        PR_ERROR(&quot;Unsupported command: %s\n&quot;, argv[optind]);<br>
        return 1;<br>
<br>
diff --git a/src/mem.c b/src/mem.c<br>
index e0327d1..29fd21e 100644<br>
--- a/src/mem.c<br>
+++ b/src/mem.c<br>
@@ -20,21 +20,34 @@<br>
 #include &lt;string.h&gt;<br>
 #include &lt;unistd.h&gt;<br>
 #include &lt;assert.h&gt;<br>
+#include &lt;stdbool.h&gt;<br>
<br>
 #include &lt;libpdbg.h&gt;<br>
<br>
 #include &quot;main.h&quot;<br>
 #include &quot;progress.h&quot;<br>
+#include &quot;optcmd.h&quot;<br>
+#include &quot;parsers.h&quot;<br>
<br>
 #define PR_ERROR(x, args...) \<br>
        pdbg_log(PDBG_ERROR, x, ##args)<br>
<br>
 #define PUTMEM_BUF_SIZE 1024<br>
-static int getmem(uint64_t addr, uint64_t size, bool ci)<br>
+<br>
+struct mem_flags {<br>
+       bool ci;<br>
+};<br>
+<br>
+#define MEM_CI_FLAG (&quot;--ci&quot;, ci, parse_flag_noarg, false)<br>
+<br>
+static int getmem(uint64_t addr, uint64_t size, struct mem_flags flags)<br>
 {<br>
        struct pdbg_target *target;<br>
        uint8_t *buf;<br>
        int rc = 0;<br>
+<br>
+       printf(&quot;getmem ci %d\n&quot;, <a href="http://flags.ci" rel="noreferrer noreferrer" target="_blank">flags.ci</a>);<br>
+<br>
        buf = malloc(size);<br>
        assert(buf);<br>
        pdbg_for_each_class_target(&quot;adu&quot;, target) {<br>
@@ -43,7 +56,7 @@ static int getmem(uint64_t addr, uint64_t size, bool ci)<br>
<br>
                pdbg_set_progress_tick(progress_tick);<br>
                progress_init();<br>
-               if (!__adu_getmem(target, addr, buf, size, ci)) {<br>
+               if (!__adu_getmem(target, addr, buf, size, <a href="http://flags.ci" rel="noreferrer noreferrer" target="_blank">flags.ci</a>)) {<br>
                        if (write(STDOUT_FILENO, buf, size) &lt; 0)<br>
                                PR_ERROR(&quot;Unable to write stdout.\n&quot;);<br>
                        else<br>
@@ -58,7 +71,10 @@ static int getmem(uint64_t addr, uint64_t size, bool ci)<br>
        return rc;<br>
<br>
 }<br>
-static int putmem(uint64_t addr, bool ci)<br>
+OPTCMD_DEFINE_CMD_WITH_FLAGS(getmem, getmem, (ADDRESS, DATA),<br>
+                            mem_flags, (MEM_CI_FLAG));<br>
+<br>
+static int putmem(uint64_t addr, struct mem_flags flags)<br>
 {<br>
        uint8_t *buf;<br>
        int read_size, rc = 0;<br>
@@ -76,7 +92,7 @@ static int putmem(uint64_t addr, bool ci)<br>
        progress_init();<br>
        do {<br>
                read_size = read(STDIN_FILENO, buf, PUTMEM_BUF_SIZE);<br>
-               if (__adu_putmem(adu_target, addr, buf, read_size, ci)) {<br>
+               if (__adu_putmem(adu_target, addr, buf, read_size, <a href="http://flags.ci" rel="noreferrer noreferrer" target="_blank">flags.ci</a>)) {<br>
                        rc = 0;<br>
                        printf(&quot;Unable to write memory.\n&quot;);<br>
                        break;<br>
@@ -89,101 +105,5 @@ static int putmem(uint64_t addr, bool ci)<br>
        free(buf);<br>
        return rc;<br>
 }<br>
-<br>
-static bool is_real_address(struct thread_regs *regs, uint64_t addr)<br>
-{<br>
-       return true;<br>
-       if ((addr &amp; 0xf000000000000000ULL) == 0xc000000000000000ULL)<br>
-               return true;<br>
-       return false;<br>
-}<br>
-<br>
-static int load8(struct pdbg_target *target, uint64_t addr, uint64_t *value)<br>
-{<br>
-       if (adu_getmem(target, addr, (uint8_t *)value, 8)) {<br>
-               PR_ERROR(&quot;Unable to read memory address=%016&quot; PRIx64 &quot;.\n&quot;, addr);<br>
-               return 0;<br>
-       }<br>
-<br>
-       return 1;<br>
-}<br>
-<br>
-int dump_stack(struct thread_regs *regs)<br>
-{<br>
-       struct pdbg_target *target;<br>
-       uint64_t sp = regs-&gt;gprs[1];<br>
-       uint64_t pc;<br>
-<br>
-       pdbg_for_each_class_target(&quot;adu&quot;, target) {<br>
-               if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)<br>
-                       continue;<br>
-               break;<br>
-       }<br>
-<br>
-       printf(&quot;STACK:\n&quot;);<br>
-       if (!target)<br>
-               PR_ERROR(&quot;Unable to read memory (no ADU found)\n&quot;);<br>
-<br>
-       if (sp &amp;&amp; is_real_address(regs, sp)) {<br>
-               if (!load8(target, sp, &amp;sp))<br>
-                       return 1;<br>
-               while (sp &amp;&amp; is_real_address(regs, sp)) {<br>
-                       if (!load8(target, sp + 16, &amp;pc))<br>
-                               return 1;<br>
-<br>
-                       printf(&quot; 0x%016&quot; PRIx64 &quot; 0x%16&quot; PRIx64 &quot;\n&quot;, sp, pc);<br>
-<br>
-                       if (!load8(target, sp, &amp;sp))<br>
-                               return 1;<br>
-               }<br>
-       }<br>
-<br>
-       return 0;<br>
-}<br>
-<br>
-int handle_mem(int optind, int argc, char *argv[])<br>
-{<br>
-       uint64_t addr;<br>
-       char *endptr;<br>
-       bool ci = false;<br>
-<br>
-       if (optind + 1 &gt;= argc) {<br>
-               printf(&quot;%s: command &#39;%s&#39; requires an address\n&quot;, argv[0], argv[optind]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       errno = 0;<br>
-<br>
-       if (strcmp(argv[optind +1], &quot;-ci&quot;) == 0) {<br>
-               /* Set cache-inhibited flag */<br>
-               ci = true;<br>
-       }<br>
-<br>
-       addr = strtoull(argv[optind + 1 + ci], &amp;endptr, 0);<br>
-       if (errno || *endptr != &#39;\0&#39;) {<br>
-               printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse address &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1 + ci]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       if (strcmp(argv[optind], &quot;getmem&quot;) == 0) {<br>
-               uint64_t size;<br>
-<br>
-               if (optind + 2 + ci &gt;= argc) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; requires data\n&quot;, argv[0], argv[optind]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               errno = 0;<br>
-               size = strtoull(argv[optind + 2 + ci], &amp;endptr, 0);<br>
-               if (errno || *endptr != &#39;\0&#39;) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse data &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1 + ci]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               return getmem(addr, size, ci);<br>
-       }<br>
-<br>
-       return putmem(addr, ci);<br>
-}<br>
+OPTCMD_DEFINE_CMD_WITH_FLAGS(putmem, putmem, (ADDRESS),<br>
+                            mem_flags, (MEM_CI_FLAG));<br>
diff --git a/src/mem.h b/src/mem.h<br>
deleted file mode 100644<br>
index 42bdc04..0000000<br>
--- a/src/mem.h<br>
+++ /dev/null<br>
@@ -1,20 +0,0 @@<br>
-/* Copyright 2017 IBM Corp.<br>
- *<br>
- * Licensed under the Apache License, Version 2.0 (the &quot;License&quot;);<br>
- * you may not use this file except in compliance with the License.<br>
- * You may obtain a copy of the License at<br>
- *<br>
- *     <a href="http://www.apache.org/licenses/LICENSE-2.0" rel="noreferrer noreferrer" target="_blank">http://www.apache.org/licenses/LICENSE-2.0</a><br>
- *<br>
- * Unless required by applicable law or agreed to in writing, software<br>
- * distributed under the License is distributed on an &quot;AS IS&quot; BASIS,<br>
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or<br>
- * implied.<br>
- * See the License for the specific language governing permissions and<br>
- * limitations under the License.<br>
- */<br>
-#include &lt;inttypes.h&gt;<br>
-#include &lt;libpdbg.h&gt;<br>
-<br>
-int dump_stack(struct thread_regs *regs);<br>
-int handle_mem(int optind, int argc, char *argv[]);<br>
diff --git a/src/reg.c b/src/reg.c<br>
index 21bec13..aa77a8a 100644<br>
--- a/src/reg.c<br>
+++ b/src/reg.c<br>
@@ -22,6 +22,7 @@<br>
 #include &lt;libpdbg.h&gt;<br>
<br>
 #include &quot;main.h&quot;<br>
+#include &quot;optcmd.h&quot;<br>
<br>
 #define REG_MEM -3<br>
 #define REG_MSR -2<br>
@@ -91,143 +92,58 @@ static int getprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg,<br>
        return !rc;<br>
 }<br>
<br>
-int handle_gpr(int optind, int argc, char *argv[])<br>
+static int getgpr(int gpr)<br>
 {<br>
-       char *endptr;<br>
-       uint64_t gpr;<br>
-<br>
-       if (optind + 1 &gt;= argc) {<br>
-               printf(&quot;%s: command &#39;%s&#39; requires a GPR\n&quot;, argv[0], argv[optind]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       errno = 0;<br>
-       gpr = strtoull(argv[optind + 1], &amp;endptr, 0);<br>
-       if (errno || *endptr != &#39;\0&#39;) {<br>
-               printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse GPR &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       if (gpr &gt; 31) {<br>
-               printf(&quot;A GPR must be between zero and 31 inclusive\n&quot;);<br>
-               return -1;<br>
-       }<br>
-<br>
-       if (strcmp(argv[optind], &quot;putgpr&quot;) == 0) {<br>
-               uint64_t data;<br>
-<br>
-               if (optind + 2 &gt;= argc) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; requires data\n&quot;, argv[0], argv[optind]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               errno = 0;<br>
-               data = strtoull(argv[optind + 2], &amp;endptr, 0);<br>
-               if (errno || *endptr != &#39;\0&#39;) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse data &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               return for_each_target(&quot;thread&quot;, putprocreg, &amp;gpr, &amp;data);<br>
-       }<br>
-<br>
-       return for_each_target(&quot;thread&quot;, getprocreg, &amp;gpr, NULL);<br>
+       uint64_t reg = gpr;<br>
+       return for_each_target(&quot;thread&quot;, getprocreg, &amp;reg, NULL);<br>
 }<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(getgpr, getgpr, (GPR));<br>
<br>
-int handle_nia(int optind, int argc, char *argv[])<br>
+static int putgpr(int gpr, uint64_t data)<br>
 {<br>
-       uint64_t reg = REG_NIA;<br>
-       char *endptr;<br>
-<br>
-       if (strcmp(argv[optind], &quot;putnia&quot;) == 0) {<br>
-               uint64_t data;<br>
-<br>
-               if (optind + 1 &gt;= argc) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; requires data\n&quot;, argv[0], argv[optind]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               errno = 0;<br>
-               data = strtoull(argv[optind + 1], &amp;endptr, 0);<br>
-               if (errno || *endptr != &#39;\0&#39;) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse data &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               return for_each_target(&quot;thread&quot;, putprocreg, &amp;reg, &amp;data);<br>
-       }<br>
+       uint64_t reg = gpr;<br>
+       return for_each_target(&quot;thread&quot;, putprocreg, &amp;reg, &amp;data);<br>
+}<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(putgpr, putgpr, (GPR, DATA));<br>
<br>
+static int getnia(void)<br>
+{<br>
+       uint64_t reg = REG_NIA;<br>
        return for_each_target(&quot;thread&quot;, getprocreg, &amp;reg, NULL);<br>
 }<br>
+OPTCMD_DEFINE_CMD(getnia, getnia);<br>
<br>
-int handle_spr(int optind, int argc, char *argv[])<br>
+static int putnia(uint64_t nia)<br>
 {<br>
-       char *endptr;<br>
-       uint64_t spr;<br>
-<br>
-       if (optind + 1 &gt;= argc) {<br>
-               printf(&quot;%s: command &#39;%s&#39; requires a GPR\n&quot;, argv[0], argv[optind]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       errno = 0;<br>
-       spr = strtoull(argv[optind + 1], &amp;endptr, 0);<br>
-       if (errno || *endptr != &#39;\0&#39;) {<br>
-               printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse GPR &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       spr += REG_R31;<br>
-<br>
-       if (strcmp(argv[optind], &quot;putspr&quot;) == 0) {<br>
-               uint64_t data;<br>
-<br>
-               if (optind + 2 &gt;= argc) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; requires data\n&quot;, argv[0], argv[optind]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               errno = 0;<br>
-               data = strtoull(argv[optind + 2], &amp;endptr, 0);<br>
-               if (errno || *endptr != &#39;\0&#39;) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse data &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               return for_each_target(&quot;thread&quot;, putprocreg, &amp;spr, &amp;data);<br>
-       }<br>
-<br>
-       return for_each_target(&quot;thread&quot;, getprocreg, &amp;spr, NULL);<br>
+       uint64_t reg = REG_NIA;<br>
+       return for_each_target(&quot;thread&quot;, getprocreg, &amp;reg, &amp;nia);<br>
 }<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">I think this should be putprocreg. There is at least one more that I found that seems to be using the wrong &lt;put/get&gt;procreg</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+OPTCMD_DEFINE_CMD_WITH_ARGS(putnia, putnia, (DATA));<br>
<br>
-int handle_msr(int optind, int argc, char *argv[])<br>
+static int getspr(int spr)<br>
 {<br>
-       uint64_t msr = REG_MSR;<br>
-       char *endptr;<br>
-<br>
-       if (strcmp(argv[optind], &quot;putmsr&quot;) == 0) {<br>
-               uint64_t data;<br>
-<br>
-               if (optind + 1 &gt;= argc) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; requires data\n&quot;, argv[0], argv[optind]);<br>
-                       return -1;<br>
-               }<br>
+       uint64_t reg = spr + REG_R31;<br>
+       return for_each_target(&quot;thread&quot;, getprocreg, &amp;reg, NULL);<br>
+}<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(getspr, getspr, (SPR));<br>
<br>
-               errno = 0;<br>
-               data = strtoull(argv[optind + 1], &amp;endptr, 0);<br>
-               if (errno || *endptr != &#39;\0&#39;) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse data &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-                       return -1;<br>
-               }<br>
+static int putspr(int spr, uint64_t data)<br>
+{<br>
+       uint64_t reg = spr + REG_R31;<br>
+       return for_each_target(&quot;thread&quot;, putprocreg, &amp;reg, &amp;data);<br>
+}<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(putspr, putspr, (SPR, DATA));<br>
<br>
-               return for_each_target(&quot;thread&quot;, putprocreg, &amp;msr, &amp;data);<br>
-       }<br>
+static int getmsr(void)<br>
+{<br>
+       uint64_t reg = REG_MSR;<br>
+       return for_each_target(&quot;thread&quot;, getprocreg, &amp;reg, NULL);<br>
+}<br>
+OPTCMD_DEFINE_CMD(getmsr, getmsr);<br>
<br>
-       return for_each_target(&quot;thread&quot;, getprocreg, &amp;msr, NULL);<br>
+static int putmsr(uint64_t data)<br>
+{<br>
+       uint64_t reg = REG_MSR;<br>
+       return for_each_target(&quot;thread&quot;, putprocreg, &amp;reg, &amp;data);<br>
 }<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(putmsr, putmsr, (DATA));<br>
diff --git a/src/reg.h b/src/reg.h<br>
deleted file mode 100644<br>
index ad41d9d..0000000<br>
--- a/src/reg.h<br>
+++ /dev/null<br>
@@ -1,20 +0,0 @@<br>
-/* Copyright 2017 IBM Corp.<br>
- *<br>
- * Licensed under the Apache License, Version 2.0 (the &quot;License&quot;);<br>
- * you may not use this file except in compliance with the License.<br>
- * You may obtain a copy of the License at<br>
- *<br>
- *     <a href="http://www.apache.org/licenses/LICENSE-2.0" rel="noreferrer noreferrer" target="_blank">http://www.apache.org/licenses/LICENSE-2.0</a><br>
- *<br>
- * Unless required by applicable law or agreed to in writing, software<br>
- * distributed under the License is distributed on an &quot;AS IS&quot; BASIS,<br>
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or<br>
- * implied.<br>
- * See the License for the specific language governing permissions and<br>
- * limitations under the License.<br>
- */<br>
-<br>
-int handle_gpr(int optind, int argc, char *argv[]);<br>
-int handle_nia(int optind, int argc, char *argv[]);<br>
-int handle_spr(int optind, int argc, char *argv[]);<br>
-int handle_msr(int optind, int argc, char *argv[]);<br>
diff --git a/src/ring.c b/src/ring.c<br>
index b0c9376..58df4d1 100644<br>
--- a/src/ring.c<br>
+++ b/src/ring.c<br>
@@ -18,11 +18,12 @@<br>
 #include &lt;stdio.h&gt;<br>
 #include &lt;stdlib.h&gt;<br>
 #include &lt;string.h&gt;<br>
+#include &lt;assert.h&gt;<br>
<br>
-#include &lt;target.h&gt;<br>
-#include &lt;operations.h&gt;<br>
+#include &lt;libpdbg.h&gt;<br>
<br>
 #include &quot;main.h&quot;<br>
+#include &quot;optcmd.h&quot;<br>
<br>
 static int pdbg_getring(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *len)<br>
 {<br>
@@ -51,31 +52,8 @@ static int pdbg_getring(struct pdbg_target *target, uint32_t index, uint64_t *ad<br>
        return 1;<br>
 }<br>
<br>
-int handle_getring(int optind, int argc, char *argv[])<br>
+static int _getring(uint64_t ring_addr, uint64_t ring_len)<br>
 {<br>
-       uint64_t ring_addr, ring_len;<br>
-       char *endptr;<br>
-<br>
-       if (optind + 2 &gt;= argc) {<br>
-               printf(&quot;%s: command &#39;%s&#39; requires two arguments (address and length)\n&quot;,<br>
-                      argv[0], argv[optind]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       errno = 0;<br>
-       ring_addr = strtoull(argv[optind + 1], &amp;endptr, 0);<br>
-       if (errno || *endptr != &#39;\0&#39;) {<br>
-               printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse ring address &#39;%s&#39;\n&quot;,<br>
-                      argv[0], argv[optind], argv[optind + 1]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       ring_len = strtoull(argv[optind + 2], &amp;endptr, 0);<br>
-       if (errno || *endptr != &#39;\0&#39;) {<br>
-               printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse ring length &#39;%s&#39;\n&quot;,<br>
-                      argv[0], argv[optind], argv[optind + 2]);<br>
-               return -1;<br>
-       }<br>
-<br>
        return for_each_target(&quot;chiplet&quot;, pdbg_getring, &amp;ring_addr, &amp;ring_len);<br>
 }<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(getring, _getring, (ADDRESS, DATA));<br>
diff --git a/src/ring.h b/src/ring.h<br>
deleted file mode 100644<br>
index a72c875..0000000<br>
--- a/src/ring.h<br>
+++ /dev/null<br>
@@ -1,17 +0,0 @@<br>
-/* Copyright 2018 IBM Corp.<br>
- *<br>
- * Licensed under the Apache License, Version 2.0 (the &quot;License&quot;);<br>
- * you may not use this file except in compliance with the License.<br>
- * You may obtain a copy of the License at<br>
- *<br>
- *      <a href="http://www.apache.org/licenses/LICENSE-2.0" rel="noreferrer noreferrer" target="_blank">http://www.apache.org/licenses/LICENSE-2.0</a><br>
- *<br>
- * Unless required by applicable law or agreed to in writing, software<br>
- * distributed under the License is distributed on an &quot;AS IS&quot; BASIS,<br>
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or<br>
- * implied.<br>
- * See the License for the specific language governing permissions and<br>
- * limitations under the License.<br>
- */<br>
-<br>
-int handle_getring(int optind, int argc, char *argv[]);<br>
diff --git a/src/scom.c b/src/scom.c<br>
index 4c59e2a..2372e91 100644<br>
--- a/src/scom.c<br>
+++ b/src/scom.c<br>
@@ -22,8 +22,9 @@<br>
 #include &lt;libpdbg.h&gt;<br>
<br>
 #include &quot;main.h&quot;<br>
+#include &quot;optcmd.h&quot;<br>
<br>
-static int getscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)<br>
+static int _getscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)<br>
 {<br>
        uint64_t value;<br>
<br>
@@ -35,7 +36,13 @@ static int getscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, u<br>
        return 1;<br>
 }<br>
<br>
-static int putscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)<br>
+ int getscom(uint64_t addr)<br>
+{<br>
+       return for_each_target(&quot;pib&quot;, _getscom, &amp;addr, NULL);<br>
+}<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(getscom, getscom, (ADDRESS));<br>
+<br>
+static int _putscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)<br>
 {<br>
        if (pib_write(target, *addr, *data))<br>
                return 0;<br>
@@ -43,44 +50,9 @@ static int putscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, u<br>
        return 1;<br>
 }<br>
<br>
-<br>
-int handle_scoms(int optind, int argc, char *argv[])<br>
+ int putscom(uint64_t addr, uint64_t data, uint64_t mask)<br>
 {<br>
-       uint64_t addr;<br>
-       char *endptr;<br>
-<br>
-       if (optind + 1 &gt;= argc) {<br>
-               printf(&quot;%s: command &#39;%s&#39; requires an address\n&quot;, argv[0], argv[optind]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       errno = 0;<br>
-       addr = strtoull(argv[optind + 1], &amp;endptr, 0);<br>
-       if (errno || *endptr != &#39;\0&#39;) {<br>
-               printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse address &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       if (strcmp(argv[optind], &quot;putscom&quot;) == 0) {<br>
-               uint64_t data;<br>
-<br>
-               if (optind + 2 &gt;= argc) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; requires data\n&quot;, argv[0], argv[optind]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               errno = 0;<br>
-               data = strtoull(argv[optind + 2], &amp;endptr, 0);<br>
-               if (errno || *endptr != &#39;\0&#39;) {<br>
-                       printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse data &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-                       return -1;<br>
-               }<br>
-<br>
-               return for_each_target(&quot;pib&quot;, putscom, &amp;addr, &amp;data);<br>
-       }<br>
-<br>
-       return for_each_target(&quot;pib&quot;, getscom, &amp;addr, NULL);<br>
+       /* TODO: Restore the &lt;mask&gt; functionality */<br>
+       return for_each_target(&quot;pib&quot;, _putscom, &amp;addr, &amp;data);<br>
 }<br>
-<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(putscom, putscom, (ADDRESS, DATA, DEFAULT_DATA(&quot;0xffffffffffffffff&quot;)));<br>
diff --git a/src/scom.h b/src/scom.h<br>
deleted file mode 100644<br>
index d4325b5..0000000<br>
--- a/src/scom.h<br>
+++ /dev/null<br>
@@ -1,18 +0,0 @@<br>
-/* Copyright 2017 IBM Corp.<br>
- *<br>
- * Licensed under the Apache License, Version 2.0 (the &quot;License&quot;);<br>
- * you may not use this file except in compliance with the License.<br>
- * You may obtain a copy of the License at<br>
- *<br>
- *     <a href="http://www.apache.org/licenses/LICENSE-2.0" rel="noreferrer noreferrer" target="_blank">http://www.apache.org/licenses/LICENSE-2.0</a><br>
- *<br>
- * Unless required by applicable law or agreed to in writing, software<br>
- * distributed under the License is distributed on an &quot;AS IS&quot; BASIS,<br>
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or<br>
- * implied.<br>
- * See the License for the specific language governing permissions and<br>
- * limitations under the License.<br>
- */<br>
-#include &lt;inttypes.h&gt;<br>
-<br>
-int handle_scoms(int optind, int argc, char *argv[]);<br>
diff --git a/src/thread.c b/src/thread.c<br>
index e8b54cb..4b95636 100644<br>
--- a/src/thread.c<br>
+++ b/src/thread.c<br>
@@ -21,7 +21,7 @@<br>
 #include &lt;libpdbg.h&gt;<br>
<br>
 #include &quot;main.h&quot;<br>
-#include &quot;mem.h&quot;<br>
+#include &quot;optcmd.h&quot;<br>
<br>
 static int print_thread_status(struct pdbg_target *target, uint32_t index, uint64_t *arg, uint64_t *unused1)<br>
 {<br>
@@ -81,6 +81,57 @@ static int print_core_thread_status(struct pdbg_target *core_target, uint32_t in<br>
        return rc;<br>
 }<br>
<br>
+static bool is_real_address(struct thread_regs *regs, uint64_t addr)<br>
+{<br>
+       return true;<br>
+       if ((addr &amp; 0xf000000000000000ULL) == 0xc000000000000000ULL)<br>
+               return true;<br>
+       return false;<br>
+}<br>
+<br>
+static int load8(struct pdbg_target *target, uint64_t addr, uint64_t *value)<br>
+{<br>
+       if (adu_getmem(target, addr, (uint8_t *)value, 8)) {<br>
+               pdbg_log(PDBG_ERROR, &quot;Unable to read memory address=%016&quot; PRIx64 &quot;.\n&quot;, addr);<br>
+               return 0;<br>
+       }<br>
+<br>
+       return 1;<br>
+}<br>
+<br>
+static int dump_stack(struct thread_regs *regs)<br>
+{<br>
+       struct pdbg_target *target;<br>
+       uint64_t sp = regs-&gt;gprs[1];<br>
+       uint64_t pc;<br>
+<br>
+       pdbg_for_each_class_target(&quot;adu&quot;, target) {<br>
+               if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)<br>
+                       continue;<br>
+               break;<br>
+       }<br>
+<br>
+       printf(&quot;STACK:\n&quot;);<br>
+       if (!target)<br>
+               pdbg_log(PDBG_ERROR, &quot;Unable to read memory (no ADU found)\n&quot;);<br>
+<br>
+       if (sp &amp;&amp; is_real_address(regs, sp)) {<br>
+               if (!load8(target, sp, &amp;sp))<br>
+                       return 1;<br>
+               while (sp &amp;&amp; is_real_address(regs, sp)) {<br>
+                       if (!load8(target, sp + 16, &amp;pc))<br>
+                               return 1;<br>
+<br>
+                       printf(&quot; 0x%016&quot; PRIx64 &quot; 0x%16&quot; PRIx64 &quot;\n&quot;, sp, pc);<br>
+<br>
+                       if (!load8(target, sp, &amp;sp))<br>
+                               return 1;<br>
+               }<br>
+       }<br>
+<br>
+       return 0;<br>
+}<br>
+<br>
 static int get_thread_max_index(struct pdbg_target *target, uint32_t index, uint64_t *maxindex, uint64_t *unused)<br>
 {<br>
        if (index &gt; *maxindex)<br>
@@ -140,48 +191,37 @@ static int state_thread(struct pdbg_target *thread_target, uint32_t index, uint6<br>
        return 1;<br>
 }<br>
<br>
-int thread_start(int optind, int argc, char *argv[])<br>
+static int thread_start(void)<br>
 {<br>
        return for_each_target(&quot;thread&quot;, start_thread, NULL, NULL);<br>
 }<br>
+OPTCMD_DEFINE_CMD(start, thread_start);<br>
<br>
-int thread_step(int optind, int argc, char *argv[])<br>
+static int thread_step(uint64_t count)<br>
 {<br>
-       uint64_t count;<br>
-       char *endptr;<br>
-<br>
-       if (optind + 1 &gt;= argc) {<br>
-               printf(&quot;%s: command &#39;%s&#39; requires a count\n&quot;, argv[0], argv[optind]);<br>
-               return -1;<br>
-       }<br>
-<br>
-       errno = 0;<br>
-       count = strtoull(argv[optind + 1], &amp;endptr, 0);<br>
-       if (errno || *endptr != &#39;\0&#39;) {<br>
-               printf(&quot;%s: command &#39;%s&#39; couldn&#39;t parse count &#39;%s&#39;\n&quot;,<br>
-                               argv[0], argv[optind], argv[optind + 1]);<br>
-               return -1;<br>
-       }<br>
-<br>
        return for_each_target(&quot;thread&quot;, step_thread, &amp;count, NULL);<br>
 }<br>
+OPTCMD_DEFINE_CMD_WITH_ARGS(step, thread_step, (DATA));<br>
<br>
-int thread_stop(int optind, int argc, char *argv[])<br>
+static int thread_stop(void)<br>
 {<br>
        return for_each_target(&quot;thread&quot;, stop_thread, NULL, NULL);<br>
 }<br>
+OPTCMD_DEFINE_CMD(stop, thread_stop);<br>
<br>
-int thread_status_print(int optind, int argc, char *argv[])<br>
+static int thread_status_print(void)<br>
 {<br>
        return for_each_target(&quot;pib&quot;, print_proc_thread_status, NULL, NULL);<br>
 }<br>
+OPTCMD_DEFINE_CMD(threadstatus, thread_status_print);<br>
<br>
-int thread_sreset(int optind, int argc, char *argv[])<br>
+static int thread_sreset(void)<br>
 {<br>
        return for_each_target(&quot;thread&quot;, sreset_thread, NULL, NULL);<br>
 }<br>
+OPTCMD_DEFINE_CMD(sreset, thread_sreset);<br>
<br>
-int thread_state(int optind, int argc, char *argv[])<br>
+static int thread_state(void)<br>
 {<br>
        int err;<br>
<br>
@@ -191,3 +231,4 @@ int thread_state(int optind, int argc, char *argv[])<br>
<br>
        return err;<br>
 }<br>
+OPTCMD_DEFINE_CMD(regs, thread_state);<br>
diff --git a/src/thread.h b/src/thread.h<br>
deleted file mode 100644<br>
index 0c1caa2..0000000<br>
--- a/src/thread.h<br>
+++ /dev/null<br>
@@ -1,23 +0,0 @@<br>
-/* Copyright 2017 IBM Corp.<br>
- *<br>
- * Licensed under the Apache License, Version 2.0 (the &quot;License&quot;);<br>
- * you may not use this file except in compliance with the License.<br>
- * You may obtain a copy of the License at<br>
- *<br>
- *     <a href="http://www.apache.org/licenses/LICENSE-2.0" rel="noreferrer noreferrer" target="_blank">http://www.apache.org/licenses/LICENSE-2.0</a><br>
- *<br>
- * Unless required by applicable law or agreed to in writing, software<br>
- * distributed under the License is distributed on an &quot;AS IS&quot; BASIS,<br>
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or<br>
- * implied.<br>
- * See the License for the specific language governing permissions and<br>
- * limitations under the License.<br>
- */<br>
-#include &lt;inttypes.h&gt;<br>
-<br>
-int thread_start(int optind, int argc, char *argv[]);<br>
-int thread_step(int optind, int argc, char *argv[]);<br>
-int thread_stop(int optind, int argc, char *argv[]);<br>
-int thread_status_print(int optind, int argc, char *argv[]);<br>
-int thread_sreset(int optind, int argc, char *argv[]);<br>
-int thread_state(int optind, int argc, char *argv[]);<br>
-- <br>
2.11.0<br>
<br>
-- <br>
Pdbg mailing list<br>
<a href="mailto:Pdbg@lists.ozlabs.org" target="_blank" rel="noreferrer">Pdbg@lists.ozlabs.org</a><br>
<a href="https://lists.ozlabs.org/listinfo/pdbg" rel="noreferrer noreferrer" target="_blank">https://lists.ozlabs.org/listinfo/pdbg</a><br>
</blockquote></div></div></div>
diff mbox series

Patch

diff --git a/Makefile.am b/Makefile.am
index 23f2080..4e0dce2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -39,6 +39,7 @@  pdbg_SOURCES = \
 	src/ring.c \
 	src/htm.c \
 	src/progress.c \
+	src/parsers.c \
 	src/optcmd.c \
 	src/options_@ARCH@.c
 
diff --git a/src/cfam.c b/src/cfam.c
index 269123e..6dab388 100644
--- a/src/cfam.c
+++ b/src/cfam.c
@@ -20,8 +20,9 @@ 
 #include <inttypes.h>
 
 #include "main.h"
+#include "optcmd.h"
 
-static int getcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)
+static int _getcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)
 {
 	uint32_t value;
 
@@ -33,7 +34,15 @@  static int getcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, u
 	return 1;
 }
 
-static int putcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)
+static int getcfam(uint32_t addr)
+{
+	uint64_t addr64 = addr;
+
+	return for_each_target("fsi", _getcfam, &addr64, NULL);
+}
+OPTCMD_DEFINE_CMD_WITH_ARGS(getcfam, getcfam, (ADDRESS32));
+
+static int _putcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)
 {
 	if (fsi_write(target, *addr, *data))
 		return 0;
@@ -41,44 +50,10 @@  static int putcfam(struct pdbg_target *target, uint32_t index, uint64_t *addr, u
 	return 1;
 }
 
-int handle_cfams(int optind, int argc, char *argv[])
+static int putcfam(uint32_t addr, uint32_t data)
 {
-	uint64_t addr;
-	char *endptr;
-
-	if (optind + 1 >= argc) {
-		printf("%s: command '%s' requires an address\n", argv[0], argv[optind]);
-		return -1;
-	}
-
-	errno = 0;
-	addr = strtoull(argv[optind + 1], &endptr, 0);
-	if (errno || *endptr != '\0') {
-		printf("%s: command '%s' couldn't parse address '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-		return -1;
-	}
+	uint64_t addr64 = addr, data64 = data;
 
-	if (strcmp(argv[optind], "putcfam") == 0) {
-		uint64_t data;
-
-		if (optind + 2 >= argc) {
-			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
-			return -1;
-		}
-
-		errno = 0;
-		data = strtoull(argv[optind + 2], &endptr, 0);
-		if (errno || *endptr != '\0') {
-			printf("%s: command '%s' couldn't parse data '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-			return -1;
-		}
-
-		return for_each_target("fsi", putcfam, &addr, &data);
-	}
-
-	return for_each_target("fsi", getcfam, &addr, NULL);
+	return for_each_target("fsi", _putcfam, &addr64, &data64);
 }
-
-
+OPTCMD_DEFINE_CMD_WITH_ARGS(putcfam, putcfam, (ADDRESS32, DATA32));
diff --git a/src/cfam.h b/src/cfam.h
deleted file mode 100644
index 997ed3d..0000000
--- a/src/cfam.h
+++ /dev/null
@@ -1,18 +0,0 @@ 
-/* Copyright 2017 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * 	http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#include <inttypes.h>
-
-int handle_cfams(int optind, int argc, char *argv[]);
diff --git a/src/main.c b/src/main.c
index cf9a3b2..1cfaca7 100644
--- a/src/main.c
+++ b/src/main.c
@@ -34,14 +34,9 @@ 
 #include <target.h>
 
 #include "main.h"
-#include "cfam.h"
-#include "scom.h"
-#include "reg.h"
-#include "ring.h"
-#include "mem.h"
-#include "thread.h"
 #include "htm.h"
 #include "options.h"
+#include "optcmd.h"
 
 #define PR_ERROR(x, args...) \
 	pdbg_log(PDBG_ERROR, x, ##args)
@@ -77,44 +72,64 @@  static int **processorsel[MAX_PROCESSORS];
 static int *chipsel[MAX_PROCESSORS][MAX_CHIPS];
 static int threadsel[MAX_PROCESSORS][MAX_CHIPS][MAX_THREADS];
 
-static int handle_probe(int optind, int argc, char *argv[]);
-static int handle_release(int optind, int argc, char *argv[]);
+static int probe(void);
+static int release(void);
+
+/* TODO: We are repeating ourselves here. A little bit more macro magic could
+ * easily fix this but I was hesitant to introduce too much magic all at
+ * once. */
+extern struct optcmd_cmd
+	optcmd_getscom, optcmd_putscom,	optcmd_getcfam, optcmd_putcfam,
+	optcmd_getgpr, optcmd_putgpr, optcmd_getspr, optcmd_putspr,
+	optcmd_getnia, optcmd_putnia, optcmd_getmsr, optcmd_putmsr,
+	optcmd_getring, optcmd_start, optcmd_stop, optcmd_step,
+	optcmd_threadstatus, optcmd_sreset, optcmd_regs, optcmd_probe,
+	optcmd_getmem, optcmd_putmem;
+
+static struct optcmd_cmd *cmds[] = {
+	&optcmd_getscom, &optcmd_putscom, &optcmd_getcfam, &optcmd_putcfam,
+	&optcmd_getgpr, &optcmd_putgpr, &optcmd_getspr, &optcmd_putspr,
+	&optcmd_getnia, &optcmd_putnia, &optcmd_getmsr, &optcmd_putmsr,
+	&optcmd_getring, &optcmd_start, &optcmd_stop, &optcmd_step,
+	&optcmd_threadstatus, &optcmd_sreset, &optcmd_regs, &optcmd_probe,
+	&optcmd_getmem, &optcmd_putmem,
+};
 
+/* Purely for printing usage text. We could integrate printing argument and flag
+ * help into optcmd if desired. */
 struct action {
 	const char *name;
 	const char *args;
 	const char *desc;
-	int (*fn)(int, int, char **);
 };
 
 static struct action actions[] = {
-	{ "getgpr",  "<gpr>", "Read General Purpose Register (GPR)", &handle_gpr },
-	{ "putgpr",  "<gpr> <value>", "Write General Purpose Register (GPR)", &handle_gpr },
-	{ "getnia",  "", "Get Next Instruction Address (NIA)", &handle_nia },
-	{ "putnia",  "<value>", "Write Next Instrution Address (NIA)", &handle_nia },
-	{ "getspr",  "<spr>", "Get Special Purpose Register (SPR)", &handle_spr },
-	{ "putspr",  "<spr> <value>", "Write Special Purpose Register (SPR)", &handle_spr },
-	{ "getmsr",  "", "Get Machine State Register (MSR)", &handle_msr },
-	{ "putmsr",  "<value>", "Write Machine State Register (MSR)", &handle_msr },
-	{ "getring", "<addr> <len>", "Read a ring. Length must be correct", &handle_getring },
-	{ "start",   "", "Start thread", &thread_start },
-	{ "step",    "<count>", "Set a thread <count> instructions", &thread_step },
-	{ "stop",    "", "Stop thread", &thread_stop },
-	{ "htm", "core|nest start|stop|status|reset|dump|trace|analyse", "Hardware Trace Macro", &run_htm },
-	{ "release", "", "Should be called after pdbg work is finished, to release special wakeups and other resources.", &handle_release},
-	{ "probe", "", "", &handle_probe },
-	{ "getcfam", "<address>", "Read system cfam", &handle_cfams },
-	{ "putcfam", "<address> <value> [<mask>]", "Write system cfam", &handle_cfams },
-	{ "getscom", "<address>", "Read system scom", &handle_scoms },
-	{ "putscom", "<address> <value> [<mask>]", "Write system scom", &handle_scoms },
-	{ "getmem",  "<address> <count>", "Read system memory", &handle_mem },
-	{ "putmem",  "<address>", "Write to system memory", &handle_mem },
-	{ "threadstatus", "", "Print the status of a thread", &thread_status_print },
-	{ "sreset",  "", "Reset", &thread_sreset },
-	{ "regs",  "", "State", &thread_state },
+	{ "getgpr",  "<gpr>", "Read General Purpose Register (GPR)" },
+	{ "putgpr",  "<gpr> <value>", "Write General Purpose Register (GPR)" },
+	{ "getnia",  "", "Get Next Instruction Address (NIA)" },
+	{ "putnia",  "<value>", "Write Next Instrution Address (NIA)" },
+	{ "getspr",  "<spr>", "Get Special Purpose Register (SPR)" },
+	{ "putspr",  "<spr> <value>", "Write Special Purpose Register (SPR)" },
+	{ "getmsr",  "", "Get Machine State Register (MSR)" },
+	{ "putmsr",  "<value>", "Write Machine State Register (MSR)" },
+	{ "getring", "<addr> <len>", "Read a ring. Length must be correct" },
+	{ "start",   "", "Start thread" },
+	{ "step",    "<count>", "Set a thread <count> instructions" },
+	{ "stop",    "", "Stop thread" },
+	{ "htm", "core|nest start|stop|status|reset|dump|trace|analyse", "Hardware Trace Macro" },
+	{ "release", "", "Should be called after pdbg work is finished" },
+	{ "probe", "", "" },
+	{ "getcfam", "<address>", "Read system cfam" },
+	{ "putcfam", "<address> <value> [<mask>]", "Write system cfam" },
+	{ "getscom", "<address>", "Read system scom" },
+	{ "putscom", "<address> <value> [<mask>]", "Write system scom" },
+	{ "getmem",  "<address> <count>", "Read system memory" },
+	{ "putmem",  "<address>", "Write to system memory" },
+	{ "threadstatus", "", "Print the status of a thread" },
+	{ "sreset",  "", "Reset" },
+	{ "regs",  "", "State" },
 };
 
-
 static void print_usage(char *pname)
 {
 	int i;
@@ -600,7 +615,7 @@  static void release_target(struct pdbg_target *target)
 	pdbg_target_release(target);
 }
 
-static void do_release(void)
+static int release(void)
 {
 	struct pdbg_target_class *target_class;
 
@@ -610,7 +625,10 @@  static void do_release(void)
 		pdbg_for_each_class_target(target_class->name, target)
 			release_target(target);
 	}
+
+	return 0;
 }
+OPTCMD_DEFINE_CMD(release, release);
 
 void print_target(struct pdbg_target *target, int level)
 {
@@ -652,7 +670,7 @@  void print_target(struct pdbg_target *target, int level)
 	}
 }
 
-static int handle_probe(int optind, int argc, char *argv[])
+static int probe(void)
 {
 	struct pdbg_target *target;
 
@@ -664,25 +682,21 @@  static int handle_probe(int optind, int argc, char *argv[])
 
 	return 1;
 }
+OPTCMD_DEFINE_CMD(probe, probe);
 
 /*
  * Release handler.
  */
 static void atexit_release(void)
 {
-	do_release();
-}
-
-static int handle_release(int optind, int argc, char *argv[])
-{
-	do_release();
-
-	return 1;
+	release();
 }
 
 int main(int argc, char *argv[])
 {
 	int i, rc = 0;
+	void **args, **flags;
+	optcmd_cmd_t *cmd;
 
 	backend = default_backend();
 	device_node = default_target(backend);
@@ -714,13 +728,28 @@  int main(int argc, char *argv[])
 
 	atexit(atexit_release);
 
-	for (i = 0; i < ARRAY_SIZE(actions); i++) {
-		if (strcmp(argv[optind], actions[i].name) == 0) {
-			rc = actions[i].fn(optind, argc, argv);
-			goto found_action;
+	for (i = 0; i < ARRAY_SIZE(cmds); i++) {
+		if (!strcmp(argv[optind], cmds[i]->cmd)) {
+			/* Found our command */
+			cmd = optcmd_parse(cmds[i], (const char **) &argv[optind + 1],
+					   argc - (optind + 1), &args, &flags);
+			if (cmd) {
+				rc = cmd(args, flags);
+				goto found_action;
+			} else {
+				/* Error parsing arguments so exit return directly */
+				return 1;
+			}
 		}
 	}
 
+	/* Process subcommands. Currently only 'htm'.
+	 * TODO: Move htm command parsing to optcmd once htm clean-up is complete */
+	if (!strcmp(argv[optind], "htm")) {
+		run_htm(optind, argc, argv);
+		goto found_action;
+	}
+
 	PR_ERROR("Unsupported command: %s\n", argv[optind]);
 	return 1;
 
diff --git a/src/mem.c b/src/mem.c
index e0327d1..29fd21e 100644
--- a/src/mem.c
+++ b/src/mem.c
@@ -20,21 +20,34 @@ 
 #include <string.h>
 #include <unistd.h>
 #include <assert.h>
+#include <stdbool.h>
 
 #include <libpdbg.h>
 
 #include "main.h"
 #include "progress.h"
+#include "optcmd.h"
+#include "parsers.h"
 
 #define PR_ERROR(x, args...) \
 	pdbg_log(PDBG_ERROR, x, ##args)
 
 #define PUTMEM_BUF_SIZE 1024
-static int getmem(uint64_t addr, uint64_t size, bool ci)
+
+struct mem_flags {
+	bool ci;
+};
+
+#define MEM_CI_FLAG ("--ci", ci, parse_flag_noarg, false)
+
+static int getmem(uint64_t addr, uint64_t size, struct mem_flags flags)
 {
 	struct pdbg_target *target;
 	uint8_t *buf;
 	int rc = 0;
+
+	printf("getmem ci %d\n", flags.ci);
+
 	buf = malloc(size);
 	assert(buf);
 	pdbg_for_each_class_target("adu", target) {
@@ -43,7 +56,7 @@  static int getmem(uint64_t addr, uint64_t size, bool ci)
 
 		pdbg_set_progress_tick(progress_tick);
 		progress_init();
-		if (!__adu_getmem(target, addr, buf, size, ci)) {
+		if (!__adu_getmem(target, addr, buf, size, flags.ci)) {
 			if (write(STDOUT_FILENO, buf, size) < 0)
 				PR_ERROR("Unable to write stdout.\n");
 			else
@@ -58,7 +71,10 @@  static int getmem(uint64_t addr, uint64_t size, bool ci)
 	return rc;
 
 }
-static int putmem(uint64_t addr, bool ci)
+OPTCMD_DEFINE_CMD_WITH_FLAGS(getmem, getmem, (ADDRESS, DATA),
+			     mem_flags, (MEM_CI_FLAG));
+
+static int putmem(uint64_t addr, struct mem_flags flags)
 {
 	uint8_t *buf;
 	int read_size, rc = 0;
@@ -76,7 +92,7 @@  static int putmem(uint64_t addr, bool ci)
 	progress_init();
 	do {
 		read_size = read(STDIN_FILENO, buf, PUTMEM_BUF_SIZE);
-		if (__adu_putmem(adu_target, addr, buf, read_size, ci)) {
+		if (__adu_putmem(adu_target, addr, buf, read_size, flags.ci)) {
 			rc = 0;
 			printf("Unable to write memory.\n");
 			break;
@@ -89,101 +105,5 @@  static int putmem(uint64_t addr, bool ci)
 	free(buf);
 	return rc;
 }
-
-static bool is_real_address(struct thread_regs *regs, uint64_t addr)
-{
-	return true;
-	if ((addr & 0xf000000000000000ULL) == 0xc000000000000000ULL)
-		return true;
-	return false;
-}
-
-static int load8(struct pdbg_target *target, uint64_t addr, uint64_t *value)
-{
-	if (adu_getmem(target, addr, (uint8_t *)value, 8)) {
-		PR_ERROR("Unable to read memory address=%016" PRIx64 ".\n", addr);
-		return 0;
-	}
-
-	return 1;
-}
-
-int dump_stack(struct thread_regs *regs)
-{
-	struct pdbg_target *target;
-	uint64_t sp = regs->gprs[1];
-	uint64_t pc;
-
-	pdbg_for_each_class_target("adu", target) {
-		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
-			continue;
-		break;
-	}
-
-	printf("STACK:\n");
-	if (!target)
-		PR_ERROR("Unable to read memory (no ADU found)\n");
-
-	if (sp && is_real_address(regs, sp)) {
-		if (!load8(target, sp, &sp))
-			return 1;
-		while (sp && is_real_address(regs, sp)) {
-			if (!load8(target, sp + 16, &pc))
-				return 1;
-
-			printf(" 0x%016" PRIx64 " 0x%16" PRIx64 "\n", sp, pc);
-
-			if (!load8(target, sp, &sp))
-				return 1;
-		}
-	}
-
-	return 0;
-}
-
-int handle_mem(int optind, int argc, char *argv[])
-{
-	uint64_t addr;
-	char *endptr;
-	bool ci = false;
-
-	if (optind + 1 >= argc) {
-		printf("%s: command '%s' requires an address\n", argv[0], argv[optind]);
-		return -1;
-	}
-
-	errno = 0;
-
-	if (strcmp(argv[optind +1], "-ci") == 0) {
-		/* Set cache-inhibited flag */
-		ci = true;
-	}
-
-	addr = strtoull(argv[optind + 1 + ci], &endptr, 0);
-	if (errno || *endptr != '\0') {
-		printf("%s: command '%s' couldn't parse address '%s'\n",
-				argv[0], argv[optind], argv[optind + 1 + ci]);
-		return -1;
-	}
-
-	if (strcmp(argv[optind], "getmem") == 0) {
-		uint64_t size;
-
-		if (optind + 2 + ci >= argc) {
-			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
-			return -1;
-		}
-
-		errno = 0;
-		size = strtoull(argv[optind + 2 + ci], &endptr, 0);
-		if (errno || *endptr != '\0') {
-			printf("%s: command '%s' couldn't parse data '%s'\n",
-				argv[0], argv[optind], argv[optind + 1 + ci]);
-			return -1;
-		}
-
-		return getmem(addr, size, ci);
-	}
-
-	return putmem(addr, ci);
-}
+OPTCMD_DEFINE_CMD_WITH_FLAGS(putmem, putmem, (ADDRESS),
+			     mem_flags, (MEM_CI_FLAG));
diff --git a/src/mem.h b/src/mem.h
deleted file mode 100644
index 42bdc04..0000000
--- a/src/mem.h
+++ /dev/null
@@ -1,20 +0,0 @@ 
-/* Copyright 2017 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * 	http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#include <inttypes.h>
-#include <libpdbg.h>
-
-int dump_stack(struct thread_regs *regs);
-int handle_mem(int optind, int argc, char *argv[]);
diff --git a/src/reg.c b/src/reg.c
index 21bec13..aa77a8a 100644
--- a/src/reg.c
+++ b/src/reg.c
@@ -22,6 +22,7 @@ 
 #include <libpdbg.h>
 
 #include "main.h"
+#include "optcmd.h"
 
 #define REG_MEM -3
 #define REG_MSR -2
@@ -91,143 +92,58 @@  static int getprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg,
 	return !rc;
 }
 
-int handle_gpr(int optind, int argc, char *argv[])
+static int getgpr(int gpr)
 {
-	char *endptr;
-	uint64_t gpr;
-
-	if (optind + 1 >= argc) {
-		printf("%s: command '%s' requires a GPR\n", argv[0], argv[optind]);
-		return -1;
-	}
-
-	errno = 0;
-	gpr = strtoull(argv[optind + 1], &endptr, 0);
-	if (errno || *endptr != '\0') {
-		printf("%s: command '%s' couldn't parse GPR '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-		return -1;
-	}
-
-	if (gpr > 31) {
-		printf("A GPR must be between zero and 31 inclusive\n");
-		return -1;
-	}
-
-	if (strcmp(argv[optind], "putgpr") == 0) {
-		uint64_t data;
-
-		if (optind + 2 >= argc) {
-			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
-			return -1;
-		}
-
-		errno = 0;
-		data = strtoull(argv[optind + 2], &endptr, 0);
-		if (errno || *endptr != '\0') {
-			printf("%s: command '%s' couldn't parse data '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-			return -1;
-		}
-
-		return for_each_target("thread", putprocreg, &gpr, &data);
-	}
-
-	return for_each_target("thread", getprocreg, &gpr, NULL);
+	uint64_t reg = gpr;
+	return for_each_target("thread", getprocreg, &reg, NULL);
 }
+OPTCMD_DEFINE_CMD_WITH_ARGS(getgpr, getgpr, (GPR));
 
-int handle_nia(int optind, int argc, char *argv[])
+static int putgpr(int gpr, uint64_t data)
 {
-	uint64_t reg = REG_NIA;
-	char *endptr;
-
-	if (strcmp(argv[optind], "putnia") == 0) {
-		uint64_t data;
-
-		if (optind + 1 >= argc) {
-			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
-			return -1;
-		}
-
-		errno = 0;
-		data = strtoull(argv[optind + 1], &endptr, 0);
-		if (errno || *endptr != '\0') {
-			printf("%s: command '%s' couldn't parse data '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-			return -1;
-		}
-
-		return for_each_target("thread", putprocreg, &reg, &data);
-	}
+	uint64_t reg = gpr;
+	return for_each_target("thread", putprocreg, &reg, &data);
+}
+OPTCMD_DEFINE_CMD_WITH_ARGS(putgpr, putgpr, (GPR, DATA));
 
+static int getnia(void)
+{
+	uint64_t reg = REG_NIA;
 	return for_each_target("thread", getprocreg, &reg, NULL);
 }
+OPTCMD_DEFINE_CMD(getnia, getnia);
 
-int handle_spr(int optind, int argc, char *argv[])
+static int putnia(uint64_t nia)
 {
-	char *endptr;
-	uint64_t spr;
-
-	if (optind + 1 >= argc) {
-		printf("%s: command '%s' requires a GPR\n", argv[0], argv[optind]);
-		return -1;
-	}
-
-	errno = 0;
-	spr = strtoull(argv[optind + 1], &endptr, 0);
-	if (errno || *endptr != '\0') {
-		printf("%s: command '%s' couldn't parse GPR '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-		return -1;
-	}
-
-	spr += REG_R31;
-
-	if (strcmp(argv[optind], "putspr") == 0) {
-		uint64_t data;
-
-		if (optind + 2 >= argc) {
-			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
-			return -1;
-		}
-
-		errno = 0;
-		data = strtoull(argv[optind + 2], &endptr, 0);
-		if (errno || *endptr != '\0') {
-			printf("%s: command '%s' couldn't parse data '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-			return -1;
-		}
-
-		return for_each_target("thread", putprocreg, &spr, &data);
-	}
-
-	return for_each_target("thread", getprocreg, &spr, NULL);
+	uint64_t reg = REG_NIA;
+	return for_each_target("thread", getprocreg, &reg, &nia);
 }
+OPTCMD_DEFINE_CMD_WITH_ARGS(putnia, putnia, (DATA));
 
-int handle_msr(int optind, int argc, char *argv[])
+static int getspr(int spr)
 {
-	uint64_t msr = REG_MSR;
-	char *endptr;
-
-	if (strcmp(argv[optind], "putmsr") == 0) {
-		uint64_t data;
-
-		if (optind + 1 >= argc) {
-			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
-			return -1;
-		}
+	uint64_t reg = spr + REG_R31;
+	return for_each_target("thread", getprocreg, &reg, NULL);
+}
+OPTCMD_DEFINE_CMD_WITH_ARGS(getspr, getspr, (SPR));
 
-		errno = 0;
-		data = strtoull(argv[optind + 1], &endptr, 0);
-		if (errno || *endptr != '\0') {
-			printf("%s: command '%s' couldn't parse data '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-			return -1;
-		}
+static int putspr(int spr, uint64_t data)
+{
+	uint64_t reg = spr + REG_R31;
+	return for_each_target("thread", putprocreg, &reg, &data);
+}
+OPTCMD_DEFINE_CMD_WITH_ARGS(putspr, putspr, (SPR, DATA));
 
-		return for_each_target("thread", putprocreg, &msr, &data);
-	}
+static int getmsr(void)
+{
+	uint64_t reg = REG_MSR;
+	return for_each_target("thread", getprocreg, &reg, NULL);
+}
+OPTCMD_DEFINE_CMD(getmsr, getmsr);
 
-	return for_each_target("thread", getprocreg, &msr, NULL);
+static int putmsr(uint64_t data)
+{
+	uint64_t reg = REG_MSR;
+	return for_each_target("thread", putprocreg, &reg, &data);
 }
+OPTCMD_DEFINE_CMD_WITH_ARGS(putmsr, putmsr, (DATA));
diff --git a/src/reg.h b/src/reg.h
deleted file mode 100644
index ad41d9d..0000000
--- a/src/reg.h
+++ /dev/null
@@ -1,20 +0,0 @@ 
-/* Copyright 2017 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * 	http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-int handle_gpr(int optind, int argc, char *argv[]);
-int handle_nia(int optind, int argc, char *argv[]);
-int handle_spr(int optind, int argc, char *argv[]);
-int handle_msr(int optind, int argc, char *argv[]);
diff --git a/src/ring.c b/src/ring.c
index b0c9376..58df4d1 100644
--- a/src/ring.c
+++ b/src/ring.c
@@ -18,11 +18,12 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <assert.h>
 
-#include <target.h>
-#include <operations.h>
+#include <libpdbg.h>
 
 #include "main.h"
+#include "optcmd.h"
 
 static int pdbg_getring(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *len)
 {
@@ -51,31 +52,8 @@  static int pdbg_getring(struct pdbg_target *target, uint32_t index, uint64_t *ad
 	return 1;
 }
 
-int handle_getring(int optind, int argc, char *argv[])
+static int _getring(uint64_t ring_addr, uint64_t ring_len)
 {
-	uint64_t ring_addr, ring_len;
-	char *endptr;
-
-	if (optind + 2 >= argc) {
-		printf("%s: command '%s' requires two arguments (address and length)\n",
-		       argv[0], argv[optind]);
-		return -1;
-	}
-
-	errno = 0;
-	ring_addr = strtoull(argv[optind + 1], &endptr, 0);
-	if (errno || *endptr != '\0') {
-		printf("%s: command '%s' couldn't parse ring address '%s'\n",
-		       argv[0], argv[optind], argv[optind + 1]);
-		return -1;
-	}
-
-	ring_len = strtoull(argv[optind + 2], &endptr, 0);
-	if (errno || *endptr != '\0') {
-		printf("%s: command '%s' couldn't parse ring length '%s'\n",
-		       argv[0], argv[optind], argv[optind + 2]);
-		return -1;
-	}
-
 	return for_each_target("chiplet", pdbg_getring, &ring_addr, &ring_len);
 }
+OPTCMD_DEFINE_CMD_WITH_ARGS(getring, _getring, (ADDRESS, DATA));
diff --git a/src/ring.h b/src/ring.h
deleted file mode 100644
index a72c875..0000000
--- a/src/ring.h
+++ /dev/null
@@ -1,17 +0,0 @@ 
-/* Copyright 2018 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-int handle_getring(int optind, int argc, char *argv[]);
diff --git a/src/scom.c b/src/scom.c
index 4c59e2a..2372e91 100644
--- a/src/scom.c
+++ b/src/scom.c
@@ -22,8 +22,9 @@ 
 #include <libpdbg.h>
 
 #include "main.h"
+#include "optcmd.h"
 
-static int getscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)
+static int _getscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *unused)
 {
 	uint64_t value;
 
@@ -35,7 +36,13 @@  static int getscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, u
 	return 1;
 }
 
-static int putscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)
+ int getscom(uint64_t addr)
+{
+	return for_each_target("pib", _getscom, &addr, NULL);
+}
+OPTCMD_DEFINE_CMD_WITH_ARGS(getscom, getscom, (ADDRESS));
+
+static int _putscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, uint64_t *data)
 {
 	if (pib_write(target, *addr, *data))
 		return 0;
@@ -43,44 +50,9 @@  static int putscom(struct pdbg_target *target, uint32_t index, uint64_t *addr, u
 	return 1;
 }
 
-
-int handle_scoms(int optind, int argc, char *argv[])
+ int putscom(uint64_t addr, uint64_t data, uint64_t mask)
 {
-	uint64_t addr;
-	char *endptr;
-
-	if (optind + 1 >= argc) {
-		printf("%s: command '%s' requires an address\n", argv[0], argv[optind]);
-		return -1;
-	}
-
-	errno = 0;
-	addr = strtoull(argv[optind + 1], &endptr, 0);
-	if (errno || *endptr != '\0') {
-		printf("%s: command '%s' couldn't parse address '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-		return -1;
-	}
-
-	if (strcmp(argv[optind], "putscom") == 0) {
-		uint64_t data;
-
-		if (optind + 2 >= argc) {
-			printf("%s: command '%s' requires data\n", argv[0], argv[optind]);
-			return -1;
-		}
-
-		errno = 0;
-		data = strtoull(argv[optind + 2], &endptr, 0);
-		if (errno || *endptr != '\0') {
-			printf("%s: command '%s' couldn't parse data '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-			return -1;
-		}
-
-		return for_each_target("pib", putscom, &addr, &data);
-	}
-
-	return for_each_target("pib", getscom, &addr, NULL);
+	/* TODO: Restore the <mask> functionality */
+	return for_each_target("pib", _putscom, &addr, &data);
 }
-
+OPTCMD_DEFINE_CMD_WITH_ARGS(putscom, putscom, (ADDRESS, DATA, DEFAULT_DATA("0xffffffffffffffff")));
diff --git a/src/scom.h b/src/scom.h
deleted file mode 100644
index d4325b5..0000000
--- a/src/scom.h
+++ /dev/null
@@ -1,18 +0,0 @@ 
-/* Copyright 2017 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * 	http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#include <inttypes.h>
-
-int handle_scoms(int optind, int argc, char *argv[]);
diff --git a/src/thread.c b/src/thread.c
index e8b54cb..4b95636 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -21,7 +21,7 @@ 
 #include <libpdbg.h>
 
 #include "main.h"
-#include "mem.h"
+#include "optcmd.h"
 
 static int print_thread_status(struct pdbg_target *target, uint32_t index, uint64_t *arg, uint64_t *unused1)
 {
@@ -81,6 +81,57 @@  static int print_core_thread_status(struct pdbg_target *core_target, uint32_t in
 	return rc;
 }
 
+static bool is_real_address(struct thread_regs *regs, uint64_t addr)
+{
+	return true;
+	if ((addr & 0xf000000000000000ULL) == 0xc000000000000000ULL)
+		return true;
+	return false;
+}
+
+static int load8(struct pdbg_target *target, uint64_t addr, uint64_t *value)
+{
+	if (adu_getmem(target, addr, (uint8_t *)value, 8)) {
+		pdbg_log(PDBG_ERROR, "Unable to read memory address=%016" PRIx64 ".\n", addr);
+		return 0;
+	}
+
+	return 1;
+}
+
+static int dump_stack(struct thread_regs *regs)
+{
+	struct pdbg_target *target;
+	uint64_t sp = regs->gprs[1];
+	uint64_t pc;
+
+	pdbg_for_each_class_target("adu", target) {
+		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
+			continue;
+		break;
+	}
+
+	printf("STACK:\n");
+	if (!target)
+		pdbg_log(PDBG_ERROR, "Unable to read memory (no ADU found)\n");
+
+	if (sp && is_real_address(regs, sp)) {
+		if (!load8(target, sp, &sp))
+			return 1;
+		while (sp && is_real_address(regs, sp)) {
+			if (!load8(target, sp + 16, &pc))
+				return 1;
+
+			printf(" 0x%016" PRIx64 " 0x%16" PRIx64 "\n", sp, pc);
+
+			if (!load8(target, sp, &sp))
+				return 1;
+		}
+	}
+
+	return 0;
+}
+
 static int get_thread_max_index(struct pdbg_target *target, uint32_t index, uint64_t *maxindex, uint64_t *unused)
 {
 	if (index > *maxindex)
@@ -140,48 +191,37 @@  static int state_thread(struct pdbg_target *thread_target, uint32_t index, uint6
 	return 1;
 }
 
-int thread_start(int optind, int argc, char *argv[])
+static int thread_start(void)
 {
 	return for_each_target("thread", start_thread, NULL, NULL);
 }
+OPTCMD_DEFINE_CMD(start, thread_start);
 
-int thread_step(int optind, int argc, char *argv[])
+static int thread_step(uint64_t count)
 {
-	uint64_t count;
-	char *endptr;
-
-	if (optind + 1 >= argc) {
-		printf("%s: command '%s' requires a count\n", argv[0], argv[optind]);
-		return -1;
-	}
-
-	errno = 0;
-	count = strtoull(argv[optind + 1], &endptr, 0);
-	if (errno || *endptr != '\0') {
-		printf("%s: command '%s' couldn't parse count '%s'\n",
-				argv[0], argv[optind], argv[optind + 1]);
-		return -1;
-	}
-
 	return for_each_target("thread", step_thread, &count, NULL);
 }
+OPTCMD_DEFINE_CMD_WITH_ARGS(step, thread_step, (DATA));
 
-int thread_stop(int optind, int argc, char *argv[])
+static int thread_stop(void)
 {
 	return for_each_target("thread", stop_thread, NULL, NULL);
 }
+OPTCMD_DEFINE_CMD(stop, thread_stop);
 
-int thread_status_print(int optind, int argc, char *argv[])
+static int thread_status_print(void)
 {
 	return for_each_target("pib", print_proc_thread_status, NULL, NULL);
 }
+OPTCMD_DEFINE_CMD(threadstatus, thread_status_print);
 
-int thread_sreset(int optind, int argc, char *argv[])
+static int thread_sreset(void)
 {
 	return for_each_target("thread", sreset_thread, NULL, NULL);
 }
+OPTCMD_DEFINE_CMD(sreset, thread_sreset);
 
-int thread_state(int optind, int argc, char *argv[])
+static int thread_state(void)
 {
 	int err;
 
@@ -191,3 +231,4 @@  int thread_state(int optind, int argc, char *argv[])
 
 	return err;
 }
+OPTCMD_DEFINE_CMD(regs, thread_state);
diff --git a/src/thread.h b/src/thread.h
deleted file mode 100644
index 0c1caa2..0000000
--- a/src/thread.h
+++ /dev/null
@@ -1,23 +0,0 @@ 
-/* Copyright 2017 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * 	http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#include <inttypes.h>
-
-int thread_start(int optind, int argc, char *argv[]);
-int thread_step(int optind, int argc, char *argv[]);
-int thread_stop(int optind, int argc, char *argv[]);
-int thread_status_print(int optind, int argc, char *argv[]);
-int thread_sreset(int optind, int argc, char *argv[]);
-int thread_state(int optind, int argc, char *argv[]);