Hi all,

My Thinkpad T60p laptop won't boot any recent kernel with nosmp.
I investigated the issue because many people have been complaining
lately that nosmp was breaking their system, and that's unfortunate
because nosmp is a valuable debugging tool.

The actual problem is that the ahci driver fails during probe.
It's essentially the same problem that has been reported here over
3 years ago:
http://marc.info/?l=linux-ide&m=112729386600155&w=2

The post above gave me the idea to try booting with "nosmp pci=nomsi",
and that worked. This makes me believe that the problem is that PCI MSI
makes expectations that are no longer valid when booting with nomsp.

So, I have come up with the following naive patch:

* * * * *

Booting with "nosmp" doesn't work on my Thinkpad T60p laptop while
booting with "nosmp pci=nomsi" works. Forcibly disabling PCI MSI
when booting with nosmp fixes the problem. I'm not sure if it's the
correct fix though.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
---
 drivers/pci/pci.c |    4 ++++
 1 file changed, 4 insertions(+)

--- linux-2.6.25-rc9.orig/drivers/pci/pci.c
+++ linux-2.6.25-rc9/drivers/pci/pci.c
@@ -1635,6 +1635,10 @@ static int __devinit pci_init(void)
 	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
 		pci_fixup_device(pci_fixup_final, dev);
 	}
+#ifdef CONFIG_SMP
+	if (setup_max_cpus == 0)
+		pci_no_msi();
+#endif	
 	return 0;
 }
 

* * * * *

I would welcome comments on the patch above. Is it even remotely
correct? Or is the bug more likely in the ahci driver and the PCI MSI
code is innocent?

FWIW, booting with noapic or nolapic without disabling PCI MSI works
fine for me, so it doesn't seem to be an APIC problem (although this was
my first suspect originally.)

Thanks,
-- 
Jean Delvare
Suse L3
--

[ message continues ]

Seems like this is fixing the symptom, not the cause.  I'll see if I can 
reproduce locally...

Jesse
--

[ message continues ]

If it helps, the device that breaks is:
00:1f.2 SATA controller: Intel Corporation 82801GBM/GHM (ICH7 Family) SATA AHCI Controller (rev 02)
i.e. SATA hard disk drive on an Intel ICH7 controller.

-- 
Jean Delvare
Suse L3
--

[ message continues ]

Ok, I see this too on my desktop machine.  It looks like we're not getting 
interrupts setup correctly in the nosmp case.  Still digging through to see 
why though...

Jesse
--

[ message continues ]

NoSMP disables the io-apic and a lot of modern systems don't work without APIC.

If you just want to run with a single cpu for testing etc. always use maxcpus=1 
(not 0, that will disable the APIC too)

-Andi
--

[ message continues ]

Right...  but it looks like the MSI code is buggy when noapic is specified via 
nosmp or maxcpus=0.  We should either fix it to work with noapic or disable 
it like we do the ioapic when nosmp or maxcpus=0:

index 99ce949..a0cd0ab 100644
--- a/init/main.c
+++ b/init/main.c
@@ -148,6 +148,7 @@ static int __init nosmp(char *str)
 {
        setup_max_cpus = 0;
        disable_ioapic_setup();
+       pci_no_msi();
        return 0;
 }

@@ -156,9 +157,10 @@ early_param("nosmp", nosmp);
 static int __init maxcpus(char *str)
 {
        get_option(&str, &setup_max_cpus);
-       if (setup_max_cpus == 0)
+       if (setup_max_cpus == 0) {
                disable_ioapic_setup();
-
+               pci_no_msi();
+       }
        return 0;
 }

--

[ message continues ]

Or if you want something that compiles & works (at least on my machine) here 
it is.  But the fact that noapic alone doesn't cause the bug means there's 
probably something depending on !setup_max_cpus that needs fixing instead.

Jesse

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index eabeb1f..9170589 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -39,14 +39,6 @@ extern struct rw_semaphore pci_bus_sem;
 
 extern unsigned int pci_pm_d3_delay;
 
