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

Re: [PATCH v3 for-4.15] x86/msr: introduce an option for compatible MSR behavior selection


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 10 Mar 2021 12:13:50 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=MSlPR975+YkePtkUfBjhUF8gMxXvL5hxSJ1uqufy44A=; b=iwv2swueLLrvwo1fd5tPhKllhFvERUjZprbGOMicXrlLUuUmoP3RQQpB4aokE6zs7npujYODCs9M4yuEd+wRsKIF2XShuNbBor8OXJxZ+2uJYd5nB2hKOSeE/XHbkeR4TrJ7PxxNSLaaFCURC0G0mEs9Sy+fGd26vsVBR9JrFQJPFnGabeUZgsTs2LlWmh9Pjs6bSd5hMXy4sfOUk8Rw6fL0KEC5984dFzkbw/Ri6IiBqDcPGl8PZmCbkS5avvCjQCndSTkDipBv4uGMMaqvnAsRn9/VBF0d8WsWugW4ykD+PjwzoZZ4A47GFThvDokzFZHzXG8l5WHkXv+mzLKQ0A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=crVL2ycZ/RlDFEqK/AFLrcGd36QF7QL3/5dkhzo/zluug191xXrrJBRQKOoZqRDUOQPAz+UUdH+lzeJPWhfAC1Ebb0XeivX4XFBnJbXu5/X1wi2Ja45jTwkJTVr0c9jhXhGOHWss5/ueaSNCMb1usnCqr8y6gYtxqB0dL+Vff8pkP2QF5xtVWARsFzsCl9f8k2v1MnfUq1peNQGeIg3cDCWthfbJO7X/EEI5m02MMr0NcNfQbDK7zuIPxewbx6LGT06i9W1aAqpo9ta8LmZNBrbm0iT1pVIxPTkkKoX9DONe39RTMf5c4fut8Iop92SKl8devjQqBOnPORDE7iwb+g==
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Christian Lindig <christian.lindig@xxxxxxxxxx>, David Scott <dave@xxxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 10 Mar 2021 11:14:28 +0000
  • Ironport-sdr: 4Se8efW+pUmNnD3Hz3E0SEdTjoZ5ZSDwSogEMbjeSSEcgNQBRmNW+X+r7i4R10ibDc2+BXIphZ bWKHWK9flUMi+3ZmY36o1rou15N7OQe7SmH0wKegcgtAF/hIoVIfgLK5g3Gf1Q5oou0kEDMHcV o1vJT5AbqQJfPWXgEb990a0kPtsz311LHXGC75Jxc3MeONEpc/eTMCbIHEycl8P0xizfjskYNM Wa0uTd/RQaLFmoxu+RGDqKiHFQWp4DD4aABSJMTR5U6SD3Fe03CMNfxRiPwTXqbrcxEKNfi9qP js4=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Tue, Mar 09, 2021 at 07:57:42PM +0000, Andrew Cooper wrote:
> On 09/03/2021 11:36, Jan Beulich wrote:
> > On 09.03.2021 11:56, Roger Pau Monne wrote:
> >> --- a/xen/arch/x86/pv/emul-priv-op.c
> >> +++ b/xen/arch/x86/pv/emul-priv-op.c
> >> @@ -875,6 +875,7 @@ static int read_msr(unsigned int reg, uint64_t *val,
> >>      const struct domain *currd = curr->domain;
> >>      const struct cpuid_policy *cp = currd->arch.cpuid;
> >>      bool vpmu_msr = false;
> >> +    uint64_t tmp;
> >>      int ret;
> >>  
> >>      if ( (ret = guest_rdmsr(curr, reg, val)) != X86EMUL_UNHANDLEABLE )
> >> @@ -986,6 +987,12 @@ static int read_msr(unsigned int reg, uint64_t *val,
> >>          }
> >>          /* fall through */
> >>      default:
> >> +        if ( currd->arch.msr_relaxed && !rdmsr_safe(reg, tmp) )
> >> +        {
> >> +            *val = 0;
> >> +            return X86EMUL_OKAY;
> >> +        }
> >> +
> >>          gdprintk(XENLOG_WARNING, "RDMSR 0x%08x unimplemented\n", reg);
> >>          break;
> >>  
> >> @@ -1148,6 +1155,9 @@ static int write_msr(unsigned int reg, uint64_t val,
> >>          }
> >>          /* fall through */
> >>      default:
> >> +        if ( currd->arch.msr_relaxed && !rdmsr_safe(reg, val) )
> >> +            return X86EMUL_OKAY;
> >> +
> >>          gdprintk(XENLOG_WARNING,
> >>                   "WRMSR 0x%08x val 0x%016"PRIx64" unimplemented\n",
> >>                   reg, val);
> > So what are your thoughts wrt my change to this file? Drop it
> > altogether and require people to use this new option? Or do you
> > see both coexist? In the latter case, since you had suggested
> > that I drop the write side of my change - does your changing of
> > the write path indicate you've changed your mind?
> 
> I don't think we should legitimise buggy PV behaviour, either by
> codifying in the ABI, or providing a knob beyond this one.

In that case - can we try to figure out which MSRs is PV Linux trying
to access without having the #GP handler setup?

Maybe we can try to handle the ones we know the buggy Linux versions
will try to access without having #GP setup?

I know it's not possible to test all possible Linux versions, but we
could at least try to get the ones we know about fixed. Is the only
offending one we know about MSR_K8_HWCR?

> A guest blindly stumbling forward with a missed #GP could very well
> worse than crashing hard.
> 
> Case in point - the 4.15 behaviour spotted a very serious bug in NetBSD
> where it tried writing MSR_PAT with its own choice (which wasn't the
> same as Xen's choice).  The consequence of this bug is getting cache
> attributes in pagetables wrong.

Was that write by NetBSD done without the #GP handler setup?

Because the patch by Jan affects only that specific scenario.

Thanks, Roger.



 


Rackspace

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