linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: tip-bot for Thomas Gleixner <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: ashok.raj@intel.com, eranian@google.com,
	linux-kernel@vger.kernel.org, peterz@infradead.org,
	ravi.v.shankar@intel.com, andi.kleen@intel.com, hpa@zytor.com,
	ricardo.neri-calderon@linux.intel.com,
	Suravee.Suthikulpanit@amd.com, tglx@linutronix.de,
	mingo@kernel.org
Subject: [tip:x86/timers] x86/hpet: Use common init for legacy clockevent
Date: Thu, 27 Jun 2019 16:53:12 -0700	[thread overview]
Message-ID: <tip-49adaa60fa75a04457d30f38321378cdc3547212@git.kernel.org> (raw)
In-Reply-To: <20190623132436.646565913@linutronix.de>

Commit-ID:  49adaa60fa75a04457d30f38321378cdc3547212
Gitweb:     https://git.kernel.org/tip/49adaa60fa75a04457d30f38321378cdc3547212
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Sun, 23 Jun 2019 15:24:08 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 28 Jun 2019 00:57:27 +0200

x86/hpet: Use common init for legacy clockevent

Replace the static initialization of the legacy clockevent with runtime
initialization utilizing the common init function as the last preparatory
step to switch the legacy clockevent over to the channel 0 storage in
hpet_base.

This comes with a twist. The static clockevent initializer has selected
support for periodic and oneshot mode unconditionally whether the HPET
config advertised periodic mode or not. Even the pre clockevents code did
this. But....

Using the conditional in hpet_init_clockevent() makes at least Qemu and one
hardware machine fail to boot.  There are two issues which cause the boot
failure:

 #1 After the timer delivery test in IOAPIC and the IOAPIC setup the next
    interrupt is not delivered despite the HPET channel being programmed
    correctly. Reprogramming the HPET after switching to IOAPIC makes it
    work again. After fixing this, the next issue surfaces:

 #2 Due to the unconditional periodic mode 'availability' the Local APIC
    timer calibration can hijack the global clockevents event handler
    without causing damage. Using oneshot at this stage makes if hang
    because the HPET does not get reprogrammed due to the handler
    hijacking. Duh, stupid me!

Both issues require major surgery and especially the kick HPET again after
enabling IOAPIC results in really nasty hackery.  This 'assume periodic
works' magic has survived since HPET support got added, so it's
questionable whether this should be fixed. Both Qemu and the failing
hardware machine support periodic mode despite the fact that both don't
advertise it in the configuration register and both need that extra kick
after switching to IOAPIC. Seems to be a feature...

Keep the 'assume periodic works' magic around and add a big fat comment.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Andi Kleen <andi.kleen@intel.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Link: https://lkml.kernel.org/r/20190623132436.646565913@linutronix.de

---
 arch/x86/kernel/hpet.c | 87 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 80497fe5354c..35633e577d21 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -66,6 +66,9 @@ bool					boot_hpet_disable;
 bool					hpet_force_user;
 static bool				hpet_verbose;
 
+/*
+ * The HPET clock event device wrapped in a channel for conversion
+ */
 static struct hpet_channel		hpet_channel0;
 
 static inline
@@ -294,22 +297,6 @@ static void hpet_enable_legacy_int(void)
 	hpet_legacy_int_enabled = true;
 }
 
-static void hpet_legacy_clockevent_register(struct hpet_channel *hc)
-{
-	/* Start HPET legacy interrupts */
-	hpet_enable_legacy_int();
-
-	/*
-	 * Start HPET with the boot CPU's cpumask and make it global after
-	 * the IO_APIC has been initialized.
-	 */
-	hc->evt.cpumask = cpumask_of(boot_cpu_data.cpu_index);
-	clockevents_config_and_register(&hc->evt, hpet_freq,
-					HPET_MIN_PROG_DELTA, 0x7FFFFFFF);
-	global_clock_event = &hc->evt;
-	pr_debug("Clockevent registered\n");
-}
-
 static int hpet_clkevt_set_state_periodic(struct clock_event_device *evt)
 {
 	unsigned int channel = clockevent_to_channel(evt)->num;
@@ -430,23 +417,57 @@ static void hpet_init_clockevent(struct hpet_channel *hc, unsigned int rating)
 	}
 }
 
