[PATCH 1/6] powerpc/kconfig: Move NR_IRQS into "Kernel Options"

[PATCH 1/6] powerpc/kconfig: Move NR_IRQS into "Kernel Options"

[Michael Ellerman]

Currently the NR_IRQS option sits at the top level, which is ugly in
menuconfig. It's not something users will commonly need to worry about
so move it into "Kernel Options".

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/Kconfig | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9a7057ec2154..c6f1aa1e8d4c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -33,16 +33,6 @@ config HAVE_SETUP_PER_CPU_AREA
 config NEED_PER_CPU_EMBED_FIRST_CHUNK
 	def_bool PPC64
 
-config NR_IRQS
-	int "Number of virtual interrupt numbers"
-	range 32 32768
-	default "512"
-	help
-	  This defines the number of virtual interrupt numbers the kernel
-	  can manage. Virtual interrupt numbers are what you see in
-	  /proc/interrupts. If you configure your system to have too few,
-	  drivers will fail to load or worse - handle with care.
-
 config STACKTRACE_SUPPORT
 	bool
 	default y
@@ -321,6 +311,16 @@ config HIGHMEM
 	bool "High memory support"
 	depends on PPC32
 
+config NR_IRQS
+	int "Number of virtual interrupt numbers"
+	range 32 32768
+	default "512"
+	help
+	  This defines the number of virtual interrupt numbers the kernel
+	  can manage. Virtual interrupt numbers are what you see in
+	  /proc/interrupts. If you configure your system to have too few,
+	  drivers will fail to load or worse - handle with care.
+
 source kernel/Kconfig.hz
 source kernel/Kconfig.preempt
 source "fs/Kconfig.binfmt"
-- 
2.1.4

[PATCH 2/6] powerpc/kconfig: Move sysdev Kconfig later

[Michael Ellerman]

Things in sysdev are generally not what we want near the top of the
menuconfig, so move them down later in the list.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c6f1aa1e8d4c..db685fa54c5f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -302,7 +302,6 @@ source "init/Kconfig"
 
 source "kernel/Kconfig.freezer"
 
-source "arch/powerpc/sysdev/Kconfig"
 source "arch/powerpc/platforms/Kconfig"
 
 menu "Kernel options"
@@ -1079,6 +1078,7 @@ source "arch/powerpc/sysdev/qe_lib/Kconfig"
 source "lib/Kconfig"
 
 source "arch/powerpc/Kconfig.debug"
+source "arch/powerpc/sysdev/Kconfig"
 
 source "security/Kconfig"
 
-- 
2.1.4

Re: [PATCH 2/6] powerpc/kconfig: Move sysdev Kconfig later

[Thomas Huth]

On 08/10/15 23:09, Michael Ellerman wrote:
> Things in sysdev are generally not what we want near the top of the
> menuconfig, so move them down later in the list.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index c6f1aa1e8d4c..db685fa54c5f 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -302,7 +302,6 @@ source "init/Kconfig"
>  
>  source "kernel/Kconfig.freezer"
>  
> -source "arch/powerpc/sysdev/Kconfig"
>  source "arch/powerpc/platforms/Kconfig"
>  
>  menu "Kernel options"
> @@ -1079,6 +1078,7 @@ source "arch/powerpc/sysdev/qe_lib/Kconfig"
>  source "lib/Kconfig"
>  
>  source "arch/powerpc/Kconfig.debug"
> +source "arch/powerpc/sysdev/Kconfig"
>  
>  source "security/Kconfig"
>  

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 3/6] powerpc/kconfig: Move bitness & endian options to the top

[Michael Ellerman]

