2020-10-08 08:31:34

by Maninder Singh

[permalink] [raw]
Subject: [PATCH 2/3] arm: introduce IRQ stacks

This patch adds code for switching to IRQ stack.
IRQ stack and Kernel SVC stack have below design.

IRQ STACK:-
------------ IRQ stack top
| |
------------
. .
. .
. .
------------
| sp | <- irq_stack_base + 0x8
------------
| fp | <- irq_stack_base + 0x4
------------
|tinfo_ptr | /* pointer to thread info */
irq_stack_ptr --> ------------ IRQ stack base

Kernel SVC stack:-
------------ Kernel stack top
| |
------------
. .
. .
. .
------------
| |
| |
------------
|tinfo_ptr | /* pointer to thread info */
------------ Kernel stack base

Co-developed-by: Vaneet Narang <[email protected]>
Signed-off-by: Vaneet Narang <[email protected]>
Signed-off-by: Maninder Singh <[email protected]>
---
arch/arm/include/asm/assembler.h | 8 ++++++++
arch/arm/include/asm/irq.h | 6 ++++++
arch/arm/kernel/entry-armv.S | 41 +++++++++++++++++++++++++++++++++++++++-
arch/arm/kernel/irq.c | 24 +++++++++++++++++++++++
4 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 8512bdc..82ee6ee 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -212,6 +212,14 @@
#endif
.endm

+ .macro this_cpu_ptr, sym, reg, tmp
+ ldr \reg, =\sym
+#if defined(CONFIG_SMP) && !defined(CONFIG_CPU_V6)
+ mrc p15, 0, \tmp, cr13, c0, 4
+ add \reg, \reg, \tmp
+#endif
+ .endm
+
/*
* Increment/decrement the preempt count.
*/
diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h
index 46d4114..f3299ab 100644
--- a/arch/arm/include/asm/irq.h
+++ b/arch/arm/include/asm/irq.h
@@ -22,10 +22,16 @@
#define NO_IRQ ((unsigned int)(-1))
#endif

+#define IRQ_STACK_SIZE THREAD_SIZE
+
#ifndef __ASSEMBLY__
struct irqaction;
struct pt_regs;

+#ifdef CONFIG_IRQ_STACK
+DECLARE_PER_CPU(unsigned long *, irq_stack_ptr);
+#endif
+
extern void asm_do_IRQ(unsigned int, struct pt_regs *);
void handle_IRQ(unsigned int, struct pt_regs *);
void init_IRQ(void);
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 55a47df..13a5889 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -32,6 +32,43 @@
#include "entry-header.S"
#include <asm/entry-macro-multi.S>
#include <asm/probes.h>
+#ifdef CONFIG_IRQ_STACK
+#include <asm/irq.h>
+#endif
+
+ .macro irq_stack_entry
+#ifdef CONFIG_IRQ_STACK
+ mov r6, sp /* preserve sp */
+
+ this_cpu_ptr irq_stack_ptr, r7, r8
+ ldr r7, [r7]
+ mov r8, r7
+
+ /*
+ * Compare sp with base of IRQ stack.
+ * if the top ~(#THREAD_SIZE_ORDER + PAGE_SHIFT) bits match,
+ * we are on a irq stack.
+ */
+ eor r8, r8, sp
+ lsrs r8, #THREAD_SIZE_ORDER + PAGE_SHIFT
+ beq 9998f
+
+ /*
+ * store thread info pointer on IRQ stack and
+ * switch to the irq stack.
+ */
+ get_thread_info r8
+ stm r7, {r8, fp, sp}
+ add sp, r7, #IRQ_STACK_SIZE
+9998:
+#endif
+ .endm
+
+ .macro irq_stack_exit
+#ifdef CONFIG_IRQ_STACK
+ mov sp, r6 /* restore sp */
+#endif
+ .endm

