Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1532914 > unrolled thread
| Started by | David Matlack <dmatlack@google.com> |
|---|---|
| First post | 2016-11-30 03:20 +0100 |
| Last post | 2016-11-30 21:50 +0100 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v3 1/5] KVM: nVMX: generate non-true VMX MSRs based on true versions David Matlack <dmatlack@google.com> - 2016-11-30 03:20 +0100
Re: [PATCH v3 1/5] KVM: nVMX: generate non-true VMX MSRs based on true versions Paolo Bonzini <pbonzini@redhat.com> - 2016-11-30 12:20 +0100
Re: [PATCH v3 1/5] KVM: nVMX: generate non-true VMX MSRs based on true versions David Matlack <dmatlack@google.com> - 2016-11-30 19:10 +0100
Re: [PATCH v3 1/5] KVM: nVMX: generate non-true VMX MSRs based on true versions Paolo Bonzini <pbonzini@redhat.com> - 2016-11-30 21:50 +0100
| From | David Matlack <dmatlack@google.com> |
|---|---|
| Date | 2016-11-30 03:20 +0100 |
| Subject | [PATCH v3 1/5] KVM: nVMX: generate non-true VMX MSRs based on true versions |
| Message-ID | <sJ2F3-8jd-11@gated-at.bofh.it> |
The "non-true" VMX capability MSRs can be generated from their "true"
counterparts, by OR-ing the default1 bits. The default1 bits are fixed
and defined in the SDM.
Since we can generate the non-true VMX MSRs from the true versions,
there's no need to store both in struct nested_vmx. This also lets
userspace avoid having to restore the non-true MSRs.
Note this does not preclude emulating MSR_IA32_VMX_BASIC[55]=0. To do so,
we simply need to set all the default1 bits in the true MSRs (such that
the true MSRs and the generated non-true MSRs are equal).
Signed-off-by: David Matlack <dmatlack@google.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/vmx.c | 45 +++++++++++++++++++--------------------------
1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 5382b82..0beb56a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -446,19 +446,21 @@ struct nested_vmx {
u16 vpid02;
u16 last_vpid;
+ /*
+ * We only store the "true" versions of the VMX capability MSRs. We
+ * generate the "non-true" versions by setting the must-be-1 bits
+ * according to the SDM.
+ */
u32 nested_vmx_procbased_ctls_low;
u32 nested_vmx_procbased_ctls_high;
- u32 nested_vmx_true_procbased_ctls_low;
u32 nested_vmx_secondary_ctls_low;
u32 nested_vmx_secondary_ctls_high;
u32 nested_vmx_pinbased_ctls_low;
u32 nested_vmx_pinbased_ctls_high;
u32 nested_vmx_exit_ctls_low;
u32 nested_vmx_exit_ctls_high;
- u32 nested_vmx_true_exit_ctls_low;
u32 nested_vmx_entry_ctls_low;
u32 nested_vmx_entry_ctls_high;
- u32 nested_vmx_true_entry_ctls_low;
u32 nested_vmx_misc_low;
u32 nested_vmx_misc_high;
u32 nested_vmx_ept_caps;
@@ -2712,9 +2714,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
vmx->nested.nested_vmx_exit_ctls_high |= VM_EXIT_CLEAR_BNDCFGS;
/* We support free control of debug control saving. */
- vmx->nested.nested_vmx_true_exit_ctls_low =
- vmx->nested.nested_vmx_exit_ctls_low &
- ~VM_EXIT_SAVE_DEBUG_CONTROLS;
+ vmx->nested.nested_vmx_exit_ctls_low &= ~VM_EXIT_SAVE_DEBUG_CONTROLS;
/* entry controls */
rdmsr(MSR_IA32_VMX_ENTRY_CTLS,
@@ -2733,9 +2733,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
vmx->nested.nested_vmx_entry_ctls_high |= VM_ENTRY_LOAD_BNDCFGS;
/* We support free control of debug control loading. */
- vmx->nested.nested_vmx_true_entry_ctls_low =
- vmx->nested.nested_vmx_entry_ctls_low &
- ~VM_ENTRY_LOAD_DEBUG_CONTROLS;
+ vmx->nested.nested_vmx_entry_ctls_low &= ~VM_ENTRY_LOAD_DEBUG_CONTROLS;
/* cpu-based controls */
rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
@@ -2768,8 +2766,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
CPU_BASED_USE_MSR_BITMAPS;
/* We support free control of CR3 access interception. */
- vmx->nested.nested_vmx_true_procbased_ctls_low =
- vmx->nested.nested_vmx_procbased_ctls_low &
+ vmx->nested.nested_vmx_procbased_ctls_low &=
~(CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING);
/* secondary cpu-based controls */
@@ -2868,36 +2865,32 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
*pdata = vmx_control_msr(
vmx->nested.nested_vmx_pinbased_ctls_low,
vmx->nested.nested_vmx_pinbased_ctls_high);
+ if (msr_index == MSR_IA32_VMX_PINBASED_CTLS)
+ *pdata |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR;
break;
case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
- *pdata = vmx_control_msr(
- vmx->nested.nested_vmx_true_procbased_ctls_low,
- vmx->nested.nested_vmx_procbased_ctls_high);
- break;
case MSR_IA32_VMX_PROCBASED_CTLS:
*pdata = vmx_control_msr(
vmx->nested.nested_vmx_procbased_ctls_low,
vmx->nested.nested_vmx_procbased_ctls_high);
+ if (msr_index == MSR_IA32_VMX_PROCBASED_CTLS)
+ *pdata |= CPU_BASED_ALWAYSON_WITHOUT_TRUE_MSR;
break;
case MSR_IA32_VMX_TRUE_EXIT_CTLS:
- *pdata = vmx_control_msr(
- vmx->nested.nested_vmx_true_exit_ctls_low,
- vmx->nested.nested_vmx_exit_ctls_high);
- break;
case MSR_IA32_VMX_EXIT_CTLS:
*pdata = vmx_control_msr(
vmx->nested.nested_vmx_exit_ctls_low,
vmx->nested.nested_vmx_exit_ctls_high);
+ if (msr_index == MSR_IA32_VMX_EXIT_CTLS)
+ *pdata |= VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR;
break;
case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
- *pdata = vmx_control_msr(
- vmx->nested.nested_vmx_true_entry_ctls_low,
- vmx->nested.nested_vmx_entry_ctls_high);
- break;
case MSR_IA32_VMX_ENTRY_CTLS:
*pdata = vmx_control_msr(
vmx->nested.nested_vmx_entry_ctls_low,
vmx->nested.nested_vmx_entry_ctls_high);
+ if (msr_index == MSR_IA32_VMX_ENTRY_CTLS)
+ *pdata |= VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR;
break;
case MSR_IA32_VMX_MISC:
*pdata = vmx_control_msr(
@@ -10184,7 +10177,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
}
if (!vmx_control_verify(vmcs12->cpu_based_vm_exec_control,
- vmx->nested.nested_vmx_true_procbased_ctls_low,
+ vmx->nested.nested_vmx_procbased_ctls_low,
vmx->nested.nested_vmx_procbased_ctls_high) ||
!vmx_control_verify(vmcs12->secondary_vm_exec_control,
vmx->nested.nested_vmx_secondary_ctls_low,
@@ -10193,10 +10186,10 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
vmx->nested.nested_vmx_pinbased_ctls_low,
vmx->nested.nested_vmx_pinbased_ctls_high) ||
!vmx_control_verify(vmcs12->vm_exit_controls,
- vmx->nested.nested_vmx_true_exit_ctls_low,
+ vmx->nested.nested_vmx_exit_ctls_low,
vmx->nested.nested_vmx_exit_ctls_high) ||
!vmx_control_verify(vmcs12->vm_entry_controls,
- vmx->nested.nested_vmx_true_entry_ctls_low,
+ vmx->nested.nested_vmx_entry_ctls_low,
vmx->nested.nested_vmx_entry_ctls_high))
{
nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_CONTROL_FIELD);
--
2.8.0.rc3.226.g39d4020
[toc] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2016-11-30 12:20 +0100 |
| Subject | Re: [PATCH v3 1/5] KVM: nVMX: generate non-true VMX MSRs based on true versions |
| Message-ID | <sJb5D-5oB-17@gated-at.bofh.it> |
| In reply to | #1532914 |
On 30/11/2016 03:14, David Matlack wrote:
> The "non-true" VMX capability MSRs can be generated from their "true"
> counterparts, by OR-ing the default1 bits. The default1 bits are fixed
> and defined in the SDM.
>
> Since we can generate the non-true VMX MSRs from the true versions,
> there's no need to store both in struct nested_vmx. This also lets
> userspace avoid having to restore the non-true MSRs.
>
> Note this does not preclude emulating MSR_IA32_VMX_BASIC[55]=0. To do so,
> we simply need to set all the default1 bits in the true MSRs (such that
> the true MSRs and the generated non-true MSRs are equal).
>
> Signed-off-by: David Matlack <dmatlack@google.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/kvm/vmx.c | 45 +++++++++++++++++++--------------------------
> 1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 5382b82..0beb56a 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -446,19 +446,21 @@ struct nested_vmx {
> u16 vpid02;
> u16 last_vpid;
>
> + /*
> + * We only store the "true" versions of the VMX capability MSRs. We
> + * generate the "non-true" versions by setting the must-be-1 bits
> + * according to the SDM.
> + */
> u32 nested_vmx_procbased_ctls_low;
> u32 nested_vmx_procbased_ctls_high;
> - u32 nested_vmx_true_procbased_ctls_low;
> u32 nested_vmx_secondary_ctls_low;
> u32 nested_vmx_secondary_ctls_high;
> u32 nested_vmx_pinbased_ctls_low;
> u32 nested_vmx_pinbased_ctls_high;
> u32 nested_vmx_exit_ctls_low;
> u32 nested_vmx_exit_ctls_high;
> - u32 nested_vmx_true_exit_ctls_low;
> u32 nested_vmx_entry_ctls_low;
> u32 nested_vmx_entry_ctls_high;
> - u32 nested_vmx_true_entry_ctls_low;
> u32 nested_vmx_misc_low;
> u32 nested_vmx_misc_high;
> u32 nested_vmx_ept_caps;
> @@ -2712,9 +2714,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
> vmx->nested.nested_vmx_exit_ctls_high |= VM_EXIT_CLEAR_BNDCFGS;
>
> /* We support free control of debug control saving. */
> - vmx->nested.nested_vmx_true_exit_ctls_low =
> - vmx->nested.nested_vmx_exit_ctls_low &
> - ~VM_EXIT_SAVE_DEBUG_CONTROLS;
> + vmx->nested.nested_vmx_exit_ctls_low &= ~VM_EXIT_SAVE_DEBUG_CONTROLS;
>
> /* entry controls */
> rdmsr(MSR_IA32_VMX_ENTRY_CTLS,
> @@ -2733,9 +2733,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
> vmx->nested.nested_vmx_entry_ctls_high |= VM_ENTRY_LOAD_BNDCFGS;
>
> /* We support free control of debug control loading. */
> - vmx->nested.nested_vmx_true_entry_ctls_low =
> - vmx->nested.nested_vmx_entry_ctls_low &
> - ~VM_ENTRY_LOAD_DEBUG_CONTROLS;
> + vmx->nested.nested_vmx_entry_ctls_low &= ~VM_ENTRY_LOAD_DEBUG_CONTROLS;
>
> /* cpu-based controls */
> rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
> @@ -2768,8 +2766,7 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
> CPU_BASED_USE_MSR_BITMAPS;
>
> /* We support free control of CR3 access interception. */
> - vmx->nested.nested_vmx_true_procbased_ctls_low =
> - vmx->nested.nested_vmx_procbased_ctls_low &
> + vmx->nested.nested_vmx_procbased_ctls_low &=
> ~(CPU_BASED_CR3_LOAD_EXITING | CPU_BASED_CR3_STORE_EXITING);
>
> /* secondary cpu-based controls */
> @@ -2868,36 +2865,32 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
> *pdata = vmx_control_msr(
> vmx->nested.nested_vmx_pinbased_ctls_low,
> vmx->nested.nested_vmx_pinbased_ctls_high);
> + if (msr_index == MSR_IA32_VMX_PINBASED_CTLS)
> + *pdata |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR;
Almost: PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR must be
added to both the low and high parts. Likewise below.
I guess you can use vmx_control_msr to generate it, too.
Paolo
> break;
> case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
> - *pdata = vmx_control_msr(
> - vmx->nested.nested_vmx_true_procbased_ctls_low,
> - vmx->nested.nested_vmx_procbased_ctls_high);
> - break;
> case MSR_IA32_VMX_PROCBASED_CTLS:
> *pdata = vmx_control_msr(
> vmx->nested.nested_vmx_procbased_ctls_low,
> vmx->nested.nested_vmx_procbased_ctls_high);
> + if (msr_index == MSR_IA32_VMX_PROCBASED_CTLS)
> + *pdata |= CPU_BASED_ALWAYSON_WITHOUT_TRUE_MSR;
> break;
> case MSR_IA32_VMX_TRUE_EXIT_CTLS:
> - *pdata = vmx_control_msr(
> - vmx->nested.nested_vmx_true_exit_ctls_low,
> - vmx->nested.nested_vmx_exit_ctls_high);
> - break;
> case MSR_IA32_VMX_EXIT_CTLS:
> *pdata = vmx_control_msr(
> vmx->nested.nested_vmx_exit_ctls_low,
> vmx->nested.nested_vmx_exit_ctls_high);
> + if (msr_index == MSR_IA32_VMX_EXIT_CTLS)
> + *pdata |= VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR;
> break;
> case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> - *pdata = vmx_control_msr(
> - vmx->nested.nested_vmx_true_entry_ctls_low,
> - vmx->nested.nested_vmx_entry_ctls_high);
> - break;
> case MSR_IA32_VMX_ENTRY_CTLS:
> *pdata = vmx_control_msr(
> vmx->nested.nested_vmx_entry_ctls_low,
> vmx->nested.nested_vmx_entry_ctls_high);
> + if (msr_index == MSR_IA32_VMX_ENTRY_CTLS)
> + *pdata |= VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR;
> break;
> case MSR_IA32_VMX_MISC:
> *pdata = vmx_control_msr(
> @@ -10184,7 +10177,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
> }
>
> if (!vmx_control_verify(vmcs12->cpu_based_vm_exec_control,
> - vmx->nested.nested_vmx_true_procbased_ctls_low,
> + vmx->nested.nested_vmx_procbased_ctls_low,
> vmx->nested.nested_vmx_procbased_ctls_high) ||
> !vmx_control_verify(vmcs12->secondary_vm_exec_control,
> vmx->nested.nested_vmx_secondary_ctls_low,
> @@ -10193,10 +10186,10 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
> vmx->nested.nested_vmx_pinbased_ctls_low,
> vmx->nested.nested_vmx_pinbased_ctls_high) ||
> !vmx_control_verify(vmcs12->vm_exit_controls,
> - vmx->nested.nested_vmx_true_exit_ctls_low,
> + vmx->nested.nested_vmx_exit_ctls_low,
> vmx->nested.nested_vmx_exit_ctls_high) ||
> !vmx_control_verify(vmcs12->vm_entry_controls,
> - vmx->nested.nested_vmx_true_entry_ctls_low,
> + vmx->nested.nested_vmx_entry_ctls_low,
> vmx->nested.nested_vmx_entry_ctls_high))
> {
> nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_CONTROL_FIELD);
>
[toc] | [prev] | [next] | [standalone]
| From | David Matlack <dmatlack@google.com> |
|---|---|
| Date | 2016-11-30 19:10 +0100 |
| Subject | Re: [PATCH v3 1/5] KVM: nVMX: generate non-true VMX MSRs based on true versions |
| Message-ID | <sJhuq-10J-21@gated-at.bofh.it> |
| In reply to | #1533232 |
On Wed, Nov 30, 2016 at 3:16 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 30/11/2016 03:14, David Matlack wrote: >> >> /* secondary cpu-based controls */ >> @@ -2868,36 +2865,32 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) >> *pdata = vmx_control_msr( >> vmx->nested.nested_vmx_pinbased_ctls_low, >> vmx->nested.nested_vmx_pinbased_ctls_high); >> + if (msr_index == MSR_IA32_VMX_PINBASED_CTLS) >> + *pdata |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR; > > Almost: PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR must be > added to both the low and high parts. Likewise below. > I guess you can use vmx_control_msr to generate it, too. SGTM. Although that would mean the true MSRs indicate a bit must-be-0 while the non-true MSRs are indicating it must-be-1, which seems odd.
[toc] | [prev] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2016-11-30 21:50 +0100 |
| Subject | Re: [PATCH v3 1/5] KVM: nVMX: generate non-true VMX MSRs based on true versions |
| Message-ID | <sJjZf-2rP-9@gated-at.bofh.it> |
| In reply to | #1533482 |
----- Original Message ----- > From: "David Matlack" <dmatlack@google.com> > To: "Paolo Bonzini" <pbonzini@redhat.com> > Cc: "kvm list" <kvm@vger.kernel.org>, linux-kernel@vger.kernel.org, "Jim Mattson" <jmattson@google.com>, "Radim > Krčmář" <rkrcmar@redhat.com> > Sent: Wednesday, November 30, 2016 7:05:04 PM > Subject: Re: [PATCH v3 1/5] KVM: nVMX: generate non-true VMX MSRs based on true versions > > On Wed, Nov 30, 2016 at 3:16 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 30/11/2016 03:14, David Matlack wrote: > >> > >> /* secondary cpu-based controls */ > >> @@ -2868,36 +2865,32 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, > >> u32 msr_index, u64 *pdata) > >> *pdata = vmx_control_msr( > >> vmx->nested.nested_vmx_pinbased_ctls_low, > >> vmx->nested.nested_vmx_pinbased_ctls_high); > >> + if (msr_index == MSR_IA32_VMX_PINBASED_CTLS) > >> + *pdata |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR; > > > > Almost: PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR must be > > added to both the low and high parts. Likewise below. > > I guess you can use vmx_control_msr to generate it, too. > > SGTM. > > Although that would mean the true MSRs indicate a bit must-be-0 while > the non-true MSRs are indicating it must-be-1, which seems odd. You're right. the high part is "can be 1", so the true MSR's high part must already include the always-on-without-true-MSR bits. Good! Paolo
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web