We want the 64/32-bit choice, and the little/big endian choice to be at
the very top of the menuconfig. That is because they have the most
effect on what other options are available, so it makes sense to set
them first.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/Kconfig                   | 36 ++++++++++++++++++++++++++++++++++
 arch/powerpc/platforms/Kconfig.cputype | 36 ----------------------------------
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index db685fa54c5f..ea9377259f15 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -1,3 +1,39 @@
+config PPC64
+	bool "64-bit kernel"
+	default n
+	select HAVE_VIRT_CPU_ACCOUNTING
+	select ZLIB_DEFLATE
+	help
+	  This option selects whether a 32-bit or a 64-bit kernel
+	  will be built.
+
+choice
+	prompt "Endianness selection"
+	default CPU_BIG_ENDIAN
+	help
+	  This option selects whether a big endian or little endian kernel will
+	  be built.
+
+config CPU_BIG_ENDIAN
+	bool "Build big endian kernel"
+	help
+	  Build a big endian kernel.
+
+	  If unsure, select this option.
+
+config CPU_LITTLE_ENDIAN
+	bool "Build little endian kernel"
+	depends on PPC_BOOK3S_64
+	select PPC64_BOOT_WRAPPER
+	help
+	  Build a little endian kernel.
+
+	  Note that if cross compiling a little endian kernel,
+	  CROSS_COMPILE must point to a toolchain capable of targeting
+	  little endian powerpc.
+
+endchoice
+
 source "arch/powerpc/platforms/Kconfig.cputype"
 
 config PPC32
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index c140e94c7c72..60b28e63c137 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -1,12 +1,3 @@
-config PPC64
-	bool "64-bit kernel"
-	default n
-	select HAVE_VIRT_CPU_ACCOUNTING
-	select ZLIB_DEFLATE
-	help
-	  This option selects whether a 32-bit or a 64-bit kernel
-	  will be built.
-
 menu "Processor support"
 choice
 	prompt "Processor Type"
@@ -415,33 +406,6 @@ config VDSO32
 	  big endian. That is because the only little endian configuration we
 	  support is ppc64le which is 64-bit only.
 
-choice
-	prompt "Endianness selection"
-	default CPU_BIG_ENDIAN
-	help
-	  This option selects whether a big endian or little endian kernel will
-	  be built.
-
-config CPU_BIG_ENDIAN
-	bool "Build big endian kernel"
-	help
-	  Build a big endian kernel.
-
-	  If unsure, select this option.
-
-config CPU_LITTLE_ENDIAN
-	bool "Build little endian kernel"
-	depends on PPC_BOOK3S_64
-	select PPC64_BOOT_WRAPPER
-	help
-	  Build a little endian kernel.
-
-	  Note that if cross compiling a little endian kernel,
-	  CROSS_COMPILE must point to a toolchain capable of targeting
-	  little endian powerpc.
-
-endchoice
-
 config PPC64_BOOT_WRAPPER
 	def_bool n
 	depends on CPU_LITTLE_ENDIAN
-- 
2.1.4

[PATCH 4/6] powerpc/kconfig: Move SCOM_DEBUGFS into "Kernel Hacking"

[Michael Ellerman]

Currently SCOM_DEBUGFS appears at the top level in menuconfig, but it's
not particularly important so shouldn't really be there. Move it under
"Kernel Hacking" which seems more appropriate.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/Kconfig.debug  | 5 +++++
 arch/powerpc/sysdev/Kconfig | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 3a510f4a6b68..807515b1a743 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -63,6 +63,11 @@ config PPC_EMULATED_STATS
 	  warnings can be printed to the console when instructions are
 	  emulated.
 
+config SCOM_DEBUGFS
+	bool "Expose SCOM controllers via debugfs"
+	depends on PPC_SCOM && DEBUG_FS
+	default n
+
 config CODE_PATCHING_SELFTEST
 	bool "Run self-tests of the code-patching code."
 	depends on DEBUG_KERNEL
diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
index a19332a38715..fd8888b43f5b 100644
--- a/arch/powerpc/sysdev/Kconfig
+++ b/arch/powerpc/sysdev/Kconfig
@@ -32,11 +32,6 @@ source "arch/powerpc/sysdev/xics/Kconfig"
 config PPC_SCOM
 	bool
 
-config SCOM_DEBUGFS
-	bool "Expose SCOM controllers via debugfs"
-	depends on PPC_SCOM && DEBUG_FS
-	default n
-
 config GE_FPGA
 	bool
 	default n
-- 
2.1.4

[PATCH 5/6] powerpc/kconfig: Move cpu options after platform options

[Michael Ellerman]

In general platforms are a more important configuration decision than
cpus, so the platforms should come first.

My basis for saying that is that our cpu selection options are generally
just about tuning for a cpu, rather than enabling/disabling support for
a cpu. On the other hand disabling a platform means you can't boot on
those machines, and any associated cpu options should vanish.

With this applied the start of menuconfig looks like:

     [*] 64-bit kernel
         Endianness selection (Build little endian kernel)
         General setup
     [*] Enable loadable module support
     -*- Enable the block layer
         Platform support
         Processor support
         Kernel options
         Bus options

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index ea9377259f15..0f8c51279147 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -34,8 +34,6 @@ config CPU_LITTLE_ENDIAN
 
 endchoice
 