/*
* Interrupt handling.
@@ -41,11 +78,13 @@
ldr r1, =handle_arch_irq
mov r0, sp
badr lr, 9997f
+ irq_stack_entry
ldr pc, [r1]
+9997:
+ irq_stack_exit
#else
arch_irq_handler_default
#endif
-9997:
.endm

.macro pabt_helper
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index 698b6f6..79872e5 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -43,6 +43,15 @@

unsigned long irq_err_count;

+#ifdef CONFIG_IRQ_STACK
+DEFINE_PER_CPU(unsigned long *, irq_stack_ptr);
+
+/*
+ * irq_stack must be IRQ_STACK_SIZE(THREAD_SIZE) aligned,
+ */
+DEFINE_PER_CPU(unsigned long [IRQ_STACK_SIZE/sizeof(long)], irq_stack) __aligned(IRQ_STACK_SIZE);
+#endif
+
int arch_show_interrupts(struct seq_file *p, int prec)
{
#ifdef CONFIG_FIQ
@@ -55,6 +64,20 @@ int arch_show_interrupts(struct seq_file *p, int prec)
return 0;
}

+#ifdef CONFIG_IRQ_STACK
+static void init_irq_stacks(void)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu)
+ per_cpu(irq_stack_ptr, cpu) = per_cpu(irq_stack, cpu);
+}
+#else
+static inline void init_irq_stacks(void)
+{
+}
+#endif
+
/*
* handle_IRQ handles all hardware IRQ's. Decoded IRQs should
* not come via this function. Instead, they should provide their
@@ -79,6 +102,7 @@ void __init init_IRQ(void)
{
int ret;

+ init_irq_stacks();
if (IS_ENABLED(CONFIG_OF) && !machine_desc->init_irq)
irqchip_init();
else
--
1.9.1


2020-10-21 14:11:19

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

On Wed, Oct 21, 2020 at 02:42:48PM +0200, Arnd Bergmann wrote:
> (also resending this reply from @kernel.org)
>
> On Fri, Oct 16, 2020 at 12:19 PM Arnd Bergmann <[email protected]> wrote:
> > On Thu, Oct 8, 2020 at 9:20 AM Maninder Singh <[email protected]> wrote:
> > >
> > > This patch adds code for switching to IRQ stack.
> > > IRQ stack and Kernel SVC stack have below design.
> > >
> > > IRQ STACK:-
> > > ------------ IRQ stack top
> > > | |
> > > ------------
> > > . .
> > > . .
> > > . .
> > > ------------
> > > | sp | <- irq_stack_base + 0x8
> > > ------------
> > > | fp | <- irq_stack_base + 0x4
> > > ------------
> > > |tinfo_ptr | /* pointer to thread info */
> > > irq_stack_ptr --> ------------ IRQ stack base
> > >
> > > Kernel SVC stack:-
> > > ------------ Kernel stack top
> > > | |
> > > ------------
> > > . .
> > > . .
> > > . .
> > > ------------
> > > | |
> > > | |
> > > ------------
> > > |tinfo_ptr | /* pointer to thread info */
> > > ------------ Kernel stack base
> >
> > The extra indirection doesn't look great, and I don't see any of the
> > other architectures need that. Since we can access percpu data
> > without going through thread_info, maybe doing the same as
> > x86 would work here:
> >
> > - define 'current' as 'this_cpu_read_stable(current_task);'
> > - convert to CONFIG_THREAD_INFO_IN_TASK

That means we need to also code that up in assembly - remember, we
need to access thread_info from assembly code.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2020-10-22 00:41:20

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

(also resending this reply from @kernel.org)

On Fri, Oct 16, 2020 at 12:19 PM Arnd Bergmann <[email protected]> wrote:
> On Thu, Oct 8, 2020 at 9:20 AM Maninder Singh <[email protected]> wrote:
> >
> > This patch adds code for switching to IRQ stack.
> > IRQ stack and Kernel SVC stack have below design.
> >
> > IRQ STACK:-
> > ------------ IRQ stack top
> > | |
> > ------------
> > . .
> > . .
> > . .
> > ------------
> > | sp | <- irq_stack_base + 0x8
> > ------------
> > | fp | <- irq_stack_base + 0x4
> > ------------
> > |tinfo_ptr | /* pointer to thread info */
> > irq_stack_ptr --> ------------ IRQ stack base
> >
> > Kernel SVC stack:-
> > ------------ Kernel stack top
> > | |
> > ------------
> > . .
> > . .
> > . .
> > ------------
> > | |
> > | |
> > ------------
> > |tinfo_ptr | /* pointer to thread info */
> > ------------ Kernel stack base
>
> The extra indirection doesn't look great, and I don't see any of the
> other architectures need that. Since we can access percpu data
> without going through thread_info, maybe doing the same as
> x86 would work here:
>
> - define 'current' as 'this_cpu_read_stable(current_task);'
> - convert to CONFIG_THREAD_INFO_IN_TASK
>
> Arnd

