linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Jon Doron <arilou@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 3/7] KVM: x86: hyper-v: make KVM_GET_SUPPORTED_HV_CPUID output independent of eVMCS enablement
Date: Fri,  7 Aug 2020 10:39:42 +0200	[thread overview]
Message-ID: <20200807083946.377654-4-vkuznets@redhat.com> (raw)
In-Reply-To: <20200807083946.377654-1-vkuznets@redhat.com>

It is really inconvenient when KVM_GET_SUPPORTED_HV_CPUID output depends on
whether some other capabilities are enabled as VMM needs to be aware of the
correct sequence (which becomes an API).

Enlightened VMCS is not the only feature which requires explicit enablement.
In particular, SynIC has to be enabled explicitly too but the corresponding
features are always present in KVM_GET_SUPPORTED_HV_CPUID, this is
inconsistent. In any case, userspace can't blindly copy the output of
KVM_GET_SUPPORTED_HV_CPUID to guest's CPUIDs and expect everything to work.

Previously, we were also using EVMCS enablement to decide whether or not to
report HYPERV_CPUID_NESTED_FEATURES feature leaf as the last available leaf
in HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES but this is already gone with
SYNDBG and nobody seemed to complain.

As a preparation to making KVM_GET_SUPPORTED_HV_CPUID a system ioctl, make
EVMCS feature bits work the same way as all other bits, nothing should break
(famous last words).

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 Documentation/virt/kvm/api.rst                |  3 --
 arch/x86/include/asm/kvm_host.h               |  2 +-
 arch/x86/kvm/hyperv.c                         |  8 ++---
 arch/x86/kvm/vmx/evmcs.c                      |  8 ++---
 arch/x86/kvm/vmx/evmcs.h                      |  2 +-
 .../selftests/kvm/x86_64/hyperv_cpuid.c       | 35 +++++--------------
 6 files changed, 15 insertions(+), 43 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index c0ccad0def5a..6acdc6f80aee 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4498,9 +4498,6 @@ Currently, the following list of CPUID leaves are returned:
  - HYPERV_CPUID_SYNDBG_INTERFACE
  - HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES
 
-HYPERV_CPUID_NESTED_FEATURES leaf is only exposed when Enlightened VMCS was
-enabled on the corresponding vCPU (KVM_CAP_HYPERV_ENLIGHTENED_VMCS).
-
 Userspace invokes KVM_GET_SUPPORTED_HV_CPUID by passing a kvm_cpuid2 structure
 with the 'nent' field indicating the number of entries in the variable-size
 array 'entries'.  If the number of entries is too low to describe all Hyper-V
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1bab87a444d7..cd82559490a5 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1243,7 +1243,7 @@ struct kvm_x86_nested_ops {
 
 	int (*enable_evmcs)(struct kvm_vcpu *vcpu,
 			    uint16_t *vmcs_version);
-	uint16_t (*get_evmcs_version)(struct kvm_vcpu *vcpu);
+	uint16_t (*get_evmcs_version)(void);
 };
 
 struct kvm_x86_init_ops {
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 4291943d1028..5a6831799544 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1961,19 +1961,15 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 		{ .function = HYPERV_CPUID_FEATURES },
 		{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
 		{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
+		{ .function = HYPERV_CPUID_NESTED_FEATURES },
 		{ .function = HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS },
 		{ .function = HYPERV_CPUID_SYNDBG_INTERFACE },
 		{ .function = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES	},
-		{ .function = HYPERV_CPUID_NESTED_FEATURES },
 	};
 	int i, nent = ARRAY_SIZE(cpuid_entries);
 
 	if (kvm_x86_ops.nested_ops->get_evmcs_version)
-		evmcs_ver = kvm_x86_ops.nested_ops->get_evmcs_version(vcpu);
-
-	/* Skip NESTED_FEATURES if eVMCS is not supported */
-	if (!evmcs_ver)
-		--nent;
+		evmcs_ver = kvm_x86_ops.nested_ops->get_evmcs_version();
 
 	if (cpuid->nent < nent)
 		return -E2BIG;
diff --git a/arch/x86/kvm/vmx/evmcs.c b/arch/x86/kvm/vmx/evmcs.c
index e5325bd0f304..4559d5851bf0 100644
--- a/arch/x86/kvm/vmx/evmcs.c
+++ b/arch/x86/kvm/vmx/evmcs.c
@@ -325,17 +325,15 @@ bool nested_enlightened_vmentry(struct kvm_vcpu *vcpu, u64 *evmcs_gpa)
 	return true;
 }
 
-uint16_t nested_get_evmcs_version(struct kvm_vcpu *vcpu)
+uint16_t nested_get_evmcs_version(void)
 {
-	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	/*
 	 * vmcs_version represents the range of supported Enlightened VMCS
 	 * versions: lower 8 bits is the minimal version, higher 8 bits is the
 	 * maximum supported version. KVM supports versions from 1 to
 	 * KVM_EVMCS_VERSION.
 	 */
-	if (kvm_cpu_cap_get(X86_FEATURE_VMX) &&
-	    vmx->nested.enlightened_vmcs_enabled)
+	if (kvm_cpu_cap_get(X86_FEATURE_VMX))
 		return (KVM_EVMCS_VERSION << 8) | 1;
 
 	return 0;
@@ -427,7 +425,7 @@ int nested_enable_evmcs(struct kvm_vcpu *vcpu,
 	vmx->nested.enlightened_vmcs_enabled = true;
 
 	if (vmcs_version)
-		*vmcs_version = nested_get_evmcs_version(vcpu);
+		*vmcs_version = nested_get_evmcs_version();
 
 	return 0;
 }
diff --git a/arch/x86/kvm/vmx/evmcs.h b/arch/x86/kvm/vmx/evmcs.h
index e5f7a7ebf27d..8e6429ec9ace 100644
--- a/arch/x86/kvm/vmx/evmcs.h
+++ b/arch/x86/kvm/vmx/evmcs.h
@@ -206,7 +206,7 @@ enum nested_evmptrld_status {
 };
 
 bool nested_enlightened_vmentry(struct kvm_vcpu *vcpu, u64 *evmcs_gpa);
-uint16_t nested_get_evmcs_version(struct kvm_vcpu *vcpu);
+uint16_t nested_get_evmcs_version(void);
 int nested_enable_evmcs(struct kvm_vcpu *vcpu,
 			uint16_t *vmcs_version);
 void nested_evmcs_filter_control_msr(u32 msr_index, u64 *pdata);
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 745b708c2d3b..8b24cb2e6a19 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -45,20 +45,16 @@ static bool smt_possible(void)
 	return res;
 }
 
-static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
-			  bool evmcs_enabled)
+static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries)
 {
 	int i;
-	int nent = 9;
+	int nent = 10;
 	u32 test_val;
 
-	if (evmcs_enabled)
-		nent += 1; /* 0x4000000A */
-
 	TEST_ASSERT(hv_cpuid_entries->nent == nent,
 		    "KVM_GET_SUPPORTED_HV_CPUID should return %d entries"
-		    " with evmcs=%d (returned %d)",
-		    nent, evmcs_enabled, hv_cpuid_entries->nent);
+		    " (returned %d)",
+		    nent, hv_cpuid_entries->nent);
 
 	for (i = 0; i < hv_cpuid_entries->nent; i++) {
 		struct kvm_cpuid_entry2 *entry = &hv_cpuid_entries->entries[i];
@@ -68,9 +64,6 @@ static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
 			    "function %x is our of supported range",
 			    entry->function);
 
