On Sat, Apr 29, 2017 at 05:24:22PM -0700, Pratik Vyas wrote:
> Hello tech@,
>
> This is a patch that extends the readregs and writeregs vmm(4) ioctl to
> read and write MSRs as well.
>
> It also sets the IA32_VMX_IA32E_MODE_GUEST entry control in
> vcpu_reset_regs based on the value of EFER_LMA.
>
> There are changes to vmmvar.h and would require a `make includes` step
> to build vmd(8). This should also not alter any behaviour of vmd(8).
>
> Context: These are the kernel changes required to implement vmctl send
> and vmctl receive. vmctl send / receive are two new options that will
> support snapshotting VMs and migrating VMs from one host to another.
> This project was undertaken at San Jose State University along with my
> three teammates CCed with mlarkin@ as our advisor.
>
> Patches to vmd(8) that implement vmctl send and vmctl receive are
> forthcoming.
>
>
> Thanks,
> Pratik
>
>
For some more color commentary - this diff is needed to provide vmd a way to
access the MSRs via the register set it sends to vmm for VM initialization.
Without that, the VM resumes with default/power on state, and that won't work
to restore a VM that had already been running.
I'll do some quick testing myself (I know this team has already tested OpenBSD
and Linux guests with this code, for both bios and non-bios boots) and if I
don't see any problems, I'll commit this tonight. Unless someone objects?
-ml
>
> Index: sys/arch/amd64/amd64/vmm.c
> ===================================================================
> RCS file: /home/pdvyas/cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.137
> diff -u -p -a -u -r1.137 vmm.c
> --- sys/arch/amd64/amd64/vmm.c 28 Apr 2017 10:09:37 -0000 1.137
> +++ sys/arch/amd64/amd64/vmm.c 30 Apr 2017 00:01:21 -0000
> @@ -1334,7 +1334,9 @@ vcpu_readregs_vmx(struct vcpu *vcpu, uin
> uint64_t sel, limit, ar;
> uint64_t *gprs = vrs->vrs_gprs;
> uint64_t *crs = vrs->vrs_crs;
> + uint64_t *msrs = vrs->vrs_msrs;
> struct vcpu_segment_info *sregs = vrs->vrs_sregs;
> + struct vmx_msr_store *msr_store;
>
> if (vcpu_reload_vmcs_vmx(&vcpu->vc_control_pa))
> return (EINVAL);
> @@ -1402,6 +1404,14 @@ vcpu_readregs_vmx(struct vcpu *vcpu, uin
> goto errout;
> }
>
> + msr_store = (struct vmx_msr_store *)vcpu->vc_vmx_msr_exit_save_va;
> +
> + if (regmask & VM_RWREGS_MSRS) {
> + for (i = 0; i < VCPU_REGS_NMSRS; i++) {
> + msrs[i] = msr_store[i].vms_data;
> + }
> + }
> +
> goto out;
>
> errout:
> @@ -1448,7 +1458,9 @@ vcpu_writeregs_vmx(struct vcpu *vcpu, ui
> uint64_t limit, ar;
> uint64_t *gprs = vrs->vrs_gprs;
> uint64_t *crs = vrs->vrs_crs;
> + uint64_t *msrs = vrs->vrs_msrs;
> struct vcpu_segment_info *sregs = vrs->vrs_sregs;
> + struct vmx_msr_store *msr_store;
>
> if (loadvmcs) {
> if (vcpu_reload_vmcs_vmx(&vcpu->vc_control_pa))
> @@ -1518,6 +1530,14 @@ vcpu_writeregs_vmx(struct vcpu *vcpu, ui
> goto errout;
> }
>
> + msr_store = (struct vmx_msr_store *)vcpu->vc_vmx_msr_exit_save_va;
> +
> + if (regmask & VM_RWREGS_MSRS) {
> + for (i = 0; i < VCPU_REGS_NMSRS; i++) {
> + msr_store[i].vms_data = msrs[i];
> + }
> + }
> +
> goto out;
>
> errout:
> @@ -2128,7 +2148,7 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s
> * IA32_VMX_LOAD_DEBUG_CONTROLS
> * IA32_VMX_LOAD_IA32_PERF_GLOBAL_CTRL_ON_ENTRY
> */
> - if (ug == 1)
> + if (ug == 1 && !(vrs->vrs_msrs[VCPU_REGS_EFER] & EFER_LMA))
> want1 = 0;
> else
> want1 = IA32_VMX_IA32E_MODE_GUEST;
> @@ -2277,26 +2297,12 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s
> */
> msr_store = (struct vmx_msr_store *)vcpu->vc_vmx_msr_exit_save_va;
>
> - /*
> - * Make sure LME is enabled in EFER if restricted guest mode is
> - * needed.
> - */
> - msr_store[0].vms_index = MSR_EFER;
> - if (ug == 1)
> - msr_store[0].vms_data = 0ULL; /* Initial value */
> - else
> - msr_store[0].vms_data = EFER_LME;
> -
> - msr_store[1].vms_index = MSR_STAR;
> - msr_store[1].vms_data = 0ULL; /* Initial value */
> - msr_store[2].vms_index = MSR_LSTAR;
> - msr_store[2].vms_data = 0ULL; /* Initial value */
> - msr_store[3].vms_index = MSR_CSTAR;
> - msr_store[3].vms_data = 0ULL; /* Initial value */
> - msr_store[4].vms_index = MSR_SFMASK;
> - msr_store[4].vms_data = 0ULL; /* Initial value */
> - msr_store[5].vms_index = MSR_KERNELGSBASE;
> - msr_store[5].vms_data = 0ULL; /* Initial value */
> + msr_store[VCPU_REGS_EFER].vms_index = MSR_EFER;
> + msr_store[VCPU_REGS_STAR].vms_index = MSR_STAR;
> + msr_store[VCPU_REGS_LSTAR].vms_index = MSR_LSTAR;
> + msr_store[VCPU_REGS_CSTAR].vms_index = MSR_CSTAR;
> + msr_store[VCPU_REGS_SFMASK].vms_index = MSR_SFMASK;
> + msr_store[VCPU_REGS_KGSBASE].vms_index = MSR_KERNELGSBASE;
>
> /*
> * Currently we have the same count of entry/exit MSRs loads/stores
> @@ -2359,6 +2365,13 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, s
> ret = vcpu_writeregs_vmx(vcpu, VM_RWREGS_ALL, 0, vrs);
>
> /*
> + * Make sure LME is enabled in EFER if restricted guest mode is
> + * needed.
> + */
> + if (ug == 0)
> + msr_store[VCPU_REGS_EFER].vms_data |= EFER_LME;
> +
> + /*
> * Set up the MSR bitmap
> */
> memset((uint8_t *)vcpu->vc_msr_bitmap_va, 0xFF, PAGE_SIZE);
> @@ -4317,7 +4330,7 @@ vmx_handle_cr0_write(struct vcpu *vcpu,
> return
> (EINVAL);
> }
>
> - if (msr_store[0].vms_data & EFER_LME)
> + if (msr_store[VCPU_REGS_EFER].vms_data & EFER_LME)
> ectls |= IA32_VMX_IA32E_MODE_GUEST;
> else
> ectls &= ~IA32_VMX_IA32E_MODE_GUEST;
> Index: sys/arch/amd64/include/vmmvar.h
> ===================================================================
> RCS file: /home/pdvyas/cvs/src/sys/arch/amd64/include/vmmvar.h,v
> retrieving revision 1.35
> diff -u -p -a -u -r1.35 vmmvar.h
> --- sys/arch/amd64/include/vmmvar.h 28 Apr 2017 07:44:36 -0000 1.35
> +++ sys/arch/amd64/include/vmmvar.h 30 Apr 2017 00:01:21 -0000
> @@ -340,9 +340,18 @@ struct vcpu_segment_info {
> #define VCPU_REGS_TR 7
> #define VCPU_REGS_NSREGS (VCPU_REGS_TR + 1)
>
> +#define VCPU_REGS_EFER 0
> +#define VCPU_REGS_STAR 1
> +#define VCPU_REGS_LSTAR 2
> +#define VCPU_REGS_CSTAR 3
> +#define VCPU_REGS_SFMASK 4
> +#define VCPU_REGS_KGSBASE 5
> +#define VCPU_REGS_NMSRS (VCPU_REGS_KGSBASE + 1)
> +
> struct vcpu_reg_state {
> uint64_t vrs_gprs[VCPU_REGS_NGPRS];
> uint64_t vrs_crs[VCPU_REGS_NCRS];
> + uint64_t vrs_msrs[VCPU_REGS_NMSRS];
> struct vcpu_segment_info vrs_sregs[VCPU_REGS_NSREGS];
> struct vcpu_segment_info vrs_gdtr;
> struct vcpu_segment_info vrs_idtr;
> @@ -427,7 +436,9 @@ struct vm_intr_params {
> #define VM_RWREGS_GPRS 0x1 /* read/write GPRs */
> #define VM_RWREGS_SREGS 0x2 /* read/write segment registers */
> #define VM_RWREGS_CRS 0x4 /* read/write CRs */
> -#define VM_RWREGS_ALL (VM_RWREGS_GPRS | VM_RWREGS_SREGS |
> VM_RWREGS_CRS)
> +#define VM_RWREGS_MSRS 0x8 /* read/write MSRs */
> +#define VM_RWREGS_ALL (VM_RWREGS_GPRS | VM_RWREGS_SREGS |
> VM_RWREGS_CRS | \
> + VM_RWREGS_MSRS)
>
> struct vm_rwregs_params {
> uint32_t vrwp_vm_id;
> Index: usr.sbin/vmd/vm.c
> ===================================================================
> RCS file: /home/pdvyas/cvs/src/usr.sbin/vmd/vm.c,v
> retrieving revision 1.13
> diff -u -p -a -u -r1.13 vm.c
> --- usr.sbin/vmd/vm.c 25 Apr 2017 06:44:35 -0000 1.13
> +++ usr.sbin/vmd/vm.c 30 Apr 2017 00:01:21 -0000
> @@ -133,6 +133,12 @@ static const struct vcpu_reg_state vcpu_
> .vrs_idtr = { 0x0, 0xFFFF, 0x0, 0x0},
> .vrs_sregs[VCPU_REGS_LDTR] = { 0x0, 0xFFFF, 0x0082, 0x0},
> .vrs_sregs[VCPU_REGS_TR] = { 0x0, 0xFFFF, 0x008B, 0x0},
> + .vrs_msrs[VCPU_REGS_EFER] = 0ULL,
> + .vrs_msrs[VCPU_REGS_STAR] = 0ULL,
> + .vrs_msrs[VCPU_REGS_LSTAR] = 0ULL,
> + .vrs_msrs[VCPU_REGS_CSTAR] = 0ULL,
> + .vrs_msrs[VCPU_REGS_SFMASK] = 0ULL,
> + .vrs_msrs[VCPU_REGS_KGSBASE] = 0ULL
> };
>
> /*
> @@ -161,6 +167,12 @@ static const struct vcpu_reg_state vcpu_
> .vrs_idtr = { 0x0, 0xFFFF, 0x0, 0x0},
> .vrs_sregs[VCPU_REGS_LDTR] = { 0x0, 0xFFFF, 0x0082, 0x0},
> .vrs_sregs[VCPU_REGS_TR] = { 0x0, 0xFFFF, 0x008B, 0x0},
> + .vrs_msrs[VCPU_REGS_EFER] = 0ULL,
> + .vrs_msrs[VCPU_REGS_STAR] = 0ULL,
> + .vrs_msrs[VCPU_REGS_LSTAR] = 0ULL,
> + .vrs_msrs[VCPU_REGS_CSTAR] = 0ULL,
> + .vrs_msrs[VCPU_REGS_SFMASK] = 0ULL,
> + .vrs_msrs[VCPU_REGS_KGSBASE] = 0ULL
> };
>
> /*
>
