From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>, JBeulich@suse.com
Cc: xen-devel@lists.xenproject.org, kevin.tian@intel.com,
dietmar.hahn@ts.fujitsu.com
Subject: Re: [PATCH 1/2] x86/VPMU: Disable VPMU when NMI watchdog is on
Date: Wed, 28 Jan 2015 21:49:36 +0000 [thread overview]
Message-ID: <54C95970.4050601@citrix.com> (raw)
In-Reply-To: <1422474991-2001-2-git-send-email-boris.ostrovsky@oracle.com>
On 28/01/2015 19:56, Boris Ostrovsky wrote:
> NMI watchdog sets APIC_LVTPC register to generate an NMI when PMU counter
> overflow occurs. This may be overwritten by VPMU code later, effectively
> turning off the watchdog.
>
> We should disable VPMU when NMI watchdog is running.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
I completely agree with the aim, but this patch is clunky and you have
missed the case where neither 'watchdog' nor 'vpmu' is specified on the
command line, but the booleans have been tweaked (which is the XenServer
way of choosing defaults while keeping the length of the command line down).
A more simple approach, which doesn't involve exposing opt_vpmu_enabled
or changing any nmi code, would be to have a check in vpmu_initialise()
which checks for opt_watchdog and opt_vpmu_enabled and bail.
Looking at the code in tree, it seems odd opt_vpmu_enabled is passed to
the sub initialise functions to be acted upon. Is this something which
is cleaned up or changed in your series? If not, it perhaps should be.
Also, under what conditions would you expect this initialise function to
be called on a vcpu, and to find its vpmu already active?
~Andrew
next prev parent reply other threads:[~2015-01-28 21:49 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-28 19:56 [PATCH 0/2] Two VPMU/watchdog-related patches Boris Ostrovsky
2015-01-28 19:56 ` [PATCH 1/2] x86/VPMU: Disable VPMU when NMI watchdog is on Boris Ostrovsky
2015-01-28 21:49 ` Andrew Cooper [this message]
2015-01-28 22:33 ` Boris Ostrovsky
2015-01-28 22:41 ` Andrew Cooper
2015-01-28 23:36 ` Boris Ostrovsky
2015-01-29 11:54 ` Andrew Cooper
2015-01-29 11:46 ` Jan Beulich
2015-01-29 15:25 ` Boris Ostrovsky
2015-01-28 19:56 ` [PATCH 2/2] x86/VPMU: Handle APIC_LVTPC accesses Boris Ostrovsky
2015-01-29 11:54 ` Jan Beulich
2015-01-29 15:28 ` Boris Ostrovsky
2015-01-29 15:44 ` Jan Beulich
2015-01-29 11:37 ` [PATCH 0/2] Two VPMU/watchdog-related patches 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=54C95970.4050601@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=boris.ostrovsky@oracle.com \
--cc=dietmar.hahn@ts.fujitsu.com \
--cc=kevin.tian@intel.com \
--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.