RFC: kprobes support for ppc32

RFC: kprobes support for ppc32

[Kumar Gala]

Here's a first cut on ppc32 support for kprobes.  I haven't tested this, 
but I think I got all the places that needed fixing up.

- k

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d6abe49..db1e118 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -1182,7 +1182,7 @@ source "arch/powerpc/oprofile/Kconfig"

  config KPROBES
  	bool "Kprobes (EXPERIMENTAL)"
-	depends on PPC64 && KALLSYMS && EXPERIMENTAL && MODULES
+	depends on !BOOKE && KALLSYMS && EXPERIMENTAL && MODULES
  	help
  	  Kprobes allows you to trap at almost any kernel address and
  	  execute a callback function.  register_kprobe() establishes
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 4657563..dd2886f 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -46,8 +46,8 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
  	if ((unsigned long)p->addr & 0x03) {
  		printk("Attempt to register kprobe at an unaligned address\n");
  		ret = -EINVAL;
-	} else if (IS_MTMSRD(insn) || IS_RFID(insn)) {
-		printk("Cannot register a kprobe on rfid or mtmsrd\n");
+	} else if (IS_MTMSRD(insn) || IS_RFID(insn) || IS_RFI(insn)) {
+		printk("Cannot register a kprobe on rfi/rfid or mtmsr[d]\n");
  		ret = -EINVAL;
  	}

@@ -483,8 +483,12 @@ int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
  	memcpy(&kcb->jprobe_saved_regs, regs, sizeof(struct pt_regs));

  	/* setup return addr to the jprobe handler routine */
+#ifdef CONFIG_PPC64
  	regs->nip = (unsigned long)(((func_descr_t *)jp->entry)->entry);
  	regs->gpr[2] = (unsigned long)(((func_descr_t *)jp->entry)->toc);
+#else
+	regs->nip = (unsigned long)jp->entry;
+#endif

  	return 1;
  }
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index a0360ae..116432f 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -16,10 +16,10 @@ obj-$(CONFIG_PPC64)	+= checksum_64.o copypage_64.o copyuser_64.o \
  			   strcase.o
  obj-$(CONFIG_QUICC_ENGINE) += rheap.o
  obj-$(CONFIG_XMON)	+= sstep.o
+obj-$(CONFIG_KPROBES) += sstep.o

  ifeq ($(CONFIG_PPC64),y)
  obj-$(CONFIG_SMP)	+= locks.o
-obj-$(CONFIG_DEBUG_KERNEL) += sstep.o
  endif

  # Temporary hack until we have migrated to asm-powerpc
diff --git a/include/asm-powerpc/kprobes.h b/include/asm-powerpc/kprobes.h
index 2dafa37..4658a95 100644
--- a/include/asm-powerpc/kprobes.h
+++ b/include/asm-powerpc/kprobes.h
@@ -66,7 +66,11 @@ typedef unsigned int kprobe_opcode_t;
  	}								\
  }

+#ifdef CONFIG_PPC64
  #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)((func_descr_t *)pentry)
+#else
+#define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)(pentry)
+#endif

  #define is_trap(instr)	(IS_TW(instr) || IS_TD(instr) || \
  			IS_TWI(instr) || IS_TDI(instr))
diff --git a/include/asm-powerpc/sstep.h b/include/asm-powerpc/sstep.h
index 630a988..f593b0f 100644
--- a/include/asm-powerpc/sstep.h
+++ b/include/asm-powerpc/sstep.h
@@ -21,6 +21,7 @@ struct pt_regs;
   */
  #define IS_MTMSRD(instr)	(((instr) & 0xfc0007be) == 0x7c000124)
  #define IS_RFID(instr)		(((instr) & 0xfc0007fe) == 0x4c000024)
+#define IS_RFI(instr)		(((instr) & 0xfc0007fe) == 0x4c000064)

  /* Emulate instructions that cause a transfer of control. */
  extern int emulate_step(struct pt_regs *regs, unsigned int instr);

Re: RFC: kprobes support for ppc32

[Ananth N Mavinakayanahalli]

On Tue, Jan 30, 2007 at 12:13:32AM -0600, Kumar Gala wrote:
> Here's a first cut on ppc32 support for kprobes.  I haven't tested this, 
> but I think I got all the places that needed fixing up.
> 
> - k

Kumar,