-source "arch/powerpc/platforms/Kconfig.cputype"
-
 config PPC32
 	bool
 	default y if !PPC64
@@ -339,6 +337,7 @@ source "init/Kconfig"
 source "kernel/Kconfig.freezer"
 
 source "arch/powerpc/platforms/Kconfig"
+source "arch/powerpc/platforms/Kconfig.cputype"
 
 menu "Kernel options"
 
-- 
2.1.4

Re: [PATCH 5/6] powerpc/kconfig: Move cpu options after platform options

[Thomas Huth]

On 08/10/15 23:09, Michael Ellerman wrote:
> In general platforms are a more important configuration decision than
> cpus, so the platforms should come first.
> 
> My basis for saying that is that our cpu selection options are generally
> just about tuning for a cpu, rather than enabling/disabling support for
> a cpu. On the other hand disabling a platform means you can't boot on
> those machines, and any associated cpu options should vanish.
> 
> With this applied the start of menuconfig looks like:
> 
>      [*] 64-bit kernel
>          Endianness selection (Build little endian kernel)
>          General setup
>      [*] Enable loadable module support
>      -*- Enable the block layer
>          Platform support
>          Processor support
>          Kernel options
>          Bus options
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/Kconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index ea9377259f15..0f8c51279147 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -34,8 +34,6 @@ config CPU_LITTLE_ENDIAN
>  
>  endchoice
>  
> -source "arch/powerpc/platforms/Kconfig.cputype"
> -
>  config PPC32
>  	bool
>  	default y if !PPC64
> @@ -339,6 +337,7 @@ source "init/Kconfig"
>  source "kernel/Kconfig.freezer"
>  
>  source "arch/powerpc/platforms/Kconfig"
> +source "arch/powerpc/platforms/Kconfig.cputype"
>  
>  menu "Kernel options"
>  

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 5/6] powerpc/kconfig: Move cpu options after platform options

[Scott Wood]

On Fri, 2015-10-09 at 08:09 +1100, Michael Ellerman wrote:
> In general platforms are a more important configuration decision than
> cpus, so the platforms should come first.
> 
> My basis for saying that is that our cpu selection options are generally
> just about tuning for a cpu, rather than enabling/disabling support for
> a cpu. On the other hand disabling a platform means you can't boot on
> those machines, and any associated cpu options should vanish.

Kconfig.cputype *is* about enabling/disabling support for a cpu.  On 32-bit, 
the first choice is between "512x/52xx/6xx/7xx/74xx/82xx/83xx/86xx", 
"Freescale 85xx", "Freescale 8xx", "AMCC 40x", "AMCC 44x, 46x or 47x", or 
"Freescale e200".  Until that is set correctly, the platforms available for 
selection will not be correct.

Likewise, on 64-bit one must choose book3s versus book3e, before going on to 
the tuning selection you describe above.

-Scott

Re: [PATCH 5/6] powerpc/kconfig: Move cpu options after platform options

[Michael Ellerman]

On Mon, 2015-10-12 at 16:47 -0500, Scott Wood wrote:
> On Fri, 2015-10-09 at 08:09 +1100, Michael Ellerman wrote:
> > In general platforms are a more important configuration decision than
> > cpus, so the platforms should come first.
> > 
> > My basis for saying that is that our cpu selection options are generally
> > just about tuning for a cpu, rather than enabling/disabling support for
> > a cpu. On the other hand disabling a platform means you can't boot on
> > those machines, and any associated cpu options should vanish.
> 
> Kconfig.cputype *is* about enabling/disabling support for a cpu.  On 32-bit, 
> the first choice is between "512x/52xx/6xx/7xx/74xx/82xx/83xx/86xx", 
> "Freescale 85xx", "Freescale 8xx", "AMCC 40x", "AMCC 44x, 46x or 47x", or 
> "Freescale e200".  Until that is set correctly, the platforms available for 
> selection will not be correct.
> 
> Likewise, on 64-bit one must choose book3s versus book3e, before going on to 
> the tuning selection you describe above.

Ugh, this Kconfig mess shits me.

You're right, my testing was running menuconfig with an existing config, so
then the options look vaguely sane, but if you start with a blank config then
it's the wrong way around as you say.

So I'll drop this.

cheers

[PATCH 6/6] powerpc/kconfig: Cell CPU options should depend on PPC_CELL

[Michael Ellerman]

