diff mbox series

[2/8] libpdbg: Add in getxer and putxer functions

Message ID 20180622045116.12059-3-rashmica.g@gmail.com
State Superseded
Headers show
Series Pre gdbserver additions | expand

Commit Message

Rashmica Gupta June 22, 2018, 4:51 a.m. UTC
Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
 libpdbg/chip.c       | 93 +++++++++++++++++++++++++++++++++++++++++++++++++---
 libpdbg/libpdbg.h    |  4 +++
 libpdbg/operations.h | 11 ++++++-
 libpdbg/p8chip.c     | 41 +++++++++++++++++++++++
 libpdbg/p9chip.c     | 32 ++++++++++++++++--
 libpdbg/target.h     |  2 ++
 6 files changed, 175 insertions(+), 8 deletions(-)

Comments

Alistair Popple June 25, 2018, 6:24 a.m. UTC | #1
Thanks Rashmica, couple of questions/comments below...

>
> +int ram_getxer_field(struct pdbg_target *thread, uint32_t *value, uint32_t field)
> +{
> +	uint64_t opcodes[] = {mfxerf(0, field), mtspr(277, 0)};
> +	uint64_t results[] = {0, 0};
> +
> +	CHECK_ERR(ram_instructions(thread, opcodes, results, ARRAY_SIZE(opcodes), 0));

What is the distinction between getxer_field and getxer? You seem to handle
getxer specially below which implies it is different between P8 vs. P9 yet the
getxer_field case is handled the same for both P8 and P9. Would just using
multiple mfxerf()'s for getxer introduce any limitations?

If it doesn't it might be preferable to just use that for getxer (like you do
for P8 anyway) rather than maintaining a special case for P9. Same would
obviously apply for putxer.

> +
> +	*value = results[1];
> +	return 0;
> +}
> +
> +int ram_getxer(struct pdbg_target *thread_target, uint32_t *value)
> +{
> +
> +	struct thread *thread;
> +
> +	assert(!strcmp(thread_target->class, "thread"));
> +	thread = target_to_thread(thread_target);
> +
> +	CHECK_ERR(thread->ram_getxer(thread_target, value));
> +
> +	return 0;
> +}
> +

<snip>