This looks fine (very similar to a patch I had done, but couldn't test
it for lack of hardware).

Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d6abe49..db1e118 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -1182,7 +1182,7 @@ source "arch/powerpc/oprofile/Kconfig"
> 
>  config KPROBES
>  	bool "Kprobes (EXPERIMENTAL)"
> -	depends on PPC64 && KALLSYMS && EXPERIMENTAL && MODULES
> +	depends on !BOOKE && KALLSYMS && EXPERIMENTAL && MODULES
>  	help
>  	  Kprobes allows you to trap at almost any kernel address and
>  	  execute a callback function.  register_kprobe() establishes
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 4657563..dd2886f 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -46,8 +46,8 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  	if ((unsigned long)p->addr & 0x03) {
>  		printk("Attempt to register kprobe at an unaligned 
>  		address\n");
>  		ret = -EINVAL;
> -	} else if (IS_MTMSRD(insn) || IS_RFID(insn)) {
> -		printk("Cannot register a kprobe on rfid or mtmsrd\n");
> +	} else if (IS_MTMSRD(insn) || IS_RFID(insn) || IS_RFI(insn)) {
> +		printk("Cannot register a kprobe on rfi/rfid or mtmsr[d]\n");
>  		ret = -EINVAL;
>  	}
> 
> @@ -483,8 +483,12 @@ int __kprobes setjmp_pre_handler(struct kprobe *p, 
> struct pt_regs *regs)
>  	memcpy(&kcb->jprobe_saved_regs, regs, sizeof(struct pt_regs));
> 
>  	/* setup return addr to the jprobe handler routine */
> +#ifdef CONFIG_PPC64
>  	regs->nip = (unsigned long)(((func_descr_t *)jp->entry)->entry);
>  	regs->gpr[2] = (unsigned long)(((func_descr_t *)jp->entry)->toc);
> +#else
> +	regs->nip = (unsigned long)jp->entry;
> +#endif
> 
>  	return 1;
>  }
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index a0360ae..116432f 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -16,10 +16,10 @@ obj-$(CONFIG_PPC64)	+= checksum_64.o 
> copypage_64.o copyuser_64.o \
>  			   strcase.o
>  obj-$(CONFIG_QUICC_ENGINE) += rheap.o
>  obj-$(CONFIG_XMON)	+= sstep.o
> +obj-$(CONFIG_KPROBES) += sstep.o
> 
>  ifeq ($(CONFIG_PPC64),y)
>  obj-$(CONFIG_SMP)	+= locks.o
> -obj-$(CONFIG_DEBUG_KERNEL) += sstep.o
>  endif
> 
>  # Temporary hack until we have migrated to asm-powerpc
> diff --git a/include/asm-powerpc/kprobes.h b/include/asm-powerpc/kprobes.h
> index 2dafa37..4658a95 100644
> --- a/include/asm-powerpc/kprobes.h
> +++ b/include/asm-powerpc/kprobes.h
> @@ -66,7 +66,11 @@ typedef unsigned int kprobe_opcode_t;
>  	}								\
>  }
> 
> +#ifdef CONFIG_PPC64
>  #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)((func_descr_t *)pentry)
> +#else
> +#define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)(pentry)
> +#endif
> 
>  #define is_trap(instr)	(IS_TW(instr) || IS_TD(instr) || \
>  			IS_TWI(instr) || IS_TDI(instr))
> diff --git a/include/asm-powerpc/sstep.h b/include/asm-powerpc/sstep.h
> index 630a988..f593b0f 100644
> --- a/include/asm-powerpc/sstep.h
> +++ b/include/asm-powerpc/sstep.h
> @@ -21,6 +21,7 @@ struct pt_regs;
>   */
>  #define IS_MTMSRD(instr)	(((instr) & 0xfc0007be) == 0x7c000124)
>  #define IS_RFID(instr)		(((instr) & 0xfc0007fe) == 
>  0x4c000024)
> +#define IS_RFI(instr)		(((instr) & 0xfc0007fe) == 0x4c000064)
> 
>  /* Emulate instructions that cause a transfer of control. */
>  extern int emulate_step(struct pt_regs *regs, unsigned int instr);

Re: RFC: kprobes support for ppc32

[Christoph Hellwig]

On Tue, Jan 30, 2007 at 12:13:32AM -0600, Kumar Gala wrote:
> +	depends on !BOOKE && KALLSYMS && EXPERIMENTAL && MODULES

Just curious: what's the problem with BOOKE?

> +#ifdef CONFIG_PPC64
>   	regs->nip = (unsigned long)(((func_descr_t *)jp->entry)->entry);
>   	regs->gpr[2] = (unsigned long)(((func_descr_t *)jp->entry)->toc);
> +#else
> +	regs->nip = (unsigned long)jp->entry;
> +#endif

This should probably be split int a macro.



also I think you should not define kprobe_lookup_name for the 32bit
case as the generic version should be fine for ppc32

Re: RFC: kprobes support for ppc32

[Ananth N Mavinakayanahalli]

On Tue, Jan 30, 2007 at 11:46:30AM +0100, Christoph Hellwig wrote:
> On Tue, Jan 30, 2007 at 12:13:32AM -0600, Kumar Gala wrote:
> > +	depends on !BOOKE && KALLSYMS && EXPERIMENTAL && MODULES
> 
> Just curious: what's the problem with BOOKE?
> 
> > +#ifdef CONFIG_PPC64
> >   	regs->nip = (unsigned long)(((func_descr_t *)jp->entry)->entry);
> >   	regs->gpr[2] = (unsigned long)(((func_descr_t *)jp->entry)->toc);
> > +#else
> > +	regs->nip = (unsigned long)jp->entry;
> > +#endif
> 
> This should probably be split int a macro.
> 
> 
> 
> also I think you should not define kprobe_lookup_name for the 32bit
> case as the generic version should be fine for ppc32

Right. 32bit powerpc uses the SVR4 ABI and hence can just use the
generic version. In addition, since 32bit powerpc implementations don't
have a TD/TDI, they can be stubbed out too in the is_trap() check.

Ananth

Re: RFC: kprobes support for ppc32

[Kumar Gala]

On Jan 30, 2007, at 4:46 AM, Christoph Hellwig wrote:

> On Tue, Jan 30, 2007 at 12:13:32AM -0600, Kumar Gala wrote:
>> +	depends on !BOOKE && KALLSYMS && EXPERIMENTAL && MODULES
>
> Just curious: what's the problem with BOOKE?

Book-e's debug handling/architecture is significantly different than  
most other PPCs.  I'll look at adding support for it in the future.

>> +#ifdef CONFIG_PPC64
>>   	regs->nip = (unsigned long)(((func_descr_t *)jp->entry)->entry);
>>   	regs->gpr[2] = (unsigned long)(((func_descr_t *)jp->entry)->toc);
>> +#else
>> +	regs->nip = (unsigned long)jp->entry;
>> +#endif
>
> This should probably be split int a macro.

Seems like a bit of a waste, for 6 lines of code.  I'm not exactly  
sure what you're envisioning here.

> also I think you should not define kprobe_lookup_name for the 32bit
> case as the generic version should be fine for ppc32

Ok, I'll look into that.

- k