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 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.