| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <YtnMIkFI469Ub9vB@google.com>
Date: Thu, 21 Jul 2022 21:58:58 +0000
From: Sean Christopherson <seanjc@...gle.com>
To: Vitaly Kuznetsov <vkuznets@...hat.com>
Cc: kvm@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
Anirudh Rayabharam <anrayabh@...ux.microsoft.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jim Mattson <jmattson@...gle.com>,
Maxim Levitsky <mlevitsk@...hat.com>,
linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 09/25] KVM: VMX: nVMX: Support TSC scaling and
PERF_GLOBAL_CTRL with enlightened VMCS
Nit of all nits, just "KVM: nVMX:" in the shortlog.
On Thu, Jul 14, 2022, Vitaly Kuznetsov wrote:
> Enlightened VMCS v1 got updated and now includes the required fields
> for TSC scaling and loading PERF_GLOBAL_CTRL upon VMENTER/VMEXIT
> features. For Hyper-V on KVM enablement, KVM can just observe VMX control
> MSRs and use the features (with or without eVMCS) when
> possible. Hyper-V on KVM case is trickier because of live migration:
> the new features require explicit enablement from VMM to not break
> it. Luckily, the updated eVMCS revision comes with a feature bit in
> CPUID.0x4000000A.EBX.
Mostly out of curiosity, what happens if the VMM parrots back the results of
kvm_get_hv_cpuid()?
> Reviewed-by: Maxim Levitsky <mlevitsk@...hat.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.com>
> ---
> arch/x86/kvm/hyperv.c | 2 +-
> arch/x86/kvm/vmx/evmcs.c | 88 +++++++++++++++++++++++++++++++--------
> arch/x86/kvm/vmx/evmcs.h | 17 ++------
> arch/x86/kvm/vmx/nested.c | 2 +-
> arch/x86/kvm/vmx/vmx.c | 2 +-
> 5 files changed, 78 insertions(+), 33 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index a8e4944ca110..995d3ab1443e 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -2552,7 +2552,7 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
> case HYPERV_CPUID_NESTED_FEATURES:
> ent->eax = evmcs_ver;
> ent->eax |= HV_X64_NESTED_MSR_BITMAP;
> -
> + ent->ebx |= HV_X64_NESTED_EVMCS1_2022_UPDATE;
> break;
>
> case HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS:
> diff --git a/arch/x86/kvm/vmx/evmcs.c b/arch/x86/kvm/vmx/evmcs.c
> index 8bea5dea0341..52a53debd806 100644
> --- a/arch/x86/kvm/vmx/evmcs.c
> +++ b/arch/x86/kvm/vmx/evmcs.c
> @@ -368,7 +368,60 @@ uint16_t nested_get_evmcs_version(struct kvm_vcpu *vcpu)
> return 0;
> }
>
> -void nested_evmcs_filter_control_msr(u32 msr_index, u64 *pdata)
> +enum evmcs_v1_revision {
> + EVMCSv1_2016,
> + EVMCSv1_2022,
> +};
Why bother with the enums? They don't make life any easier, e.g. if there's a
2023 update then it's easy to do:
unsupported = <baseline>;
if (!has_evmcs_2022_features)
unsupported |= <2022 features>;
if (!has_evmcs_2023_features)
unsupported |= <2023 features>;
return unsupported;
diff --git a/arch/x86/kvm/vmx/evmcs.c b/arch/x86/kvm/vmx/evmcs.c
index b5cfbf7d487b..7b348a03e096 100644
--- a/arch/x86/kvm/vmx/evmcs.c
+++ b/arch/x86/kvm/vmx/evmcs.c
@@ -355,11 +355,6 @@ uint16_t nested_get_evmcs_version(struct kvm_vcpu *vcpu)
return 0;
}
-enum evmcs_v1_revision {
- EVMCSv1_2016,
- EVMCSv1_2022,
-};
-
enum evmcs_unsupported_ctrl_type {
EVMCS_EXIT_CTLS,
EVMCS_ENTRY_CTLS,
@@ -372,29 +367,29 @@ static u32 evmcs_get_unsupported_ctls(struct kvm_vcpu *vcpu,
enum evmcs_unsupported_ctrl_type ctrl_type)
{
struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
- enum evmcs_v1_revision evmcs_rev = EVMCSv1_2016;
+ bool has_evmcs_2022_features;
if (!hv_vcpu)
return 0;
- if (hv_vcpu->cpuid_cache.nested_ebx & HV_X64_NESTED_EVMCS1_2022_UPDATE)
- evmcs_rev = EVMCSv1_2022;
+ has_evmcs_2022_features = hv_vcpu->cpuid_cache.nested_ebx &
+ HV_X64_NESTED_EVMCS1_2022_UPDATE;
switch (ctrl_type) {
case EVMCS_EXIT_CTLS:
- if (evmcs_rev == EVMCSv1_2016)
+ if (!has_evmcs_2022_features)
return EVMCS1_UNSUPPORTED_VMEXIT_CTRL |
VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL;
else
return EVMCS1_UNSUPPORTED_VMEXIT_CTRL;
case EVMCS_ENTRY_CTLS:
- if (evmcs_rev == EVMCSv1_2016)
+ if (!has_evmcs_2022_features)
return EVMCS1_UNSUPPORTED_VMENTRY_CTRL |
VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL;
else
return EVMCS1_UNSUPPORTED_VMENTRY_CTRL;
case EVMCS_2NDEXEC:
- if (evmcs_rev == EVMCSv1_2016)
+ if (!has_evmcs_2022_features)
return EVMCS1_UNSUPPORTED_2NDEXEC |
SECONDARY_EXEC_TSC_SCALING;
else
> +
> +enum evmcs_unsupported_ctrl_type {
> + EVMCS_EXIT_CTLS,
> + EVMCS_ENTRY_CTLS,
> + EVMCS_2NDEXEC,
> + EVMCS_PINCTRL,
> + EVMCS_VMFUNC,
> +};
Same question here, it just makes life more difficult. I.e. do
static u32 evmcs_get_unsupported_ctls(struct kvm_vcpu *vcpu, int msr_index)
and then
void nested_evmcs_filter_control_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
{
u32 ctl_low = (u32)*pdata;
u32 ctl_high = (u32)(*pdata >> 32);
/*
* Hyper-V 2016 and 2019 try using unsupported features when eVMCS is
* enabled but there are no corresponding fields.
*/
ctl_high &= ~evmcs_get_unsupported_ctls(vcpu, msr_index);
*pdata = ctl_low | ((u64)ctl_high << 32);
}
int nested_evmcs_check_controls(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
{
int ret = 0;
u32 unsupp_ctl;
unsupp_ctl = vmcs12->pin_based_vm_exec_control &
evmcs_get_unsupported_ctls(vcpu, MSR_IA32_VMX_TRUE_PINBASED_CTLS);
<and so on and so forth>
}
Powered by blists - more mailing lists