-#ifdef CONFIG_PCI_MSI
-void pci_no_msi(void);
-extern void pci_msi_init_pci_dev(struct pci_dev *dev);
-#else
-static inline void pci_no_msi(void) { }
-static inline void pci_msi_init_pci_dev(struct pci_dev *dev) { }
-#endif
-
 #ifdef CONFIG_PCIEAER
 void pci_no_aer(void);
 #else
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ea760e5..0ed53d6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -590,6 +590,13 @@ int pci_try_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
 void pci_intx(struct pci_dev *dev, int enable);
 void pci_msi_off(struct pci_dev *dev);
+#ifdef CONFIG_PCI_MSI
+void pci_no_msi(void);
+extern void pci_msi_init_pci_dev(struct pci_dev *dev);
+#else
+static inline void pci_no_msi(void) { }
+static inline void pci_msi_init_pci_dev(struct pci_dev *dev) { }
+#endif
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size);
diff --git a/init/main.c b/init/main.c
index 99ce949..a9436b7 100644
--- a/init/main.c
+++ b/init/main.c
@@ -58,6 +58,7 @@
 #include <linux/kthread.h>
 #include <linux/sched.h>
 #include <linux/signal.h>
+#include <linux/pci.h>
 
 #include <asm/io.h>
 #include <asm/bugs.h>
@@ -148,6 +149,7 @@ static int __init nosmp(char *str)
 {
 	setup_max_cpus = 0;
 	disable_ioapic_setup();
+	pci_no_msi();
 	return 0;
 }
 
@@ -156,9 +158,10 @@ early_param("nosmp", nosmp);
 static int __init ...
[ message continues ]

First that would likely not compile on architectures without PCI?

Also there is more code in the guts of arch/x86 that disables the
IO-APIC and likely has the same problem. Best probably you put it all
into a single function that does it all properly instead of continuning
to open code it.

Just don't break the other architectures in main.c

-Andi
--

[ message continues ]

Are you sure? I still boot DOS on very recent boxes, and they seem to
work. How can PC-compatible machine require an APIC?
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--

[ message continues ]

The machine doesn't, but the drivers do. DOS likely doesn't use all
hardware.

-Andi
--

[ message continues ]

Should we be fixing drivers?

Do drivers even know? I'd expect core code in arch/x86 to shield
details of interrupt routing from them..

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--

[ message continues ]

See my patch in the other sub-thread; I try to do just that.  MSI support 
should only depend on local APIC support, so even if we don't setup the 
IOAPIC(s) we should be able to leave MSI support enabled.

The patch works for me, but I haven't tried it on 32 bit yet.  Any comments 
Andi?

Thanks,
Jesse
--

[ message continues ]

Hi Andi,


In my case, booting with noapic works fine, so I don't think that the problem
is related to APIC being disabled.

-- 
Jean Delvare
Suse L3
--

[ message continues ]

noapic is not the same as no ioapic. maxcpus=0 just disables the IO-APIC
but not the local APIC.

-Andi
--

[ message continues ]

Hi Andi,


From init/main.c:

static int __init maxcpus(char *str)
{
	get_option(&str, &setup_max_cpus);
	if (setup_max_cpus == 0)
		disable_ioapic_setup();

	return 0;
}
early_param("maxcpus", maxcpus);

From arch/x86/kernel/io_apic_32.c:

static int __init parse_noapic(char *arg)
{
	/* disable IO-APIC */
	disable_ioapic_setup();
	return 0;
}
early_param("noapic", parse_noapic);

Both call disable_ioapic_setup(), so how can they not be the same?

-- 
Jean Delvare
Suse L3
--

[ message continues ]

> Both call disable_ioapic_setup(), so how can they not be the same?

See Jesse's explanation. Basically there is special code in the smp 
boot up to handle 0 CPUs and it disables all APICs.

This is very old and crufty and somewhat obsolete logic,
but it is like that.

-Andi
--

