| Message ID | 20180531052915.31171-3-rashmica.g@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [01/10] libpdbg: Print the name of the instruction when erroring | expand |
On Thursday, 31 May 2018 3:29:08 PM AEST Rashmica Gupta wrote: > diff --git a/src/reg.c b/src/reg.c > index 002cfe9..416236b 100644 > --- a/src/reg.c > +++ b/src/reg.c > @@ -18,11 +18,13 @@ > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > +#include <debug.h> > > #include <libpdbg.h> > > #include "main.h" > > +#define REG_XER -4 > #define REG_MEM -3 > #define REG_MSR -2 > #define REG_NIA -1 > @@ -41,6 +43,8 @@ static void print_proc_reg(struct pdbg_target *target, uint64_t reg, uint64_t va > printf("msr: "); > else if (reg == REG_NIA) > printf("nia: "); > + else if (reg == REG_XER) > + printf("xer: "); > else if (reg > REG_R31) > printf("spr%03" PRIu64 ": ", reg - REG_R31); > else if (reg >= 0 && reg <= 31) > @@ -62,6 +66,8 @@ static int putprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg, > rc = ram_putmsr(target, *value); > else if (*reg == REG_NIA) > rc = ram_putnia(target, *value); > + else if (*reg == REG_XER) > + rc = ram_putxer(target, *value); > else if (*reg > REG_R31) > rc = ram_putspr(target, *reg - REG_R31, *value); > else if (*reg >= 0 && *reg <= 31) > @@ -81,6 +87,8 @@ static int getprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg, > rc = ram_getmsr(target, &value); > else if (*reg == REG_NIA) > rc = ram_getnia(target, &value); > + else if (*reg == REG_XER) > + rc = ram_getxer(target, &value); > else if (*reg > REG_R31) > rc = ram_getspr(target, *reg - REG_R31, &value); > else if (*reg >= 0 && *reg <= 31) Most of the above exists for like it does for weird historical reasons, so if you can see ways of refactoring it feel free to! > @@ -231,3 +239,32 @@ int handle_msr(int optind, int argc, char *argv[]) > > return for_each_target("thread", getprocreg, &msr, NULL); > } > + > +int handle_xer(int optind, int argc, char *argv[]) > +{ > + uint64_t xer = REG_XER; > + char *endptr; > + > + if (strcmp(argv[optind], "putxer") == 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; > + } > + > + PR_WARNING("We can only set part of the XER register.\n"); Why? And which part of the XER register? It might be nice if we specified to the user which bits we can get/set to avoid confusion. Otherwise everything else looks pretty good! Thanks. - Alistair > + return for_each_target("thread", putprocreg, &xer, &data); > + } > + > + PR_WARNING("We can only get part of the XER register.\n"); > + return for_each_target("thread", getprocreg, &xer, NULL); > +} > diff --git a/src/reg.h b/src/reg.h > index ad41d9d..ca548a6 100644 > --- a/src/reg.h > +++ b/src/reg.h > @@ -18,3 +18,4 @@ 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[]); > +int handle_xer(int optind, int argc, char *argv[]); >
On 04/06/18 16:26, Alistair Popple wrote: > On Thursday, 31 May 2018 3:29:08 PM AEST Rashmica Gupta wrote: >> diff --git a/src/reg.c b/src/reg.c >> index 002cfe9..416236b 100644 >> --- a/src/reg.c >> +++ b/src/reg.c >> @@ -18,11 +18,13 @@ >> #include <stdio.h> >> #include <stdlib.h> >> #include <string.h> >> +#include <debug.h> >> >> #include <libpdbg.h> >> >> #include "main.h" >> >> +#define REG_XER -4 >> #define REG_MEM -3 >> #define REG_MSR -2 >> #define REG_NIA -1 >> @@ -41,6 +43,8 @@ static void print_proc_reg(struct pdbg_target *target, uint64_t reg, uint64_t va >> printf("msr: "); >> else if (reg == REG_NIA) >> printf("nia: "); >> + else if (reg == REG_XER) >> + printf("xer: "); >> else if (reg > REG_R31) >> printf("spr%03" PRIu64 ": ", reg - REG_R31); >> else if (reg >= 0 && reg <= 31) >> @@ -62,6 +66,8 @@ static int putprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg, >> rc = ram_putmsr(target, *value); >> else if (*reg == REG_NIA) >> rc = ram_putnia(target, *value); >> + else if (*reg == REG_XER) >> + rc = ram_putxer(target, *value); >> else if (*reg > REG_R31) >> rc = ram_putspr(target, *reg - REG_R31, *value); >> else if (*reg >= 0 && *reg <= 31) >> @@ -81,6 +87,8 @@ static int getprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg, >> rc = ram_getmsr(target, &value); >> else if (*reg == REG_NIA) >> rc = ram_getnia(target, &value); >> + else if (*reg == REG_XER) >> + rc = ram_getxer(target, &value); >> else if (*reg > REG_R31) >> rc = ram_getspr(target, *reg - REG_R31, &value); >> else if (*reg >= 0 && *reg <= 31) > Most of the above exists for like it does for weird historical reasons, so if > you can see ways of refactoring it feel free to! > >> @@ -231,3 +239,32 @@ int handle_msr(int optind, int argc, char *argv[]) >> >> return for_each_target("thread", getprocreg, &msr, NULL); >> } >> + >> +int handle_xer(int optind, int argc, char *argv[]) >> +{ >> + uint64_t xer = REG_XER; >> + char *endptr; >> + >> + if (strcmp(argv[optind], "putxer") == 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; >> + } >> + >> + PR_WARNING("We can only set part of the XER register.\n"); > Why? And which part of the XER register? It might be nice if we specified to the > user which bits we can get/set to avoid confusion. The bits we can set are different for p8 and p9 when in ramming mode. On p8 machines we can't seem to get or set the whole XER register with mtxer/mfxer/mtspr etc. We can only access four fields of the register with mtxerf/mfxerf. However these fields only cover (IBM) bits 32-39 and 41-43, and only the two bits 33 and 34 seem to be set-able. These are the only two bits across the four fields that are specified in the ISA. On p9 we can use mtxer or mtspr, however only (IBM) bits 32-34 and 44-63 were writable on the machine I used. These are all the bits specified in the ISA plus some 'reserved' ones. Once I put the chip specific get/put xer behaviour into pxchip.c I could put more specific warnings there. However I'm not sure I'm comfortable definitively stating that the above bits are the only ones we can set based off my limited tests and knowledge... > > Otherwise everything else looks pretty good! Thanks. > > - Alistair > >> + return for_each_target("thread", putprocreg, &xer, &data); >> + } >> + >> + PR_WARNING("We can only get part of the XER register.\n"); >> + return for_each_target("thread", getprocreg, &xer, NULL); >> +} >> diff --git a/src/reg.h b/src/reg.h >> index ad41d9d..ca548a6 100644 >> --- a/src/reg.h >> +++ b/src/reg.h >> @@ -18,3 +18,4 @@ 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[]); >> +int handle_xer(int optind, int argc, char *argv[]); >> >
diff --git a/src/main.c b/src/main.c index 07c3677..90fb729 100644 --- a/src/main.c +++ b/src/main.c @@ -96,6 +96,8 @@ static struct action expert_actions[] = { { "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 }, + { "getxer", "", "Get Fixed Point Exception Register (XER)", &handle_xer }, + { "putxer", "<value>", "Write Fixed Point Exception Register (XER)", &handle_xer }, { "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 }, diff --git a/src/reg.c b/src/reg.c index 002cfe9..416236b 100644 --- a/src/reg.c +++ b/src/reg.c @@ -18,11 +18,13 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <debug.h> #include <libpdbg.h> #include "main.h" +#define REG_XER -4 #define REG_MEM -3 #define REG_MSR -2 #define REG_NIA -1 @@ -41,6 +43,8 @@ static void print_proc_reg(struct pdbg_target *target, uint64_t reg, uint64_t va printf("msr: "); else if (reg == REG_NIA) printf("nia: "); + else if (reg == REG_XER) + printf("xer: "); else if (reg > REG_R31) printf("spr%03" PRIu64 ": ", reg - REG_R31); else if (reg >= 0 && reg <= 31) @@ -62,6 +66,8 @@ static int putprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg, rc = ram_putmsr(target, *value); else if (*reg == REG_NIA) rc = ram_putnia(target, *value); + else if (*reg == REG_XER) + rc = ram_putxer(target, *value); else if (*reg > REG_R31) rc = ram_putspr(target, *reg - REG_R31, *value); else if (*reg >= 0 && *reg <= 31) @@ -81,6 +87,8 @@ static int getprocreg(struct pdbg_target *target, uint32_t index, uint64_t *reg, rc = ram_getmsr(target, &value); else if (*reg == REG_NIA) rc = ram_getnia(target, &value); + else if (*reg == REG_XER) + rc = ram_getxer(target, &value); else if (*reg > REG_R31) rc = ram_getspr(target, *reg - REG_R31, &value); else if (*reg >= 0 && *reg <= 31) @@ -231,3 +239,32 @@ int handle_msr(int optind, int argc, char *argv[]) return for_each_target("thread", getprocreg, &msr, NULL); } + +int handle_xer(int optind, int argc, char *argv[]) +{ + uint64_t xer = REG_XER; + char *endptr; + + if (strcmp(argv[optind], "putxer") == 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; + } + + PR_WARNING("We can only set part of the XER register.\n"); + return for_each_target("thread", putprocreg, &xer, &data); + } + + PR_WARNING("We can only get part of the XER register.\n"); + return for_each_target("thread", getprocreg, &xer, NULL); +} diff --git a/src/reg.h b/src/reg.h index ad41d9d..ca548a6 100644 --- a/src/reg.h +++ b/src/reg.h @@ -18,3 +18,4 @@ 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[]); +int handle_xer(int optind, int argc, char *argv[]);
Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com> --- src/main.c | 2 ++ src/reg.c | 37 +++++++++++++++++++++++++++++++++++++ src/reg.h | 1 + 3 files changed, 40 insertions(+)