2020-10-22 00:45:54

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

On Wed, Oct 21, 2020 at 2:45 PM Russell King - ARM Linux admin
<[email protected]> wrote:
> On Wed, Oct 21, 2020 at 02:42:48PM +0200, Arnd Bergmann wrote:
> > >
> > > - define 'current' as 'this_cpu_read_stable(current_task);'
> > > - convert to CONFIG_THREAD_INFO_IN_TASK
>
> That means we need to also code that up in assembly - remember, we
> need to access thread_info from assembly code.

Are there any obvious places that need patching aside from
.macro get_thread_info? I did expect the above conversion to
be somewhat more complicated than Maninder's original patch,
but that part seems easy enough.

Arnd

2020-10-22 00:47:30

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

On Wed, Oct 21, 2020 at 01:45:42PM +0100, Russell King - ARM Linux admin wrote:
> On Wed, Oct 21, 2020 at 02:42:48PM +0200, Arnd Bergmann wrote:
> > (also resending this reply from @kernel.org)
> >
> > On Fri, Oct 16, 2020 at 12:19 PM Arnd Bergmann <[email protected]> wrote:
> > > On Thu, Oct 8, 2020 at 9:20 AM Maninder Singh <[email protected]> wrote:
> > > >
> > > > This patch adds code for switching to IRQ stack.
> > > > IRQ stack and Kernel SVC stack have below design.
> > > >
> > > > IRQ STACK:-
> > > > ------------ IRQ stack top
> > > > | |
> > > > ------------
> > > > . .
> > > > . .
> > > > . .
> > > > ------------
> > > > | sp | <- irq_stack_base + 0x8
> > > > ------------
> > > > | fp | <- irq_stack_base + 0x4
> > > > ------------
> > > > |tinfo_ptr | /* pointer to thread info */
> > > > irq_stack_ptr --> ------------ IRQ stack base
> > > >
> > > > Kernel SVC stack:-
> > > > ------------ Kernel stack top
> > > > | |
> > > > ------------
> > > > . .
> > > > . .
> > > > . .
> > > > ------------
> > > > | |
> > > > | |
> > > > ------------
> > > > |tinfo_ptr | /* pointer to thread info */
> > > > ------------ Kernel stack base
> > >
> > > The extra indirection doesn't look great, and I don't see any of the
> > > other architectures need that. Since we can access percpu data
> > > without going through thread_info, maybe doing the same as
> > > x86 would work here:
> > >
> > > - define 'current' as 'this_cpu_read_stable(current_task);'
> > > - convert to CONFIG_THREAD_INFO_IN_TASK
>
> That means we need to also code that up in assembly - remember, we
> need to access thread_info from assembly code.

Note also that there is a circular dependency involved. If you make
thread_info accessible via per-cpu, then:

#ifndef __my_cpu_offset
#define __my_cpu_offset per_cpu_offset(raw_smp_processor_id())
#endif
#ifdef CONFIG_DEBUG_PREEMPT
#define my_cpu_offset per_cpu_offset(smp_processor_id())
#else
#define my_cpu_offset __my_cpu_offset
#endif

smp_processor_id() ultimately ends up as raw_smp_processor_id() which
is:

#define raw_smp_processor_id() (current_thread_info()->cpu)

and if current_thread_info() itself involves reading from per-cpu data,
we end up recursing... infinitely.

This is why I said in the other thread:

