| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <2409A0E9-376A-4B49-8C5F-73084843096F@gmail.com>
Date: Tue, 8 Sep 2015 23:54:09 +0900
From: Jungseok Lee <jungseoklee85@...il.com>
To: James Morse <james.morse@....com>
Cc: Catalin Marinas <Catalin.Marinas@....com>,
Will Deacon <Will.Deacon@....com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: kernel: Use a separate stack for irq interrupts.
On Sep 7, 2015, at 11:36 PM, James Morse wrote:
Hi James,
> Having to handle interrupts on top of an existing kernel stack means the
> kernel stack must be large enough to accomodate both the maximum kernel
> usage, and the maximum irq handler usage. Switching to a different stack
> when processing irqs allows us to make the stack size smaller.
>
> Maximum kernel stack usage (running ltp and generating usb+ethernet
> interrupts) was 7256 bytes. With this patch, the same workload gives
> a maximum stack usage of 5816 bytes.
>
> Signed-off-by: James Morse <james.morse@....com>
> ---
> arch/arm64/include/asm/irq.h | 12 +++++++++
> arch/arm64/include/asm/thread_info.h | 8 ++++--
> arch/arm64/kernel/entry.S | 33 ++++++++++++++++++++---
> arch/arm64/kernel/irq.c | 52 ++++++++++++++++++++++++++++++++++++
> arch/arm64/kernel/smp.c | 4 +++
> arch/arm64/kernel/stacktrace.c | 4 ++-
> 6 files changed, 107 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
> index bbb251b14746..050d4196c736 100644
> --- a/arch/arm64/include/asm/irq.h
> +++ b/arch/arm64/include/asm/irq.h
> @@ -2,14 +2,20 @@
> #define __ASM_IRQ_H
>
> #include <linux/irqchip/arm-gic-acpi.h>
> +#include <linux/percpu.h>
>
> #include <asm-generic/irq.h>
> +#include <asm/thread_info.h>
> +
> +DECLARE_PER_CPU(unsigned long, irq_sp);
>
> struct pt_regs;
>
> extern void migrate_irqs(void);
> extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
>
> +extern int alloc_irq_stack(unsigned int cpu);
> +
> static inline void acpi_irq_init(void)
> {
> /*
> @@ -21,4 +27,10 @@ static inline void acpi_irq_init(void)
> }
> #define acpi_irq_init acpi_irq_init
>
> +static inline bool is_irq_stack(unsigned long sp)
> +{
> + struct thread_info *ti = get_thread_info(sp);
> + return (get_thread_info(per_cpu(irq_sp, ti->cpu)) == ti);
> +}
> +
> #endif
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index dcd06d18a42a..b906254fc400 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -69,12 +69,16 @@ register unsigned long current_stack_pointer asm ("sp");
> /*
> * how to get the thread information struct from C
> */
> +static inline struct thread_info *get_thread_info(unsigned long sp)
> +{
> + return (struct thread_info *)(sp & ~(THREAD_SIZE - 1));
> +}
> +
> static inline struct thread_info *current_thread_info(void) __attribute_const__;
>
> static inline struct thread_info *current_thread_info(void)
> {
> - return (struct thread_info *)
> - (current_stack_pointer & ~(THREAD_SIZE - 1));
> + return get_thread_info(current_stack_pointer);
> }
>
> #define thread_saved_pc(tsk) \
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index e16351819fed..d42371f3f5a1 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -190,10 +190,37 @@ tsk .req x28 // current thread_info
> * Interrupt handling.
> */
> .macro irq_handler
> - adrp x1, handle_arch_irq
> - ldr x1, [x1, #:lo12:handle_arch_irq]
> - mov x0, sp
> + mrs x21, tpidr_el1
> + adr_l x20, irq_sp
> + add x20, x20, x21
> +
> + ldr x21, [x20]
> + mov x20, sp
> +
> + mov x0, x21
> + mov x1, x20
> + bl irq_copy_thread_info
> +
> + /* test for recursive use of irq_sp */
> + cbz w0, 1f
> + mrs x30, elr_el1
> + mov sp, x21
> +
> + /*
> + * Create a fake stack frame to bump unwind_frame() onto the original
> + * stack. This relies on x29 not being clobbered by kernel_entry().
> + */
> + push x29, x30
It is the most challenging item to handle a frame pointer in this context.
As I mentioned previously, a stack tracer on ftrace is not supported yet.
How about decoupling "IRQ Stack feature" from "Stack Tracer Fix"?
> +
> +1: ldr_l x1, handle_arch_irq
> + mov x0, x20
> blr x1
> +
> + mov x0, x20
> + mov x1, x21
> + bl irq_copy_thread_info
> + mov sp, x20
> +
> .endm
>
> .text
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index 463fa2e7e34c..10b57a006da8 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -26,11 +26,14 @@
> #include <linux/smp.h>
> #include <linux/init.h>
> #include <linux/irqchip.h>
> +#include <linux/percpu.h>
> #include <linux/seq_file.h>
> #include <linux/ratelimit.h>
>
> unsigned long irq_err_count;
>
> +DEFINE_PER_CPU(unsigned long, irq_sp) = 0;
> +
> int arch_show_interrupts(struct seq_file *p, int prec)
> {
> #ifdef CONFIG_SMP
> @@ -55,6 +58,10 @@ void __init init_IRQ(void)
> irqchip_init();
> if (!handle_arch_irq)
> panic("No interrupt controller found.");
> +
> + /* Allocate an irq stack for the boot cpu */
> + if (alloc_irq_stack(smp_processor_id()))
> + panic("Failed to allocate irq stack for boot cpu.");
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
> @@ -117,3 +124,48 @@ void migrate_irqs(void)
> local_irq_restore(flags);
> }
> #endif /* CONFIG_HOTPLUG_CPU */
> +
> +/* Allocate an irq_stack for a cpu that is about to be brought up. */
> +int alloc_irq_stack(unsigned int cpu)
> +{
> + struct page *irq_stack_page;
> + union thread_union *irq_stack;
> +
> + /* reuse stack allocated previously */
> + if (per_cpu(irq_sp, cpu))
> + return 0;
I'd like to avoid even this simple check since CPU hotplug could be heavily
used for power management.
> +
> + irq_stack_page = alloc_kmem_pages(THREADINFO_GFP, THREAD_SIZE_ORDER);
> + if (!irq_stack_page) {
> + pr_crit("CPU%u: failed to allocate irq stack for cpu %u\n",
> + smp_processor_id(), cpu);
> + return -ENOMEM;
> + }
> + irq_stack = page_address(irq_stack_page);
> +
> + per_cpu(irq_sp, cpu) = (unsigned long)irq_stack->stack
> + + THREAD_START_SP;
> +
> + return 0;
> +}
> +
> +/*
> + * Copy struct thread_info between two stacks, and update current->stack.
> + * This is used when moving to the irq exception stack.
> + * Changing current->stack is necessary so that non-arch thread_info writers
> + * don't use the new thread_info->task->stack to find the old thread_info when
> + * setting flags like TIF_NEED_RESCHED...
> + */
> +asmlinkage int irq_copy_thread_info(unsigned long dst_sp, unsigned long src_sp)
> +{
> + struct thread_info *src = get_thread_info(src_sp);
> + struct thread_info *dst = get_thread_info(dst_sp);
> +
> + if (dst == src)
> + return 0;
> +
> + *dst = *src;
> + current->stack = dst;
> +
> + return 1;
> +}
Although this is 32-byte memcpy, sizeof(struct thread_info), this function is called
twice to handle a single interrupt. Isn's it too expensive?
This is a major difference between my approach and this patch. I think we should get
some feedbacks on struct thread_info information management for v2 preparation.
Best Regards
Jungseok Lee--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists