Re: vmm(4) patch - iniatialise eptp to zero for vmx like svm

2020-02-06 Thread Mike Larkin
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

2020-02-06 Thread Mike Larkin
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

2020-02-05 Thread Adam Steen
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)) {