Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Masami Hiramatsu]

(2013/08/23 17:09), Heiko Carstens wrote:
> On Fri, Aug 23, 2013 at 01:31:23PM +0900, Masami Hiramatsu wrote:
>> (2013/08/22 14:52), Heiko Carstens wrote:
>>> Therefore I need to different insn slot caches, where the slots are either
>>> allocated with __get_free_page(GFP_KERNEL | GFP_DMA) (for the kernel image)
>>> or module_alloc(PAGE_SIZE) for modules.
>>>
>>> I can't have a single cache which satifies both areas.
>>
>> Oh, I see.
>> Indeed, that enough reason to add a new cache... By the way, is there
>> any way to implement it without new kconfig like DMAPROBE and dma flag?
>> AFAICS, since such flag is strongly depends on the s390 arch, I don't
>> like to put it in kernel/kprobes.c.
>>
>> Perhaps, we can make insn slot more generic, e.g. create new slot type
>> with passing page allocator.
> 
> Something like below?
> (only compile tested and on top of the previous patches).

Yes! this is exactly what I thought :)

> I'm not sure, since that would expose struct kprobe_insn_cache.

That is OK, since it is required for some arch.
Could you update the series with this change?

Thank you!

> 
>  arch/Kconfig   |  7 ---
>  arch/s390/Kconfig  |  1 -
>  arch/s390/kernel/kprobes.c | 20 ++
>  include/linux/kprobes.h| 14 -
>  kernel/kprobes.c   | 51 
> --
>  5 files changed, 47 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 7010d68..1feb169 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -76,13 +76,6 @@ config OPTPROBES
>   depends on KPROBES && HAVE_OPTPROBES
>   depends on !PREEMPT
>  
> -config DMAPROBES
> - bool
> - help
> -   Architectures may want to put kprobes instruction slots into
> -   the dma memory region. E.g. s390 has the kernel image in the
> -   dma memory region but the module area far away.
> -
>  config KPROBES_ON_FTRACE
>   def_bool y
>   depends on KPROBES && HAVE_KPROBES_ON_FTRACE
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index ce389a9..8a4cae7 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -96,7 +96,6 @@ config S390
>   select ARCH_WANT_IPC_PARSE_VERSION
>   select BUILDTIME_EXTABLE_SORT
>   select CLONE_BACKWARDS2
> - select DMAPROBES if KPROBES
>   select GENERIC_CLOCKEVENTS
>   select GENERIC_CPU_DEVICES if !SMP
>   select GENERIC_SMP_IDLE_THREAD
> diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
> index bc1071c..cb7ac9e 100644
> --- a/arch/s390/kernel/kprobes.c
> +++ b/arch/s390/kernel/kprobes.c
> @@ -37,6 +37,26 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>  
>  struct kretprobe_blackpoint kretprobe_blacklist[] = { };
>  
> +DEFINE_INSN_CACHE_OPS(dmainsn);
> +
> +static void *alloc_dmainsn_page(void)
> +{
> + return (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
> +}
> +
> +static void free_dmainsn_page(void *page)
> +{
> + free_page((unsigned long)page);
> +}
> +
> +struct kprobe_insn_cache kprobe_dmainsn_slots = {
> + .mutex = __MUTEX_INITIALIZER(kprobe_dmainsn_slots.mutex),
> + .alloc = alloc_dmainsn_page,
> + .free = free_dmainsn_page,
> + .pages = LIST_HEAD_INIT(kprobe_dmainsn_slots.pages),
> + .insn_size = MAX_INSN_SIZE,
> +};
> +
>  static int __kprobes is_prohibited_opcode(kprobe_opcode_t *insn)
>  {
>   switch (insn[0] >> 8) {
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index a5290f6..4e96827 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -266,7 +266,15 @@ extern int arch_init_kprobes(void);
>  extern void show_registers(struct pt_regs *regs);
>  extern void kprobes_inc_nmissed_count(struct kprobe *p);
>  
> -struct kprobe_insn_cache;
> +struct kprobe_insn_cache {
> + struct mutex mutex;
> + void *(*alloc)(void);   /* allocate insn page */
> + void (*free)(void *);   /* free insn page */
> + struct list_head pages; /* list of kprobe_insn_page */
> + size_t insn_size;   /* size of instruction slot */
> + int nr_garbage;
> +};
> +
>  extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c);
>  extern void __free_insn_slot(struct kprobe_insn_cache *c,
>kprobe_opcode_t *slot, int dirty);
> @@ -321,10 +329,6 @@ extern int proc_kprobes_optimization_handler(struct 
> ctl_table *table,
>  
>  #endif /* CONFIG_OPTPROBES */
>  
> -#ifdef CONFIG_DMAPROBES
> -DEFINE_INSN_CACHE_OPS(dmainsn);
> -#endif
> -
>  #ifdef CONFIG_KPROBES_ON_FTRACE
>  extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *ops, struct pt_regs *regs);
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 3b8b073..a0d367a 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -112,9 +112,9 @@ static struct kprobe_blackpoint kprobe_blacklist[] = {
>  struct kprobe_insn_page {
>   struct list_head list;

Re: Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Heiko Carstens]

On Fri, Aug 23, 2013 at 01:31:23PM +0900, Masami Hiramatsu wrote:
> (2013/08/22 14:52), Heiko Carstens wrote:
> > Therefore I need to different insn slot caches, where the slots are either
> > allocated with __get_free_page(GFP_KERNEL | GFP_DMA) (for the kernel image)
> > or module_alloc(PAGE_SIZE) for modules.
> > 
> > I can't have a single cache which satifies both areas.
> 
> Oh, I see.
> Indeed, that enough reason to add a new cache... By the way, is there
> any way to implement it without new kconfig like DMAPROBE and dma flag?
> AFAICS, since such flag is strongly depends on the s390 arch, I don't
> like to put it in kernel/kprobes.c.
> 
> Perhaps, we can make insn slot more generic, e.g. create new slot type
> with passing page allocator.

Something like below?
(only compile tested and on top of the previous patches).

I'm not sure, since that would expose struct kprobe_insn_cache.

 arch/Kconfig   |  7 ---
 arch/s390/Kconfig  |  1 -
 arch/s390/kernel/kprobes.c | 20 ++
 include/linux/kprobes.h| 14 -
 kernel/kprobes.c   | 51 --
 5 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 7010d68..1feb169 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -76,13 +76,6 @@ config OPTPROBES
depends on KPROBES && HAVE_OPTPROBES
depends on !PREEMPT
 
-config DMAPROBES
-   bool
-   help
- Architectures may want to put kprobes instruction slots into
- the dma memory region. E.g. s390 has the kernel image in the
- dma memory region but the module area far away.
-
 config KPROBES_ON_FTRACE
def_bool y
depends on KPROBES && HAVE_KPROBES_ON_FTRACE
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index ce389a9..8a4cae7 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -96,7 +96,6 @@ config S390
select ARCH_WANT_IPC_PARSE_VERSION
select BUILDTIME_EXTABLE_SORT
select CLONE_BACKWARDS2
-   select DMAPROBES if KPROBES
select GENERIC_CLOCKEVENTS
select GENERIC_CPU_DEVICES if !SMP
select GENERIC_SMP_IDLE_THREAD
diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
index bc1071c..cb7ac9e 100644
--- a/arch/s390/kernel/kprobes.c
+++ b/arch/s390/kernel/kprobes.c
@@ -37,6 +37,26 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
 
 struct kretprobe_blackpoint kretprobe_blacklist[] = { };
 
+DEFINE_INSN_CACHE_OPS(dmainsn);
+
+static void *alloc_dmainsn_page(void)
+{
+   return (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
+}
+
+static void free_dmainsn_page(void *page)
+{
+   free_page((unsigned long)page);
+}
+
+struct kprobe_insn_cache kprobe_dmainsn_slots = {
+   .mutex = __MUTEX_INITIALIZER(kprobe_dmainsn_slots.mutex),
+   .alloc = alloc_dmainsn_page,
+   .free = free_dmainsn_page,
+   .pages = LIST_HEAD_INIT(kprobe_dmainsn_slots.pages),
+   .insn_size = MAX_INSN_SIZE,
+};
+
 static int __kprobes is_prohibited_opcode(kprobe_opcode_t *insn)
 {
switch (insn[0] >> 8) {
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index a5290f6..4e96827 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -266,7 +266,15 @@ extern int arch_init_kprobes(void);
 extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 
-struct kprobe_insn_cache;
+struct kprobe_insn_cache {
+   struct mutex mutex;
+   void *(*alloc)(void);   /* allocate insn page */
+   void (*free)(void *);   /* free insn page */
+   struct list_head pages; /* list of kprobe_insn_page */
+   size_t insn_size;   /* size of instruction slot */
+   int nr_garbage;
+};
+
 extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c);
 extern void __free_insn_slot(struct kprobe_insn_cache *c,
 kprobe_opcode_t *slot, int dirty);
@@ -321,10 +329,6 @@ extern int proc_kprobes_optimization_handler(struct 
ctl_table *table,
 
 #endif /* CONFIG_OPTPROBES */
 
-#ifdef CONFIG_DMAPROBES
-DEFINE_INSN_CACHE_OPS(dmainsn);
-#endif
-
 #ifdef CONFIG_KPROBES_ON_FTRACE
 extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
  struct ftrace_ops *ops, struct pt_regs *regs);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 3b8b073..a0d367a 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -112,9 +112,9 @@ static struct kprobe_blackpoint kprobe_blacklist[] = {
 struct kprobe_insn_page {
struct list_head list;
kprobe_opcode_t *insns; /* Page of instruction slots */
+   struct kprobe_insn_cache *cache;
int nused;
int ngarbage;
-   bool dma_alloc;
char slot_used[];
 };
 
@@ -122,14 +122,6 @@ struct kprobe_insn_page {
(offsetof(struct kprobe_insn_page, slot_used) + \
 (sizeof(char) * (slots)))
 
-struct 

Re: Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Heiko Carstens]

On Fri, Aug 23, 2013 at 01:31:23PM +0900, Masami Hiramatsu wrote:
 (2013/08/22 14:52), Heiko Carstens wrote:
  Therefore I need to different insn slot caches, where the slots are either
  allocated with __get_free_page(GFP_KERNEL | GFP_DMA) (for the kernel image)
  or module_alloc(PAGE_SIZE) for modules.
  
  I can't have a single cache which satifies both areas.
 
 Oh, I see.
 Indeed, that enough reason to add a new cache... By the way, is there
 any way to implement it without new kconfig like DMAPROBE and dma flag?
 AFAICS, since such flag is strongly depends on the s390 arch, I don't
 like to put it in kernel/kprobes.c.
 
 Perhaps, we can make insn slot more generic, e.g. create new slot type
 with passing page allocator.

Something like below?
(only compile tested and on top of the previous patches).

I'm not sure, since that would expose struct kprobe_insn_cache.

 arch/Kconfig   |  7 ---
 arch/s390/Kconfig  |  1 -
 arch/s390/kernel/kprobes.c | 20 ++
 include/linux/kprobes.h| 14 -
 kernel/kprobes.c   | 51 --
 5 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 7010d68..1feb169 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -76,13 +76,6 @@ config OPTPROBES
depends on KPROBES  HAVE_OPTPROBES
depends on !PREEMPT
 
-config DMAPROBES
-   bool
-   help
- Architectures may want to put kprobes instruction slots into
- the dma memory region. E.g. s390 has the kernel image in the
- dma memory region but the module area far away.
-
 config KPROBES_ON_FTRACE
def_bool y
depends on KPROBES  HAVE_KPROBES_ON_FTRACE
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index ce389a9..8a4cae7 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -96,7 +96,6 @@ config S390
select ARCH_WANT_IPC_PARSE_VERSION
select BUILDTIME_EXTABLE_SORT
select CLONE_BACKWARDS2
-   select DMAPROBES if KPROBES
select GENERIC_CLOCKEVENTS
select GENERIC_CPU_DEVICES if !SMP
select GENERIC_SMP_IDLE_THREAD
diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
index bc1071c..cb7ac9e 100644
--- a/arch/s390/kernel/kprobes.c
+++ b/arch/s390/kernel/kprobes.c
@@ -37,6 +37,26 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
 
 struct kretprobe_blackpoint kretprobe_blacklist[] = { };
 
+DEFINE_INSN_CACHE_OPS(dmainsn);
+
+static void *alloc_dmainsn_page(void)
+{
+   return (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
+}
+
+static void free_dmainsn_page(void *page)
+{
+   free_page((unsigned long)page);
+}
+
+struct kprobe_insn_cache kprobe_dmainsn_slots = {
+   .mutex = __MUTEX_INITIALIZER(kprobe_dmainsn_slots.mutex),
+   .alloc = alloc_dmainsn_page,
+   .free = free_dmainsn_page,
+   .pages = LIST_HEAD_INIT(kprobe_dmainsn_slots.pages),
+   .insn_size = MAX_INSN_SIZE,
+};
+
 static int __kprobes is_prohibited_opcode(kprobe_opcode_t *insn)
 {
switch (insn[0]  8) {
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index a5290f6..4e96827 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -266,7 +266,15 @@ extern int arch_init_kprobes(void);
 extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 
-struct kprobe_insn_cache;
+struct kprobe_insn_cache {
+   struct mutex mutex;
+   void *(*alloc)(void);   /* allocate insn page */
+   void (*free)(void *);   /* free insn page */
+   struct list_head pages; /* list of kprobe_insn_page */
+   size_t insn_size;   /* size of instruction slot */
+   int nr_garbage;
+};
+
 extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c);
 extern void __free_insn_slot(struct kprobe_insn_cache *c,
 kprobe_opcode_t *slot, int dirty);
@@ -321,10 +329,6 @@ extern int proc_kprobes_optimization_handler(struct 
ctl_table *table,
 
 #endif /* CONFIG_OPTPROBES */
 
-#ifdef CONFIG_DMAPROBES
-DEFINE_INSN_CACHE_OPS(dmainsn);
-#endif
-
 #ifdef CONFIG_KPROBES_ON_FTRACE
 extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
  struct ftrace_ops *ops, struct pt_regs *regs);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 3b8b073..a0d367a 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -112,9 +112,9 @@ static struct kprobe_blackpoint kprobe_blacklist[] = {
 struct kprobe_insn_page {
struct list_head list;
kprobe_opcode_t *insns; /* Page of instruction slots */
+   struct kprobe_insn_cache *cache;
int nused;
int ngarbage;
-   bool dma_alloc;
char slot_used[];
 };
 
@@ -122,14 +122,6 @@ struct kprobe_insn_page {
(offsetof(struct kprobe_insn_page, slot_used) + \
 (sizeof(char) * (slots)))
 
-struct kprobe_insn_cache {
-   

Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Masami Hiramatsu]

(2013/08/23 17:09), Heiko Carstens wrote:
 On Fri, Aug 23, 2013 at 01:31:23PM +0900, Masami Hiramatsu wrote:
 (2013/08/22 14:52), Heiko Carstens wrote:
 Therefore I need to different insn slot caches, where the slots are either
 allocated with __get_free_page(GFP_KERNEL | GFP_DMA) (for the kernel image)
 or module_alloc(PAGE_SIZE) for modules.

 I can't have a single cache which satifies both areas.

 Oh, I see.
 Indeed, that enough reason to add a new cache... By the way, is there
 any way to implement it without new kconfig like DMAPROBE and dma flag?
 AFAICS, since such flag is strongly depends on the s390 arch, I don't
 like to put it in kernel/kprobes.c.

 Perhaps, we can make insn slot more generic, e.g. create new slot type
 with passing page allocator.
 
 Something like below?
 (only compile tested and on top of the previous patches).

Yes! this is exactly what I thought :)

 I'm not sure, since that would expose struct kprobe_insn_cache.

That is OK, since it is required for some arch.
Could you update the series with this change?

Thank you!

 
  arch/Kconfig   |  7 ---
  arch/s390/Kconfig  |  1 -
  arch/s390/kernel/kprobes.c | 20 ++
  include/linux/kprobes.h| 14 -
  kernel/kprobes.c   | 51 
 --
  5 files changed, 47 insertions(+), 46 deletions(-)
 
 diff --git a/arch/Kconfig b/arch/Kconfig
 index 7010d68..1feb169 100644
 --- a/arch/Kconfig
 +++ b/arch/Kconfig
 @@ -76,13 +76,6 @@ config OPTPROBES
   depends on KPROBES  HAVE_OPTPROBES
   depends on !PREEMPT
  
 -config DMAPROBES
 - bool
 - help
 -   Architectures may want to put kprobes instruction slots into
 -   the dma memory region. E.g. s390 has the kernel image in the
 -   dma memory region but the module area far away.
 -
  config KPROBES_ON_FTRACE
   def_bool y
   depends on KPROBES  HAVE_KPROBES_ON_FTRACE
 diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
 index ce389a9..8a4cae7 100644
 --- a/arch/s390/Kconfig
 +++ b/arch/s390/Kconfig
 @@ -96,7 +96,6 @@ config S390
   select ARCH_WANT_IPC_PARSE_VERSION
   select BUILDTIME_EXTABLE_SORT
   select CLONE_BACKWARDS2
 - select DMAPROBES if KPROBES
   select GENERIC_CLOCKEVENTS
   select GENERIC_CPU_DEVICES if !SMP
   select GENERIC_SMP_IDLE_THREAD
 diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
 index bc1071c..cb7ac9e 100644
 --- a/arch/s390/kernel/kprobes.c
 +++ b/arch/s390/kernel/kprobes.c
 @@ -37,6 +37,26 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
  
  struct kretprobe_blackpoint kretprobe_blacklist[] = { };
  
 +DEFINE_INSN_CACHE_OPS(dmainsn);
 +
 +static void *alloc_dmainsn_page(void)
 +{
 + return (void *)__get_free_page(GFP_KERNEL | GFP_DMA);
 +}
 +
 +static void free_dmainsn_page(void *page)
 +{
 + free_page((unsigned long)page);
 +}
 +
 +struct kprobe_insn_cache kprobe_dmainsn_slots = {
 + .mutex = __MUTEX_INITIALIZER(kprobe_dmainsn_slots.mutex),
 + .alloc = alloc_dmainsn_page,
 + .free = free_dmainsn_page,
 + .pages = LIST_HEAD_INIT(kprobe_dmainsn_slots.pages),
 + .insn_size = MAX_INSN_SIZE,
 +};
 +
  static int __kprobes is_prohibited_opcode(kprobe_opcode_t *insn)
  {
   switch (insn[0]  8) {
 diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
 index a5290f6..4e96827 100644
 --- a/include/linux/kprobes.h
 +++ b/include/linux/kprobes.h
 @@ -266,7 +266,15 @@ extern int arch_init_kprobes(void);
  extern void show_registers(struct pt_regs *regs);
  extern void kprobes_inc_nmissed_count(struct kprobe *p);
  
 -struct kprobe_insn_cache;
 +struct kprobe_insn_cache {
 + struct mutex mutex;
 + void *(*alloc)(void);   /* allocate insn page */
 + void (*free)(void *);   /* free insn page */
 + struct list_head pages; /* list of kprobe_insn_page */
 + size_t insn_size;   /* size of instruction slot */
 + int nr_garbage;
 +};
 +
  extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c);
  extern void __free_insn_slot(struct kprobe_insn_cache *c,
kprobe_opcode_t *slot, int dirty);
 @@ -321,10 +329,6 @@ extern int proc_kprobes_optimization_handler(struct 
 ctl_table *table,
  
  #endif /* CONFIG_OPTPROBES */
  
 -#ifdef CONFIG_DMAPROBES
 -DEFINE_INSN_CACHE_OPS(dmainsn);
 -#endif
 -
  #ifdef CONFIG_KPROBES_ON_FTRACE
  extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
 struct ftrace_ops *ops, struct pt_regs *regs);
 diff --git a/kernel/kprobes.c b/kernel/kprobes.c
 index 3b8b073..a0d367a 100644
 --- a/kernel/kprobes.c
 +++ b/kernel/kprobes.c
 @@ -112,9 +112,9 @@ static struct kprobe_blackpoint kprobe_blacklist[] = {
  struct kprobe_insn_page {
   struct list_head list;
   kprobe_opcode_t *insns; /* Page of instruction slots */
 + struct kprobe_insn_cache *cache;
   int nused;
   int ngarbage;
 - 

Re: Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Masami Hiramatsu]

(2013/08/22 14:52), Heiko Carstens wrote:
> Hi Masami,
> 
>> (2013/08/21 21:01), Heiko Carstens wrote:
>>> The current kpropes insn caches allocate memory areas for insn slots with
>>> module_alloc(). The assumption is that the kernel image and module area
>>> are both within the same +/- 2GB memory area.
>>> This however is not true for s390 where the kernel image resides within
>>> the first 2GB (DMA memory area), but the module area is far away in the
>>> vmalloc area, usually somewhere close below the 4TB area.
>>>
>>> For new pc relative instructions s390 needs insn slots that are within
>>> +/- 2GB of each area. That way we can patch displacements of pc-relative
>>> instructions within the insn slots just like x86 and powerpc.
>>>
>>> The module area works already with the normal insn slot allocator, however
>>> there is currently no way to get insn slots that are within the first 2GB
>>> on s390 (aka DMA area).
>>
>> The reason why we allocate instruction buffers from module area is
>> to execute a piece of code on the buffer, which should be executable.
>> I'm not good for s390, is that allows kernel to execute the code
>> on such DMA buffer?
> 
> Yes, the kernel image itself resides in DMA capable memory and it is all
> executable.
> 
>>> Therefore this patch set introduces a third insn slot cache besides the
>>> normal insn and optinsn slot caches: the dmainsn slot cache. Slots can be
>>> allocated and freed with get_dmainsn_slot() and free_dmainsn_slot().
>>
>> OK, but it seems that your patch introduced unneeded complexity. Perhaps,
>> you just have to introduce 2 weak functions to allocate/release such
>> executable and jump-able buffers, like below,
>>
>> void * __weak arch_allocate_executable_page(void)
>> {
>>  return module_alloc(PAGE_SIZE);
>> }
>>
>> void __weak arch_free_executable_page(void *page)
>> {
>>  module_free(NULL, page);
>> }
>>
>> Thus, all you need to do is implementing dmaalloc() version of above
>> functions on s390. No kconfig, no ifdefs are needed. :)
> 
> Hm, I don't see how that can work, or maybe I just don't get your idea ;)
> Or maybe my intention was not clear? So let me try again:
> 
> If the to be probed instruction resides within the first 2GB of memory
> (aka DMA memory, aka kernel image) the insn slot must be within the first
> 2GB as well, otherwise I can't patch pc-relative instructions.
> 
> On the other hand if the to be probed instruction resides in a module
> (aka part of the vmalloc area), the insn slot must reside within the same
> 2GB area as well.
> 
> Therefore I need to different insn slot caches, where the slots are either
> allocated with __get_free_page(GFP_KERNEL | GFP_DMA) (for the kernel image)
> or module_alloc(PAGE_SIZE) for modules.
> 
> I can't have a single cache which satifies both areas.

Oh, I see.
Indeed, that enough reason to add a new cache... By the way, is there
any way to implement it without new kconfig like DMAPROBE and dma flag?
AFAICS, since such flag is strongly depends on the s390 arch, I don't
like to put it in kernel/kprobes.c.

Perhaps, we can make insn slot more generic, e.g. create new slot type
with passing page allocator.

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com


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

Re: Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Masami Hiramatsu]

(2013/08/22 14:52), Heiko Carstens wrote:
 Hi Masami,
 
 (2013/08/21 21:01), Heiko Carstens wrote:
 The current kpropes insn caches allocate memory areas for insn slots with
 module_alloc(). The assumption is that the kernel image and module area
 are both within the same +/- 2GB memory area.
 This however is not true for s390 where the kernel image resides within
 the first 2GB (DMA memory area), but the module area is far away in the
 vmalloc area, usually somewhere close below the 4TB area.

 For new pc relative instructions s390 needs insn slots that are within
 +/- 2GB of each area. That way we can patch displacements of pc-relative
 instructions within the insn slots just like x86 and powerpc.

 The module area works already with the normal insn slot allocator, however
 there is currently no way to get insn slots that are within the first 2GB
 on s390 (aka DMA area).

 The reason why we allocate instruction buffers from module area is
 to execute a piece of code on the buffer, which should be executable.
 I'm not good for s390, is that allows kernel to execute the code
 on such DMA buffer?
 
 Yes, the kernel image itself resides in DMA capable memory and it is all
 executable.
 
 Therefore this patch set introduces a third insn slot cache besides the
 normal insn and optinsn slot caches: the dmainsn slot cache. Slots can be
 allocated and freed with get_dmainsn_slot() and free_dmainsn_slot().

 OK, but it seems that your patch introduced unneeded complexity. Perhaps,
 you just have to introduce 2 weak functions to allocate/release such
 executable and jump-able buffers, like below,

 void * __weak arch_allocate_executable_page(void)
 {
  return module_alloc(PAGE_SIZE);
 }

 void __weak arch_free_executable_page(void *page)
 {
  module_free(NULL, page);
 }

 Thus, all you need to do is implementing dmaalloc() version of above
 functions on s390. No kconfig, no ifdefs are needed. :)
 
 Hm, I don't see how that can work, or maybe I just don't get your idea ;)
 Or maybe my intention was not clear? So let me try again:
 
 If the to be probed instruction resides within the first 2GB of memory
 (aka DMA memory, aka kernel image) the insn slot must be within the first
 2GB as well, otherwise I can't patch pc-relative instructions.
 
 On the other hand if the to be probed instruction resides in a module
 (aka part of the vmalloc area), the insn slot must reside within the same
 2GB area as well.
 
 Therefore I need to different insn slot caches, where the slots are either
 allocated with __get_free_page(GFP_KERNEL | GFP_DMA) (for the kernel image)
 or module_alloc(PAGE_SIZE) for modules.
 
 I can't have a single cache which satifies both areas.

Oh, I see.
Indeed, that enough reason to add a new cache... By the way, is there
any way to implement it without new kconfig like DMAPROBE and dma flag?
AFAICS, since such flag is strongly depends on the s390 arch, I don't
like to put it in kernel/kprobes.c.

Perhaps, we can make insn slot more generic, e.g. create new slot type
with passing page allocator.

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com


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

Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Heiko Carstens]

Hi Masami,

> (2013/08/21 21:01), Heiko Carstens wrote:
> > The current kpropes insn caches allocate memory areas for insn slots with
> > module_alloc(). The assumption is that the kernel image and module area
> > are both within the same +/- 2GB memory area.
> > This however is not true for s390 where the kernel image resides within
> > the first 2GB (DMA memory area), but the module area is far away in the
> > vmalloc area, usually somewhere close below the 4TB area.
> > 
> > For new pc relative instructions s390 needs insn slots that are within
> > +/- 2GB of each area. That way we can patch displacements of pc-relative
> > instructions within the insn slots just like x86 and powerpc.
> > 
> > The module area works already with the normal insn slot allocator, however
> > there is currently no way to get insn slots that are within the first 2GB
> > on s390 (aka DMA area).
> 
> The reason why we allocate instruction buffers from module area is
> to execute a piece of code on the buffer, which should be executable.
> I'm not good for s390, is that allows kernel to execute the code
> on such DMA buffer?

Yes, the kernel image itself resides in DMA capable memory and it is all
executable.

> > Therefore this patch set introduces a third insn slot cache besides the
> > normal insn and optinsn slot caches: the dmainsn slot cache. Slots can be
> > allocated and freed with get_dmainsn_slot() and free_dmainsn_slot().
> 
> OK, but it seems that your patch introduced unneeded complexity. Perhaps,
> you just have to introduce 2 weak functions to allocate/release such
> executable and jump-able buffers, like below,
> 
> void * __weak arch_allocate_executable_page(void)
> {
>   return module_alloc(PAGE_SIZE);
> }
> 
> void __weak arch_free_executable_page(void *page)
> {
>   module_free(NULL, page);
> }
> 
> Thus, all you need to do is implementing dmaalloc() version of above
> functions on s390. No kconfig, no ifdefs are needed. :)

Hm, I don't see how that can work, or maybe I just don't get your idea ;)
Or maybe my intention was not clear? So let me try again:

If the to be probed instruction resides within the first 2GB of memory
(aka DMA memory, aka kernel image) the insn slot must be within the first
2GB as well, otherwise I can't patch pc-relative instructions.

On the other hand if the to be probed instruction resides in a module
(aka part of the vmalloc area), the insn slot must reside within the same
2GB area as well.

Therefore I need to different insn slot caches, where the slots are either
allocated with __get_free_page(GFP_KERNEL | GFP_DMA) (for the kernel image)
or module_alloc(PAGE_SIZE) for modules.

I can't have a single cache which satifies both areas.

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

Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Masami Hiramatsu]

Hi Heiko,

(2013/08/21 21:01), Heiko Carstens wrote:
> The current kpropes insn caches allocate memory areas for insn slots with
> module_alloc(). The assumption is that the kernel image and module area
> are both within the same +/- 2GB memory area.
> This however is not true for s390 where the kernel image resides within
> the first 2GB (DMA memory area), but the module area is far away in the
> vmalloc area, usually somewhere close below the 4TB area.
> 
> For new pc relative instructions s390 needs insn slots that are within
> +/- 2GB of each area. That way we can patch displacements of pc-relative
> instructions within the insn slots just like x86 and powerpc.
> 
> The module area works already with the normal insn slot allocator, however
> there is currently no way to get insn slots that are within the first 2GB
> on s390 (aka DMA area).

The reason why we allocate instruction buffers from module area is
to execute a piece of code on the buffer, which should be executable.
I'm not good for s390, is that allows kernel to execute the code
on such DMA buffer?

> Therefore this patch set introduces a third insn slot cache besides the
> normal insn and optinsn slot caches: the dmainsn slot cache. Slots can be
> allocated and freed with get_dmainsn_slot() and free_dmainsn_slot().

OK, but it seems that your patch introduced unneeded complexity. Perhaps,
you just have to introduce 2 weak functions to allocate/release such
executable and jump-able buffers, like below,

void * __weak arch_allocate_executable_page(void)
{
return module_alloc(PAGE_SIZE);
}

void __weak arch_free_executable_page(void *page)
{
module_free(NULL, page);
}

Thus, all you need to do is implementing dmaalloc() version of above
functions on s390. No kconfig, no ifdefs are needed. :)

> 
> Patch 1 unifies the current insn and optinsn caches implementation so we
> don't end up with a lot of code duplication when adding a third cache.
> 
> Patch 2 simply adds the new dmainsn slot cache.
> 
> Patch 3 is the s390 usage of the new cache.
> 
> Looking at the last couple of sign-off chains I'm not sure how kprobes
> patches should go upstream.. Andrew, Ingo, or simply via the s390 tree?

Hmm, AFAIK, currently all noarch kprobes works go to -tip tree, and
the arch dependent parts go to each arch tree (only x86 goes to -tip tree).

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com


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

Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Masami Hiramatsu]