>
> +static int p8_ram_getxer(struct pdbg_target *thread, uint32_t *value)
> +{
> +	uint32_t fields[] = {0, 0, 0, 0};
> +	int i;
> +
> +	/* On POWER8 we can't get xer with getspr. We can only get IBM
> +	 * bits 33-39 and 41-43 using the xer fields. The rest of the
> +	 * bits are in latches somewhere. */
> +	PR_WARNING("Can only get IBM bits 33-39 and 41-43 of the XER register\n");
> +	for (i = 0; i < 4; i++) {
> +		CHECK_ERR(ram_getxer_field(thread, &fields[i], i));

This could be made slightly more efficient by generating the mfxerf opcodes for
each field here and sending them as a single call to ram_instructions() rather
than the multiple calls we have here at present.

- Alistair

> +	}
> +	*value = fields[0] | fields[1] | fields[2] | fields[3];
> +
> +	return 0;
> +}
> +
> +static int p8_ram_putxer(struct pdbg_target *thread, uint32_t value)
> +{
> +	uint32_t fields[] = {0, 0, 0, 0};
> +	int i;
> +
> +	/* On POWER8 we seem to be only able to set IBM bits 33 and 34. This is
> +	 * f0 and the first bit of f1, the only bits in the four XER fields
> +	 * that are publicly documented.
> +	 */
> +	PR_WARNING("Can only set IBM bits 33 and 34 of the XER register\n");
> +	fields[0] = (value & (0x1 << 30));
> +	fields[1] = (value & (0x3 << 28));
> +	fields[2] = (value & (0xf << 24));
> +	fields[3] = (value & (0x7 << 20 ));
> +
> +	for (i = 0; i < 4; i++) {
> +		CHECK_ERR(ram_putxer_field(thread, fields[i], i));
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Initialise all viable threads for ramming on the given core.
>   */
> @@ -404,6 +443,8 @@ static struct thread p8_thread = {
>  	.ram_setup = p8_ram_setup,
>  	.ram_instruction = p8_ram_instruction,
>  	.ram_destroy = p8_ram_destroy,
> +	.ram_getxer = p8_ram_getxer,
> +	.ram_putxer = p8_ram_putxer,
>  };
>  DECLARE_HW_UNIT(p8_thread);
>  
> diff --git a/libpdbg/p9chip.c b/libpdbg/p9chip.c
> index c5de3bb..a266188 100644
> --- a/libpdbg/p9chip.c
> +++ b/libpdbg/p9chip.c
> @@ -275,7 +275,6 @@ static int __p9_ram_instruction(struct thread *thread, uint64_t opcode, uint64_t
>  	switch(opcode & OPCODE_MASK) {
>  	case MTNIA_OPCODE:
>  		predecode = 8;
> -
>  		/* Not currently supported as we can only MTNIA from LR */
>  		PR_ERROR("MTNIA is not currently supported\n");
>  		break;
> @@ -290,7 +289,18 @@ static int __p9_ram_instruction(struct thread *thread, uint64_t opcode, uint64_t
>  		break;
>  
>  	case MFSPR_OPCODE:
> -		switch(MFSPR_SPR(opcode)) {
> +		switch(MXSPR_SPR(opcode)) {
> +		case 1: /* XER */
> +			predecode = 4;
> +			break;
> +		default:
> +			predecode = 0;
> +			break;
> +		}
> +		break;
> +
> +	case MTSPR_OPCODE:
> +		switch(MXSPR_SPR(opcode)) {
>  		case 1: /* XER */
>  			predecode = 4;
>  			break;
> @@ -373,6 +383,22 @@ static int p9_ram_destroy(struct thread *thread)
>  	return 0;
>  }
>  
> +static int p9_ram_getxer(struct pdbg_target *thread, uint32_t *value)
> +{
> +	CHECK_ERR(ram_getspr(thread, 1, (uint64_t *)value));
> +
> +	return 0;
> +}
> +
> +static int p9_ram_putxer(struct pdbg_target *thread, uint32_t value)
> +{
> +	/* On POWER9 we can only set bits 32-34 and 44-63.*/
> +	CHECK_ERR(ram_putspr(thread, 1, (uint64_t)value));
> +
> +	return 0;
> +
> +}
> +
>  static struct thread p9_thread = {
>  	.target = {
>  		.name = "POWER9 Thread",
> @@ -387,6 +413,8 @@ static struct thread p9_thread = {
>  	.ram_setup = p9_ram_setup,
>  	.ram_instruction = p9_ram_instruction,
>  	.ram_destroy = p9_ram_destroy,
> +	.ram_getxer = p9_ram_getxer,
> +	.ram_putxer = p9_ram_putxer,
>  };
>  DECLARE_HW_UNIT(p9_thread);
>  
> diff --git a/libpdbg/target.h b/libpdbg/target.h
> index e4a3ed4..87e9f25 100644
> --- a/libpdbg/target.h
> +++ b/libpdbg/target.h
> @@ -152,6 +152,8 @@ struct thread {
>  	int (*ram_setup)(struct thread *);
>  	int (*ram_instruction)(struct thread *, uint64_t opcode, uint64_t *scratch);
>  	int (*ram_destroy)(struct thread *);
> +	int (*ram_getxer)(struct pdbg_target *, uint32_t *value);
> +	int (*ram_putxer)(struct pdbg_target *, uint32_t value);
>  };
>  #define target_to_thread(x) container_of(x, struct thread, target)
>  
>
Rashmica Gupta June 25, 2018, 8 a.m. UTC | #2
On 25/06/18 16:24, Alistair Popple wrote:
> Thanks Rashmica, couple of questions/comments below...
>
>> +int ram_getxer_field(struct pdbg_target *thread, uint32_t *value, uint32_t field)
>> +{
>> +	uint64_t opcodes[] = {mfxerf(0, field), mtspr(277, 0)};
>> +	uint64_t results[] = {0, 0};
>> +
>> +	CHECK_ERR(ram_instructions(thread, opcodes, results, ARRAY_SIZE(opcodes), 0));
> What is the distinction between getxer_field and getxer? You seem to handle
> getxer specially below which implies it is different between P8 vs. P9 yet the
> getxer_field case is handled the same for both P8 and P9.

getxer should return the XER register regardless of implementation,
while getxer_field is technically only for P8. I left getxer_field in
the chip.c file as to move it to the p8 specific file required changing the
scope of a few things like mfspr, mtspr, ram_instructions.


>  Would just using
> multiple mfxerf()'s for getxer introduce any limitations?

I don't think that wouldn't work for p9 and if it did
would take more time than just getspr 1. But I will try.

>
> If it doesn't it might be preferable to just use that for getxer (like you do
> for P8 anyway) rather than maintaining a special case for P9. Same would
> obviously apply for putxer.


Not sure I follow. Will talk to you about this in person tomorrow?


>
>> +
>> +	*value = results[1];
>> +	return 0;
>> +}
>> +
>> +int ram_getxer(struct pdbg_target *thread_target, uint32_t *value)
>> +{
>> +
>> +	struct thread *thread;
>> +
>> +	assert(!strcmp(thread_target->class, "thread"));
>> +	thread = target_to_thread(thread_target);
>> +
>> +	CHECK_ERR(thread->ram_getxer(thread_target, value));
>> +
>> +	return 0;
>> +}
>> +
> <snip>
>
>> +static int p8_ram_getxer(struct pdbg_target *thread, uint32_t *value)
>> +{
>> +	uint32_t fields[] = {0, 0, 0, 0};
>> +	int i;
>> +
>> +	/* On POWER8 we can't get xer with getspr. We can only get IBM
>> +	 * bits 33-39 and 41-43 using the xer fields. The rest of the
>> +	 * bits are in latches somewhere. */
>> +	PR_WARNING("Can only get IBM bits 33-39 and 41-43 of the XER register\n");
>> +	for (i = 0; i < 4; i++) {
>> +		CHECK_ERR(ram_getxer_field(thread, &fields[i], i));
> This could be made slightly more efficient by generating the mfxerf opcodes for
> each field here and sending them as a single call to ram_instructions() rather
> than the multiple calls we have here at present.

That would require me to change the scope of ram_instructions, are
you ok with that?

>
> - Alistair
>
>> +	}
>> +	*value = fields[0] | fields[1] | fields[2] | fields[3];
>> +
>> +	return 0;
>> +}
>> +
>> +static int p8_ram_putxer(struct pdbg_target *thread, uint32_t value)
>> +{
>> +	uint32_t fields[] = {0, 0, 0, 0};
>> +	int i;
>> +
>> +	/* On POWER8 we seem to be only able to set IBM bits 33 and 34. This is
>> +	 * f0 and the first bit of f1, the only bits in the four XER fields
>> +	 * that are publicly documented.
>> +	 */
>> +	PR_WARNING("Can only set IBM bits 33 and 34 of the XER register\n");
>> +	fields[0] = (value & (0x1 << 30));
>> +	fields[1] = (value & (0x3 << 28));
>> +	fields[2] = (value & (0xf << 24));
>> +	fields[3] = (value & (0x7 << 20 ));
>> +
>> +	for (i = 0; i < 4; i++) {
>> +		CHECK_ERR(ram_putxer_field(thread, fields[i], i));
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  /*
>>   * Initialise all viable threads for ramming on the given core.
>>   */
>> @@ -404,6 +443,8 @@ static struct thread p8_thread = {
>>  	.ram_setup = p8_ram_setup,
>>  	.ram_instruction = p8_ram_instruction,
>>  	.ram_destroy = p8_ram_destroy,
>> +	.ram_getxer = p8_ram_getxer,
>> +	.ram_putxer = p8_ram_putxer,
>>  };
>>  DECLARE_HW_UNIT(p8_thread);
>>  
>> diff --git a/libpdbg/p9chip.c b/libpdbg/p9chip.c
>> index c5de3bb..a266188 100644
>> --- a/libpdbg/p9chip.c
>> +++ b/libpdbg/p9chip.c
>> @@ -275,7 +275,6 @@ static int __p9_ram_instruction(struct thread *thread, uint64_t opcode, uint64_t
>>  	switch(opcode & OPCODE_MASK) {
>>  	case MTNIA_OPCODE:
>>  		predecode = 8;
>> -
>>  		/* Not currently supported as we can only MTNIA from LR */
>>  		PR_ERROR("MTNIA is not currently supported\n");
>>  		break;
>> @@ -290,7 +289,18 @@ static int __p9_ram_instruction(struct thread *thread, uint64_t opcode, uint64_t
>>  		break;
>>  
>>  	case MFSPR_OPCODE:
>> -		switch(MFSPR_SPR(opcode)) {
>> +		switch(MXSPR_SPR(opcode)) {
>> +		case 1: /* XER */
>> +			predecode = 4;
>> +			break;
>> +		default:
>> +			predecode = 0;
>> +			break;
>> +		}
>> +		break;
>> +
>> +	case MTSPR_OPCODE:
>> +		switch(MXSPR_SPR(opcode)) {
>>  		case 1: /* XER */
>>  			predecode = 4;
>>  			break;
>> @@ -373,6 +383,22 @@ static int p9_ram_destroy(struct thread *thread)
>>  	return 0;
>>  }
>>  
>> +static int p9_ram_getxer(struct pdbg_target *thread, uint32_t *value)
>> +{
>> +	CHECK_ERR(ram_getspr(thread, 1, (uint64_t *)value));
>> +
>> +	return 0;
>> +}
>> +
>> +static int p9_ram_putxer(struct pdbg_target *thread, uint32_t value)
>> +{
>> +	/* On POWER9 we can only set bits 32-34 and 44-63.*/
>> +	CHECK_ERR(ram_putspr(thread, 1, (uint64_t)value));
>> +
>> +	return 0;
>> +
>> +}
>> +
>>  static struct thread p9_thread = {
>>  	.target = {
>>  		.name = "POWER9 Thread",
>> @@ -387,6 +413,8 @@ static struct thread p9_thread = {
>>  	.ram_setup = p9_ram_setup,
>>  	.ram_instruction = p9_ram_instruction,
>>  	.ram_destroy = p9_ram_destroy,
>> +	.ram_getxer = p9_ram_getxer,
>> +	.ram_putxer = p9_ram_putxer,
>>  };
>>  DECLARE_HW_UNIT(p9_thread);
>>  
>> diff --git a/libpdbg/target.h b/libpdbg/target.h
>> index e4a3ed4..87e9f25 100644
>> --- a/libpdbg/target.h
>> +++ b/libpdbg/target.h
>> @@ -152,6 +152,8 @@ struct thread {
>>  	int (*ram_setup)(struct thread *);
>>  	int (*ram_instruction)(struct thread *, uint64_t opcode, uint64_t *scratch);
>>  	int (*ram_destroy)(struct thread *);
>> +	int (*ram_getxer)(struct pdbg_target *, uint32_t *value);
>> +	int (*ram_putxer)(struct pdbg_target *, uint32_t value);
>>  };
>>  #define target_to_thread(x) container_of(x, struct thread, target)
>>  
>>
>
diff mbox series

Patch

diff --git a/libpdbg/chip.c b/libpdbg/chip.c
index 4226cbf..6907765 100644
--- a/libpdbg/chip.c
+++ b/libpdbg/chip.c
@@ -84,6 +84,46 @@  static uint64_t mtmsr(uint64_t reg)
 	return MTMSR_OPCODE | (reg << 21);
 }
 
+static uint64_t mfxerf(uint64_t reg, uint64_t field)
+{
+	if (reg > 31)
+		PR_ERROR("Invalid register specified for mfxerf\n");
+
+	switch(field) {
+	case 0:
+		return MFXERF0_OPCODE | (reg << 21);
+	case 1:
+		return MFXERF1_OPCODE | (reg << 21);
+	case 2:
+		return MFXERF2_OPCODE | (reg << 21);
+	case 3:
+		return MFXERF3_OPCODE | (reg << 21);
+	default:
+		PR_ERROR("Invalid XER field specified\n");
+	}
+	return 0;
+}
+
+static uint64_t mtxerf(uint64_t reg, uint64_t field)
+{
+	if (reg > 31)
+		PR_ERROR("Invalid register specified for mtxerf\n");
+
+	switch(field) {
+	case 0:
+		return MTXERF0_OPCODE | (reg << 21);
+	case 1:
+		return MTXERF1_OPCODE | (reg << 21);
+	case 2:
+		return MTXERF2_OPCODE | (reg << 21);
+	case 3:
+		return MTXERF3_OPCODE | (reg << 21);
+	default:
+		PR_ERROR("Invalid XER field specified\n");
+	}
+	return 0;
+}
+
 static uint64_t ld(uint64_t rt, uint64_t ds, uint64_t ra)
 {
 	if ((rt > 31) | (ra > 31) | (ds > 0x3fff))
@@ -168,6 +208,7 @@  static int ram_instructions(struct pdbg_target *thread_target, uint64_t *opcodes
 	/* RAM instructions */
 	for (i = -2; i < len + 2; i++) {
 		if (i == -2)
+			/* Save r1 (assumes opcodes don't touch other registers) */
 			opcode = mtspr(277, 1);
 		else if (i == -1)
 			/* Save r0 (assumes opcodes don't touch other registers) */
@@ -309,6 +350,52 @@  int ram_getmem(struct pdbg_target *thread, uint64_t addr, uint64_t *value)
 	return 0;
 }
 
+int ram_getxer_field(struct pdbg_target *thread, uint32_t *value, uint32_t field)
+{
+	uint64_t opcodes[] = {mfxerf(0, field), mtspr(277, 0)};
+	uint64_t results[] = {0, 0};
+
+	CHECK_ERR(ram_instructions(thread, opcodes, results, ARRAY_SIZE(opcodes), 0));
+
+	*value = results[1];
+	return 0;
+}
+
+int ram_getxer(struct pdbg_target *thread_target, uint32_t *value)
+{
+
+	struct thread *thread;
+
+	assert(!strcmp(thread_target->class, "thread"));
+	thread = target_to_thread(thread_target);
+
+	CHECK_ERR(thread->ram_getxer(thread_target, value));
+
+	return 0;
+}
+
+int ram_putxer_field(struct pdbg_target *thread, uint32_t value, uint32_t field)
+{
+	uint64_t opcodes[] = {mfspr(0, 277), mtxerf(0, field)};
+	uint64_t results[] = {value, 0};
+
+	CHECK_ERR(ram_instructions(thread, opcodes, results, ARRAY_SIZE(opcodes), 0));
+
+	return 0;
+}
+
+int ram_putxer(struct pdbg_target *thread_target, uint32_t value)
+{
+	struct thread *thread;
+
+	assert(!strcmp(thread_target->class, "thread"));
+	thread = target_to_thread(thread_target);
+
+	CHECK_ERR(thread->ram_putxer(thread_target, value));
+
+	return 0;
+}
+
 /*
  * Read the given ring from the given chiplet. Result must be large enough to hold ring_len bits.
  */
@@ -368,12 +455,8 @@  int ram_state_thread(struct pdbg_target *thread, struct thread_regs *regs)
 	}
 	printf("CR    : 0x%08" PRIx32 "\n", regs->cr);
 
-#if 0
-	/* TODO: Disabling because reading SPR 0x1 reliably checkstops a P8 */
-	ram_getspr(thread, 0x1, &value);
-	regs->xer = value;
+	ram_getxer(thread, &regs->xer);
 	printf("XER   : 0x%08" PRIx32 "\n", regs->xer);
-#endif
 
 	printf("GPRS  :\n");
 	for (i = 0; i < 32; i++) {
diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
index c0990d9..81d6170 100644
--- a/libpdbg/libpdbg.h
+++ b/libpdbg/libpdbg.h
@@ -147,6 +147,10 @@  int ram_stop_thread(struct pdbg_target *target);
 int ram_sreset_thread(struct pdbg_target *target);
 int ram_state_thread(struct pdbg_target *target, struct thread_regs *regs);
 struct thread_state thread_status(struct pdbg_target *target);
+int ram_getxer(struct pdbg_target *thread, uint32_t *value);
+int ram_putxer(struct pdbg_target *thread, uint32_t value);
+int ram_getxer_field(struct pdbg_target *thread, uint32_t *value, uint32_t field);
+int ram_putxer_field(struct pdbg_target *thread, uint32_t value, uint32_t field);
 int getring(struct pdbg_target *chiplet_target, uint64_t ring_addr, uint64_t ring_len, uint32_t result[]);
 
 enum pdbg_sleep_state {PDBG_THREAD_STATE_RUN, PDBG_THREAD_STATE_DOZE,
diff --git a/libpdbg/operations.h b/libpdbg/operations.h
index 52bfe7e..c7e3218 100644
--- a/libpdbg/operations.h
+++ b/libpdbg/operations.h
@@ -64,12 +64,21 @@ 
 #define MTMSR_OPCODE 0x7c000124UL
 #define MFSPR_OPCODE 0x7c0002a6UL
 #define MTSPR_OPCODE 0x7c0003a6UL
+#define MTXERF0_OPCODE 0x00000008UL
+#define MTXERF1_OPCODE 0x00000108UL
+#define MTXERF2_OPCODE 0x00000208UL
+#define MTXERF3_OPCODE 0x00000308UL
+#define MFXERF0_OPCODE 0x00000010UL
+#define MFXERF1_OPCODE 0x00000110UL
+#define MFXERF2_OPCODE 0x00000210UL
+#define MFXERF3_OPCODE 0x00000310UL
 #define MFOCRF_OPCODE 0x7c100026UL
 #define MFSPR_MASK (MFSPR_OPCODE | ((0x1f) << 16) | ((0x3e0) << 6))
 #define MFXER_OPCODE (MFSPR_OPCODE | ((1 & 0x1f) << 16) | ((1 & 0x3e0) << 6))
+#define MTXER_OPCODE (MTSPR_OPCODE | ((1 & 0x1f) << 16) | ((1 & 0x3e0) << 6))
 #define LD_OPCODE 0xe8000000UL
 
-#define MFSPR_SPR(opcode) (((opcode >> 16) & 0x1f) | ((opcode >> 6) & 0x3e0))
+#define MXSPR_SPR(opcode) (((opcode >> 16) & 0x1f) | ((opcode >> 6) & 0x3e0))
 
 /* GDB server functionality */
 int gdbserver_start(uint16_t port);
diff --git a/libpdbg/p8chip.c b/libpdbg/p8chip.c
index 6640bb4..5626943 100644
--- a/libpdbg/p8chip.c
+++ b/libpdbg/p8chip.c
@@ -377,6 +377,45 @@  static int p8_ram_destroy(struct thread *thread)
 	return 0;
 }
 
+static int p8_ram_getxer(struct pdbg_target *thread, uint32_t *value)
+{
+	uint32_t fields[] = {0, 0, 0, 0};
+	int i;
+
+	/* On POWER8 we can't get xer with getspr. We can only get IBM
+	 * bits 33-39 and 41-43 using the xer fields. The rest of the
+	 * bits are in latches somewhere. */
+	PR_WARNING("Can only get IBM bits 33-39 and 41-43 of the XER register\n");
+	for (i = 0; i < 4; i++) {
+		CHECK_ERR(ram_getxer_field(thread, &fields[i], i));
+	}
+	*value = fields[0] | fields[1] | fields[2] | fields[3];
+
+	return 0;
+}
+
+static int p8_ram_putxer(struct pdbg_target *thread, uint32_t value)
+{
+	uint32_t fields[] = {0, 0, 0, 0};
+	int i;
+
+	/* On POWER8 we seem to be only able to set IBM bits 33 and 34. This is
+	 * f0 and the first bit of f1, the only bits in the four XER fields
+	 * that are publicly documented.
+	 */
+	PR_WARNING("Can only set IBM bits 33 and 34 of the XER register\n");
+	fields[0] = (value & (0x1 << 30));
+	fields[1] = (value & (0x3 << 28));
+	fields[2] = (value & (0xf << 24));
+	fields[3] = (value & (0x7 << 20 ));
+
+	for (i = 0; i < 4; i++) {
+		CHECK_ERR(ram_putxer_field(thread, fields[i], i));
+	}
+
+	return 0;
+}
+
 /*
  * Initialise all viable threads for ramming on the given core.
  */
@@ -404,6 +443,8 @@  static struct thread p8_thread = {
 	.ram_setup = p8_ram_setup,
 	.ram_instruction = p8_ram_instruction,
 	.ram_destroy = p8_ram_destroy,
+	.ram_getxer = p8_ram_getxer,
+	.ram_putxer = p8_ram_putxer,
 };
 DECLARE_HW_UNIT(p8_thread);
 
diff --git a/libpdbg/p9chip.c b/libpdbg/p9chip.c
index c5de3bb..a266188 100644
--- a/libpdbg/p9chip.c
+++ b/libpdbg/p9chip.c
@@ -275,7 +275,6 @@  static int __p9_ram_instruction(struct thread *thread, uint64_t opcode, uint64_t
 	switch(opcode & OPCODE_MASK) {
 	case MTNIA_OPCODE:
 		predecode = 8;
-
 		/* Not currently supported as we can only MTNIA from LR */
 		PR_ERROR("MTNIA is not currently supported\n");
 		break;
@@ -290,7 +289,18 @@  static int __p9_ram_instruction(struct thread *thread, uint64_t opcode, uint64_t
 		break;
 
 	case MFSPR_OPCODE:
-		switch(MFSPR_SPR(opcode)) {
+		switch(MXSPR_SPR(opcode)) {
+		case 1: /* XER */
+			predecode = 4;
+			break;
+		default:
+			predecode = 0;
+			break;
+		}
+		break;
+
+	case MTSPR_OPCODE:
+		switch(MXSPR_SPR(opcode)) {
 		case 1: /* XER */
 			predecode = 4;
 			break;
@@ -373,6 +383,22 @@  static int p9_ram_destroy(struct thread *thread)
 	return 0;
 }
 
+static int p9_ram_getxer(struct pdbg_target *thread, uint32_t *value)
+{
+	CHECK_ERR(ram_getspr(thread, 1, (uint64_t *)value));
+
+	return 0;
+}
+
+static int p9_ram_putxer(struct pdbg_target *thread, uint32_t value)
+{
+	/* On POWER9 we can only set bits 32-34 and 44-63.*/
+	CHECK_ERR(ram_putspr(thread, 1, (uint64_t)value));
+
+	return 0;
+
+}
+
 static struct thread p9_thread = {
 	.target = {
 		.name = "POWER9 Thread",
@@ -387,6 +413,8 @@  static struct thread p9_thread = {
 	.ram_setup = p9_ram_setup,
 	.ram_instruction = p9_ram_instruction,
 	.ram_destroy = p9_ram_destroy,
+	.ram_getxer = p9_ram_getxer,
+	.ram_putxer = p9_ram_putxer,
 };
 DECLARE_HW_UNIT(p9_thread);
 
diff --git a/libpdbg/target.h b/libpdbg/target.h
index e4a3ed4..87e9f25 100644
--- a/libpdbg/target.h
+++ b/libpdbg/target.h
@@ -152,6 +152,8 @@  struct thread {
 	int (*ram_setup)(struct thread *);
 	int (*ram_instruction)(struct thread *, uint64_t opcode, uint64_t *scratch);
 	int (*ram_destroy)(struct thread *);
+	int (*ram_getxer)(struct pdbg_target *, uint32_t *value);
+	int (*ram_putxer)(struct pdbg_target *, uint32_t value);
 };
 #define target_to_thread(x) container_of(x, struct thread, target)