| 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
| ||
|
Date: Tue, 19 Jul 2022 20:11:07 +0800
From: Qi Zheng <zhengqi.arch@...edance.com>
To: Mark Rutland <mark.rutland@....com>
Cc: Arnd Bergmann <arnd@...db.de>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] arm64: support HAVE_IRQ_EXIT_ON_IRQ_STACK
On 2022/7/19 19:24, Mark Rutland wrote:
> On Thu, Jul 14, 2022 at 01:37:31PM +0200, Arnd Bergmann wrote:
>> On Fri, Jul 8, 2022 at 11:49 AM Qi Zheng <zhengqi.arch@...edance.com> wrote:
>>>
>>> Since softirqs are handled on the per-CPU IRQ stack,
>>> let's support HAVE_IRQ_EXIT_ON_IRQ_STACK which causes
>>> the core code to invoke __do_softirq() directly without
>>> going through do_softirq_own_stack().
>>>
>>> Signed-off-by: Qi Zheng <zhengqi.arch@...edance.com>
>>
>> Adding Mark Rutland to Cc, he's the one that worked on this area the most in the
>> past and should probably review your patch. I still feel like there
>> should be a way
>> to improve readability of the IRQ entry path rather than just adding another
>> level of indirection, but the ideas I had so far have not led to
>> anything useful.
>>
>> Overall I suppose your version is an improvement over the extra double stack
>> switch when entering softirq.
Hi Mark,
>
> The cost of the switch is fairly trivial, so performance-wise I would expect
> that to fall within the noise. So if this is just about avoiding the extra
> switch, I'd prefer to leave this as-is.
In this patch, I just want to avoid the extra switch.
>
> If we want to move more work after the stack switch, I think we'd want a more
> in depth rethink of the structure of this code (since e.g. we could make the
> common case have almost everything on the IRQ stack, at the cost of making
For now, I don't think it's necessary to put anything other than
irq_{enter|exit}_rcu() on the IRQ stack. For things that we want
to run on the IRQ stack in the future, we can also easily stuff
them into do_handler(). So I think the method of nesting pointers
is good for extensibility except that it is not easy to read. And
this method does not change much. :)
> preemption require more work).
Overall, I'm ok to drop this optimized patch if we don't care about
the performance overhead of extra switching.
Thanks,
Qi
>
> Thanks,
> Mark.
>
>>
>> Arnd
>>
>>> ---
>>> arch/arm64/Kconfig | 1 +
>>> arch/arm64/include/asm/exception.h | 4 +++-
>>> arch/arm64/kernel/entry-common.c | 30 ++++++++++++++++++++----------
>>> arch/arm64/kernel/entry.S | 6 ++++--
>>> arch/arm64/kernel/irq.c | 5 +++--
>>> 5 files changed, 31 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index be0a9f0052ee..d2cc7daecce3 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -231,6 +231,7 @@ config ARM64
>>> select TRACE_IRQFLAGS_SUPPORT
>>> select TRACE_IRQFLAGS_NMI_SUPPORT
>>> select HAVE_SOFTIRQ_ON_OWN_STACK
>>> + select HAVE_IRQ_EXIT_ON_IRQ_STACK
>>> help
>>> ARM 64-bit (AArch64) Linux support.
>>>
>>> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
>>> index d94aecff9690..8bff0aa7ab50 100644
>>> --- a/arch/arm64/include/asm/exception.h
>>> +++ b/arch/arm64/include/asm/exception.h
>>> @@ -54,7 +54,9 @@ asmlinkage void el0t_32_fiq_handler(struct pt_regs *regs);
>>> asmlinkage void el0t_32_error_handler(struct pt_regs *regs);
>>>
>>> asmlinkage void call_on_irq_stack(struct pt_regs *regs,
>>> - void (*func)(struct pt_regs *));
>>> + void (*func)(struct pt_regs *),
>>> + void (*do_func)(struct pt_regs *,
>>> + void (*)(struct pt_regs *)));
>>> asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
>>>
>>> void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
>>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
>>> index c75ca36b4a49..935d1ab150b5 100644
>>> --- a/arch/arm64/kernel/entry-common.c
>>> +++ b/arch/arm64/kernel/entry-common.c
>>> @@ -266,14 +266,16 @@ static void __sched arm64_preempt_schedule_irq(void)
>>> }
>>>
>>> static void do_interrupt_handler(struct pt_regs *regs,
>>> - void (*handler)(struct pt_regs *))
>>> + void (*handler)(struct pt_regs *),
>>> + void (*do_handler)(struct pt_regs *,
>>> + void (*)(struct pt_regs *)))
>>> {
>>> struct pt_regs *old_regs = set_irq_regs(regs);
>>>
>>> if (on_thread_stack())
>>> - call_on_irq_stack(regs, handler);
>>> + call_on_irq_stack(regs, handler, do_handler);
>>> else
>>> - handler(regs);
>>> + do_handler(regs, handler);
>>>
>>> set_irq_regs(old_regs);
>>> }
>>> @@ -441,22 +443,32 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
>>> }
>>> }
>>>
>>> +static void nmi_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
>>> +{
>>> + handler(regs);
>>> +}
>>> +
>>> static __always_inline void __el1_pnmi(struct pt_regs *regs,
>>> void (*handler)(struct pt_regs *))
>>> {
>>> arm64_enter_nmi(regs);
>>> - do_interrupt_handler(regs, handler);
>>> + do_interrupt_handler(regs, handler, nmi_handler);
>>> arm64_exit_nmi(regs);
>>> }
>>>
>>> +static void irq_handler(struct pt_regs *regs, void (*handler)(struct pt_regs *))
>>> +{
>>> + irq_enter_rcu();
>>> + handler(regs);
>>> + irq_exit_rcu();
>>> +}
>>> +
>>> static __always_inline void __el1_irq(struct pt_regs *regs,
>>> void (*handler)(struct pt_regs *))
>>> {
>>> enter_from_kernel_mode(regs);
>>>
>>> - irq_enter_rcu();
>>> - do_interrupt_handler(regs, handler);
>>> - irq_exit_rcu();
>>> + do_interrupt_handler(regs, handler, irq_handler);
>>>
>>> arm64_preempt_schedule_irq();
>>>
>>> @@ -699,9 +711,7 @@ static void noinstr el0_interrupt(struct pt_regs *regs,
>>> if (regs->pc & BIT(55))
>>> arm64_apply_bp_hardening();
>>>
>>> - irq_enter_rcu();
>>> - do_interrupt_handler(regs, handler);
>>> - irq_exit_rcu();
>>> + do_interrupt_handler(regs, handler, irq_handler);
>>>
>>> exit_to_user_mode(regs);
>>> }
>>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>>> index 254fe31c03a0..1c351391f6bd 100644
>>> --- a/arch/arm64/kernel/entry.S
>>> +++ b/arch/arm64/kernel/entry.S
>>> @@ -867,7 +867,9 @@ NOKPROBE(ret_from_fork)
>>>
>>> /*
>>> * void call_on_irq_stack(struct pt_regs *regs,
>>> - * void (*func)(struct pt_regs *));
>>> + * void (*func)(struct pt_regs *)
>>> + * void (*do_func)(struct pt_regs *,
>>> + * void (*)(struct pt_regs *)));
>>> *
>>> * Calls func(regs) using this CPU's irq stack and shadow irq stack.
>>> */
>>> @@ -886,7 +888,7 @@ SYM_FUNC_START(call_on_irq_stack)
>>>
>>> /* Move to the new stack and call the function there */
>>> mov sp, x16
>>> - blr x1
>>> + blr x2
>>>
>>> /*
>>> * Restore the SP from the FP, and restore the FP and LR from the frame
>>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
>>> index c36ad20a52f3..003db605bc4f 100644
>>> --- a/arch/arm64/kernel/irq.c
>>> +++ b/arch/arm64/kernel/irq.c
>>> @@ -73,14 +73,15 @@ static void init_irq_stacks(void)
>>> #endif
>>>
>>> #ifndef CONFIG_PREEMPT_RT
>>> -static void ____do_softirq(struct pt_regs *regs)
>>> +static void ____do_softirq(struct pt_regs *regs,
>>> + void (*handler)(struct pt_regs *))
>>> {
>>> __do_softirq();
>>> }
>>>
>>> void do_softirq_own_stack(void)
>>> {
>>> - call_on_irq_stack(NULL, ____do_softirq);
>>> + call_on_irq_stack(NULL, NULL, ____do_softirq);
>>> }
>>> #endif
>>>
>>> --
>>> 2.20.1
>>>
--
Thanks,
Qi
Powered by blists - more mailing lists