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

[Mohit Gambhir <mohit.gambhir@xxxxxxxxxx>]

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
> > > General
> > > > > > > > > Protection Fault and thus results in a hypervisor crash. This patch
> > > fixes
> > > > > the
> > > > > > > > > crash by masking PC bit and returning an error in case any guest
> > tries
> > > to
> > > > > write
> > > > > > > > > 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
> > > > > > everywhere.
> > > > > 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.
> Thanks
> Kevin
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel