Re: vmm(4) patch - iniatialise eptp to zero for vmx like svm
On Thu, Feb 06, 2020 at 01:05:01AM -0800, Mike Larkin wrote: > On Thu, Feb 06, 2020 at 02:34:47AM +, Adam Steen wrote: > > Hi > > > > Again while working on a larger patch i noticed that the eptp for vmx > > was not getting initialised to zero like the svm code path, as part of > > a VMM_IOC_RESETCPU ioctl call. > > > > please see the attach patch to initialise eptp to zero > > > > cheers > > Adam > > > > ? div > > Index: sys/arch/amd64/amd64/vmm.c > > === > > RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v > > retrieving revision 1.258 > > diff -u -p -u -p -r1.258 vmm.c > > --- sys/arch/amd64/amd64/vmm.c 31 Jan 2020 01:51:27 - 1.258 > > +++ sys/arch/amd64/amd64/vmm.c 6 Feb 2020 02:18:30 - > > @@ -2895,6 +2895,8 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s > > /* xcr0 power on default sets bit 0 (x87 state) */ > > vcpu->vc_gueststate.vg_xcr0 = XCR0_X87 & xsave_mask; > > > > + vcpu->vc_parent->vm_map->pmap->eptp = 0; > > + > > exit: > > /* Flush the VMCS */ > > if (vmclear(>vc_control_pa)) { > > > > > > I do not believe this is what you want to do. > > The SVM path *should* reset the eptp to 0, since there is no EPTP in RVI based > scenarios. > > If you reset the eptp to 0 in an EPT environment, you'll lose the VPID and > the PA of the EPT itself (which, if you look earlier in that function, is > properly initialized; you're going to be whacking it back to 0 here): > > Around line 2620: > > ... > DPRINTF("Guest EPTP = 0x%llx\n", eptp); > if (vmwrite(VMCS_GUEST_IA32_EPTP, eptp)) { > DPRINTF("%s: error setting guest EPTP\n", __func__); > ret = EINVAL; > goto exit; > } > > vcpu->vc_parent->vm_map->pmap->eptp = eptp; > ... > > > -ml > PS - Note that although the EPTP is written to the VMCS in the previous code, we do use the cached eptp value in the VM's pmap to do EPT TLB flushes. If you clear it like this, you won't get the proper behaviour (note, the behaviour today isn't actually 100% correct to begin with, I have a diff for that but it is growing out of control it seems...)
Re: vmm(4) patch - iniatialise eptp to zero for vmx like svm
On Thu, Feb 06, 2020 at 02:34:47AM +, Adam Steen wrote: > Hi > > Again while working on a larger patch i noticed that the eptp for vmx > was not getting initialised to zero like the svm code path, as part of > a VMM_IOC_RESETCPU ioctl call. > > please see the attach patch to initialise eptp to zero > > cheers > Adam > > ? div > Index: sys/arch/amd64/amd64/vmm.c > === > RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v > retrieving revision 1.258 > diff -u -p -u -p -r1.258 vmm.c > --- sys/arch/amd64/amd64/vmm.c31 Jan 2020 01:51:27 - 1.258 > +++ sys/arch/amd64/amd64/vmm.c6 Feb 2020 02:18:30 - > @@ -2895,6 +2895,8 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s > /* xcr0 power on default sets bit 0 (x87 state) */ > vcpu->vc_gueststate.vg_xcr0 = XCR0_X87 & xsave_mask; > > + vcpu->vc_parent->vm_map->pmap->eptp = 0; > + > exit: > /* Flush the VMCS */ > if (vmclear(>vc_control_pa)) { > > I do not believe this is what you want to do. The SVM path *should* reset the eptp to 0, since there is no EPTP in RVI based scenarios. If you reset the eptp to 0 in an EPT environment, you'll lose the VPID and the PA of the EPT itself (which, if you look earlier in that function, is properly initialized; you're going to be whacking it back to 0 here): Around line 2620: ... DPRINTF("Guest EPTP = 0x%llx\n", eptp); if (vmwrite(VMCS_GUEST_IA32_EPTP, eptp)) { DPRINTF("%s: error setting guest EPTP\n", __func__); ret = EINVAL; goto exit; } vcpu->vc_parent->vm_map->pmap->eptp = eptp; ... -ml
vmm(4) patch - iniatialise eptp to zero for vmx like svm
Hi Again while working on a larger patch i noticed that the eptp for vmx was not getting initialised to zero like the svm code path, as part of a VMM_IOC_RESETCPU ioctl call. please see the attach patch to initialise eptp to zero cheers Adam ? div Index: sys/arch/amd64/amd64/vmm.c === RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v retrieving revision 1.258 diff -u -p -u -p -r1.258 vmm.c --- sys/arch/amd64/amd64/vmm.c 31 Jan 2020 01:51:27 - 1.258 +++ sys/arch/amd64/amd64/vmm.c 6 Feb 2020 02:18:30 - @@ -2895,6 +2895,8 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s /* xcr0 power on default sets bit 0 (x87 state) */ vcpu->vc_gueststate.vg_xcr0 = XCR0_X87 & xsave_mask; + vcpu->vc_parent->vm_map->pmap->eptp = 0; + exit: /* Flush the VMCS */ if (vmclear(>vc_control_pa)) {