"We don't do it because we don't have a separate register to be able
to store the thread_info pointer, and copying that lump between the
SVC and IRQ stack will add massively to IRQ latency, especially for
older machines."

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2020-10-22 01:59:01

by Vaneet Narang

[permalink] [raw]
Subject: RE: [PATCH 2/3] arm: introduce IRQ stacks

Hi Russel & Arnd,

> > > - define 'current' as 'this_cpu_read_stable(current_task);'
> > > - convert to CONFIG_THREAD_INFO_IN_TASK
>
unlike ARM64 ARM doesn't suport dedicated register like "sp_el0", so
arm still stores thread info on stack and fetches using current task using
stack pointer (SP) so CONFIG_THREAD_INFO_IN_TASK is not supported on ARM.
To implement solution inline with current ARM design, we have used indirection.

> That means we need to also code that up in assembly - remember, we
> need to access thread_info from assembly code.

> Note also that there is a circular dependency involved. If you make
> thread_info accessible via per-cpu, then:

> "We don't do it because we don't have a separate register to be able
> to store the thread_info pointer, and copying that lump between the
> SVC and IRQ stack will add massively to IRQ latency, especially for
> older machines."

We tried to minimize latency in switching between SVC stack and IRQ stack
by just copying extra thread info pointer to IRQ stack. Apart from this,
Most of the code of SVC to IRQ switching is similar to ARM64 implementation.

Also We tried to minimize latency of get_current() by introducting self pointer in
SVC stack to avoid if check to determine, is current is called from SVC context
or IRQ context. This way will always do one extra indirection in get_current,
irrespective get_current is called from SVC or IRQ context.

Yes we agree still there will be minor additional latency on accessing current
task and SVC to IRQ switching which might be significant for older machines,
So this is the reason why we kept this sol. under kconfig. This solution
provides extra stack to kernel developers for further development,instead of increasing
stack size to 16KB or spending time on debugging stack overflow issues.

PS: We have debugged and resolved stack overflow issue but we still implemented this sol. to avoid
debugging such issues in future also it gives us flexibility for kernel enhancement on ARM.

Thanks & Regards,
Vaneet Narang

2020-10-22 10:53:36

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

On Wed, Oct 21, 2020 at 2:57 PM Russell King - ARM Linux admin
<[email protected]> wrote:
> On Wed, Oct 21, 2020 at 01:45:42PM +0100, Russell King - ARM Linux admin wrote:
> > > > - define 'current' as 'this_cpu_read_stable(current_task);'
> > > > - convert to CONFIG_THREAD_INFO_IN_TASK
> >
> > That means we need to also code that up in assembly - remember, we
> > need to access thread_info from assembly code.
>
> Note also that there is a circular dependency involved. If you make
> thread_info accessible via per-cpu, then:
>
> #ifndef __my_cpu_offset
> #define __my_cpu_offset per_cpu_offset(raw_smp_processor_id())
> #endif
> #ifdef CONFIG_DEBUG_PREEMPT
> #define my_cpu_offset per_cpu_offset(smp_processor_id())
> #else
> #define my_cpu_offset __my_cpu_offset
> #endif

Right, I had missed the fallback path using asm-generic/percpu.h
that is used with CONFIG_SMP && CONFIG_CPU_V6
Almost everything either uses fixed percpu data (on UP builds)
or TPIDRPRW when building a v7-only or v6k/v7 kernel without
v6 support.

> smp_processor_id() ultimately ends up as raw_smp_processor_id() which
> is:
>
> #define raw_smp_processor_id() (current_thread_info()->cpu)
>
> and if current_thread_info() itself involves reading from per-cpu data,
> we end up recursing... infinitely.
>
> This is why I said in the other thread:
>
> "We don't do it because we don't have a separate register to be able
> to store the thread_info pointer, and copying that lump between the
> SVC and IRQ stack will add massively to IRQ latency, especially for
> older machines."

As discussed on IRC, I think it can still be done in one of these
ways, though admittedly none of them are perfect:

