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

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

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

Thanks
Kevin

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