All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Aaron Janse" <aaron@ajanse.me>,
	"Jason Andryuk" <jandryuk@gmail.com>,
	"Ondrej Balaz" <blami@blami.net>,
	"Tamas K Lengyel" <tamas@tklengyel.com>,
	"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Subject: [PATCH v2] x86/timer: Fix boot on Intel systems using ITSSPRC static PIT clock gating
Date: Wed, 27 Jan 2021 15:06:15 +0000	[thread overview]
Message-ID: <20210127150615.641-1-andrew.cooper3@citrix.com> (raw)

Recent Intel client devices have disabled the legacy PIT for powersaving
reasons, breaking compatibility with a traditional IBM PC.  Xen depends on a
legacy timer interrupt to check that the IO-APIC/PIC routing is configured
correctly, and fails to boot with:

  (XEN) *******************************
  (XEN) Panic on CPU 0:
  (XEN) IO-APIC + timer doesn't work!  Boot with apic_verbosity=debug and send report.  Then try booting with the `noapic` option
  (XEN) *******************************

While this setting can be undone by Xen, the details of how to differ by
chipset, and would be very short sighted for battery based devices.  See bit 2
"8254 Static Clock Gating Enable" in:

  https://edc.intel.com/content/www/us/en/design/products-and-solutions/processors-and-chipsets/comet-lake-u/intel-400-series-chipset-on-package-platform-controller-hub-register-database/itss-power-reduction-control-itssprc-offset-3300/

All impacted systems have an HPET, but there is no indication of the absence
of PIT functionality, nor a suitable way to probe for its absence.  As a short
term fix, reconfigure the HPET into legacy replacement mode.  A better
longterm fix would be to avoid the reliance on the timer interrupt entirely.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Aaron Janse <aaron@ajanse.me>
CC: Jason Andryuk <jandryuk@gmail.com>
CC: Ondrej Balaz <blami@blami.net>
CC: Tamas K Lengyel <tamas@tklengyel.com>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

v2:
 * Fix build - misplaced bracket.
 * Tweak description of SETVAL.

Slightly RFC.  On older platforms this does generate some spurious PIC
interrupts during boot, but they're benign.  I was hoping to have time to fix
those too, but I'm getting an increasing number of requests to post this
patch.

Other followup actions:
 * Overhaul our setup logic.  Large quantities of it is legacy for pre-64bit
   systems, and not applicable to Xen these days.
 * Have Xen turn the PIT off when it isn't being used as the timesource.  Its
   a waste of time servicing useless interrupts.
 * Make `clocksource=pit` not enter an infinite loop on these systems
 * Drop all references to `noapic`.  These days, the only thing it will ever
   do is make a bad situation worse.
---
 xen/arch/x86/hpet.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index e6fab8acd8..876c5e4769 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -758,7 +758,7 @@ static u32 *hpet_boot_cfg;
 u64 __init hpet_setup(void)
 {
     static u64 __initdata hpet_rate;
-    u32 hpet_id, hpet_period;
+    unsigned int hpet_id, hpet_period, hpet_cfg;
     unsigned int last, rem;
 
     if ( hpet_rate )
@@ -793,6 +793,68 @@ u64 __init hpet_setup(void)
     if ( (rem * 2) > hpet_period )
         hpet_rate++;
 
+    /*
+     * Intel chipsets from Skylake/ApolloLake onwards can statically clock
+     * gate the 8259 PIT.  This option is enabled by default in slightly later
+     * systems, as turning the PIT off is a prerequisite to entering the C11
+     * power saving state.
+     *
+     * Xen currently depends on the legacy timer interrupt being active while
+     * IRQ routing is configured.
+     *
+     * Reconfigure the HPET into legacy mode to re-establish the timer
+     * interrupt.
+     */
+    if ( hpet_id & HPET_ID_LEGSUP &&
+         !((hpet_cfg = hpet_read32(HPET_CFG)) & HPET_CFG_LEGACY) )
+    {
+        unsigned int c0_cfg, ticks, count;
+
+        /* Stop the main counter. */
+        hpet_write32(hpet_cfg & ~HPET_CFG_ENABLE, HPET_CFG);
+
+        /* Reconfigure channel 0 to be 32bit periodic. */
+        c0_cfg = hpet_read32(HPET_Tn_CFG(0));
+        c0_cfg |= (HPET_TN_ENABLE | HPET_TN_PERIODIC | HPET_TN_SETVAL |
+                   HPET_TN_32BIT);
+        hpet_write32(c0_cfg, HPET_Tn_CFG(0));
+
+        /*
+         * The exact period doesn't have to match a legacy PIT.  All we need
+         * is an interrupt queued up via the IO-APIC to check routing.
+         *
+         * Use HZ as the frequency.
+         */
+        ticks = ((SECONDS(1) / HZ) * div_sc(hpet_rate, SECONDS(1), 32)) >> 32;
+
+        count = hpet_read32(HPET_COUNTER);
+
+        /*
+         * HPET_TN_SETVAL above is atrociously documented in the spec.
+         *
+         * Periodic HPET channels have a main comparator register, and
+         * separate accumulator register.  Each time an interrupt is
+         * generated, the accumulator register is re-added to the comparator
+         * set up the new period.
+         *
+         * Normally, writes to the CMP register update both registers.
+         * However, under these semantics, it is impossible to set up a
+         * periodic timer correctly without the main HPET counter being at 0.
+         *
+         * Instead, HPET_TN_SETVAL is a self-clearing control bit which we can
+         * use for periodic timers to mean that the second write to CMP
+         * updates the accumulator only, and not the absolute comparator
+         * value.
+         *
+         * This lets us set a period when the main counter isn't at 0.
+         */
+        hpet_write32(count + ticks, HPET_Tn_CMP(0));
+        hpet_write32(ticks,         HPET_Tn_CMP(0));
+
+        /* Restart the main counter, and legacy mode. */
+        hpet_write32(hpet_cfg | HPET_CFG_ENABLE | HPET_CFG_LEGACY, HPET_CFG);
+    }
+
     return hpet_rate;
 }
 
-- 
2.11.0



                 reply	other threads:[~2021-01-27 15:07 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20210127150615.641-1-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=aaron@ajanse.me \
    --cc=blami@blami.net \
    --cc=jandryuk@gmail.com \
    --cc=marmarek@invisiblethingslab.com \
    --cc=roger.pau@citrix.com \
    --cc=tamas@tklengyel.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.