a) add runtime patching for __my_cpu_offset() when
CONFIG_SMP_ON_UP is set. This adds complexity but avoids the
fallback for for SMP&&CPU_V6. It possibly also speeds up
running on single-cpu systems if the TPIDRPRW access adds
any measurable runtime overhead compared to patching it out.

b) If irq stacks are left as a compile-time option, that could be
made conditional on "!(SMP&&CPU_V6)". Presumably very
few people still run kernels built that way any more. The only
supported platforms are i.MX3, OMAP2 and Realview-eb, all of
which are fairly uncommon these days and would usually
run v6-only non-SMP kernels.

c) If we decide that we no longer care about that configuration
at all, we could decide to just make SMP depend on !CPU_V6,
and possibly kill off the entire SMP_ON_UP patching logic.
I suspect we still want to keep SMP_ON_UP for performance
reasons, but I don't know how significant they are to start with.

Arnd

2020-11-09 14:48:21

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

* Arnd Bergmann <[email protected]> [201021 16:07]:
> On Wed, Oct 21, 2020 at 2:57 PM Russell King - ARM Linux admin
> <[email protected]> wrote:
> > On Wed, Oct 21, 2020 at 01:45:42PM +0100, Russell King - ARM Linux admin wrote:
> > > > > - define 'current' as 'this_cpu_read_stable(current_task);'
> > > > > - convert to CONFIG_THREAD_INFO_IN_TASK
> > >
> > > That means we need to also code that up in assembly - remember, we
> > > need to access thread_info from assembly code.
> >
> > Note also that there is a circular dependency involved. If you make
> > thread_info accessible via per-cpu, then:
> >
> > #ifndef __my_cpu_offset
> > #define __my_cpu_offset per_cpu_offset(raw_smp_processor_id())
> > #endif
> > #ifdef CONFIG_DEBUG_PREEMPT
> > #define my_cpu_offset per_cpu_offset(smp_processor_id())
> > #else
> > #define my_cpu_offset __my_cpu_offset
> > #endif
>
> Right, I had missed the fallback path using asm-generic/percpu.h
> that is used with CONFIG_SMP && CONFIG_CPU_V6
> Almost everything either uses fixed percpu data (on UP builds)
> or TPIDRPRW when building a v7-only or v6k/v7 kernel without
> v6 support.
>
> > smp_processor_id() ultimately ends up as raw_smp_processor_id() which
> > is:
> >
> > #define raw_smp_processor_id() (current_thread_info()->cpu)
> >
> > and if current_thread_info() itself involves reading from per-cpu data,
> > we end up recursing... infinitely.
> >
> > This is why I said in the other thread:
> >
> > "We don't do it because we don't have a separate register to be able
> > to store the thread_info pointer, and copying that lump between the
> > SVC and IRQ stack will add massively to IRQ latency, especially for
> > older machines."
>
> As discussed on IRC, I think it can still be done in one of these
> ways, though admittedly none of them are perfect:
>
> a) add runtime patching for __my_cpu_offset() when
> CONFIG_SMP_ON_UP is set. This adds complexity but avoids the
> fallback for for SMP&&CPU_V6. It possibly also speeds up
> running on single-cpu systems if the TPIDRPRW access adds
> any measurable runtime overhead compared to patching it out.

Out of these options a) sounds best to me.

> b) If irq stacks are left as a compile-time option, that could be
> made conditional on "!(SMP&&CPU_V6)". Presumably very
> few people still run kernels built that way any more. The only
> supported platforms are i.MX3, OMAP2 and Realview-eb, all of
> which are fairly uncommon these days and would usually
> run v6-only non-SMP kernels.

This has been working just fine for years though. In general,
removing the conditional compile ifdefferey has made things quite
a bit easier for us, so let's continue on that.

> c) If we decide that we no longer care about that configuration
> at all, we could decide to just make SMP depend on !CPU_V6,
> and possibly kill off the entire SMP_ON_UP patching logic.
> I suspect we still want to keep SMP_ON_UP for performance
> reasons, but I don't know how significant they are to start with.

And this too has been working just fine for years :)

Regards,

Tony

2020-11-09 19:13:04

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

