On Mon, Mar 12, 2012 at 05:04:48PM +0100, Igor Mammedov wrote:
On 03/12/2012 02:27 PM, Gleb Natapov wrote:
On Mon, Mar 12, 2012 at 02:09:02PM +0100, Igor Mammedov wrote:
On 03/12/2012 11:14 AM, Gleb Natapov wrote:
On Mon, Mar 12, 2012 at 10:52:18AM +0100, Igor Mammedov wrote:
On 03/11/2012 11:36 AM, Gleb Natapov wrote:
On Sat, Mar 10, 2012 at 12:47:26PM +0100, Igor Mammedov wrote: >Reduce amount of consumed cpu time (i.e. don't spin forever) if >number of started cpus are more then expected. And print a bug >message into debug port. > >Signed-off-by: Igor Mammedovimammedo@redhat.com >--- > src/smp.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > >diff --git a/src/smp.c b/src/smp.c >index 8c077a1..9933ac6 100644 >--- a/src/smp.c >+++ b/src/smp.c >@@ -115,8 +115,14 @@ smp_probe(void) > msleep(10); > } else { > u8 cmos_smp_count = inb_cmos(CMOS_BIOS_SMP_COUNT); >- while (cmos_smp_count + 1 != readl(&CountCPUs)) >+ while (cmos_smp_count + 1 != readl(&CountCPUs)) { >+ if (cmos_smp_count + 1< readl(&CountCPUs)) { >+ dprintf(1, "BUG: Expected %d cpu(s) but %d cpus started\n", >+ cmos_smp_count + 1, readl(&CountCPUs)); Shouldn't we print it to the console too?
I'm not sure if it's possible to print at this stage, but I'll try and re-post if it works.
If it is not possible currently may be we should move smp detection to later stage. Or do not hlt here and report error later. The easier for a user to see this error the easier it is for us to debug user's problems.
It looks safe to move vga_setup() right after pci_setup() in post(). However I can't tell what structures various option roms might access so I'm not completely sure if it right thing to do.
Well you move it above "Setup interfaces that option roms may need" comment, so we can be sure it is not safe :)
Yep, sorry my bad.
I think moving smp detection to later stage should be safer. But now I am
we can't move it past init_bios_tables() and this function is is under comment "// Setup interfaces that option roms may need" so I'm not sure we can move vga_setup before it.
I think init_bios_tables() is not required for option rom to work. Not sure though. Kevin?
not even sure we should halt at all. May be warn the user and continue
We need to stop because otherwise user won't event notice that there was an error and only later may notice not expected cpus count in OS. BIOSes usually stop in case of error and you have to override this behavior in settings (something like this: boot if keyboard is not present).
Perhaps we could safely clamp CountCPUs to 1 + alter smp_ap_boot_code() to stop counting in case of error and continue to boot till we will be able to print error message and stop after that. Yep, error message is nice to have but I'm not sure if this one case is worth of inventing deferred print infrastructure. Support personnel might just enable logging from debug port if BIOS is suspected (it is easy to spot, we hang right on boot). That's what they have to do now in case of any BIOS related problems.
Since this error should not really happen IRL I agree we should not spend to much time on new infrastructure.
booting with cmos_smp_count = CountCPUs?
I guess is was this way some time ago 31bfad632b0ca and with large cpu counts it wasn't reliable to wait XX ms while all cpus initialize, that why cmos_smp_count was introduced and bios waits on exact count.
The halt condition is meant for buggy emulators so if we get more cpus then expected it is safer to just stop then experience inexplicable bugs later (I can imagine having mp, smbios and acpi tables built with different CountCPUs, and maybe there is other dependencies).
-- Gleb.
--
Igor
-- Gleb.