Hi Heiko,

(2013/08/21 21:01), Heiko Carstens wrote:
 The current kpropes insn caches allocate memory areas for insn slots with
 module_alloc(). The assumption is that the kernel image and module area
 are both within the same +/- 2GB memory area.
 This however is not true for s390 where the kernel image resides within
 the first 2GB (DMA memory area), but the module area is far away in the
 vmalloc area, usually somewhere close below the 4TB area.
 
 For new pc relative instructions s390 needs insn slots that are within
 +/- 2GB of each area. That way we can patch displacements of pc-relative
 instructions within the insn slots just like x86 and powerpc.
 
 The module area works already with the normal insn slot allocator, however
 there is currently no way to get insn slots that are within the first 2GB
 on s390 (aka DMA area).

The reason why we allocate instruction buffers from module area is
to execute a piece of code on the buffer, which should be executable.
I'm not good for s390, is that allows kernel to execute the code
on such DMA buffer?

 Therefore this patch set introduces a third insn slot cache besides the
 normal insn and optinsn slot caches: the dmainsn slot cache. Slots can be
 allocated and freed with get_dmainsn_slot() and free_dmainsn_slot().

OK, but it seems that your patch introduced unneeded complexity. Perhaps,
you just have to introduce 2 weak functions to allocate/release such
executable and jump-able buffers, like below,