On Mon, Nov 9, 2020 at 3:45 PM Tony Lindgren <[email protected]> wrote:
> >
> > As discussed on IRC, I think it can still be done in one of these
> > ways, though admittedly none of them are perfect:
> >
> > a) add runtime patching for __my_cpu_offset() when
> > CONFIG_SMP_ON_UP is set. This adds complexity but avoids the
> > fallback for for SMP&&CPU_V6. It possibly also speeds up
> > running on single-cpu systems if the TPIDRPRW access adds
> > any measurable runtime overhead compared to patching it out.
>
> Out of these options a) sounds best to me.

Ok. Maninder, would you like to give implementing this a try?

> > b) If irq stacks are left as a compile-time option, that could be
> > made conditional on "!(SMP&&CPU_V6)". Presumably very
> > few people still run kernels built that way any more. The only
> > supported platforms are i.MX3, OMAP2 and Realview-eb, all of
> > which are fairly uncommon these days and would usually
> > run v6-only non-SMP kernels.
>
> This has been working just fine for years though. In general,
> removing the conditional compile ifdefferey has made things quite
> a bit easier for us, so let's continue on that.
>
> > c) If we decide that we no longer care about that configuration
> > at all, we could decide to just make SMP depend on !CPU_V6,
> > and possibly kill off the entire SMP_ON_UP patching logic.
> > I suspect we still want to keep SMP_ON_UP for performance
> > reasons, but I don't know how significant they are to start with.
>
> And this too has been working just fine for years :)

I know it works, my point was that I'm not sure anyone cares
any more ;-)

I suppose the existence of omap2plus_defconfig and
imx_v6_v7_defconfig means it does at least get tested
regularly.

Arnd

2020-11-10 09:21:02

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

* Arnd Bergmann <[email protected]> [201109 19:10]:
> On Mon, Nov 9, 2020 at 3:45 PM Tony Lindgren <[email protected]> wrote:
> > >
> > > As discussed on IRC, I think it can still be done in one of these
> > > ways, though admittedly none of them are perfect:
> > >
> > > a) add runtime patching for __my_cpu_offset() when
> > > CONFIG_SMP_ON_UP is set. This adds complexity but avoids the
> > > fallback for for SMP&&CPU_V6. It possibly also speeds up
> > > running on single-cpu systems if the TPIDRPRW access adds
> > > any measurable runtime overhead compared to patching it out.
> >
> > Out of these options a) sounds best to me.
>
> Ok. Maninder, would you like to give implementing this a try?
>
> > > b) If irq stacks are left as a compile-time option, that could be
> > > made conditional on "!(SMP&&CPU_V6)". Presumably very
> > > few people still run kernels built that way any more. The only
> > > supported platforms are i.MX3, OMAP2 and Realview-eb, all of
> > > which are fairly uncommon these days and would usually
> > > run v6-only non-SMP kernels.
> >
> > This has been working just fine for years though. In general,
> > removing the conditional compile ifdefferey has made things quite
> > a bit easier for us, so let's continue on that.
> >
> > > c) If we decide that we no longer care about that configuration
> > > at all, we could decide to just make SMP depend on !CPU_V6,
> > > and possibly kill off the entire SMP_ON_UP patching logic.
> > > I suspect we still want to keep SMP_ON_UP for performance
> > > reasons, but I don't know how significant they are to start with.
> >
> > And this too has been working just fine for years :)
>
> I know it works, my point was that I'm not sure anyone cares
> any more ;-)

Well for example whatever Linux running ARMv6 LTE modems out there might
need to be supported for quite some time. Not sure how many of them are
able to update kernels though. Certainly network security related issues
would be a good reason to update the kernels.

> I suppose the existence of omap2plus_defconfig and
> imx_v6_v7_defconfig means it does at least get tested
> regularly.

Yes. I probably would just stop any random ARMv6 related testing if
it it needs a custom .config.

Regards,

Tony

2020-11-10 10:09:10

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