If none of the Cell platforms are enabled then the resulting kernel can
never boot on a Cell machine, and so there's no sense offering to tune
or build for Cell CPUs.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/platforms/Kconfig.cputype | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 60b28e63c137..03c53f4418ad 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -91,7 +91,7 @@ config GENERIC_CPU
 
 config CELL_CPU
 	bool "Cell Broadband Engine"
-	depends on PPC_BOOK3S_64 && !CPU_LITTLE_ENDIAN
+	depends on PPC_CELL
 
 config POWER4_CPU
 	bool "POWER4"
@@ -140,7 +140,7 @@ config 6xx
 
 config TUNE_CELL
 	bool "Optimize for Cell Broadband Engine"
-	depends on PPC64 && PPC_BOOK3S
+	depends on PPC_CELL
 	help
 	  Cause the compiler to optimize for the PPE of the Cell Broadband
 	  Engine. This will make the code run considerably faster on Cell
-- 
2.1.4

Re: [PATCH 6/6] powerpc/kconfig: Cell CPU options should depend on PPC_CELL

[Thomas Huth]

On 08/10/15 23:09, Michael Ellerman wrote:
> If none of the Cell platforms are enabled then the resulting kernel can
> never boot on a Cell machine, and so there's no sense offering to tune
> or build for Cell CPUs.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/platforms/Kconfig.cputype | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 60b28e63c137..03c53f4418ad 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -91,7 +91,7 @@ config GENERIC_CPU
>  
>  config CELL_CPU
>  	bool "Cell Broadband Engine"
> -	depends on PPC_BOOK3S_64 && !CPU_LITTLE_ENDIAN
> +	depends on PPC_CELL
>  
>  config POWER4_CPU
>  	bool "POWER4"
> @@ -140,7 +140,7 @@ config 6xx
>  
>  config TUNE_CELL
>  	bool "Optimize for Cell Broadband Engine"
> -	depends on PPC64 && PPC_BOOK3S
> +	depends on PPC_CELL
>  	help
>  	  Cause the compiler to optimize for the PPE of the Cell Broadband
>  	  Engine. This will make the code run considerably faster on Cell

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 1/6] powerpc/kconfig: Move NR_IRQS into "Kernel Options"

[Arnd Bergmann]

On Friday 09 October 2015 08:09:12 Michael Ellerman wrote:
> Currently the NR_IRQS option sits at the top level, which is ugly in
> menuconfig. It's not something users will commonly need to worry about
> so move it into "Kernel Options".
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> 

Is this option actually still meaningful at all, when you select CONFIG_SPARSE_IRQ
unconditionally?

	Arnd

Re: [PATCH 1/6] powerpc/kconfig: Move NR_IRQS into "Kernel Options"

[Michael Ellerman]

On Thu, 2015-10-08 at 23:30 +0200, Arnd Bergmann wrote:
> On Friday 09 October 2015 08:09:12 Michael Ellerman wrote:
> > Currently the NR_IRQS option sits at the top level, which is ugly in
> > menuconfig. It's not something users will commonly need to worry about
> > so move it into "Kernel Options".
> 
> Is this option actually still meaningful at all, when you select CONFIG_SPARSE_IRQ
> unconditionally?

It's still used for #define NR_IRQS, which is still used by the generic irq
code and also some drivers.

So we need some value for that, whether it needs to be user defined or if we
could just pick a value I'm not sure. x86 seem to just define it based on
NR_CPUs and some other factors.

cheers

Re: [PATCH 1/6] powerpc/kconfig: Move NR_IRQS into "Kernel Options"

[Arnd Bergmann]

On Monday 12 October 2015 21:00:25 Michael Ellerman wrote:
> On Thu, 2015-10-08 at 23:30 +0200, Arnd Bergmann wrote:
> > On Friday 09 October 2015 08:09:12 Michael Ellerman wrote:
> > > Currently the NR_IRQS option sits at the top level, which is ugly in
> > > menuconfig. It's not something users will commonly need to worry about
> > > so move it into "Kernel Options".
> > 
> > Is this option actually still meaningful at all, when you select CONFIG_SPARSE_IRQ
> > unconditionally?
> 
> It's still used for #define NR_IRQS, which is still used by the generic irq
> code and also some drivers.

Drivers shouldn't use it, and I don't see any driver using it that can be
enabled on powerpc.

The question is rather whether you have any devices on powerpc that
get a hardwired IRQ number from a statically defined platform device
rather from DT.

If there are any ISA devices, the driver might try to use an interrupt
number that is hardcoded in the driver as a number from 0 to 15.
I guess that could happen on old CHRP or 6xx machines.

