[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1.1 2/2] x86/hpet: Don't enable legacy replacement mode unconditionally


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Fri, 26 Mar 2021 16:53:27 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=VHIoGsfWwIgGaEP5O372bN7pg6+tTs0nJuFQXbDMSu8=; b=lH8gJCA8avoxxpB33uELaAD+vfyvCIZIs6KfCbHR+fg3HdMV3KYUQd7va82Qsc64ySCPbAThLccatRxidYKV/edYI5XZTCdY6KgyNfd0/tX3jRkkFpIcxirvTnEVpLnEU+qtXJTJEDgrdGITDG8Mgwr4mA6BjT/8XbR1t92I6UU91jRO1/ucycYuCtd8IGbWZuLoFBwaMvlrMlsSg2iS6pp1oiyIEVLkJhCKMdp8h/Q3iOJ7vuBkvSOrfjTMgJYaHG1NH86psCX39vmq9stbxtFbKK/UOtV1+EbtaSxEqh1Qo8ixwYZe0aeYaLSh9uGoUpL96b4DpgI133o7mit5oQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BZak5iYt2rOgO9wWAYtxzGyDkIfDNZ3bjHUR/mk82WDostawMnTcPii4zZIJHAgG97MmMMNI4salqVWwZX1HWI80UpqhajIBPBMYDJdjmRzQMES+zxPGJ6v1SNIcTp0ncKSUsySv2nEm513qoIA2Qjf9jszUCZkwRyizVNwCF5Xwl6X6JERegOnsAMp7kbS+osHrJRGHCfFqC29R3DgFOE22uFDUpKbaeorW6GnqZvGy4rTurrhwT9wG3R0g99pW3BaMN2b5nQwokIcWI+fTOtQBvsSCAQ65QTQF4YUlLDHjF77Vvu+B2Drj8wJf0XD8UMY1aySGm9+q10D6j77HfQ==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>, Frédéric Pierret <frederic.pierret@xxxxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 26 Mar 2021 16:54:23 +0000
  • Ironport-hdrordr: A9a23:8BWd7amh/IABgyHUzGMsZywHrN3pDfODj2dD5ilNYBxZY6Wkvu iUtrAyyQL0hDENWHsphNCHP+26TWnB8INuiLN+AZ6LZyOjnGezNolt4c/ZwzPmEzDj7eI178 hdWoBEIpnLAVB+5PyX3CCRD8sgzN6b8KqhmOfZyDNXQRt3brx7hj0YNi+wOCRNNW57LLA+E4 eR4dcCgjKmd2geYMjTPAh6Y8HoodrXmJX6JSMcDxk85wWUyR+u4rj2Ex+Xty1uLA9n67Ek7G TDjkjF9ryu2svLtiP0+k3yy9BtmNXnwsZeH8DksKkoAxjllwrAXvUbZ5SspzYwydvfkWoCsN 6JmBs4OtQ21nW5RBDJnTLI+y3NlAkj8GXjz1jwuwqQneXcSCghA8RMwaJ1GyGpk3YIh9133K JV02/xjfM+Znms7UeNham9azhQmkW5unYkm+II5kYvN7c2U7NNsZcZuHpcDZZoJlOI1KkcDO JsAMvAjcwmFG+yUnaxhBgK/PWRRHgpWj+JTk8e0/blqQR+rTRSyksVw9EnhXEQ9J4xYIks3Z W1Do1Y0J5JVcMYdqR7GaMoRta2EHXERVb2PHuVOkmPLtBIB1v977rMpJkl7uCjf5IFiLM0hZ T6SVtd8Uo/YVjnB8Gi1IBCmyq9AVmVbHDI8IVz9pJ5srrzSP7AKiuYUm0jlMOmvrE2HtDbc+ zbAuMVP9bTaU/VXapZ1Qz3XJdfbVMEVtcOh9o9U1WS5urWN4zRsPDBevq7HsuvLR8UHkfERl cTVjn6I8tNqmqxXGXjvRTXU3TxPmPl+5ZdF7Xb4vgzxIABOpYkiHlQtX2JouWwbRFSuK0/e0 VzZJn9lLmgmGWw9WHUq0VlUyAtSXp90fHFaTdntAUKO0T7ffIooNOEY11f23OBO1taR8PSGw hPmkRv9cuMXtqt7BFnL+jiHnORjnMVqn7PZYwbgLe/6cDsfY59KZo6RqprF0HuGwZukQhn7E dPATV0BnP3J3fLs+GInZYUDObQe51XmwGwO/NZrnrZqAG7vsEgRnwSWha0Ss6JiQMSRz5Z72 cBtpM3sf6lo3KCOGE/iOM3PBlnc2KMGo9LCwyDecFpgLzxQRpxSm2LnDSerBk2dgPRhhwvr1 2kCRfRVeDAA1JbtHwd9qrx6lt7el+QeF9KZmlgvZdwEnnHvXhPwfaGD5DDple5Wx8n+KUwIT vFaTwdLkdVy9e72AW8tRyCGX8lr69ec9D1PfAGSfX+y3mtIIqHmeU6BPdS5o9iL82rmPQMS/ ijdwicKy7YB+sl1xeOnGssPDB5pRAf4KvV8SygyFL98G80APLULlgjeqoSJMuE6XP4A9mPy5 d0gLsOzKONG1S0TuTD767ZbzROcEyO5UG3SvwlspBSs+YZsqBpE5zSTDvP0zVm0XwFXbHJvX JbZJ4+xrbLfrJLVYg1XQly+1IyjtSBLEcxqGXNc6UDVGBoq0WeBs+D5rrDlKEmDUKArjbhIF X3yVwrw971Gw+4kYMAA60+IW5qeFExxXRr8uSFbZDRAmyRBqh+1Wv/Fn+2a7lGTqeZXZ0Wsx Zh+tmN9tXnOxbQ6UT1vTFhJLhJ/HviacSuABiUEeoN19ChI1yDju+L58G05Q2HBgeTWgA9hY dfc1YXYdkGoj4+jJcv2iz3c5fJmCse4hBjyAAisEXs1Iig6HraGk8DETSxuOQmYRBjdl6Sjc rE9uCE0m/a+zYt4+iZKHtt
  • Ironport-sdr: L43dheEaLnqrwktK5rHaqU+iD1O1IgU8K1hCI7eTH6614rs6A4qVgrWeFN3bt9Oy/IbHYPFyYP ZuvvtmipQZNGFeliha49kws3NHrDFFsONEKAEW7WsOyNgMpkASoD2j4hasp08iNN3QH9Cuby5g 39xWAHsOPXEU5b/JcpxQC1sJDUwpFm6taT0sikJWeeR3I7j7Ndx6oZLzIA/8kwTbRqES1EoYxT va9nzLkKTa2XJsZ91B5VTYRMT+dwi494cav0ldtcL+6ev2geNB73oax0lidnLv7a6Lzat60XWb acw=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 26/03/2021 16:48, Jan Beulich wrote:
> On 26.03.2021 17:32, Andrew Cooper wrote:
>> On 26/03/2021 09:51, Jan Beulich wrote:
>>> On 25.03.2021 18:21, Andrew Cooper wrote:
>>>> @@ -1922,14 +1924,38 @@ static void __init check_timer(void)
>>>>             vector, apic1, pin1, apic2, pin2);
>>>>  
>>>>      if (pin1 != -1) {
>>>> +        bool hpet_changed = false;
>>>> +
>>>>          /*
>>>>           * Ok, does IRQ0 through the IOAPIC work?
>>>>           */
>>>>          unmask_IO_APIC_irq(irq_to_desc(0));
>>>> +    retry_ioapic_pin:
>>>>          if (timer_irq_works()) {
>>>>              local_irq_restore(flags);
>>>>              return;
>>>>          }
>>>> +
>>>> +        /*
>>>> +         * 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.
>>>> +         *
>>>> +         * If the user hasn't made an explicit option, attempt to 
>>>> reconfigure
>>>> +         * the HPET into legacy mode to re-establish the timer interrupt.
>>>> +         */
>>>> +        if ( opt_hpet_legacy_replacement < 0 &&
>>>> +             !hpet_changed && hpet_enable_legacy_replacement_mode() )
>>>> +        {
>>>> +            printk(XENLOG_ERR "..no 8254 timer found - trying HPET Legacy 
>>>> Replacement Mode\n");
>>>> +            hpet_changed = true;
>>>> +            goto retry_ioapic_pin;
>>>> +        }
>>>> +
>>>>          clear_IO_APIC_pin(apic1, pin1);
>>>>          printk(KERN_ERR "..MP-BIOS bug: 8254 timer not connected to "
>>>>                 "IO-APIC\n");
>>> As mentioned on irc already, I'm somewhat concerned by doing this change
>>> first (and also not undoing it if it didn't work). An AMD Turion based
>>> laptop I was using many years ago required one of the other fallbacks to
>>> be engaged, and hence I'd expect certain other (old?) systems to be
>>> similarly affected. Sadly (for the purposes here) I don't have this
>>> laptop anymore, so wouldn't be able to verify whether the above actually
>>> breaks there.
>> Turion is K8, so very obsolete these days.  If it doesn't have an
>> IO-APIC, its even less likely to have an HPET.
> It did have an IO-APIC, but required one of the virtual-wire modes to
> be enabled iirc.
>
>> Even if it does have an HPET, there isn't anything to suggest that
>> legacy replacement mode is broken.
> With one firmware flaw there is about as much chance for another one
> as there is for HPET to be working, I'd say. Iirc (very vaguely) it
> did have a HPET, but no ACPI table entry for it, so we wouldn't have
> used it.
>
>> Would you prefer me to undo the change?  Its not easy - we have the boot
>> time config stashed, but if it was periodic before, the accumulator is
>> broken because we can never read that value back out.
> I didn't think the accumulator change would matter. I did think though
> not having been in legacy replacement mode before might be better to
> also not be in after, if its enabling didn't help anyway.

The accumulator matters if chan0 was configured as periodic previously.

Then again, this is broken anyway generally (e.g. the S3 path), so I
suppose we're not making it any worse here.

~Andrew



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.