-/*
- * The HPET clock event device wrapped in a channel for conversion
- */
-static struct hpet_channel hpet_channel0 = {
-	.evt = {
-		.name			= "hpet",
-		.features		= CLOCK_EVT_FEAT_PERIODIC |
-					  CLOCK_EVT_FEAT_ONESHOT,
-		.set_state_periodic	= hpet_clkevt_set_state_periodic,
-		.set_state_oneshot	= hpet_clkevt_set_state_oneshot,
-		.set_state_shutdown	= hpet_clkevt_set_state_shutdown,
-		.tick_resume		= hpet_clkevt_legacy_resume,
-		.set_next_event		= hpet_clkevt_set_next_event,
-		.irq			= 0,
-		.rating			= 50,
-	}
-};
+static void __init hpet_legacy_clockevent_register(struct hpet_channel *hc)
+{
+	/*
+	 * Start HPET with the boot CPU's cpumask and make it global after
+	 * the IO_APIC has been initialized.
+	 */
+	hc->cpu = boot_cpu_data.cpu_index;
+	strncpy(hc->name, "hpet", sizeof(hc->name));
+	hpet_init_clockevent(hc, 50);
+
+	hc->evt.tick_resume	= hpet_clkevt_legacy_resume;
+
+	/*
+	 * Legacy horrors and sins from the past. HPET used periodic mode
+	 * unconditionally forever on the legacy channel 0. Removing the
+	 * below hack and using the conditional in hpet_init_clockevent()
+	 * makes at least Qemu and one hardware machine fail to boot.
+	 * There are two issues which cause the boot failure:
+	 *
+	 * #1 After the timer delivery test in IOAPIC and the IOAPIC setup
+	 *    the next interrupt is not delivered despite the HPET channel
+	 *    being programmed correctly. Reprogramming the HPET after
+	 *    switching to IOAPIC makes it work again. After fixing this,
+	 *    the next issue surfaces:
+	 *
+	 * #2 Due to the unconditional periodic mode availability the Local
+	 *    APIC timer calibration can hijack the global clockevents
+	 *    event handler without causing damage. Using oneshot at this
+	 *    stage makes if hang because the HPET does not get
+	 *    reprogrammed due to the handler hijacking. Duh, stupid me!
+	 *
+	 * Both issues require major surgery and especially the kick HPET
+	 * again after enabling IOAPIC results in really nasty hackery.
+	 * This 'assume periodic works' magic has survived since HPET
+	 * support got added, so it's questionable whether this should be
+	 * fixed. Both Qemu and the failing hardware machine support
+	 * periodic mode despite the fact that both don't advertise it in
+	 * the configuration register and both need that extra kick after
+	 * switching to IOAPIC. Seems to be a feature...
+	 */
+	hc->evt.features		|= CLOCK_EVT_FEAT_PERIODIC;
+	hc->evt.set_state_periodic	= hpet_clkevt_set_state_periodic;
+
+	/* Start HPET legacy interrupts */
+	hpet_enable_legacy_int();
+
+	clockevents_config_and_register(&hc->evt, hpet_freq,
+					HPET_MIN_PROG_DELTA, 0x7FFFFFFF);
+	global_clock_event = &hc->evt;
+	pr_debug("Clockevent registered\n");
+}
 
 /*
  * HPET MSI Support

  parent reply	other threads:[~2019-06-27 23:53 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-23 13:23 [patch 00/29] x86/hpet: Cleanup the channel management Thomas Gleixner
2019-06-23 13:23 ` [patch 01/29] x86/hpet: Simplify CPU online code Thomas Gleixner
2019-06-27 23:34   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 02/29] x86/hpet: Replace printk(KERN...) with pr_...() Thomas Gleixner
2019-06-27 23:34   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 03/29] x86/hpet: Restructure init code Thomas Gleixner
2019-06-27 23:35   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 04/29] x86/hpet: Remove pointless x86-64 specific #include Thomas Gleixner
2019-06-27 23:36   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 05/29] x86/hpet: Remove unused parameter from hpet_next_event() Thomas Gleixner
2019-06-27 23:36   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 06/29] x86/hpet: Remove the unused hpet_msi_read() function Thomas Gleixner
2019-06-27 23:37   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 07/29] x86/hpet: Mark init functions __init Thomas Gleixner
2019-06-27 23:38   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 08/29] x86/hpet: Sanitize stub functions Thomas Gleixner
2019-06-27 23:39   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 09/29] x86/hpet: Move static and global variables to one place Thomas Gleixner
2019-06-27 23:39   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 10/29] x86/hpet: Shuffle code around for readability sake Thomas Gleixner
2019-06-27 23:40   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 11/29] x86/hpet: Separate counter check out of clocksource register code Thomas Gleixner
2019-06-27 23:41   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 12/29] x86/hpet: Simplify counter validation Thomas Gleixner
2019-06-27 23:41   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 13/29] x86/hpet: Decapitalize and rename EVT_TO_HPET_DEV Thomas Gleixner
2019-06-27 23:42   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 14/29] x86/hpet: Remove not required includes Thomas Gleixner
2019-06-27 23:43   ` [tip:x86/timers] " tip-bot for Ingo Molnar
2019-06-23 13:23 ` [patch 15/29] x86/hpet: Make naming consistent Thomas Gleixner
2019-06-27 23:44   ` [tip:x86/timers] " tip-bot for Ingo Molnar
2019-06-23 13:23 ` [patch 16/29] x86/hpet: Clean up comments Thomas Gleixner
2019-06-27 23:44   ` [tip:x86/timers] " tip-bot for Ingo Molnar
2019-06-23 13:23 ` [patch 17/29] x86/hpet: Coding style cleanup Thomas Gleixner
2019-06-27 23:45   ` [tip:x86/timers] " tip-bot for Ingo Molnar
2019-06-23 13:23 ` [patch 18/29] x86/hpet: Introduce struct hpet_base and struct hpet_channel Thomas Gleixner
2019-06-27 23:46   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:23 ` [patch 19/29] x86/hpet: Use cached channel data Thomas Gleixner
2019-06-27 23:46   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:24 ` [patch 20/29] x86/hpet: Add mode information to struct hpet_channel Thomas Gleixner
2019-06-27 23:47   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:24 ` [patch 21/29] x86/hpet: Add function to select a /dev/hpet channel Thomas Gleixner
2019-06-27 23:48   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:24 ` [patch 22/29] x86/hpet: Rename variables to prepare for switching to channels Thomas Gleixner
2019-06-27 23:49   ` [tip:x86/timers] " tip-bot for Ingo Molnar
2019-06-23 13:24 ` [patch 23/29] x86/hpet: Move clockevents into channels Thomas Gleixner
2019-06-27 23:49   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:24 ` [patch 24/29] x86/hpet: Use cached info instead of extra flags Thomas Gleixner
2019-06-26 21:20   ` Ingo Molnar
2019-06-27 23:50   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:24 ` [patch 25/29] x86/hpet: Wrap legacy clockevent in hpet_channel Thomas Gleixner
2019-06-27 23:51   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:24 ` [patch 26/29] x86/hpet: Consolidate clockevent functions Thomas Gleixner
2019-06-26 21:17   ` Ingo Molnar
2019-06-27 22:44     ` Thomas Gleixner
2019-06-27 22:53       ` Thomas Gleixner
2019-06-28  8:32         ` Ingo Molnar
2019-06-27 23:51   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:24 ` [patch 27/29] x86/hpet: Carve out shareable parts of init_one_hpet_msi_clockevent() Thomas Gleixner
2019-06-27 23:52   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-23 13:24 ` [patch 28/29] x86/hpet: Use common init for legacy clockevent Thomas Gleixner
2019-06-26 21:13   ` Ingo Molnar
2019-06-27 23:53   ` tip-bot for Thomas Gleixner [this message]
2019-06-23 13:24 ` [patch 29/29] x86/hpet: Use channel for legacy clockevent storage Thomas Gleixner
2019-06-27 23:53   ` [tip:x86/timers] " tip-bot for Thomas Gleixner
2019-06-26 21:22 ` [patch 00/29] x86/hpet: Cleanup the channel management Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tip-49adaa60fa75a04457d30f38321378cdc3547212@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=Suravee.Suthikulpanit@amd.com \
    --cc=andi.kleen@intel.com \
    --cc=ashok.raj@intel.com \
    --cc=eranian@google.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=ravi.v.shankar@intel.com \
    --cc=ricardo.neri-calderon@linux.intel.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).