Hello tech@,

I have a question (or maybe a suggestion) about vmm(4).

I'm writing a small additional feature to sys/arch/amd64/amd64/vmm.c
and found a seemingly unneeded vmclear() at the end of
vcpu_readregs_vmx(). This function didn't seem to affect VM state at
first glance because, obviously, its name is _read_. Against this
intuition, this function clears VM state, which makes the vmexit
handling stop (eg. "advance rip" failure) and the VM die if it is used
in there. Possibly this was added as a counterpart of
vcpu_reload_vmcs_vmx() at the very beginning of the read function, but
I don't think it does undo the reload.

This vmclear can be removed in my understanding and also I confirmed
that Alpine Linux and OpenBSD guest VMs can run on the vmm(4) without
this vmclear call.

Or I am just wrong and it may be actually needed in some corner cases.
If so, I will restore (vmptrld) vcpu->vc_control_pa every time the VM
context continues after calling readregs function or add another flag
to indicate whether vmclear is needed here if all of the corner cases
are known.

Would you mind to tell me I am correct or not, or how it has to be
dealt with?


Index: sys/arch/amd64/amd64/vmm.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.254
diff -u -p -r1.254 vmm.c
--- sys/arch/amd64/amd64/vmm.c    22 Sep 2019 08:47:54 -0000    1.254
+++ sys/arch/amd64/amd64/vmm.c    20 Oct 2019 18:04:17 -0000
@@ -1602,8 +1602,10 @@ vcpu_readregs_vmx(struct vcpu *vcpu, uin
 errout:
     ret = EINVAL;
 out:
+    /* XXX Why do we need vmclear here?
     if (vmclear(&vcpu->vc_control_pa))
         ret = EINVAL;
+    */
     return (ret);
 }

Reply via email to