void * __weak arch_allocate_executable_page(void)
{
return module_alloc(PAGE_SIZE);
}

void __weak arch_free_executable_page(void *page)
{
module_free(NULL, page);
}

Thus, all you need to do is implementing dmaalloc() version of above
functions on s390. No kconfig, no ifdefs are needed. :)

 
 Patch 1 unifies the current insn and optinsn caches implementation so we
 don't end up with a lot of code duplication when adding a third cache.
 
 Patch 2 simply adds the new dmainsn slot cache.
 
 Patch 3 is the s390 usage of the new cache.
 
 Looking at the last couple of sign-off chains I'm not sure how kprobes
 patches should go upstream.. Andrew, Ingo, or simply via the s390 tree?

Hmm, AFAIK, currently all noarch kprobes works go to -tip tree, and
the arch dependent parts go to each arch tree (only x86 goes to -tip tree).

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com


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

Re: [PATCH 0/3] kprobes: add new dma insn slot cache for s390

[Heiko Carstens]

Hi Masami,

 (2013/08/21 21:01), Heiko Carstens wrote:
  The current kpropes insn caches allocate memory areas for insn slots with
  module_alloc(). The assumption is that the kernel image and module area
  are both within the same +/- 2GB memory area.
  This however is not true for s390 where the kernel image resides within
  the first 2GB (DMA memory area), but the module area is far away in the
  vmalloc area, usually somewhere close below the 4TB area.
  
  For new pc relative instructions s390 needs insn slots that are within
  +/- 2GB of each area. That way we can patch displacements of pc-relative
  instructions within the insn slots just like x86 and powerpc.
  
  The module area works already with the normal insn slot allocator, however
  there is currently no way to get insn slots that are within the first 2GB
  on s390 (aka DMA area).
 
 The reason why we allocate instruction buffers from module area is
 to execute a piece of code on the buffer, which should be executable.
 I'm not good for s390, is that allows kernel to execute the code
 on such DMA buffer?

