[v1,2/2] x86: kconfig: remove X86_UP_APIC

From: "Luis R. Rodriguez" <mcgrof@suse.com>

X86_UP_APIC is used for two reasons on 32-bit systems:

1) set a series of dependencies under which we would like
   to express we want X86_LOCAL_APIC enabled
2) under the above conditions if PCI_MSI is enabled always
   force X86_LOCAL_APIC to be enabled
3) Let users opt in or out of X86_LOCAL_APIC if PCI_MSI is
   not enabled

We don't really need a Kconfig entry to address 1) for 2) and 3)
as X86_LOCAL_APIC already has the dependencies which we wish to
match. Instead since 2) is desirable we can just select
X86_LOCAL_APIC on PCI_MSI and it will be enabled when the
dependencies are met. The only thing we loose with this then
is letting users elect to enable or not X86_LOCAL_APIC on
UP 32-bit systems but since:

a) enabling X86_LOCAL_APIC will in no way slow down your kernel
b) enabling X86_LOCAL_APIC only increases your kernel
   by 19264 bytes (19 KiB)
c) x86 is not in a state of flux

We can compromise here and just always enable X86_LOCAL_APIC
when PCI_MSI is enabled.

Using:

(a hacked up patch to enable X86_LOCAL_APIC any time)

export ARCH=i386
make allnoconfig
--> Enabling or disabling X86_LOCAL_APIC
make localyesconfig

With X86_LOCAL_APIC:
mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage
508496  arch/x86/boot/bzImage

With X86_LOCAL_APIC:
mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage
489232  arch/x86/boot/bzImage

Cc: David Rientjes <rientjes@google.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: linux-pci@vger.kernel.org
Cc: x86@kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 arch/x86/Kconfig    | 17 +----------------
 drivers/pci/Kconfig |  2 ++
 2 files changed, 3 insertions(+), 16 deletions(-)

>>> On 12.03.15 at 00:10, <mcgrof@do-not-panic.com> wrote:
>  config X86_LOCAL_APIC
>  	def_bool y
> -	depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI
> +	depends on X86_64 || SMP || X86_32_NON_STANDARD || PCI_MSI

I.e. building a 32-bit kernel with APIC support but with !SMP, !PCI_MSI,
and !X86_32_NON_STANDARD will now be impossible. Surely not
what the patch description says.

> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -5,6 +5,8 @@ config PCI_MSI
>  	bool "Message Signaled Interrupts (MSI and MSI-X)"
>  	depends on PCI
>  	select GENERIC_MSI_IRQ
> +	select X86_LOCAL_APIC
> +	select X86_IO_APIC

I don't see the need for the latter - MSI specifically works without
any IO-APIC interaction. And for the former you should decide
which way you want it - PCI_MSI select X86_LOCAL_APIC
(probably the right thing in x86, but surely wrong everwhere
else, i.e. this at least needs a condition tagged onto it) or
X86_LOCAL_APIC depend on PCI_MSI; in no case should this
be a forward _and_ reverse dependency.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Mar 12, 2015 at 02:42:01AM -0600, Jan Beulich wrote:
> >>> On 12.03.15 at 00:10, <mcgrof@do-not-panic.com> wrote:
> >  config X86_LOCAL_APIC
> >  	def_bool y
> > -	depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI
> > +	depends on X86_64 || SMP || X86_32_NON_STANDARD || PCI_MSI
> 
> I.e. building a 32-bit kernel with APIC support but with !SMP, !PCI_MSI,
> and !X86_32_NON_STANDARD will now be impossible.

These are the requirements for X86_UP_APIC though.

> Surely not what the patch description says.

The only removal I see here is the option to opt-in or out of
X86_UP_APIC when PCI_MSI is *not* enabled provided you meet the
requirements.

> > --- a/drivers/pci/Kconfig
> > +++ b/drivers/pci/Kconfig
> > @@ -5,6 +5,8 @@ config PCI_MSI
> >  	bool "Message Signaled Interrupts (MSI and MSI-X)"
> >  	depends on PCI
> >  	select GENERIC_MSI_IRQ
> > +	select X86_LOCAL_APIC
> > +	select X86_IO_APIC
> 
> I don't see the need for the latter - MSI specifically works without
> any IO-APIC interaction.

Although it can some MSI platforms need it as Bryan originally
had expressed in his original patch [0] that Intel CE, Intel MID
and Intel Quark are all 32-bit uniprocessor systems with IO-APICs
that also use PCI-MSI, so this is not a hard requirement but rather
compromising on enabling it since the the X86_IOAPIC cost is only
~12 KiB at with no performance penalty.

https://lkml.org/lkml/2015/1/22/718

> And for the former you should decide
> which way you want it - PCI_MSI select X86_LOCAL_APIC
> (probably the right thing in x86, but surely wrong everwhere
> else, i.e. this at least needs a condition tagged onto it)

selects are done *iff* the dependencies are met, otherwise we can't
select it, that's all, so the *iff* is implicit.

> or
> X86_LOCAL_APIC depend on PCI_MSI; in no case should this
> be a forward _and_ reverse dependency.

There is no reverse requirement here because of the select.
The trick here is the select implies the depends the value
you are selecting has.

mcgrof@ergon ~/linux-next (git::master)$ export ARCH=alpha
mcgrof@ergon ~/linux-next (git::master)$ make allnoconfig
mcgrof@ergon ~/linux-next (git::master)$ make menuconfig
  --> System setup  --->
      [*] Message Signaled Interrupts (MSI and MSI-X)
mcgrof@ergon ~/linux-next (git::master)$ grep -i APIC .config
mcgrof@ergon ~/linux-next (git::master)$ echo $?
1

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html