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

Re: [Xen-devel] [PATCH v2] x86/vpmu_intel: Fix hypervisor crash by masking PC bit in MSR_P6_EVNTSEL

On 04/28/2017 02:52 AM, Tian, Kevin wrote:
From: Tian, Kevin
Sent: Friday, April 28, 2017 2:43 PM

From: Boris Ostrovsky [mailto:boris.ostrovsky@xxxxxxxxxx]
Sent: Thursday, April 27, 2017 11:18 PM

On 04/27/2017 11:05 AM, Jan Beulich wrote:
On 27.04.17 at 16:57, <boris.ostrovsky@xxxxxxxxxx> wrote:
On 04/27/2017 03:32 AM, Jan Beulich wrote:
On 26.04.17 at 20:50, <mohit.gambhir@xxxxxxxxxx> wrote:
On 04/26/2017 02:19 PM, Andrew Cooper wrote:
On 26/04/17 19:11, Mohit Gambhir wrote:
Setting Pin Control (PC) bit (19) in MSR_P6_EVNTSEL results in a
Protection Fault and thus results in a hypervisor crash. This patch
crash by masking PC bit and returning an error in case any guest
to it.

Signed-off-by: Mohit Gambhir <mohit.gambhir@xxxxxxxxxx>
Out of interest, which hardware has this been observed on?
I have tested this on two Intel Broadwell machines.
Since by now all we have are indications that this is an erratum,
this information belongs into the commit message. As it is written
now, it means the bit can't be set on any hardware. If there are
reasons beyond this erratum to uniformly disallow the bit to be
set by guests, these should be named here too. After all the
way you do the change, you now refuse values with the bit set
I don't think this is specific to Broadwell. I tried this on a Haswell
(model 60) and got a #GPF as well.

If I understand what this bit does, it is pretty pointless in a guest.
It is only useful in some sort of embedded setup, where something is
hooked up to a particular pin on the board. So perhaps this is not an
erratum but rather a not fully documented feature, where if nothing is
connected to that pin this bit should not be set.

Or maybe it is documented but I can't find anything on that.
Kevin, Jun?
I asked internally but didn't get a clarification yet.

Either way, we should mask this bit.
Sure, but: Refuse attempts to set it, or silently ignore such?
I think the former, especially if what I surmised above is correct ---
the user shouldn't try to set it.

I'm with the former too.

btw regardless of clarification which I'm trying to get, I think we do
need disallow such guest operation going to physical MSR. It's not
good to have guest impact physical PMU interrupt behavior. Even
when we want to support guest PC operation in the future, it needs
be emulated as the comment given in KVM side. If others also
agree with this assumption, we could check-in this patch w/o
mentioning any possible erratum...
Do we have a consensus on this? Should we push through with this patch or any other
changes/clarifications are required?

+ my personal email id.

Xen-devel mailing list

Xen-devel mailing list



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