linux-kernel - Re: No 8254 PIT & no HPET on new Intel N3350 platforms causes kernel panic during early boot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190627085419.27854-1-drake@endlessm.com>
Date:   Thu, 27 Jun 2019 16:54:19 +0800
From:   Daniel Drake <drake@...lessm.com>
To:     tglx@...utronix.de
Cc:     linux-kernel@...r.kernel.org, mingo@...hat.com, bp@...en8.de,
        hdegoede@...hat.com, david.e.box@...ux.intel.com,
        linux@...lessm.com, rafael.j.wysocki@...el.com, x86@...nel.org
Subject: Re: No 8254 PIT & no HPET on new Intel N3350 platforms causes kernel panic during early boot

Hi Thomas,

Picking up this issue again after a break!

We made some progress last time on reducing PIT usage in the TSC
calibration code, but we still have the bigger issue to resolve:
IO-APIC code panicing when the PIT isn't ticking.

On Wed, Apr 3, 2019 at 7:21 PM Thomas Gleixner <tglx@...utronix.de> wrote:
> For newer CPUs we might assume that:
>
>  1) The TSC and APIC timer are actually usable
>
>  2) The frequencies can be retrieved from CPUID or MSRs
>
> If #1 and #2 are reliable we can avoid the whole calibration and interrupt
> delivery mess.
>
> That means we need the following decision logic:
>
>   1) If HPET is available in ACPI, boot normal.
>
>   2) If HPET is not available, verify that the PIT actually counts. If it
>      does, boot normal.
>
>      If it does not either:
>
>      2A) Verify that this is a PCH 300/C240 and fiddle with that ISST bit.
>
>          But that means that we need to chase PCH ids forever...
>
>      2B) Shrug and just avoid the whole PIT/HPET magic all over the place:
>
>          - Avoid the interrupt delivery check in the IOAPIC code as it's
>            uninteresting in that case. Trivial to do.

I tried to explore this idea here:
https://lore.kernel.org/patchwork/patch/1064972/
https://lore.kernel.org/patchwork/patch/1064971/

But I can't say I really knew what I was doing there, and you
pointed out some problems.

Any new ideas that I can experiment with?

Being more conservative, how about something like this?
---
 arch/x86/kernel/apic/io_apic.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 53aa234a6803..36b1e7d5b657 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2073,7 +2073,7 @@ static int mp_alloc_timer_irq(int ioapic, int pin)
  *
  * FIXME: really need to revamp this for all platforms.
  */
-static inline void __init check_timer(void)
+static inline void __init check_timer(int timer_was_working)
 {
 	struct irq_data *irq_data = irq_get_irq_data(0);
 	struct mp_chip_data *data = irq_data->chip_data;
@@ -2216,8 +2216,15 @@ static inline void __init check_timer(void)
 		apic_printk(APIC_QUIET, KERN_INFO
 			    "Perhaps problem with the pre-enabled x2apic mode\n"
 			    "Try booting with x2apic and interrupt-remapping disabled in the bios.\n");
-	panic("IO-APIC + timer doesn't work!  Boot with apic=debug and send a "
-		"report.  Then try booting with the 'noapic' option.\n");
+
+	if (timer_was_working)
+		panic("IO-APIC + timer doesn't work!  Boot with apic=debug and send a "
+			"report.  Then try booting with the 'noapic' option.\n");
+	else
+		apic_printk(APIC_QUIET, KERN_INFO
+			    "Continuing anyway with no 8254 timer, as it was not working even before IO-APIC setup was attempted.\n"
+			    "Boot will fail unless another working clocksource is found.\n");
+
 out:
 	local_irq_restore(flags);
 }
@@ -2304,12 +2311,20 @@ static void ioapic_destroy_irqdomain(int idx)
 void __init setup_IO_APIC(void)
 {
 	int ioapic;
+	int timer_was_working = 0;
 
 	if (skip_ioapic_setup || !nr_ioapics)
 		return;
 
 	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
 
+	/*
+	 * Record if the timer was in working state before we do any
+	 * IO-APIC setup.
+	 */
+	if (nr_legacy_irqs())
+		timer_was_working = timer_irq_works();
+
 	apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
 	for_each_ioapic(ioapic)
 		BUG_ON(mp_irqdomain_create(ioapic));
@@ -2323,7 +2338,7 @@ void __init setup_IO_APIC(void)
 	setup_IO_APIC_irqs();
 	init_IO_APIC_traps();
 	if (nr_legacy_irqs())
-		check_timer();
+		check_timer(timer_was_working);
 
 	ioapic_initialized = 1;
 }
-- 
2.20.1