On Sun, Mar 28, 2021 at 09:28:11AM -0400, Bryan Steele wrote:
> On Sun, Mar 28, 2021 at 08:38:13AM -0400, Dave Voutila wrote:
> > abieber@ found the latest 9front release ends up in a boot loop if
> > hosted on an AMD system. I tracked it down to 9front (oddly) trying to
> > read the PAT msr prior to writing it. [1] The problem is vmm(4)'s msr
> > handling for svm injects #GP exceptions into the guest for most msr
> > reads (since we don't emulate more than a few).
> >
> > For those (two? few? dozen?) 9front users of AMD hardware and -current,
> > can you try the below diff?
> >
> > vmm(4)'s vmx msr handlers ignores this instruction and only logs the
> > rdmsr information if the kernel is built with VMM_DEBUG. vmm(4) will
> > advance the instruction pointer regardless and it's up to the guest to
> > deal with any resulting issues.
> >
> > The diff syncs the logic between the svm and vmx msr vm-exit handlers by
> > injecting #GP *ONLY* on attempts to read the SMBASE msr.
> >
> > For context, this is the vmx rdmsr handler's (vmx_handle_rdmsr) logic:
> >
> > switch (*rcx) {
> > case MSR_SMBASE:
> > /*
> > * 34.15.6.3 - Saving Guest State (SMM)
> > *
> > * Unsupported, so inject #GP and return without
> > * advancing %rip.
> > */
> > ret = vmm_inject_gp(vcpu);
> > return (ret);
> > }
> >
> > It is *not* a design for emulating PAT access and manipulation by a
> > guest.
> >
> > (As an aside, OpenBSD doesn't bother reading the msr [2] before writing
> > to it, neither does Linux. Why is 9front special? ¯\_(ツ)_/¯)
> >
> > -Dave
> >
> > [1] https://code.9front.org/hg/plan9front/rev/10cd3e23a8c1
> > [2]
> > https://github.com/openbsd/src/blob/36fd90dcf1acf2ddb4ef5dbabe5313b3a8d46ee2/sys/arch/amd64/amd64/cpu.c#L1145-L1168
> >
IIRC I had to advertise PAT support or some guest OS didn't work. I can't recall
what OS that was though (this was years ago).
I'd say just allow reading of the host PAT but discard all writes. Do the same
on
both SVM and VMX, for now. See if this helps 9front.
Bonus points: there are rules for accessing and manipulating the PAT in a guest,
we could probably emulate that if desired.
-ml
> >
> > Index: sys/arch/amd64/amd64/vmm.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> > retrieving revision 1.278
> > diff -u -p -r1.278 vmm.c
> > --- sys/arch/amd64/amd64/vmm.c 11 Mar 2021 11:16:55 -0000 1.278
> > +++ sys/arch/amd64/amd64/vmm.c 28 Mar 2021 00:45:08 -0000
> > @@ -6545,10 +6545,16 @@ svm_handle_msr(struct vcpu *vcpu)
> > *rax = 0;
> > *rdx = 0;
> > break;
> > - default:
> > - DPRINTF("%s: guest read msr 0x%llx, injecting "
> > - "#GP\n", __func__, *rcx);
> > + case MSR_SMBASE:
> > + /* Unsupported, inject #GP w/o advancing %rip */
> > ret = vmm_inject_gp(vcpu);
> > return (ret);
> > +#ifdef VMM_DEBUG
> > + default:
> > + /* Log the access to identify unknown MSRs */
> > + DPRINTF("%s: rdmsr exit, msr=0x%llx, data "
> > + "returned to guest=0x%llx:0x%llx\n",
> > + __func__, *rcx, *rdx, *rax);
> > +#endif /* VMM_DEBUG */
> > }
> > }
>
> I'm not sure this is correct, doesn't this mean that registers will
> contain whatevever garbage that was in them beforehand, without
> injecting #GP host does the guest kernel to know the MSR read failed?
>
> I was initially concerned as this touches the codepath pd@ fixed last
> Feb where MSR reads were being passed through to the host, but still
> I think that injecting the #GP for unsupported MSR reads is right.
>
> -Bryan.
>