Yes, the kernel image itself resides in DMA capable memory and it is all
executable.

  Therefore this patch set introduces a third insn slot cache besides the
  normal insn and optinsn slot caches: the dmainsn slot cache. Slots can be
  allocated and freed with get_dmainsn_slot() and free_dmainsn_slot().
 
 OK, but it seems that your patch introduced unneeded complexity. Perhaps,
 you just have to introduce 2 weak functions to allocate/release such
 executable and jump-able buffers, like below,
 
 void * __weak arch_allocate_executable_page(void)
 {
   return module_alloc(PAGE_SIZE);
 }
 
 void __weak arch_free_executable_page(void *page)
 {
   module_free(NULL, page);
 }
 
 Thus, all you need to do is implementing dmaalloc() version of above
 functions on s390. No kconfig, no ifdefs are needed. :)

Hm, I don't see how that can work, or maybe I just don't get your idea ;)
Or maybe my intention was not clear? So let me try again:

If the to be probed instruction resides within the first 2GB of memory
(aka DMA memory, aka kernel image) the insn slot must be within the first
2GB as well, otherwise I can't patch pc-relative instructions.

On the other hand if the to be probed instruction resides in a module
(aka part of the vmalloc area), the insn slot must reside within the same
2GB area as well.

Therefore I need to different insn slot caches, where the slots are either
allocated with __get_free_page(GFP_KERNEL | GFP_DMA) (for the kernel image)
or module_alloc(PAGE_SIZE) for modules.

I can't have a single cache which satifies both areas.

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