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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.