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

[rashmica]

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