-		TEST_ASSERT(evmcs_enabled || (entry->function != 0x4000000A),
-			    "0x4000000A leaf should not be reported");
-
 		TEST_ASSERT(entry->index == 0,
 			    ".index field should be zero");
 
@@ -85,9 +78,8 @@ static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
 			test_val = 0x40000082;
 
 			TEST_ASSERT(entry->eax == test_val,
-				    "Wrong max leaf report in 0x40000000.EAX: %x"
-				    " (evmcs=%d)",
-				    entry->eax, evmcs_enabled
+				    "Wrong max leaf report in 0x40000000.EAX: %x",
+				    entry->eax
 				);
 			break;
 		case 0x40000004:
@@ -148,7 +140,6 @@ int main(int argc, char *argv[])
 	struct kvm_vm *vm;
 	int rv, stage;
 	struct kvm_cpuid2 *hv_cpuid_entries;
-	bool evmcs_enabled;
 
 	/* Tell stdout not to buffer its content */
 	setbuf(stdout, NULL);
@@ -159,8 +150,7 @@ int main(int argc, char *argv[])
 		exit(KSFT_SKIP);
 	}
 
-	for (stage = 0; stage < 3; stage++) {
-		evmcs_enabled = false;
+	for (stage = 0; stage < 2; stage++) {
 
 		vm = vm_create_default(VCPU_ID, 0, guest_code);
 		switch (stage) {
@@ -169,19 +159,10 @@ int main(int argc, char *argv[])
 			continue;
 		case 1:
 			break;
-		case 2:
-			if (!nested_vmx_supported() ||
-			    !kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
-				print_skip("Enlightened VMCS is unsupported");
-				continue;
-			}
-			vcpu_enable_evmcs(vm, VCPU_ID);
-			evmcs_enabled = true;
-			break;
 		}
 
 		hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
-		test_hv_cpuid(hv_cpuid_entries, evmcs_enabled);
+		test_hv_cpuid(hv_cpuid_entries);
 		free(hv_cpuid_entries);
 		kvm_vm_free(vm);
 	}
-- 
2.25.4


  parent reply	other threads:[~2020-08-07  8:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-07  8:39 [PATCH 0/7] KVM: x86: hyper-v: make KVM_GET_SUPPORTED_HV_CPUID more useful Vitaly Kuznetsov
2020-08-07  8:39 ` [PATCH 1/7] KVM: x86: hyper-v: Mention SynDBG CPUID leaves in api.rst Vitaly Kuznetsov
2020-08-07  8:39 ` [PATCH 2/7] KVM: x86: hyper-v: disallow configuring SynIC timers with no SynIC Vitaly Kuznetsov
2020-08-07  8:39 ` Vitaly Kuznetsov [this message]
2020-08-07  8:39 ` [PATCH 4/7] KVM: x86: hyper-v: always advertise HV_STIMER_DIRECT_MODE_AVAILABLE Vitaly Kuznetsov
2020-08-07  8:39 ` [PATCH 5/7] KVM: x86: hyper-v: drop now unneeded vcpu parameter from kvm_vcpu_ioctl_get_hv_cpuid() Vitaly Kuznetsov
2020-08-07  8:39 ` [PATCH 6/7] KVM: x86: hyper-v: allow KVM_GET_SUPPORTED_HV_CPUID as a system ioctl Vitaly Kuznetsov
2020-08-07  8:39 ` [PATCH 7/7] KVM: selftests: test " Vitaly Kuznetsov
2020-09-01 14:55 ` [PATCH 0/7] KVM: x86: hyper-v: make KVM_GET_SUPPORTED_HV_CPUID more useful Vitaly Kuznetsov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200807083946.377654-4-vkuznets@redhat.com \
    --to=vkuznets@redhat.com \
    --cc=arilou@gmail.com \
    --cc=ehabkost@redhat.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=wanpengli@tencent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).