On Tue, Nov 10, 2020 at 10:19 AM Tony Lindgren <[email protected]> wrote:
> * Arnd Bergmann <[email protected]> [201109 19:10]:
> > On Mon, Nov 9, 2020 at 3:45 PM Tony Lindgren <[email protected]> wrote:
> >
> > I know it works, my point was that I'm not sure anyone cares
> > any more ;-)
>
> Well for example whatever Linux running ARMv6 LTE modems out there might
> need to be supported for quite some time. Not sure how many of them are
> able to update kernels though. Certainly network security related issues
> would be a good reason to update the kernels.

While I agree they should update their kernels, I suspect none of those
modems do. I am however certain that none of them are running an
SMP-enabled multiplatform kernel on an ARM1136r0!

Are these actually ARMv6? Most ARM11 cores you'd come across
in practice are ARMv6K (ARM1136r1, ARM1167, ARM11MPCore),
in particular every SoC that has any mainline support except for
the ARM1136r0 based OMAP2 and i.MX3.

Arnd

2020-11-10 12:06:13

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

* Arnd Bergmann <[email protected]> [201110 10:04]:
> On Tue, Nov 10, 2020 at 10:19 AM Tony Lindgren <[email protected]> wrote:
> > * Arnd Bergmann <[email protected]> [201109 19:10]:
> > > On Mon, Nov 9, 2020 at 3:45 PM Tony Lindgren <[email protected]> wrote:
> > >
> > > I know it works, my point was that I'm not sure anyone cares
> > > any more ;-)
> >
> > Well for example whatever Linux running ARMv6 LTE modems out there might
> > need to be supported for quite some time. Not sure how many of them are
> > able to update kernels though. Certainly network security related issues
> > would be a good reason to update the kernels.
>
> While I agree they should update their kernels, I suspect none of those
> modems do. I am however certain that none of them are running an
> SMP-enabled multiplatform kernel on an ARM1136r0!

Nope, AFAIK all the SMP parts are ARMv6K :)

> Are these actually ARMv6? Most ARM11 cores you'd come across
> in practice are ARMv6K (ARM1136r1, ARM1167, ARM11MPCore),
> in particular every SoC that has any mainline support except for
> the ARM1136r0 based OMAP2 and i.MX3.

I've been only using smp_on_up for the ARMv6 ARM1136r0 variants
for omap2, no SMP on those.

Regards,

Tony

2020-11-10 13:37:16

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

On Tue, Nov 10, 2020 at 1:06 PM Tony Lindgren <[email protected]> wrote:

> > Are these actually ARMv6? Most ARM11 cores you'd come across
> > in practice are ARMv6K (ARM1136r1, ARM1167, ARM11MPCore),
> > in particular every SoC that has any mainline support except for
> > the ARM1136r0 based OMAP2 and i.MX3.
>
> I've been only using smp_on_up for the ARMv6 ARM1136r0 variants
> for omap2, no SMP on those.

Obviously all SMP hardware is ARMv6K, the only question I raised
in point "c)" is what we would lose by making ARMv6 (ARM1136r0)
support and SMP mutually exclusive in a kernel configuration, and
I suppose the answer remains "testing".

Arnd

2020-11-11 06:59:16

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm: introduce IRQ stacks

* Arnd Bergmann <[email protected]> [201110 13:35]:
> On Tue, Nov 10, 2020 at 1:06 PM Tony Lindgren <[email protected]> wrote:
>
> > > Are these actually ARMv6? Most ARM11 cores you'd come across
> > > in practice are ARMv6K (ARM1136r1, ARM1167, ARM11MPCore),
> > > in particular every SoC that has any mainline support except for
> > > the ARM1136r0 based OMAP2 and i.MX3.
> >
> > I've been only using smp_on_up for the ARMv6 ARM1136r0 variants
> > for omap2, no SMP on those.
>
> Obviously all SMP hardware is ARMv6K, the only question I raised
> in point "c)" is what we would lose by making ARMv6 (ARM1136r0)
> support and SMP mutually exclusive in a kernel configuration, and
> I suppose the answer remains "testing".

Agreed that is probably the biggest reason to keep it at this point.

Regards,

Tony