[ message continues ]

This might be closer to a real fix.  Can you confirm that this works for you 
Jean?

Thanks,
Jesse

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index e6abe8a..7cdf930 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1138,14 +1138,10 @@ static int __init smp_sanity_check(unsigned max_cpus)
 				 "forcing use of dummy APIC emulation.\n");
 		smpboot_clear_io_apic();
 #ifdef CONFIG_X86_32
-		if (nmi_watchdog == NMI_LOCAL_APIC) {
-			printk(KERN_INFO "activating minimal APIC for"
-					 "NMI watchdog use.\n");
-			connect_bsp_APIC();
-			setup_local_APIC();
-			end_local_APIC_setup();
-		}
+		connect_bsp_APIC();
 #endif
+		setup_local_APIC();
+		end_local_APIC_setup();
 		return -1;
 	}
 

--

[ message continues ]

Hi Jesse,


What tree is this patch against? I have no arch/x86/kernel/smpboot.c
in 2.6.25.

-- 
Jean Delvare
Suse L3
--

[ message continues ]

It was against the git tip of yesterday afternoon.

Jesse
--

[ message continues ]

Hi Jesse,


Sorry for the late reply. I tested the patch above on top of 2.6.25-git4
and yes, it fixes my problem: booting with "nosmp" works now. Thanks!

Is this patch good enough to go upstream, and if so, through whose
tree? Adding Thomas Gleixner to Cc.

-- 
Jean Delvare
Suse L3
--

[ message continues ]

Yeah I think the patch is reasonable, would be good to get feedback from 
Thomas/Andi/Ingo though...

Jesse
--

[ message continues ]

FWIW, the original idea behind "nosmp" or "maxcpus=0" (just as an
implementation detail) vs "maxcpus=1" was that the two formers would
disable the APIC circuitry altogether (including resisting from switching
from the PIC compatibility mode on systems supporting it), while the
latter would still boot UP, but with interrupts routed through the APICs.  
Essentially SMP implied all the MP circuitry/provisions in this context,
the APICs being an inherent part of which.  Therefore I think the original
idea of implying "pci=nomsi" with "nosmp" certainly looks more in the
spirit of the original setup to me.

 However we have "nolapic" these days as well and with this new proposal
this option could effectively take over the old meaning of "nosmp" (you
cannot do SMP without the local APIC, so "nolapic nosmp" is redundant).  
I am not entirely convinced it is the right way though...

  Maciej
--

[ message continues ]

Yeah, I'm not particularly attached to either meaning.  It looks like we'll 
setup the local apic on 32 bit if the NMI vector is a local apic one, so in 
that case at least the behavior will be the same.

Anyway, we have two options:
  1) make nosmp/maxcpus=1 imply nolapic (and therefore disable MSI too)
  2) make nosmp enable the lapic (so MSI will work)

Jesse
--

[ message continues ]

No opinion. As long as I can boot with "nosmp" and things work, I'm
happy.

-- 
Jean Delvare
Suse L3
--

[ message continues ]

Err, nosmp/maxcpus=0...
--

[ message continues ]

Well, I did not follow changes to the code over the years and I think
enabling the local APIC for the NMI watchdog does not fit the semantics of
"nosmp" very well either.  If you want the watchdog, then you can use 

 Well, I have a slight affinity towards the first one (with "maxcpus=0"  
that is; I'm assuming it's a typo above), but I do not think that is
something to be fiercely fought either.

  MAciej
--

[ message continues ]

One difference between noapic and nosmp is that in the nosmp case even the 
local APIC setup won't occur in native_smp_prepare_cpus(), due to 
smp_sanity_check() returning -1 in the setup_max_cpus == 0 case.

So we either need smp_sanity_check to do a little more APIC setup if max_cpus 
== 0 or shuffle things around in native_smp_prepare_cpus().  Since the former 
is already done for 32 bit builds for the NMI vector, maybe we should just 
make it unconditional so that MSI works?

Jesse
--

[ message continues ]