Re: [PATCH] kprobes for s390 architecture

[Jan Glauber <jan.glauber@de.ibm.com>]

On Mon, 2006-06-12 at 21:40 +0200, Martin Schwidefsky wrote:
> On Mon, 2006-06-12 at 09:15 -0400, Mike Grundy wrote:
> > +/* get_instruction type will return 0 if only the regular offset adjustments
> > + * after out of line singlestep are required. If a register needs to be fixed,
> > + * bits 16-23 will contain the register number, bits 24-31 contain the length
> > + * of the instruction unit. If fixup is only required when the branch is not
> > + * taken, bits 0-16 will all be set.
> > + */
> > +int __kprobes get_instruction_type(kprobe_opcode_t * instruction)
> > +{
> > +	__u8 opcode[6];
> > +	int ret = 0;
> > +
> > +	memcpy(opcode, instruction, 6 * sizeof(__u8));
> 
> Again that memcpy. Why don't you just cast the instruction pointer to
> __u8 and __u16 and deference it? 
> 
> The following switch deals with all the instructions that need special
> handling. Please get rid of the BALR/BASR/BCR/BCTR/... defines and use
> the opcode number directly. Add a comment which instruction it is like
> in the new is_prohibited_opcode. All these defines are only used in
> get_instruction_type and the opcodes will certainly not change, only new
> ones will get added. No point in all those #defines.
> 
> > +
> > +	switch (opcode[0]) {
> > +		/* RR Format - instruction unit length = 2
> > +		 *  ________ ____ ____
> > +		 * |Op Code | R1 | R2 |
> > +		 * |________|_M1_|____|
> > +		 * 0         8   12  15
> > +		 */
> > +	case BALR:	/* PSW addr saved in R1, branch address in R2 */
> > +		ret = (opcode[1] & 0xf0) + 2;
> > +		/* Special non branching use of BALR */
> > +		if ((opcode[1] & 0x0f) == 0)
> > +			ret &= FIXUP_NOBRANCH;
> > +		break;
> 
> ((opcode[1] & 0xf0) + 2) & FIXUP_NOBRANCH is always 0. If the target
> register is 0 no branch takes place but R1 still needs fixup.
> resume_execution will just fixup the psw address in that case. I think
> you meant "ret |= FIXUP_NOBRANCH".
> 
> 
> > +	case BASR:	/* PSW addr saved in R1, branch address in R2 */
> > +		ret = (opcode[1] & 0xf0) + 2;
> > +		/* Special non branching use of BASR */
> > +		if ((opcode[1] & 0x0f) == 0)
> > +			ret &= FIXUP_NOBRANCH;
> > +		break;
> 
> Same here..
> 
> > +	case BCR:	/* M1 is mask val (condition), branch addr in R2 */
> > +		ret = FIXUP_NOBRANCH & 2;
> > +		break;
> 
> ..here..
> 
> > +	case BCTR:	/* R1 is count down, R2 is branch addr until R1 = 0 */
> > +		ret = FIXUP_NOBRANCH & 2;
> > +		break;
> 
> ..here..
> 
> > +		/* RX Format - instruction unit length = 4
> > +		 *  ________ ____ ____ ____ ____________
> > +		 * |Op Code | R1 | X2 | B2 |     D2     |
> > +		 * |________|_M1_|____|____|____________|
> > +		 * 0         8   12   16   20          31
> > +		 */
> > +	case BAL:	/* PSW addr saved in R1, branch addr D2(X2,B2) */
> > +		ret = (opcode[1] & 0xf0) + 4;
> > +		break;
> > +	case BAS:	/* PSW addr saved in R1, branch addr D2(X2,B2) */
> > +		ret = (opcode[1] & 0xf0) + 4;
> > +		break;
> > +	case BC:	/* M1 is mask val (condition), branch addr D2(X2,B2) */
> > +		ret = FIXUP_NOBRANCH & 4;
> > +		break;
> 
> ..here..
> 
> > +	case BCT:	/* R1 is count down, D2(X2,B2) is branch addr */
> > +		ret = FIXUP_NOBRANCH & 4;
> > +		break;
> 
> ..here..
> 
> > +		/* RI Format - instruction unit length = 4
> > +		 *  ________ ____ ____ _________________
> > +		 * |Op Code | R1 |OpCd|       I2        |
> > +		 * |________|____|____|_________________|
> > +		 * 0         8   12   16               31
> > +		 */
> > +	case 0xA7:	/* first byte (multiple ops have same 1st byte) */
> > +		if ((opcode[1] & 0x0f) == BRAS) {
> > +			ret = (opcode[1] & 0xf0) + 4;
> > +		}
> > +		break;
> > +		/* RS Format - instruction unit length = 4
> > +		 *  ________ ____ ____ ____ ____________
> > +		 * |Op Code | R1 | R3 | B2 |     D2     |
> > +		 * |________|____|_M3_|____|____________|
> > +		 * 0         8   12   16   20          31
> > +		 */
> > +	case BXH:
> > +		ret = FIXUP_NOBRANCH & 4;
> > +		break;
> 
> ..here..
> 
> > +	case BXLE:
> > +		ret = FIXUP_NOBRANCH & 4;
> > +		break;
> 
> ..here..
> 
> > +		/* RIL Format - instruction unit length = 6
> > +		 *  ________ ____ ____ _____________/______________
> > +		 * |Op Code | R1 |OpCd|            I2              |
> > +		 * |________|_M1_|____|_____________/______________|
> > +		 * 0         8   12   16                          47
> > +		 */
> > +	case 0xC0:
> > +		if ((opcode[1] & 0x0f) == BRASL) {
> > +			ret = (opcode[1] & 0xf0) + 6;
> > +		} else if ((opcode[1] & 0x0f) == BRCL) {
> > +			ret = FIXUP_NOBRANCH & 6;
> > +		}
> > +		break;
> 
> ..here..
> 
> > +		/* RSY Format - instruction unit length = 6
> > +		 *  ________ ____ ____ ____ __/__ ________ ________
> > +		 * |Op Code | R1 | R3 | B2 | DL2 |  DH2   |Op Code |
> > +		 * |________|____|_M3_|____|__/__|________|________|
> > +		 * 0         8   12   16   20    32       40      47
> > +		 */
> > +	case 0xEB:
> > +		if (opcode[5] == BXHG || opcode[5] == BXLEG) {
> > +			ret = FIXUP_NOBRANCH & 6;
> > +		}
> > +		break;
> 
> ..here..
> 
> > +		/* RXY Format - instruction unit length = 6
> > +		 *  ________ ____ ____ ____ __/__ ________ ________
> > +		 * |Op Code | R1 | X2 | B2 | DL2 |  DH2   |Op Code |
> > +		 * |________|____|____|____|__/__|________|________|
> > +		 * 0         8   12   16   20    32       40      47
> > +		 */
> > +	case 0xE3:
> > +		if (opcode[5] == BCTG) {
> > +			ret = FIXUP_NOBRANCH & 6;
> > +		}
> > +		break;
> 
> ..and here.
> 
> > +	default:
> > +		break;
> > +	}
> > +	return ret;
> > +}
> > +
> 
> There are some more instructions missing that need fixup:
> "brxh" 0x84??????, "brxle" 0x85??????, "brc" 0xa7?4????,
> "brct" 0xa7?6????, "brctg" 0xa7?7????, "bctgr" 0xb946????,
> "brxhg" 0xec????????44 and "brxlg" 0xec??????45.

We need to handle lpsw and larl too, since they change the instruction
pointer.

Jan

---
Jan Glauber
IBM Linux Technology Center
Linux on zSeries Development, Boeblingen