> So we need some value for that, whether it needs to be user defined or if we
> could just pick a value I'm not sure. x86 seem to just define it based on
> NR_CPUs and some other factors.

What happens if you set it to 16?

	Arnd

Re: [PATCH 1/6] powerpc/kconfig: Move NR_IRQS into "Kernel Options"

[Michael Ellerman]

On Mon, 2015-10-12 at 12:30 +0200, Arnd Bergmann wrote:
> On Monday 12 October 2015 21:00:25 Michael Ellerman wrote:
> > On Thu, 2015-10-08 at 23:30 +0200, Arnd Bergmann wrote:
> > > On Friday 09 October 2015 08:09:12 Michael Ellerman wrote:
> > > > Currently the NR_IRQS option sits at the top level, which is ugly in
> > > > menuconfig. It's not something users will commonly need to worry about
> > > > so move it into "Kernel Options".
> > > 
> > > Is this option actually still meaningful at all, when you select CONFIG_SPARSE_IRQ
> > > unconditionally?
> > 
> > It's still used for #define NR_IRQS, which is still used by the generic irq
> > code and also some drivers.
> 
> Drivers shouldn't use it, and I don't see any driver using it that can be
> enabled on powerpc.
> 
> The question is rather whether you have any devices on powerpc that
> get a hardwired IRQ number from a statically defined platform device
> rather from DT.
> 
> If there are any ISA devices, the driver might try to use an interrupt
> number that is hardcoded in the driver as a number from 0 to 15.
> I guess that could happen on old CHRP or 6xx machines.
> 
> > So we need some value for that, whether it needs to be user defined or if we
> > could just pick a value I'm not sure. x86 seem to just define it based on
> > NR_CPUs and some other factors.
> 
> What happens if you set it to 16?

Yeah, this builds and boots at least on pseries KVM.

diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index e8e3a0a04eb0..35fba282b7f9 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -23,11 +23,8 @@ extern atomic_t ppc_n_lost_interrupts;
 /* This number is used when no interrupt has been assigned */
 #define NO_IRQ                 (0)
 
-/* Total number of virq in the platform */
-#define NR_IRQS                CONFIG_NR_IRQS
-
-/* Same thing, used by the generic IRQ code */
 #define NR_IRQS_LEGACY         NUM_ISA_INTERRUPTS
+#define NR_IRQS                        NR_IRQS_LEGACY
 
 extern irq_hw_number_t virq_to_hw(unsigned int virq);
 

cheers

Re: [PATCH 1/6] powerpc/kconfig: Move NR_IRQS into "Kernel Options"

[Arnd Bergmann]

On Monday 12 October 2015 22:07:45 Michael Ellerman wrote:
> Yeah, this builds and boots at least on pseries KVM.
> 
> diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
> index e8e3a0a04eb0..35fba282b7f9 100644
> --- a/arch/powerpc/include/asm/irq.h
> +++ b/arch/powerpc/include/asm/irq.h
> @@ -23,11 +23,8 @@ extern atomic_t ppc_n_lost_interrupts;
>  /* This number is used when no interrupt has been assigned */
>  #define NO_IRQ                 (0)
>  
> -/* Total number of virq in the platform */
> -#define NR_IRQS                CONFIG_NR_IRQS
> -
> -/* Same thing, used by the generic IRQ code */
>  #define NR_IRQS_LEGACY         NUM_ISA_INTERRUPTS
> +#define NR_IRQS                        NR_IRQS_LEGACY
>  
>  extern irq_hw_number_t virq_to_hw(unsigned int virq);
>  

This looks like the way it's intended. I've also checked that no powerpc
platform defines a static IRQ larger than NUM_ISA_INTERRUPTS, and only
two files do it at all (arch/powerpc/platforms/chrp/pegasos_eth.c and arch/powerpc/sysdev/rtc_cmos_setup.c).

	Arnd

Re: [PATCH 1/6] powerpc/kconfig: Move NR_IRQS into "Kernel Options"

[Michael Ellerman]

