From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>,
Xen-devel <xen-devel@lists.xen.org>
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
Keir Fraser <keir@xen.org>, Tim Deegan <tim@xen.org>,
Jan Beulich <JBeulich@suse.com>
Subject: Re: [Patch] x86/HVM: Fix RTC interrupt modelling
Date: Mon, 10 Feb 2014 16:17:38 +0100 [thread overview]
Message-ID: <52F8ED92.3050505@citrix.com> (raw)
In-Reply-To: <1392031068-21943-1-git-send-email-andrew.cooper3@citrix.com>
On 10/02/14 12:17, Andrew Cooper wrote:
> This reverts large amounts of:
> 9607327abbd3e77bde6cc7b5327f3efd781fc06e
> "x86/HVM: properly handle RTC periodic timer even when !RTC_PIE"
> 620d5dad54008e40798c4a0c4322aef274c36fa3
> "x86/HVM: assorted RTC emulation adjustments"
>
> and by extentsion:
> f3347f520cb4d8aa4566182b013c6758d80cbe88
> "x86/HVM: adjust IRQ (de-)assertion"
> c2f79c464849e5f796aa9d1d0f26fe356abd1a1a
> "x86/HVM: fix processing of RTC REG_B writes"
> 527824f41f5fac9cba3d4441b2e73d3118d98837
> "x86/hvm: Centralize and simplify the RTC IRQ logic."
>
> The current code has a pathological case, tickled by the access pattern of
> Windows 2003 Server SP2. Occasonally on boot (which I presume is during a
> time calibration against the RTC Periodic Timer), Windows gets stuck in an
> infinite loop reading RTC REG_C. This affects 32 and 64 bit guests.
>
> In the pathological case, the VM state looks like this:
> * RTC: 64Hz period, periodic interrupts enabled
> * RTC_IRQ in IOAPIC as vector 0xd1, edge triggered, not pending
> * vector 0xd1 set in LAPIC IRR and ISR, TPR at 0xd0
> * Reads from REG_C return 'RTC_PF | RTC_IRQF'
>
> With an intstrumented Xen, dumping the periodic timers with a guest in this
> state shows a single timer with pt->irq_issued=1 and pt->pending_intr_nr=2.
>
> Windows is presumably waiting for reads of REG_C to drop to 0, and reading
> REG_C clears the value each time in the emulated RTC. However:
>
> * {svm,vmx}_intr_assist() calls pt_update_irq() unconditionally.
> * pt_update_irq() always finds the RTC as earliest_pt.
> * rtc_periodic_interrupt() unconditionally sets RTC_PF in no_ack mode. It
> returns true, indicating that pt_update_irq() should really inject the
> interrupt.
> * pt_update_irq() decides that it doesn't need to fake up part of
> pt_intr_post() because this is a real interrupt.
> * {svm,vmx}_intr_assist() can't inject the interrupt as it is already
> pending, so exits early without calling pt_intr_post().
>
> The underlying problem here comes because the AF and UF bits of RTC interrupt
> state is modelled by the RTC code, but the PF is modelled by the pt code. The
> root cause of windows infinite loop is that RTC_PF is being re-set on vmentry
> before the interrupt logic has worked out that it can't actually inject an RTC
> interrupt, causing Windows to erroniously read (RTC_PF|RTC_IRQF) when it
> should be reading 0.
>
> This patch reverts the RTC_PF logic handling to its former state, whereby
> rtc_periodic_cb() is called strictly when the periodic timer logic has
> successfully injected a periodic interrupt. In doing so, it is important that
> the RTC code itself never directly triggers an interrupt for the periodic
> timer (other than the case when setting REG_B.PIE, where the pt code will have
> dropped the interrupt).
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Tim Deegan <tim@xen.org>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> ---
>
> I still dont know exactly what condition causes windows to tickle this
> behavour. It is seen about 1 or 2 times in 9 tests running a 12 hour VM
> lifecycle test. Over the weekend, 100 of these tests have passed without a
> single reoccurence of the infinite loop. The change has also passed a windows
> extended regression test, so it would appear that other versions of windows
> are still fine with the change.
>
> Roger: as this caused issues for FreeBSD, would you mind testing it again
> please?
Tested-by: Roger Pau Monné <roger.pau@citrix.com>
On FreeBSD 10.0, 9.2 and 8.4
No apparent regressions AFAICT.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2014-02-10 15:17 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-10 11:17 [Patch] x86/HVM: Fix RTC interrupt modelling Andrew Cooper
2014-02-10 12:19 ` Tim Deegan
2014-02-10 15:17 ` Roger Pau Monné [this message]
2014-02-10 15:33 ` Keir Fraser
2014-02-10 16:34 ` Jan Beulich
2014-02-10 17:13 ` Andrew Cooper
2014-02-11 9:28 ` Jan Beulich
2014-02-10 17:21 ` Tim Deegan
2014-02-11 9:15 ` Jan Beulich
2014-02-11 12:11 ` Tim Deegan
2014-02-11 12:50 ` Jan Beulich
2014-02-11 13:15 ` Tim Deegan
2014-02-11 13:31 ` Jan Beulich
2014-02-11 13:57 ` Tim Deegan
2014-02-11 14:35 ` Jan Beulich
2014-02-11 13:59 ` Andrew Cooper
2014-02-11 14:10 ` Tim Deegan
2014-02-11 14:52 ` Andrew Cooper
2014-02-10 17:46 ` George Dunlap
2014-02-10 18:43 ` Andrew Cooper
2014-02-11 8:47 ` Tim Deegan
2014-02-11 9:02 ` Jan Beulich
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=52F8ED92.3050505@citrix.com \
--to=roger.pau@citrix.com \
--cc=JBeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=keir@xen.org \
--cc=tim@xen.org \
--cc=xen-devel@lists.xen.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 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).