Re: [RFC PATCH v6 07/36] KVM: nVMX: Refactor unsupported eVMCS controls logic to use 2-d array

Vitaly Kuznetsov
2022-08-25T10:24:39Z

Sean Christopherson <seanjc@google.com> writes:

> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>
> Refactor the handling of unsupported eVMCS to use a 2-d array to store
> the set of unsupported controls. KVM's handling of eVMCS is completely
> broken as there is no way for userspace to query which features are
> unsupported, nor does KVM prevent userspace from attempting to enable
> unsupported features. A future commit will remedy that by filtering and
> enforcing unsupported features when eVMCS, but that needs to be opt-in
> from userspace to avoid breakage, i.e. KVM needs to maintain its legacy
> behavior by snapshotting the exact set of controls that are currently
> (un)supported by eVMCS.
>
> No functional change intended.
>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> [sean: split to standalone patch, write changelog]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/vmx/evmcs.c | 60 +++++++++++++++++++++++++++++++++-------
> 1 file changed, 50 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/evmcs.c b/arch/x86/kvm/vmx/evmcs.c
> index 9139c70b6008..10fc0be49f96 100644
> --- a/arch/x86/kvm/vmx/evmcs.c
> +++ b/arch/x86/kvm/vmx/evmcs.c
> @@ -345,6 +345,45 @@ uint16_t nested_get_evmcs_version(struct kvm_vcpu *vcpu)
> return 0;
> }
>
> +enum evmcs_revision {
> + EVMCSv1_LEGACY,
> + NR_EVMCS_REVISIONS,
> +};
> +
> +enum evmcs_ctrl_type {
> + EVMCS_EXIT_CTRLS,
> + EVMCS_ENTRY_CTRLS,
> + EVMCS_2NDEXEC,
> + EVMCS_PINCTRL,
> + EVMCS_VMFUNC,
> + NR_EVMCS_CTRLS,
> +};
> +
> +static const u32 evmcs_unsupported_ctrls[NR_EVMCS_CTRLS][NR_EVMCS_REVISIONS] = {
> + [EVMCS_EXIT_CTRLS] = {
> + [EVMCSv1_LEGACY] = EVMCS1_UNSUPPORTED_VMEXIT_CTRL | VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL,
> + },
> + [EVMCS_ENTRY_CTRLS] = {
> + [EVMCSv1_LEGACY] = EVMCS1_UNSUPPORTED_VMENTRY_CTRL | VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL,
> + },
> + [EVMCS_2NDEXEC] = {
> + [EVMCSv1_LEGACY] = EVMCS1_UNSUPPORTED_2NDEXEC | SECONDARY_EXEC_TSC_SCALING,

By the time of this patch, VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL,
VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL, SECONDARY_EXEC_TSC_SCALING are
still in 'EVMCS1_UNSUPPORTED_*' lists.

> + },
> + [EVMCS_PINCTRL] = {
> + [EVMCSv1_LEGACY] = EVMCS1_UNSUPPORTED_PINCTRL,
> + },
> + [EVMCS_VMFUNC] = {
> + [EVMCSv1_LEGACY] = EVMCS1_UNSUPPORTED_VMFUNC,
> + },
> +};
> +
> +static u32 evmcs_get_unsupported_ctls(enum evmcs_ctrl_type ctrl_type)
> +{
> + enum evmcs_revision evmcs_rev = EVMCSv1_LEGACY;
> +
> + return evmcs_unsupported_ctrls[ctrl_type][evmcs_rev];
> +}
> +
> void nested_evmcs_filter_control_msr(u32 msr_index, u64 *pdata)
> {
> u32 ctl_low = (u32)*pdata;
> @@ -357,21 +396,21 @@ void nested_evmcs_filter_control_msr(u32 msr_index, u64 *pdata)
> switch (msr_index) {
> case MSR_IA32_VMX_EXIT_CTLS:
> case MSR_IA32_VMX_TRUE_EXIT_CTLS:
> - ctl_high &= ~EVMCS1_UNSUPPORTED_VMEXIT_CTRL;
> + ctl_high &= ~evmcs_get_unsupported_ctls(EVMCS_EXIT_CTRLS);
> break;
> case MSR_IA32_VMX_ENTRY_CTLS:
> case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> - ctl_high &= ~EVMCS1_UNSUPPORTED_VMENTRY_CTRL;
> + ctl_high &= ~evmcs_get_unsupported_ctls(EVMCS_ENTRY_CTRLS);
> break;
> case MSR_IA32_VMX_PROCBASED_CTLS2:
> - ctl_high &= ~EVMCS1_UNSUPPORTED_2NDEXEC;
> + ctl_high &= ~evmcs_get_unsupported_ctls(EVMCS_2NDEXEC);
> break;
> case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
> case MSR_IA32_VMX_PINBASED_CTLS:
> - ctl_high &= ~EVMCS1_UNSUPPORTED_PINCTRL;
> + ctl_high &= ~evmcs_get_unsupported_ctls(EVMCS_PINCTRL);
> break;
> case MSR_IA32_VMX_VMFUNC:
> - ctl_low &= ~EVMCS1_UNSUPPORTED_VMFUNC;
> + ctl_low &= ~evmcs_get_unsupported_ctls(EVMCS_VMFUNC);
> break;
> }
>
> @@ -384,7 +423,7 @@ int nested_evmcs_check_controls(struct vmcs12 *vmcs12)
> u32 unsupp_ctl;
>
> unsupp_ctl = vmcs12->pin_based_vm_exec_control &
> - EVMCS1_UNSUPPORTED_PINCTRL;
> + evmcs_get_unsupported_ctls(EVMCS_PINCTRL);
> if (unsupp_ctl) {
> trace_kvm_nested_vmenter_failed(
> "eVMCS: unsupported pin-based VM-execution controls",
> @@ -393,7 +432,7 @@ int nested_evmcs_check_controls(struct vmcs12 *vmcs12)
> }
>
> unsupp_ctl = vmcs12->secondary_vm_exec_control &
> - EVMCS1_UNSUPPORTED_2NDEXEC;
> + evmcs_get_unsupported_ctls(EVMCS_2NDEXEC);
> if (unsupp_ctl) {
> trace_kvm_nested_vmenter_failed(
> "eVMCS: unsupported secondary VM-execution controls",
> @@ -402,7 +441,7 @@ int nested_evmcs_check_controls(struct vmcs12 *vmcs12)
> }
>
> unsupp_ctl = vmcs12->vm_exit_controls &
> - EVMCS1_UNSUPPORTED_VMEXIT_CTRL;
> + evmcs_get_unsupported_ctls(EVMCS_EXIT_CTRLS);
> if (unsupp_ctl) {
> trace_kvm_nested_vmenter_failed(
> "eVMCS: unsupported VM-exit controls",
> @@ -411,7 +450,7 @@ int nested_evmcs_check_controls(struct vmcs12 *vmcs12)
> }
>
> unsupp_ctl = vmcs12->vm_entry_controls &
> - EVMCS1_UNSUPPORTED_VMENTRY_CTRL;
> + evmcs_get_unsupported_ctls(EVMCS_ENTRY_CTRLS);
> if (unsupp_ctl) {
> trace_kvm_nested_vmenter_failed(
> "eVMCS: unsupported VM-entry controls",
> @@ -419,7 +458,8 @@ int nested_evmcs_check_controls(struct vmcs12 *vmcs12)
> ret = -EINVAL;
> }
>
> - unsupp_ctl = vmcs12->vm_function_control & EVMCS1_UNSUPPORTED_VMFUNC;
> + unsupp_ctl = vmcs12->vm_function_control &
> + evmcs_get_unsupported_ctls(EVMCS_VMFUNC);
> if (unsupp_ctl) {
> trace_kvm_nested_vmenter_failed(
> "eVMCS: unsupported VM-function controls",

--
Vitaly