On Mon, 2015-10-12 at 13:50 +0200, Arnd Bergmann wrote:
> On Monday 12 October 2015 22:07:45 Michael Ellerman wrote:
> > Yeah, this builds and boots at least on pseries KVM.
> > 
> > diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
> > index e8e3a0a04eb0..35fba282b7f9 100644
> > --- a/arch/powerpc/include/asm/irq.h
> > +++ b/arch/powerpc/include/asm/irq.h
> > @@ -23,11 +23,8 @@ extern atomic_t ppc_n_lost_interrupts;
> >  /* This number is used when no interrupt has been assigned */
> >  #define NO_IRQ                 (0)
> >  
> > -/* Total number of virq in the platform */
> > -#define NR_IRQS                CONFIG_NR_IRQS
> > -
> > -/* Same thing, used by the generic IRQ code */
> >  #define NR_IRQS_LEGACY         NUM_ISA_INTERRUPTS
> > +#define NR_IRQS                        NR_IRQS_LEGACY
> >  
> >  extern irq_hw_number_t virq_to_hw(unsigned int virq);
> >  
> 
> This looks like the way it's intended.

Hmm, though IRQ_BITMAP_BITS is still a hard limit, and by default it's set to
NR_IRQS + 8196.

So we'd be relying on the + 8196 to get us up to a reasonable number. That
looks like it was added as a bit of a hack/bug-fix, so I'm not sure we want to
rely on it. (c1ee6264280e ("genirq: Prevent access beyond allocated_irqs
bitmap"))

All the distros seem to run with NR_IRQS=512, so it seems no one has needed
more than that or we would have heard about it presumably.

At the same time if we're only spending a single bit for each interrupt number
maybe we should just set it to min(512, NR_CPUS) or something like that to make
sure it's always reasonably big.


> I've also checked that no powerpc platform defines a static IRQ larger than
> NUM_ISA_INTERRUPTS, and only two files do it at all
> (arch/powerpc/platforms/chrp/pegasos_eth.c and arch/powerpc/sysdev/rtc_cmos_setup.c).

Thanks. You clearly have better grep fu than me :)

cheers

Re: [PATCH 1/6] powerpc/kconfig: Move NR_IRQS into "Kernel Options"

[Arnd Bergmann]

On Tuesday 13 October 2015 11:28:06 Michael Ellerman wrote:
> On Mon, 2015-10-12 at 13:50 +0200, Arnd Bergmann wrote:
> > On Monday 12 October 2015 22:07:45 Michael Ellerman wrote:
> > > Yeah, this builds and boots at least on pseries KVM.
> > > 
> > > diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
> > > index e8e3a0a04eb0..35fba282b7f9 100644
> > > --- a/arch/powerpc/include/asm/irq.h
> > > +++ b/arch/powerpc/include/asm/irq.h
> > > @@ -23,11 +23,8 @@ extern atomic_t ppc_n_lost_interrupts;
> > >  /* This number is used when no interrupt has been assigned */
> > >  #define NO_IRQ                 (0)
> > >  
> > > -/* Total number of virq in the platform */
> > > -#define NR_IRQS                CONFIG_NR_IRQS
> > > -
> > > -/* Same thing, used by the generic IRQ code */
> > >  #define NR_IRQS_LEGACY         NUM_ISA_INTERRUPTS
> > > +#define NR_IRQS                        NR_IRQS_LEGACY
> > >  
> > >  extern irq_hw_number_t virq_to_hw(unsigned int virq);
> > >  
> > 
> > This looks like the way it's intended.
> 
> Hmm, though IRQ_BITMAP_BITS is still a hard limit, and by default it's set to
> NR_IRQS + 8196.

Ah, I wasn't aware of this limit.

> So we'd be relying on the + 8196 to get us up to a reasonable number. That
> looks like it was added as a bit of a hack/bug-fix, so I'm not sure we want to
> rely on it. (c1ee6264280e ("genirq: Prevent access beyond allocated_irqs
> bitmap"))
> 
> All the distros seem to run with NR_IRQS=512, so it seems no one has needed
> more than that or we would have heard about it presumably.
> 
> At the same time if we're only spending a single bit for each interrupt number
> maybe we should just set it to min(512, NR_CPUS) or something like that to make
> sure it's always reasonably big.

The NR_IRQS part is what consumes actual kernel memory, and it may hurt you
on some of the older embedded machines with small RAM. Reducing this is of
course the main idea behind CONFIG_SPARSE_IRQ, and I don't see any indication
that we ever need more than NR_IRQS_LEGACY here.

It's the +8196 part I'd be more worried about for large systems, as you say
it has been picked arbitrarily, and can easily be too small if you have
hundreds or thousands of CPU cores.

Maybe we could change the IRQ_BITMAP_BITS to have use

#define IRQ_BITMAP_BITS (NR_IRQS_LEGACY + MAX_NUMNODES * 8192)

which leaves it as it is on small machines, but gives large NUMA machines
(which should always have enough RAM anyway) some more room.

	Arnd