Jordan Niethe's on February 26, 2020 2:07 pm:
> @@ -136,11 +148,14 @@ int arch_prepare_kprobe(struct kprobe *p)
>       }
>  
>       if (!ret) {
> -             patch_instruction(p->ainsn.insn, *p->addr);
> +             patch_instruction(&p->ainsn.insn[0], p->addr[0]);
> +             if (IS_PREFIX(insn))
> +                     patch_instruction(&p->ainsn.insn[1], p->addr[1]);
>               p->opcode = *p->addr;

Not to single out this hunk or this patch even, but what do you reckon
about adding an instruction data type, and then use that in all these
call sites rather than adding the extra arg or doing the extra copy
manually in each place depending on prefix?

instrs_are_equal, get_user_instr, analyse_instr, patch_instruction,
etc., would all take this new instr. Places that open code a memory
access like your MCE change need some accessor

               instr = *(unsigned int *)(instr_addr);
-               if (!analyse_instr(&op, &tmp, instr, PPC_NO_SUFFIX)) {
+               if (IS_PREFIX(instr))
+                       suffix = *(unsigned int *)(instr_addr + 4);

Becomes
               read_instr(instr_addr, &instr);
               if (!analyse_instr(&op, &tmp, instr)) ...

etc.

Thanks,
Nick

Reply via email to