Re: [PATCH] kprobes for s390 architecture

From: Jan Glauber
Date: Wed Jun 21 2006 - 05:39:19 EST

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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/