linux-hyperv.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger
@ 2020-04-24 11:37 Jon Doron
  2020-04-24 11:37 ` [PATCH v11 1/7] x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit Jon Doron
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
  To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron

Add support for the synthetic debugger interface of hyper-v, the
synthetic debugger has 2 modes.
1. Use a set of MSRs to send/recv information (undocumented so it's not
   going to the hyperv-tlfs.h)
2. Use hypercalls

The first mode is based the following MSRs:
1. Control/Status MSRs which either asks for a send/recv .
2. Send/Recv MSRs each holds GPA where the send/recv buffers are.
3. Pending MSR, holds a GPA to a PAGE that simply has a boolean that
   indicates if there is data pending to issue a recv VMEXIT.

The first mode implementation is to simply exit to user-space when
either the control MSR or the pending MSR are being set.
Then it's up-to userspace to implement the rest of the logic of sending/recving.

In the second mode instead of using MSRs KNet will simply issue
Hypercalls with the information to send/recv, in this mode the data
being transferred is UDP encapsulated, unlike in the previous mode in
which you get just the data to send.

The new hypercalls will exit to userspace which will be incharge of
re-encapsulating if needed the UDP packets to be sent.

There is an issue though in which KDNet does not respect the hypercall
page and simply issues vmcall/vmmcall instructions depending on the cpu
type expecting them to be handled as it a real hypercall was issued.

It's important to note that part of this feature has been subject to be
removed in future versions of Windows, which is why some of the
defintions will not be present the the TLFS but in the kvm hyperv header
instead.

v11:
Fixed all reviewed by and rebased on latest origin/master

Jon Doron (6):
  x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit
  x86/kvm/hyper-v: Simplify addition for custom cpuid leafs
  x86/hyper-v: Add synthetic debugger definitions
  x86/kvm/hyper-v: Add support for synthetic debugger capability
  x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
  x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls

Vitaly Kuznetsov (1):
  KVM: selftests: update hyperv_cpuid with SynDBG tests

 Documentation/virt/kvm/api.rst                |  18 ++
 arch/x86/include/asm/hyperv-tlfs.h            |   6 +
 arch/x86/include/asm/kvm_host.h               |  14 +
 arch/x86/kvm/hyperv.c                         | 242 ++++++++++++++++--
 arch/x86/kvm/hyperv.h                         |  33 +++
 arch/x86/kvm/trace.h                          |  51 ++++
 arch/x86/kvm/x86.c                            |  13 +
 include/uapi/linux/kvm.h                      |  13 +
 .../selftests/kvm/x86_64/hyperv_cpuid.c       | 143 +++++++----
 9 files changed, 468 insertions(+), 65 deletions(-)

-- 
2.24.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v11 1/7] x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit
  2020-04-24 11:37 [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
@ 2020-04-24 11:37 ` Jon Doron
  2020-05-13  8:42   ` Roman Kagan
  2020-04-24 11:37 ` [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs Jon Doron
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
  To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron

The problem the patch is trying to address is the fact that 'struct
kvm_hyperv_exit' has different layout on when compiling in 32 and 64 bit
modes.

In 64-bit mode the default alignment boundary is 64 bits thus
forcing extra gaps after 'type' and 'msr' but in 32-bit mode the
boundary is at 32 bits thus no extra gaps.

This is an issue as even when the kernel is 64 bit, the userspace using
the interface can be both 32 and 64 bit but the same 32 bit userspace has
to work with 32 bit kernel.

The issue is fixed by forcing the 64 bit layout, this leads to ABI
change for 32 bit builds and while we are obviously breaking '32 bit
userspace with 32 bit kernel' case, we're fixing the '32 bit userspace
with 64 bit kernel' one.

As the interface has no (known) users and 32 bit KVM is rather baroque
nowadays, this seems like a reasonable decision.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
 Documentation/virt/kvm/api.rst | 2 ++
 include/uapi/linux/kvm.h       | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index efbbe570aa9b..750d005a75bc 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5067,9 +5067,11 @@ EOI was received.
   #define KVM_EXIT_HYPERV_SYNIC          1
   #define KVM_EXIT_HYPERV_HCALL          2
 			__u32 type;
+			__u32 pad1;
 			union {
 				struct {
 					__u32 msr;
+					__u32 pad2;
 					__u64 control;
 					__u64 evt_page;
 					__u64 msg_page;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 428c7dde6b4b..9cdc5356f542 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -189,9 +189,11 @@ struct kvm_hyperv_exit {
 #define KVM_EXIT_HYPERV_SYNIC          1
 #define KVM_EXIT_HYPERV_HCALL          2
 	__u32 type;
+	__u32 pad1;
 	union {
 		struct {
 			__u32 msr;
+			__u32 pad2;
 			__u64 control;
 			__u64 evt_page;
 			__u64 msg_page;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs
  2020-04-24 11:37 [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
  2020-04-24 11:37 ` [PATCH v11 1/7] x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit Jon Doron
@ 2020-04-24 11:37 ` Jon Doron
  2020-05-13  9:24   ` Roman Kagan
  2020-04-24 11:37 ` [PATCH v11 3/7] x86/hyper-v: Add synthetic debugger definitions Jon Doron
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
  To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron

Simlify the code to define a new cpuid leaf group by enabled feature.

This also fixes a bug in which the max cpuid leaf was always set to
HYPERV_CPUID_NESTED_FEATURES regardless if nesting is supported or not.

Any new CPUID group needs to consider the max leaf and be added in the
correct order, in this method there are two rules:
1. Each cpuid leaf group must be order in an ascending order
2. The appending for the cpuid leafs by features also needs to happen by
   ascending order.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
 arch/x86/kvm/hyperv.c | 46 ++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index bcefa9d4e57e..ab3e9dbcabbe 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1785,27 +1785,45 @@ int kvm_vm_ioctl_hv_eventfd(struct kvm *kvm, struct kvm_hyperv_eventfd *args)
 	return kvm_hv_eventfd_assign(kvm, args->conn_id, args->fd);
 }
 
+/* Must be sorted in ascending order by function */
+static struct kvm_cpuid_entry2 core_cpuid_entries[] = {
+	{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
+	{ .function = HYPERV_CPUID_INTERFACE },
+	{ .function = HYPERV_CPUID_VERSION },
+	{ .function = HYPERV_CPUID_FEATURES },
+	{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
+	{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
+};
+
+static struct kvm_cpuid_entry2 evmcs_cpuid_entries[] = {
+	{ .function = HYPERV_CPUID_NESTED_FEATURES },
+};
+
+#define HV_MAX_CPUID_ENTRIES \
+	(ARRAY_SIZE(core_cpuid_entries) +\
+	 ARRAY_SIZE(evmcs_cpuid_entries))
+
 int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 				struct kvm_cpuid_entry2 __user *entries)
 {
 	uint16_t evmcs_ver = 0;
-	struct kvm_cpuid_entry2 cpuid_entries[] = {
-		{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
-		{ .function = HYPERV_CPUID_INTERFACE },
-		{ .function = HYPERV_CPUID_VERSION },
-		{ .function = HYPERV_CPUID_FEATURES },
-		{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
-		{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
-		{ .function = HYPERV_CPUID_NESTED_FEATURES },
-	};
-	int i, nent = ARRAY_SIZE(cpuid_entries);
+	struct kvm_cpuid_entry2 cpuid_entries[HV_MAX_CPUID_ENTRIES];
+	int i, nent = 0;
+
+	/* Set the core cpuid entries required for Hyper-V */
+	memcpy(&cpuid_entries[nent], &core_cpuid_entries,
+	       sizeof(core_cpuid_entries));
+	nent = ARRAY_SIZE(core_cpuid_entries);
 
 	if (kvm_x86_ops.nested_get_evmcs_version)
 		evmcs_ver = kvm_x86_ops.nested_get_evmcs_version(vcpu);
 
-	/* Skip NESTED_FEATURES if eVMCS is not supported */
-	if (!evmcs_ver)
-		--nent;
+	if (evmcs_ver) {
+		/* EVMCS is enabled, add the required EVMCS CPUID leafs */
+		memcpy(&cpuid_entries[nent], &evmcs_cpuid_entries,
+		       sizeof(evmcs_cpuid_entries));
+		nent += ARRAY_SIZE(evmcs_cpuid_entries);
+	}
 
 	if (cpuid->nent < nent)
 		return -E2BIG;
@@ -1821,7 +1839,7 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 		case HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS:
 			memcpy(signature, "Linux KVM Hv", 12);
 
-			ent->eax = HYPERV_CPUID_NESTED_FEATURES;
+			ent->eax = cpuid_entries[nent - 1].function;
 			ent->ebx = signature[0];
 			ent->ecx = signature[1];
 			ent->edx = signature[2];
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v11 3/7] x86/hyper-v: Add synthetic debugger definitions
  2020-04-24 11:37 [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
  2020-04-24 11:37 ` [PATCH v11 1/7] x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit Jon Doron
  2020-04-24 11:37 ` [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs Jon Doron
@ 2020-04-24 11:37 ` Jon Doron
  2020-04-24 11:37 ` [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability Jon Doron
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
  To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron, Michael Kelley

Hyper-V synthetic debugger has two modes, one that uses MSRs and
the other that use Hypercalls.

Add all the required definitions to both types of synthetic debugger
interface.

Some of the required new CPUIDs and MSRs are not documented in the TLFS
so they are in hyperv.h instead.

The reason they are not documented is because they are subjected to be
removed in future versions of Windows.

Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
 arch/x86/include/asm/hyperv-tlfs.h |  6 ++++++
 arch/x86/kvm/hyperv.h              | 27 +++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 29336574d0bc..53ef6b7bd380 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -131,6 +131,8 @@
 #define HV_FEATURE_FREQUENCY_MSRS_AVAILABLE		BIT(8)
 /* Crash MSR available */
 #define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE		BIT(10)
+/* Support for debug MSRs available */
+#define HV_FEATURE_DEBUG_MSRS_AVAILABLE			BIT(11)
 /* stimer Direct Mode is available */
 #define HV_STIMER_DIRECT_MODE_AVAILABLE			BIT(19)
 
@@ -376,6 +378,9 @@ struct hv_tsc_emulation_status {
 #define HVCALL_SEND_IPI_EX			0x0015
 #define HVCALL_POST_MESSAGE			0x005c
 #define HVCALL_SIGNAL_EVENT			0x005d
+#define HVCALL_POST_DEBUG_DATA			0x0069
+#define HVCALL_RETRIEVE_DEBUG_DATA		0x006a
+#define HVCALL_RESET_DEBUG_SESSION		0x006b
 #define HVCALL_RETARGET_INTERRUPT		0x007e
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
@@ -422,6 +427,7 @@ enum HV_GENERIC_SET_FORMAT {
 #define HV_STATUS_INVALID_HYPERCALL_INPUT	3
 #define HV_STATUS_INVALID_ALIGNMENT		4
 #define HV_STATUS_INVALID_PARAMETER		5
+#define HV_STATUS_OPERATION_DENIED		8
 #define HV_STATUS_INSUFFICIENT_MEMORY		11
 #define HV_STATUS_INVALID_PORT_ID		17
 #define HV_STATUS_INVALID_CONNECTION_ID		18
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 757cb578101c..7f50ff0bad07 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -23,6 +23,33 @@
 
 #include <linux/kvm_host.h>
 
+/*
+ * The #defines related to the synthetic debugger are required by KDNet, but
+ * they are not documented in the Hyper-V TLFS because the synthetic debugger
+ * functionality has been deprecated and is subject to removal in future
+ * versions of Windows.
+ */
+#define HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS	0x40000080
+#define HYPERV_CPUID_SYNDBG_INTERFACE			0x40000081
+#define HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES	0x40000082
+
+/*
+ * Hyper-V synthetic debugger platform capabilities
+ * These are HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES.EAX bits.
+ */
+#define HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING	BIT(1)
+
+/* Hyper-V Synthetic debug options MSR */
+#define HV_X64_MSR_SYNDBG_CONTROL		0x400000F1
+#define HV_X64_MSR_SYNDBG_STATUS		0x400000F2
+#define HV_X64_MSR_SYNDBG_SEND_BUFFER		0x400000F3
+#define HV_X64_MSR_SYNDBG_RECV_BUFFER		0x400000F4
+#define HV_X64_MSR_SYNDBG_PENDING_BUFFER	0x400000F5
+#define HV_X64_MSR_SYNDBG_OPTIONS		0x400000FF
+
+/* Hyper-V HV_X64_MSR_SYNDBG_OPTIONS bits */
+#define HV_X64_SYNDBG_OPTION_USE_HCALLS		BIT(2)
+
 static inline struct kvm_vcpu_hv *vcpu_to_hv_vcpu(struct kvm_vcpu *vcpu)
 {
 	return &vcpu->arch.hyperv;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability
  2020-04-24 11:37 [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
                   ` (2 preceding siblings ...)
  2020-04-24 11:37 ` [PATCH v11 3/7] x86/hyper-v: Add synthetic debugger definitions Jon Doron
@ 2020-04-24 11:37 ` Jon Doron
  2020-05-29 10:46   ` Paolo Bonzini
  2020-04-24 11:37 ` [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg Jon Doron
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
  To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron

Add support for Hyper-V synthetic debugger (syndbg) interface.
The syndbg interface is using MSRs to emulate a way to send/recv packets
data.

The debug transport dll (kdvm/kdnet) will identify if Hyper-V is enabled
and if it supports the synthetic debugger interface it will attempt to
use it, instead of trying to initialize a network adapter.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
 Documentation/virt/kvm/api.rst  |  16 ++++
 arch/x86/include/asm/kvm_host.h |  14 +++
 arch/x86/kvm/hyperv.c           | 165 +++++++++++++++++++++++++++++++-
 arch/x86/kvm/hyperv.h           |   6 ++
 arch/x86/kvm/trace.h            |  51 ++++++++++
 arch/x86/kvm/x86.c              |  13 +++
 include/uapi/linux/kvm.h        |  11 +++
 7 files changed, 273 insertions(+), 3 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 750d005a75bc..52ba12758f7c 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -5066,6 +5066,7 @@ EOI was received.
 		struct kvm_hyperv_exit {
   #define KVM_EXIT_HYPERV_SYNIC          1
   #define KVM_EXIT_HYPERV_HCALL          2
+  #define KVM_EXIT_HYPERV_SYNDBG         3
 			__u32 type;
 			__u32 pad1;
 			union {
@@ -5081,6 +5082,15 @@ EOI was received.
 					__u64 result;
 					__u64 params[2];
 				} hcall;
+				struct {
+					__u32 msr;
+					__u32 pad2;
+					__u64 control;
+					__u64 status;
+					__u64 send_page;
+					__u64 recv_page;
+					__u64 pending_page;
+				} syndbg;
 			} u;
 		};
 		/* KVM_EXIT_HYPERV */
@@ -5097,6 +5107,12 @@ Hyper-V SynIC state change. Notification is used to remap SynIC
 event/message pages and to enable/disable SynIC messages/events processing
 in userspace.
 
+	- KVM_EXIT_HYPERV_SYNDBG -- synchronously notify user-space about
+
+Hyper-V Synthetic debugger state change. Notification is used to either update
+the pending_page location or to send a control command (send the buffer located
+in send_page or recv a buffer to recv_page).
+
 ::
 
 		/* KVM_EXIT_ARM_NISV */
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 42a2d0d3984a..563a9e69f113 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -854,6 +854,19 @@ struct kvm_apic_map {
 	struct kvm_lapic *phys_map[];
 };
 
+/* Hyper-V synthetic debugger (SynDbg)*/
+struct kvm_hv_syndbg {
+	struct {
+		u64 control;
+		u64 status;
+		u64 send_page;
+		u64 recv_page;
+		u64 pending_page;
+	} control;
+	u64 options;
+	bool active;
+};
+
 /* Hyper-V emulation context */
 struct kvm_hv {
 	struct mutex hv_lock;
@@ -877,6 +890,7 @@ struct kvm_hv {
 	atomic_t num_mismatched_vp_indexes;
 
 	struct hv_partition_assist_pg *hv_pa_pg;
+	struct kvm_hv_syndbg hv_syndbg;
 };
 
 enum kvm_irqchip_mode {
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index ab3e9dbcabbe..435516595090 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -266,6 +266,117 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
 	return ret;
 }
 
+void kvm_hv_activate_syndbg(struct kvm_vcpu *vcpu)
+{
+	struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+	syndbg->active = true;
+}
+
+static int kvm_hv_syndbg_complete_userspace(struct kvm_vcpu *vcpu)
+{
+	struct kvm *kvm = vcpu->kvm;
+	struct kvm_hv *hv = &kvm->arch.hyperv;
+
+	if (vcpu->run->hyperv.u.syndbg.msr == HV_X64_MSR_SYNDBG_CONTROL)
+		hv->hv_syndbg.control.status =
+			vcpu->run->hyperv.u.syndbg.status;
+	return 1;
+}
+
+static void syndbg_exit(struct kvm_vcpu *vcpu, u32 msr)
+{
+	struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+	struct kvm_vcpu_hv *hv_vcpu = &vcpu->arch.hyperv;
+
+	hv_vcpu->exit.type = KVM_EXIT_HYPERV_SYNDBG;
+	hv_vcpu->exit.u.syndbg.msr = msr;
+	hv_vcpu->exit.u.syndbg.control = syndbg->control.control;
+	hv_vcpu->exit.u.syndbg.send_page = syndbg->control.send_page;
+	hv_vcpu->exit.u.syndbg.recv_page = syndbg->control.recv_page;
+	hv_vcpu->exit.u.syndbg.pending_page = syndbg->control.pending_page;
+	vcpu->arch.complete_userspace_io =
+			kvm_hv_syndbg_complete_userspace;
+
+	kvm_make_request(KVM_REQ_HV_EXIT, vcpu);
+}
+
+static int syndbg_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
+{
+	struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+	if (!syndbg->active && !host)
+		return 1;
+
+	trace_kvm_hv_syndbg_set_msr(vcpu->vcpu_id,
+				    vcpu_to_hv_vcpu(vcpu)->vp_index, msr, data);
+	switch (msr) {
+	case HV_X64_MSR_SYNDBG_CONTROL:
+		syndbg->control.control = data;
+		if (!host)
+			syndbg_exit(vcpu, msr);
+		break;
+	case HV_X64_MSR_SYNDBG_STATUS:
+		syndbg->control.status = data;
+		break;
+	case HV_X64_MSR_SYNDBG_SEND_BUFFER:
+		syndbg->control.send_page = data;
+		break;
+	case HV_X64_MSR_SYNDBG_RECV_BUFFER:
+		syndbg->control.recv_page = data;
+		break;
+	case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+		syndbg->control.pending_page = data;
+		if (!host)
+			syndbg_exit(vcpu, msr);
+		break;
+	case HV_X64_MSR_SYNDBG_OPTIONS:
+		syndbg->options = data;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
+{
+	struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+	if (!syndbg->active && !host)
+		return 1;
+
+	switch (msr) {
+	case HV_X64_MSR_SYNDBG_CONTROL:
+		*pdata = syndbg->control.control;
+		break;
+	case HV_X64_MSR_SYNDBG_STATUS:
+		*pdata = syndbg->control.status;
+		break;
+	case HV_X64_MSR_SYNDBG_SEND_BUFFER:
+		*pdata = syndbg->control.send_page;
+		break;
+	case HV_X64_MSR_SYNDBG_RECV_BUFFER:
+		*pdata = syndbg->control.recv_page;
+		break;
+	case HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+		*pdata = syndbg->control.pending_page;
+		break;
+	case HV_X64_MSR_SYNDBG_OPTIONS:
+		*pdata = syndbg->options;
+		break;
+	default:
+		break;
+	}
+
+	trace_kvm_hv_syndbg_get_msr(vcpu->vcpu_id,
+				    vcpu_to_hv_vcpu(vcpu)->vp_index, msr,
+				    *pdata);
+
+	return 0;
+}
+
 static int synic_get_msr(struct kvm_vcpu_hv_synic *synic, u32 msr, u64 *pdata,
 			 bool host)
 {
@@ -800,6 +911,8 @@ static bool kvm_hv_msr_partition_wide(u32 msr)
 	case HV_X64_MSR_REENLIGHTENMENT_CONTROL:
 	case HV_X64_MSR_TSC_EMULATION_CONTROL:
 	case HV_X64_MSR_TSC_EMULATION_STATUS:
+	case HV_X64_MSR_SYNDBG_OPTIONS:
+	case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
 		r = true;
 		break;
 	}
@@ -1061,6 +1174,9 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data,
 		if (!host)
 			return 1;
 		break;
+	case HV_X64_MSR_SYNDBG_OPTIONS:
+	case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+		return syndbg_set_msr(vcpu, msr, data, host);
 	default:
 		vcpu_unimpl(vcpu, "Hyper-V unhandled wrmsr: 0x%x data 0x%llx\n",
 			    msr, data);
@@ -1190,7 +1306,8 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
 	return 0;
 }
 
-static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata,
+			     bool host)
 {
 	u64 data = 0;
 	struct kvm *kvm = vcpu->kvm;
@@ -1227,6 +1344,9 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case HV_X64_MSR_TSC_EMULATION_STATUS:
 		data = hv->hv_tsc_emulation_status;
 		break;
+	case HV_X64_MSR_SYNDBG_OPTIONS:
+	case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+		return syndbg_get_msr(vcpu, msr, pdata, host);
 	default:
 		vcpu_unimpl(vcpu, "Hyper-V unhandled rdmsr: 0x%x\n", msr);
 		return 1;
@@ -1316,7 +1436,7 @@ int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
 		int r;
 
 		mutex_lock(&vcpu->kvm->arch.hyperv.hv_lock);
-		r = kvm_hv_get_msr_pw(vcpu, msr, pdata);
+		r = kvm_hv_get_msr_pw(vcpu, msr, pdata, host);
 		mutex_unlock(&vcpu->kvm->arch.hyperv.hv_lock);
 		return r;
 	} else
@@ -1799,9 +1919,16 @@ static struct kvm_cpuid_entry2 evmcs_cpuid_entries[] = {
 	{ .function = HYPERV_CPUID_NESTED_FEATURES },
 };
 
+static struct kvm_cpuid_entry2 syndbg_cpuid_entries[] = {
+	{ .function = HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS },
+	{ .function = HYPERV_CPUID_SYNDBG_INTERFACE },
+	{ .function = HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES	},
+};
+
 #define HV_MAX_CPUID_ENTRIES \
 	(ARRAY_SIZE(core_cpuid_entries) +\
-	 ARRAY_SIZE(evmcs_cpuid_entries))
+	 ARRAY_SIZE(evmcs_cpuid_entries) +\
+	 ARRAY_SIZE(syndbg_cpuid_entries))
 
 int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 				struct kvm_cpuid_entry2 __user *entries)
@@ -1809,6 +1936,7 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 	uint16_t evmcs_ver = 0;
 	struct kvm_cpuid_entry2 cpuid_entries[HV_MAX_CPUID_ENTRIES];
 	int i, nent = 0;
+	struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
 
 	/* Set the core cpuid entries required for Hyper-V */
 	memcpy(&cpuid_entries[nent], &core_cpuid_entries,
@@ -1825,6 +1953,13 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 		nent += ARRAY_SIZE(evmcs_cpuid_entries);
 	}
 
+	if (syndbg->active) {
+		/* Syndbg is enabled, add the required Syndbg CPUID leafs */
+		memcpy(&cpuid_entries[nent], &syndbg_cpuid_entries,
+		       sizeof(syndbg_cpuid_entries));
+		nent += ARRAY_SIZE(syndbg_cpuid_entries);
+	}
+
 	if (cpuid->nent < nent)
 		return -E2BIG;
 
@@ -1878,6 +2013,12 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 			ent->edx |= HV_FEATURE_FREQUENCY_MSRS_AVAILABLE;
 			ent->edx |= HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
 
+			if (syndbg->active) {
+				ent->ebx |= HV_X64_DEBUGGING;
+				ent->edx |= HV_X64_GUEST_DEBUGGING_AVAILABLE;
+				ent->edx |= HV_FEATURE_DEBUG_MSRS_AVAILABLE;
+			}
+
 			/*
 			 * Direct Synthetic timers only make sense with in-kernel
 			 * LAPIC
@@ -1921,6 +2062,24 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 
 			break;
 
+		case HYPERV_CPUID_SYNDBG_VENDOR_AND_MAX_FUNCTIONS:
+			memcpy(signature, "Linux KVM Hv", 12);
+
+			ent->eax = 0;
+			ent->ebx = signature[0];
+			ent->ecx = signature[1];
+			ent->edx = signature[2];
+			break;
+
+		case HYPERV_CPUID_SYNDBG_INTERFACE:
+			memcpy(signature, "VS#1\0\0\0\0\0\0\0\0", 12);
+			ent->eax = signature[0];
+			break;
+
+		case HYPERV_CPUID_SYNDBG_PLATFORM_CAPABILITIES:
+			ent->eax |= HV_X64_SYNDBG_CAP_ALLOW_KERNEL_DEBUGGING;
+			break;
+
 		default:
 			break;
 		}
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 7f50ff0bad07..50cca85b5e48 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -73,6 +73,11 @@ static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
 	return hv_vcpu_to_vcpu(container_of(synic, struct kvm_vcpu_hv, synic));
 }
 
+static inline struct kvm_hv_syndbg *vcpu_to_hv_syndbg(struct kvm_vcpu *vcpu)
+{
+	return &vcpu->kvm->arch.hyperv.hv_syndbg;
+}
+
 int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
 int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host);
 
@@ -83,6 +88,7 @@ void kvm_hv_irq_routing_update(struct kvm *kvm);
 int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
 void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector);
 int kvm_hv_activate_synic(struct kvm_vcpu *vcpu, bool dont_zero_synic_pages);
+void kvm_hv_activate_syndbg(struct kvm_vcpu *vcpu);
 
 void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu);
 void kvm_hv_vcpu_postcreate(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 249062f24b94..df95c45ec3bb 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1539,6 +1539,57 @@ TRACE_EVENT(kvm_nested_vmenter_failed,
 		__print_symbolic(__entry->err, VMX_VMENTER_INSTRUCTION_ERRORS))
 );
 
+/*
+ * Tracepoint for syndbg_set_msr.
+ */
+TRACE_EVENT(kvm_hv_syndbg_set_msr,
+	TP_PROTO(int vcpu_id, u32 vp_index, u32 msr, u64 data),
+	TP_ARGS(vcpu_id, vp_index, msr, data),
+
+	TP_STRUCT__entry(
+		__field(int, vcpu_id)
+		__field(u32, vp_index)
+		__field(u32, msr)
+		__field(u64, data)
+	),
+
+	TP_fast_assign(
+		__entry->vcpu_id = vcpu_id;
+		__entry->vp_index = vp_index;
+		__entry->msr = msr;
+		__entry->data = data;
+	),
+
+	TP_printk("vcpu_id %d vp_index %u msr 0x%x data 0x%llx",
+		  __entry->vcpu_id, __entry->vp_index, __entry->msr,
+		  __entry->data)
+);
+
+/*
+ * Tracepoint for syndbg_get_msr.
+ */
+TRACE_EVENT(kvm_hv_syndbg_get_msr,
+	TP_PROTO(int vcpu_id, u32 vp_index, u32 msr, u64 data),
+	TP_ARGS(vcpu_id, vp_index, msr, data),
+
+	TP_STRUCT__entry(
+		__field(int, vcpu_id)
+		__field(u32, vp_index)
+		__field(u32, msr)
+		__field(u64, data)
+	),
+
+	TP_fast_assign(
+		__entry->vcpu_id = vcpu_id;
+		__entry->vp_index = vp_index;
+		__entry->msr = msr;
+		__entry->data = data;
+	),
+
+	TP_printk("vcpu_id %d vp_index %u msr 0x%x data 0x%llx",
+		  __entry->vcpu_id, __entry->vp_index, __entry->msr,
+		  __entry->data)
+);
 #endif /* _TRACE_KVM_H */
 
 #undef TRACE_INCLUDE_PATH
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3bf2ecafd027..28b304be5419 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1241,6 +1241,10 @@ static const u32 emulated_msrs_all[] = {
 	HV_X64_MSR_VP_ASSIST_PAGE,
 	HV_X64_MSR_REENLIGHTENMENT_CONTROL, HV_X64_MSR_TSC_EMULATION_CONTROL,
 	HV_X64_MSR_TSC_EMULATION_STATUS,
+	HV_X64_MSR_SYNDBG_OPTIONS,
+	HV_X64_MSR_SYNDBG_CONTROL, HV_X64_MSR_SYNDBG_STATUS,
+	HV_X64_MSR_SYNDBG_SEND_BUFFER, HV_X64_MSR_SYNDBG_RECV_BUFFER,
+	HV_X64_MSR_SYNDBG_PENDING_BUFFER,
 
 	MSR_KVM_ASYNC_PF_EN, MSR_KVM_STEAL_TIME,
 	MSR_KVM_PV_EOI_EN,
@@ -2940,6 +2944,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		 */
 		break;
 	case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
+	case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+	case HV_X64_MSR_SYNDBG_OPTIONS:
 	case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
 	case HV_X64_MSR_CRASH_CTL:
 	case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
@@ -3184,6 +3190,8 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		msr_info->data = 0x20000000;
 		break;
 	case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
+	case HV_X64_MSR_SYNDBG_CONTROL ... HV_X64_MSR_SYNDBG_PENDING_BUFFER:
+	case HV_X64_MSR_SYNDBG_OPTIONS:
 	case HV_X64_MSR_CRASH_P0 ... HV_X64_MSR_CRASH_P4:
 	case HV_X64_MSR_CRASH_CTL:
 	case HV_X64_MSR_STIMER0_CONFIG ... HV_X64_MSR_STIMER3_COUNT:
@@ -3355,6 +3363,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_HYPERV_TLBFLUSH:
 	case KVM_CAP_HYPERV_SEND_IPI:
 	case KVM_CAP_HYPERV_CPUID:
+	case KVM_CAP_HYPERV_SYNDBG:
 	case KVM_CAP_PCI_SEGMENT:
 	case KVM_CAP_DEBUGREGS:
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
@@ -4209,6 +4218,10 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 		return -EINVAL;
 
 	switch (cap->cap) {
+	case KVM_CAP_HYPERV_SYNDBG:
+		kvm_hv_activate_syndbg(vcpu);
+		return 0;
+
 	case KVM_CAP_HYPERV_SYNIC2:
 		if (cap->args[0])
 			return -EINVAL;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 9cdc5356f542..ec1b2c7b449e 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -188,6 +188,7 @@ struct kvm_s390_cmma_log {
 struct kvm_hyperv_exit {
 #define KVM_EXIT_HYPERV_SYNIC          1
 #define KVM_EXIT_HYPERV_HCALL          2
+#define KVM_EXIT_HYPERV_SYNDBG         3
 	__u32 type;
 	__u32 pad1;
 	union {
@@ -203,6 +204,15 @@ struct kvm_hyperv_exit {
 			__u64 result;
 			__u64 params[2];
 		} hcall;
+		struct {
+			__u32 msr;
+			__u32 pad2;
+			__u64 control;
+			__u64 status;
+			__u64 send_page;
+			__u64 recv_page;
+			__u64 pending_page;
+		} syndbg;
 	} u;
 };
 
@@ -1019,6 +1029,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_S390_VCPU_RESETS 179
 #define KVM_CAP_S390_PROTECTED 180
 #define KVM_CAP_PPC_SECURE_GUEST 181
+#define KVM_CAP_HYPERV_SYNDBG 182
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
  2020-04-24 11:37 [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
                   ` (3 preceding siblings ...)
  2020-04-24 11:37 ` [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability Jon Doron
@ 2020-04-24 11:37 ` Jon Doron
  2020-05-13  9:57   ` Roman Kagan
  2020-05-29 10:48   ` Paolo Bonzini
  2020-04-24 11:37 ` [PATCH v11 6/7] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls Jon Doron
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 22+ messages in thread
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
  To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron

Microsoft's kdvm.dll dbgtransport module does not respect the hypercall
page and simply identifies the CPU being used (AMD/Intel) and according
to it simply makes hypercalls with the relevant instruction
(vmmcall/vmcall respectively).

The relevant function in kdvm is KdHvConnectHypervisor which first checks
if the hypercall page has been enabled via HV_X64_MSR_HYPERCALL_ENABLE,
and in case it was not it simply sets the HV_X64_MSR_GUEST_OS_ID to
0x1000101010001 which means:
build_number = 0x0001
service_version = 0x01
minor_version = 0x01
major_version = 0x01
os_id = 0x00 (Undefined)
vendor_id = 1 (Microsoft)
os_type = 0 (A value of 0 indicates a proprietary, closed source OS)

and starts issuing the hypercall without setting the hypercall page.

To resolve this issue simply enable hypercalls also if the guest_os_id
is not 0 and the syndbg feature is enabled.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
 arch/x86/kvm/hyperv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 435516595090..524b5466a515 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1650,7 +1650,10 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *current_vcpu, u64 ingpa, u64 outgpa,
 
 bool kvm_hv_hypercall_enabled(struct kvm *kvm)
 {
-	return READ_ONCE(kvm->arch.hyperv.hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE;
+	struct kvm_hv *hv = &kvm->arch.hyperv;
+
+	return READ_ONCE(hv->hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE ||
+	       (hv->hv_syndbg.active && READ_ONCE(hv->hv_guest_os_id) != 0);
 }
 
 static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v11 6/7] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
  2020-04-24 11:37 [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
                   ` (4 preceding siblings ...)
  2020-04-24 11:37 ` [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg Jon Doron
@ 2020-04-24 11:37 ` Jon Doron
  2020-05-12 15:33   ` Roman Kagan
  2020-04-24 11:37 ` [PATCH v11 7/7] KVM: selftests: update hyperv_cpuid with SynDBG tests Jon Doron
  2020-05-07  3:01 ` [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
  7 siblings, 1 reply; 22+ messages in thread
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
  To: kvm, linux-hyperv; +Cc: vkuznets, Jon Doron

There is another mode for the synthetic debugger which uses hypercalls
to send/recv network data instead of the MSR interface.

This interface is much slower and less recommended since you might get
a lot of VMExits while KDVM polling for new packets to recv, rather
than simply checking the pending page to see if there is data avialble
and then request.

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Jon Doron <arilou@gmail.com>
---
 arch/x86/kvm/hyperv.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 524b5466a515..744bcef88c70 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1832,6 +1832,34 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 		}
 		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, true, false);
 		break;
+	case HVCALL_POST_DEBUG_DATA:
+	case HVCALL_RETRIEVE_DEBUG_DATA:
+		if (unlikely(fast)) {
+			ret = HV_STATUS_INVALID_PARAMETER;
+			break;
+		}
+		fallthrough;
+	case HVCALL_RESET_DEBUG_SESSION: {
+		struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
+
+		if (!syndbg->active) {
+			ret = HV_STATUS_INVALID_HYPERCALL_CODE;
+			break;
+		}
+
+		if (!(syndbg->options & HV_X64_SYNDBG_OPTION_USE_HCALLS)) {
+			ret = HV_STATUS_OPERATION_DENIED;
+			break;
+		}
+		vcpu->run->exit_reason = KVM_EXIT_HYPERV;
+		vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
+		vcpu->run->hyperv.u.hcall.input = param;
+		vcpu->run->hyperv.u.hcall.params[0] = ingpa;
+		vcpu->run->hyperv.u.hcall.params[1] = outgpa;
+		vcpu->arch.complete_userspace_io =
+				kvm_hv_hypercall_complete_userspace;
+		return 0;
+	}
 	default:
 		ret = HV_STATUS_INVALID_HYPERCALL_CODE;
 		break;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v11 7/7] KVM: selftests: update hyperv_cpuid with SynDBG tests
  2020-04-24 11:37 [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
                   ` (5 preceding siblings ...)
  2020-04-24 11:37 ` [PATCH v11 6/7] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls Jon Doron
@ 2020-04-24 11:37 ` Jon Doron
  2020-05-07  3:01 ` [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
  7 siblings, 0 replies; 22+ messages in thread
From: Jon Doron @ 2020-04-24 11:37 UTC (permalink / raw)
  To: kvm, linux-hyperv; +Cc: vkuznets

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Test all four combinations with eVMCS and SynDBG capabilities,
check that we get the right number of entries and that
0x40000000.EAX always returns the correct max leaf.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 .../selftests/kvm/x86_64/hyperv_cpuid.c       | 143 ++++++++++++------
 1 file changed, 95 insertions(+), 48 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 83323f3d7ca0..5268abf9ad80 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -26,18 +26,18 @@ static void guest_code(void)
 {
 }
 
-static int smt_possible(void)
+static bool smt_possible(void)
 {
 	char buf[16];
 	FILE *f;
-	bool res = 1;
+	bool res = true;
 
 	f = fopen("/sys/devices/system/cpu/smt/control", "r");
 	if (f) {
 		if (fread(buf, sizeof(*buf), sizeof(buf), f) > 0) {
 			if (!strncmp(buf, "forceoff", 8) ||
 			    !strncmp(buf, "notsupported", 12))
-				res = 0;
+				res = false;
 		}
 		fclose(f);
 	}
@@ -45,30 +45,48 @@ static int smt_possible(void)
 	return res;
 }
 
+void vcpu_enable_syndbg(struct kvm_vm *vm, int vcpu_id)
+{
+	struct kvm_enable_cap enable_syndbg_cap = {
+		.cap = KVM_CAP_HYPERV_SYNDBG,
+	};
+
+	vcpu_ioctl(vm, vcpu_id, KVM_ENABLE_CAP, &enable_syndbg_cap);
+}
+
 static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
-			  int evmcs_enabled)
+			  bool evmcs_enabled, bool syndbg_enabled)
 {
 	int i;
+	int nent = 6;
+	u32 test_val;
+
+	if (evmcs_enabled)
+		nent += 1; /* 0x4000000A */
 
-	if (!evmcs_enabled)
-		TEST_ASSERT(hv_cpuid_entries->nent == 6,
-			    "KVM_GET_SUPPORTED_HV_CPUID should return 6 entries"
-			    " when Enlightened VMCS is disabled (returned %d)",
-			    hv_cpuid_entries->nent);
-	else
-		TEST_ASSERT(hv_cpuid_entries->nent == 7,
-			    "KVM_GET_SUPPORTED_HV_CPUID should return 7 entries"
-			    " when Enlightened VMCS is enabled (returned %d)",
-			    hv_cpuid_entries->nent);
+	if (syndbg_enabled)
+		nent += 3; /* 0x40000080 - 0x40000082 */
+
+	TEST_ASSERT(hv_cpuid_entries->nent == nent,
+		    "KVM_GET_SUPPORTED_HV_CPUID should return %d entries"
+		    " with evmcs=%d syndbg=%d (returned %d)",
+		    nent, evmcs_enabled, syndbg_enabled,
+		    hv_cpuid_entries->nent);
 
 	for (i = 0; i < hv_cpuid_entries->nent; i++) {
 		struct kvm_cpuid_entry2 *entry = &hv_cpuid_entries->entries[i];
 
 		TEST_ASSERT((entry->function >= 0x40000000) &&
-			    (entry->function <= 0x4000000A),
+			    (entry->function <= 0x40000082),
 			    "function %x is our of supported range",
 			    entry->function);
 
+		TEST_ASSERT(evmcs_enabled || (entry->function != 0x4000000A),
+			    "0x4000000A leaf should not be reported");
+
+		TEST_ASSERT(syndbg_enabled || (entry->function <= 0x4000000A),
+			    "SYNDBG leaves should not be reported");
+
 		TEST_ASSERT(entry->index == 0,
 			    ".index field should be zero");
 
@@ -78,12 +96,27 @@ static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
 		TEST_ASSERT(!entry->padding[0] && !entry->padding[1] &&
 			    !entry->padding[2], "padding should be zero");
 
-		if (entry->function == 0x40000004) {
-			int nononarchcs = !!(entry->eax & (1UL << 18));
-
-			TEST_ASSERT(nononarchcs == !smt_possible(),
+		switch (entry->function) {
+		case 0x40000000:
+			test_val = 0x40000005;
+			if (evmcs_enabled)
+				test_val = 0x4000000A;
+			if (syndbg_enabled)
+				test_val = 0x40000082;
+
+			TEST_ASSERT(entry->eax == test_val,
+				    "Wrong max leaf report in 0x40000000.EAX: %x"
+				    " (evmcs=%d syndbg=%d)",
+				    entry->eax, evmcs_enabled, syndbg_enabled
+				);
+			break;
+		case 0x40000004:
+			test_val = entry->eax & (1UL << 18);
+
+			TEST_ASSERT(!!test_val == !smt_possible(),
 				    "NoNonArchitecturalCoreSharing bit"
 				    " doesn't reflect SMT setting");
+			break;
 		}
 
 		/*
@@ -133,8 +166,9 @@ struct kvm_cpuid2 *kvm_get_supported_hv_cpuid(struct kvm_vm *vm)
 int main(int argc, char *argv[])
 {
 	struct kvm_vm *vm;
-	int rv;
+	int rv, stage;
 	struct kvm_cpuid2 *hv_cpuid_entries;
+	bool evmcs_enabled, syndbg_enabled;
 
 	/* Tell stdout not to buffer its content */
 	setbuf(stdout, NULL);
@@ -145,36 +179,49 @@ int main(int argc, char *argv[])
 		exit(KSFT_SKIP);
 	}
 
-	/* Create VM */
-	vm = vm_create_default(VCPU_ID, 0, guest_code);
-
-	test_hv_cpuid_e2big(vm);
-
-	hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
-	if (!hv_cpuid_entries)
-		return 1;
-
-	test_hv_cpuid(hv_cpuid_entries, 0);
-
-	free(hv_cpuid_entries);
+	for (stage = 0; stage < 5; stage++) {
+		evmcs_enabled = false;
+		syndbg_enabled = false;
+
+		vm = vm_create_default(VCPU_ID, 0, guest_code);
+		switch (stage) {
+		case 0:
+			test_hv_cpuid_e2big(vm);
+			continue;
+		case 1:
+			break;
+		case 2:
+			if (!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;
+		case 3:
+			if (!kvm_check_cap(KVM_CAP_HYPERV_SYNDBG)) {
+				print_skip("Synthetic debugger is unsupported");
+				continue;
+			}
+			vcpu_enable_syndbg(vm, VCPU_ID);
+			syndbg_enabled = true;
+			break;
+		case 4:
+			if (!kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS) ||
+			    !kvm_check_cap(KVM_CAP_HYPERV_SYNDBG))
+				continue;
+			vcpu_enable_evmcs(vm, VCPU_ID);
+			vcpu_enable_syndbg(vm, VCPU_ID);
+			evmcs_enabled = true;
+			syndbg_enabled = true;
+			break;
+		}
 
-	if (!kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
-		print_skip("Enlightened VMCS is unsupported");
-		goto vm_free;
+		hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
+		test_hv_cpuid(hv_cpuid_entries, evmcs_enabled, syndbg_enabled);
+		free(hv_cpuid_entries);
+		kvm_vm_free(vm);
 	}
 
-	vcpu_enable_evmcs(vm, VCPU_ID);
-
-	hv_cpuid_entries = kvm_get_supported_hv_cpuid(vm);
-	if (!hv_cpuid_entries)
-		return 1;
-
-	test_hv_cpuid(hv_cpuid_entries, 1);
-
-	free(hv_cpuid_entries);
-
-vm_free:
-	kvm_vm_free(vm);
-
 	return 0;
 }
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger
  2020-04-24 11:37 [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
                   ` (6 preceding siblings ...)
  2020-04-24 11:37 ` [PATCH v11 7/7] KVM: selftests: update hyperv_cpuid with SynDBG tests Jon Doron
@ 2020-05-07  3:01 ` Jon Doron
  2020-05-07  7:57   ` Paolo Bonzini
  7 siblings, 1 reply; 22+ messages in thread
From: Jon Doron @ 2020-05-07  3:01 UTC (permalink / raw)
  To: kvm, linux-hyperv; +Cc: vkuznets, pbonzini

Paolo was this merged in or by any chance in the queue?

Thanks,
-- Jon.

On 24/04/2020, Jon Doron wrote:
>Add support for the synthetic debugger interface of hyper-v, the
>synthetic debugger has 2 modes.
>1. Use a set of MSRs to send/recv information (undocumented so it's not
>   going to the hyperv-tlfs.h)
>2. Use hypercalls
>
>The first mode is based the following MSRs:
>1. Control/Status MSRs which either asks for a send/recv .
>2. Send/Recv MSRs each holds GPA where the send/recv buffers are.
>3. Pending MSR, holds a GPA to a PAGE that simply has a boolean that
>   indicates if there is data pending to issue a recv VMEXIT.
>
>The first mode implementation is to simply exit to user-space when
>either the control MSR or the pending MSR are being set.
>Then it's up-to userspace to implement the rest of the logic of sending/recving.
>
>In the second mode instead of using MSRs KNet will simply issue
>Hypercalls with the information to send/recv, in this mode the data
>being transferred is UDP encapsulated, unlike in the previous mode in
>which you get just the data to send.
>
>The new hypercalls will exit to userspace which will be incharge of
>re-encapsulating if needed the UDP packets to be sent.
>
>There is an issue though in which KDNet does not respect the hypercall
>page and simply issues vmcall/vmmcall instructions depending on the cpu
>type expecting them to be handled as it a real hypercall was issued.
>
>It's important to note that part of this feature has been subject to be
>removed in future versions of Windows, which is why some of the
>defintions will not be present the the TLFS but in the kvm hyperv header
>instead.
>
>v11:
>Fixed all reviewed by and rebased on latest origin/master
>
>Jon Doron (6):
>  x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit
>  x86/kvm/hyper-v: Simplify addition for custom cpuid leafs
>  x86/hyper-v: Add synthetic debugger definitions
>  x86/kvm/hyper-v: Add support for synthetic debugger capability
>  x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
>  x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
>
>Vitaly Kuznetsov (1):
>  KVM: selftests: update hyperv_cpuid with SynDBG tests
>
> Documentation/virt/kvm/api.rst                |  18 ++
> arch/x86/include/asm/hyperv-tlfs.h            |   6 +
> arch/x86/include/asm/kvm_host.h               |  14 +
> arch/x86/kvm/hyperv.c                         | 242 ++++++++++++++++--
> arch/x86/kvm/hyperv.h                         |  33 +++
> arch/x86/kvm/trace.h                          |  51 ++++
> arch/x86/kvm/x86.c                            |  13 +
> include/uapi/linux/kvm.h                      |  13 +
> .../selftests/kvm/x86_64/hyperv_cpuid.c       | 143 +++++++----
> 9 files changed, 468 insertions(+), 65 deletions(-)
>
>-- 
>2.24.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger
  2020-05-07  3:01 ` [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
@ 2020-05-07  7:57   ` Paolo Bonzini
  0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2020-05-07  7:57 UTC (permalink / raw)
  To: Jon Doron, kvm, linux-hyperv; +Cc: vkuznets

On 07/05/20 05:01, Jon Doron wrote:
> Paolo was this merged in or by any chance in the queue?

No, I'll get to it today.

Paolo

> Thanks,
> -- Jon.
> 
> On 24/04/2020, Jon Doron wrote:
>> Add support for the synthetic debugger interface of hyper-v, the
>> synthetic debugger has 2 modes.
>> 1. Use a set of MSRs to send/recv information (undocumented so it's not
>>   going to the hyperv-tlfs.h)
>> 2. Use hypercalls
>>
>> The first mode is based the following MSRs:
>> 1. Control/Status MSRs which either asks for a send/recv .
>> 2. Send/Recv MSRs each holds GPA where the send/recv buffers are.
>> 3. Pending MSR, holds a GPA to a PAGE that simply has a boolean that
>>   indicates if there is data pending to issue a recv VMEXIT.
>>
>> The first mode implementation is to simply exit to user-space when
>> either the control MSR or the pending MSR are being set.
>> Then it's up-to userspace to implement the rest of the logic of
>> sending/recving.
>>
>> In the second mode instead of using MSRs KNet will simply issue
>> Hypercalls with the information to send/recv, in this mode the data
>> being transferred is UDP encapsulated, unlike in the previous mode in
>> which you get just the data to send.
>>
>> The new hypercalls will exit to userspace which will be incharge of
>> re-encapsulating if needed the UDP packets to be sent.
>>
>> There is an issue though in which KDNet does not respect the hypercall
>> page and simply issues vmcall/vmmcall instructions depending on the cpu
>> type expecting them to be handled as it a real hypercall was issued.
>>
>> It's important to note that part of this feature has been subject to be
>> removed in future versions of Windows, which is why some of the
>> defintions will not be present the the TLFS but in the kvm hyperv header
>> instead.
>>
>> v11:
>> Fixed all reviewed by and rebased on latest origin/master
>>
>> Jon Doron (6):
>>  x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit
>>  x86/kvm/hyper-v: Simplify addition for custom cpuid leafs
>>  x86/hyper-v: Add synthetic debugger definitions
>>  x86/kvm/hyper-v: Add support for synthetic debugger capability
>>  x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
>>  x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
>>
>> Vitaly Kuznetsov (1):
>>  KVM: selftests: update hyperv_cpuid with SynDBG tests
>>
>> Documentation/virt/kvm/api.rst                |  18 ++
>> arch/x86/include/asm/hyperv-tlfs.h            |   6 +
>> arch/x86/include/asm/kvm_host.h               |  14 +
>> arch/x86/kvm/hyperv.c                         | 242 ++++++++++++++++--
>> arch/x86/kvm/hyperv.h                         |  33 +++
>> arch/x86/kvm/trace.h                          |  51 ++++
>> arch/x86/kvm/x86.c                            |  13 +
>> include/uapi/linux/kvm.h                      |  13 +
>> .../selftests/kvm/x86_64/hyperv_cpuid.c       | 143 +++++++----
>> 9 files changed, 468 insertions(+), 65 deletions(-)
>>
>> -- 
>> 2.24.1
>>
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 6/7] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
  2020-04-24 11:37 ` [PATCH v11 6/7] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls Jon Doron
@ 2020-05-12 15:33   ` Roman Kagan
  2020-05-13 12:39     ` Jon Doron
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Kagan @ 2020-05-12 15:33 UTC (permalink / raw)
  To: Jon Doron; +Cc: kvm, linux-hyperv, vkuznets

On Fri, Apr 24, 2020 at 02:37:45PM +0300, Jon Doron wrote:
> There is another mode for the synthetic debugger which uses hypercalls
> to send/recv network data instead of the MSR interface.
> 
> This interface is much slower and less recommended since you might get
> a lot of VMExits while KDVM polling for new packets to recv, rather
> than simply checking the pending page to see if there is data avialble
> and then request.
> 
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
>  arch/x86/kvm/hyperv.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 524b5466a515..744bcef88c70 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1832,6 +1832,34 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  		}
>  		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, true, false);
>  		break;
> +	case HVCALL_POST_DEBUG_DATA:
> +	case HVCALL_RETRIEVE_DEBUG_DATA:
> +		if (unlikely(fast)) {
> +			ret = HV_STATUS_INVALID_PARAMETER;
> +			break;
> +		}
> +		fallthrough;
> +	case HVCALL_RESET_DEBUG_SESSION: {
> +		struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> +
> +		if (!syndbg->active) {
> +			ret = HV_STATUS_INVALID_HYPERCALL_CODE;
> +			break;
> +		}
> +
> +		if (!(syndbg->options & HV_X64_SYNDBG_OPTION_USE_HCALLS)) {
> +			ret = HV_STATUS_OPERATION_DENIED;
> +			break;
> +		}
> +		vcpu->run->exit_reason = KVM_EXIT_HYPERV;
> +		vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
> +		vcpu->run->hyperv.u.hcall.input = param;
> +		vcpu->run->hyperv.u.hcall.params[0] = ingpa;
> +		vcpu->run->hyperv.u.hcall.params[1] = outgpa;
> +		vcpu->arch.complete_userspace_io =
> +				kvm_hv_hypercall_complete_userspace;
> +		return 0;
> +	}

I'd personally just push every hyperv hypercall not recognized by the
kernel to userspace.  Smth like this:

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index bcefa9d4e57e..f0404df0f488 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1644,6 +1644,48 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 		}
 		kvm_vcpu_on_spin(vcpu, true);
 		break;
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
+		if (unlikely(fast || !rep_cnt || rep_idx)) {
+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
+			break;
+		}
+		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, false);
+		break;
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
+		if (unlikely(fast || rep)) {
+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
+			break;
+		}
+		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, false);
+		break;
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
+		if (unlikely(fast || !rep_cnt || rep_idx)) {
+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
+			break;
+		}
+		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, true);
+		break;
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
+		if (unlikely(fast || rep)) {
+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
+			break;
+		}
+		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, true);
+		break;
+	case HVCALL_SEND_IPI:
+		if (unlikely(rep)) {
+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
+			break;
+		}
+		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, false, fast);
+		break;
+	case HVCALL_SEND_IPI_EX:
+		if (unlikely(fast || rep)) {
+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
+			break;
+		}
+		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, true, false);
+		break;
 	case HVCALL_SIGNAL_EVENT:
 		if (unlikely(rep)) {
 			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
@@ -1653,12 +1695,8 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 		if (ret != HV_STATUS_INVALID_PORT_ID)
 			break;
 		/* fall through - maybe userspace knows this conn_id. */
-	case HVCALL_POST_MESSAGE:
-		/* don't bother userspace if it has no way to handle it */
-		if (unlikely(rep || !vcpu_to_synic(vcpu)->active)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
+	default:
+		/* forward unrecognized hypercalls to userspace */
 		vcpu->run->exit_reason = KVM_EXIT_HYPERV;
 		vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
 		vcpu->run->hyperv.u.hcall.input = param;
@@ -1667,51 +1705,6 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 		vcpu->arch.complete_userspace_io =
 				kvm_hv_hypercall_complete_userspace;
 		return 0;
-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
-		if (unlikely(fast || !rep_cnt || rep_idx)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, false);
-		break;
-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
-		if (unlikely(fast || rep)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, false);
-		break;
-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
-		if (unlikely(fast || !rep_cnt || rep_idx)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, true);
-		break;
-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
-		if (unlikely(fast || rep)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, true);
-		break;
-	case HVCALL_SEND_IPI:
-		if (unlikely(rep)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, false, fast);
-		break;
-	case HVCALL_SEND_IPI_EX:
-		if (unlikely(fast || rep)) {
-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
-			break;
-		}
-		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, true, false);
-		break;
-	default:
-		ret = HV_STATUS_INVALID_HYPERCALL_CODE;
-		break;
 	}
 
 	return kvm_hv_hypercall_complete(vcpu, ret);

(would also need a kvm cap for that)

Roman.

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 1/7] x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit
  2020-04-24 11:37 ` [PATCH v11 1/7] x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit Jon Doron
@ 2020-05-13  8:42   ` Roman Kagan
  0 siblings, 0 replies; 22+ messages in thread
From: Roman Kagan @ 2020-05-13  8:42 UTC (permalink / raw)
  To: Jon Doron; +Cc: kvm, linux-hyperv, vkuznets

On Fri, Apr 24, 2020 at 02:37:40PM +0300, Jon Doron wrote:
> The problem the patch is trying to address is the fact that 'struct
> kvm_hyperv_exit' has different layout on when compiling in 32 and 64 bit
> modes.
> 
> In 64-bit mode the default alignment boundary is 64 bits thus
> forcing extra gaps after 'type' and 'msr' but in 32-bit mode the
> boundary is at 32 bits thus no extra gaps.
> 
> This is an issue as even when the kernel is 64 bit, the userspace using
> the interface can be both 32 and 64 bit but the same 32 bit userspace has
> to work with 32 bit kernel.
> 
> The issue is fixed by forcing the 64 bit layout, this leads to ABI
> change for 32 bit builds and while we are obviously breaking '32 bit
> userspace with 32 bit kernel' case, we're fixing the '32 bit userspace
> with 64 bit kernel' one.
> 
> As the interface has no (known) users and 32 bit KVM is rather baroque
> nowadays, this seems like a reasonable decision.
> 
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
>  Documentation/virt/kvm/api.rst | 2 ++
>  include/uapi/linux/kvm.h       | 2 ++
>  2 files changed, 4 insertions(+)

Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs
  2020-04-24 11:37 ` [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs Jon Doron
@ 2020-05-13  9:24   ` Roman Kagan
  2020-05-13 12:49     ` Jon Doron
  0 siblings, 1 reply; 22+ messages in thread
From: Roman Kagan @ 2020-05-13  9:24 UTC (permalink / raw)
  To: Jon Doron; +Cc: kvm, linux-hyperv, vkuznets

On Fri, Apr 24, 2020 at 02:37:41PM +0300, Jon Doron wrote:
> Simlify the code to define a new cpuid leaf group by enabled feature.
> 
> This also fixes a bug in which the max cpuid leaf was always set to
> HYPERV_CPUID_NESTED_FEATURES regardless if nesting is supported or not.

I'm not sure the bug is there.  My understanding is that
HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS is supposed to provide the range
of leaves that return meaningful information.
HYPERV_CPUID_NESTED_FEATURES *can* return meaningful information
regardless of whether nested virt is active.

So I'd rather skip reducing the returned set if !evmcs_ver.  The
returned set is sparse in .function anyway; anything that isn't there
will just return zeros to the guest.

Changing the cpuid is also guest-visible so care must be taken with
this.

> Any new CPUID group needs to consider the max leaf and be added in the
> correct order, in this method there are two rules:
> 1. Each cpuid leaf group must be order in an ascending order
> 2. The appending for the cpuid leafs by features also needs to happen by
>    ascending order.

It looks like unnecessary complication.  I think all you need to do to
simplify adding new leaves is to add a macro to hyperv-tlfs.h, say,
HYPERV_CPUID_MAX_PRESENT_LEAF, define it to
HYPERV_CPUID_NESTED_FEATURES, and redefine once another leaf is added
(compat may need to be taken care of).

Thanks,
Roman.

> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
>  arch/x86/kvm/hyperv.c | 46 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index bcefa9d4e57e..ab3e9dbcabbe 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1785,27 +1785,45 @@ int kvm_vm_ioctl_hv_eventfd(struct kvm *kvm, struct kvm_hyperv_eventfd *args)
>  	return kvm_hv_eventfd_assign(kvm, args->conn_id, args->fd);
>  }
>  
> +/* Must be sorted in ascending order by function */
> +static struct kvm_cpuid_entry2 core_cpuid_entries[] = {
> +	{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
> +	{ .function = HYPERV_CPUID_INTERFACE },
> +	{ .function = HYPERV_CPUID_VERSION },
> +	{ .function = HYPERV_CPUID_FEATURES },
> +	{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
> +	{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
> +};
> +
> +static struct kvm_cpuid_entry2 evmcs_cpuid_entries[] = {
> +	{ .function = HYPERV_CPUID_NESTED_FEATURES },
> +};
> +
> +#define HV_MAX_CPUID_ENTRIES \
> +	(ARRAY_SIZE(core_cpuid_entries) +\
> +	 ARRAY_SIZE(evmcs_cpuid_entries))
> +
>  int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>  				struct kvm_cpuid_entry2 __user *entries)
>  {
>  	uint16_t evmcs_ver = 0;
> -	struct kvm_cpuid_entry2 cpuid_entries[] = {
> -		{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
> -		{ .function = HYPERV_CPUID_INTERFACE },
> -		{ .function = HYPERV_CPUID_VERSION },
> -		{ .function = HYPERV_CPUID_FEATURES },
> -		{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
> -		{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
> -		{ .function = HYPERV_CPUID_NESTED_FEATURES },
> -	};
> -	int i, nent = ARRAY_SIZE(cpuid_entries);
> +	struct kvm_cpuid_entry2 cpuid_entries[HV_MAX_CPUID_ENTRIES];
> +	int i, nent = 0;
> +
> +	/* Set the core cpuid entries required for Hyper-V */
> +	memcpy(&cpuid_entries[nent], &core_cpuid_entries,
> +	       sizeof(core_cpuid_entries));
> +	nent = ARRAY_SIZE(core_cpuid_entries);
>  
>  	if (kvm_x86_ops.nested_get_evmcs_version)
>  		evmcs_ver = kvm_x86_ops.nested_get_evmcs_version(vcpu);
>  
> -	/* Skip NESTED_FEATURES if eVMCS is not supported */
> -	if (!evmcs_ver)
> -		--nent;
> +	if (evmcs_ver) {
> +		/* EVMCS is enabled, add the required EVMCS CPUID leafs */
> +		memcpy(&cpuid_entries[nent], &evmcs_cpuid_entries,
> +		       sizeof(evmcs_cpuid_entries));
> +		nent += ARRAY_SIZE(evmcs_cpuid_entries);
> +	}
>  
>  	if (cpuid->nent < nent)
>  		return -E2BIG;
> @@ -1821,7 +1839,7 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>  		case HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS:
>  			memcpy(signature, "Linux KVM Hv", 12);
>  
> -			ent->eax = HYPERV_CPUID_NESTED_FEATURES;
> +			ent->eax = cpuid_entries[nent - 1].function;
>  			ent->ebx = signature[0];
>  			ent->ecx = signature[1];
>  			ent->edx = signature[2];
> -- 
> 2.24.1
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
  2020-04-24 11:37 ` [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg Jon Doron
@ 2020-05-13  9:57   ` Roman Kagan
  2020-05-13 12:37     ` Jon Doron
  2020-05-29 10:48   ` Paolo Bonzini
  1 sibling, 1 reply; 22+ messages in thread
From: Roman Kagan @ 2020-05-13  9:57 UTC (permalink / raw)
  To: Jon Doron; +Cc: kvm, linux-hyperv, vkuznets

On Fri, Apr 24, 2020 at 02:37:44PM +0300, Jon Doron wrote:
> Microsoft's kdvm.dll dbgtransport module does not respect the hypercall
> page and simply identifies the CPU being used (AMD/Intel) and according
> to it simply makes hypercalls with the relevant instruction
> (vmmcall/vmcall respectively).
> 
> The relevant function in kdvm is KdHvConnectHypervisor which first checks
> if the hypercall page has been enabled via HV_X64_MSR_HYPERCALL_ENABLE,
> and in case it was not it simply sets the HV_X64_MSR_GUEST_OS_ID to
> 0x1000101010001 which means:
> build_number = 0x0001
> service_version = 0x01
> minor_version = 0x01
> major_version = 0x01
> os_id = 0x00 (Undefined)
> vendor_id = 1 (Microsoft)
> os_type = 0 (A value of 0 indicates a proprietary, closed source OS)
> 
> and starts issuing the hypercall without setting the hypercall page.

I guess this is to avoid interfering with the OS being debugged
requesting its own hypercall page at a different address.

> To resolve this issue simply enable hypercalls also if the guest_os_id
> is not 0 and the syndbg feature is enabled.
> 
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Jon Doron <arilou@gmail.com>
> ---
>  arch/x86/kvm/hyperv.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 435516595090..524b5466a515 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1650,7 +1650,10 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *current_vcpu, u64 ingpa, u64 outgpa,
>  
>  bool kvm_hv_hypercall_enabled(struct kvm *kvm)
>  {
> -	return READ_ONCE(kvm->arch.hyperv.hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE;
> +	struct kvm_hv *hv = &kvm->arch.hyperv;
> +
> +	return READ_ONCE(hv->hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE ||
> +	       (hv->hv_syndbg.active && READ_ONCE(hv->hv_guest_os_id) != 0);

This function is meant to tell if the hypercall should be interpreted as
following KVM or HyperV conventions.  Quoting from the spec

  3.5 Legal Hypercall Environments

  ...
  All hypercalls should be invoked through the architecturally-defined
  hypercall interface. (See the following sections for instructions on
  discovering and establishing this interface.) An attempt to invoke a
  hypercall by any other means (for example, copying the code from the
  hypercall code page to an alternate location and executing it from
  there) might result in an undefined operation (#UD) exception.  The
  hypervisor is not guaranteed to deliver this exception.

so I think we can simply test for hv_guest_os_id != 0 and ignore
HV_X64_MSR_HYPERCALL_ENABLE (it's about hypercall page being enabled,
not the hypercalls per se).

Thanks,
Roman.

>  }
>  
>  static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
> -- 
> 2.24.1
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
  2020-05-13  9:57   ` Roman Kagan
@ 2020-05-13 12:37     ` Jon Doron
  0 siblings, 0 replies; 22+ messages in thread
From: Jon Doron @ 2020-05-13 12:37 UTC (permalink / raw)
  To: Roman Kagan, kvm, linux-hyperv, vkuznets

On 13/05/2020, Roman Kagan wrote:
>On Fri, Apr 24, 2020 at 02:37:44PM +0300, Jon Doron wrote:
>> Microsoft's kdvm.dll dbgtransport module does not respect the hypercall
>> page and simply identifies the CPU being used (AMD/Intel) and according
>> to it simply makes hypercalls with the relevant instruction
>> (vmmcall/vmcall respectively).
>>
>> The relevant function in kdvm is KdHvConnectHypervisor which first checks
>> if the hypercall page has been enabled via HV_X64_MSR_HYPERCALL_ENABLE,
>> and in case it was not it simply sets the HV_X64_MSR_GUEST_OS_ID to
>> 0x1000101010001 which means:
>> build_number = 0x0001
>> service_version = 0x01
>> minor_version = 0x01
>> major_version = 0x01
>> os_id = 0x00 (Undefined)
>> vendor_id = 1 (Microsoft)
>> os_type = 0 (A value of 0 indicates a proprietary, closed source OS)
>>
>> and starts issuing the hypercall without setting the hypercall page.
>
>I guess this is to avoid interfering with the OS being debugged
>requesting its own hypercall page at a different address.
>
>> To resolve this issue simply enable hypercalls also if the guest_os_id
>> is not 0 and the syndbg feature is enabled.
>>
>> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Signed-off-by: Jon Doron <arilou@gmail.com>
>> ---
>>  arch/x86/kvm/hyperv.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>> index 435516595090..524b5466a515 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -1650,7 +1650,10 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *current_vcpu, u64 ingpa, u64 outgpa,
>>
>>  bool kvm_hv_hypercall_enabled(struct kvm *kvm)
>>  {
>> -	return READ_ONCE(kvm->arch.hyperv.hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE;
>> +	struct kvm_hv *hv = &kvm->arch.hyperv;
>> +
>> +	return READ_ONCE(hv->hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE ||
>> +	       (hv->hv_syndbg.active && READ_ONCE(hv->hv_guest_os_id) != 0);
>
>This function is meant to tell if the hypercall should be interpreted as
>following KVM or HyperV conventions.  Quoting from the spec
>
>  3.5 Legal Hypercall Environments
>
>  ...
>  All hypercalls should be invoked through the architecturally-defined
>  hypercall interface. (See the following sections for instructions on
>  discovering and establishing this interface.) An attempt to invoke a
>  hypercall by any other means (for example, copying the code from the
>  hypercall code page to an alternate location and executing it from
>  there) might result in an undefined operation (#UD) exception.  The
>  hypervisor is not guaranteed to deliver this exception.
>
>so I think we can simply test for hv_guest_os_id != 0 and ignore
>HV_X64_MSR_HYPERCALL_ENABLE (it's about hypercall page being enabled,
>not the hypercalls per se).
>
>Thanks,
>Roman.
>
>>  }
>>
>>  static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
>> --
>> 2.24.1
>>

Hi Roman,

I agree this was the original implementation of this patchset (see v1) I 
will send a v12 with the suggested change, but I would prefer that you 
will review the mailing list previous comments which caused to this 
specific behaviour.

Thanks,
-- Jon.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 6/7] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls
  2020-05-12 15:33   ` Roman Kagan
@ 2020-05-13 12:39     ` Jon Doron
  0 siblings, 0 replies; 22+ messages in thread
From: Jon Doron @ 2020-05-13 12:39 UTC (permalink / raw)
  To: Roman Kagan, kvm, linux-hyperv, vkuznets

On 12/05/2020, Roman Kagan wrote:
>On Fri, Apr 24, 2020 at 02:37:45PM +0300, Jon Doron wrote:
>> There is another mode for the synthetic debugger which uses hypercalls
>> to send/recv network data instead of the MSR interface.
>>
>> This interface is much slower and less recommended since you might get
>> a lot of VMExits while KDVM polling for new packets to recv, rather
>> than simply checking the pending page to see if there is data avialble
>> and then request.
>>
>> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Signed-off-by: Jon Doron <arilou@gmail.com>
>> ---
>>  arch/x86/kvm/hyperv.c | 28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>> index 524b5466a515..744bcef88c70 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -1832,6 +1832,34 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>>  		}
>>  		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, true, false);
>>  		break;
>> +	case HVCALL_POST_DEBUG_DATA:
>> +	case HVCALL_RETRIEVE_DEBUG_DATA:
>> +		if (unlikely(fast)) {
>> +			ret = HV_STATUS_INVALID_PARAMETER;
>> +			break;
>> +		}
>> +		fallthrough;
>> +	case HVCALL_RESET_DEBUG_SESSION: {
>> +		struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
>> +
>> +		if (!syndbg->active) {
>> +			ret = HV_STATUS_INVALID_HYPERCALL_CODE;
>> +			break;
>> +		}
>> +
>> +		if (!(syndbg->options & HV_X64_SYNDBG_OPTION_USE_HCALLS)) {
>> +			ret = HV_STATUS_OPERATION_DENIED;
>> +			break;
>> +		}
>> +		vcpu->run->exit_reason = KVM_EXIT_HYPERV;
>> +		vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
>> +		vcpu->run->hyperv.u.hcall.input = param;
>> +		vcpu->run->hyperv.u.hcall.params[0] = ingpa;
>> +		vcpu->run->hyperv.u.hcall.params[1] = outgpa;
>> +		vcpu->arch.complete_userspace_io =
>> +				kvm_hv_hypercall_complete_userspace;
>> +		return 0;
>> +	}
>
>I'd personally just push every hyperv hypercall not recognized by the
>kernel to userspace.  Smth like this:
>
>diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>index bcefa9d4e57e..f0404df0f488 100644
>--- a/arch/x86/kvm/hyperv.c
>+++ b/arch/x86/kvm/hyperv.c
>@@ -1644,6 +1644,48 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
> 		}
> 		kvm_vcpu_on_spin(vcpu, true);
> 		break;
>+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
>+		if (unlikely(fast || !rep_cnt || rep_idx)) {
>+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>+			break;
>+		}
>+		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, false);
>+		break;
>+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
>+		if (unlikely(fast || rep)) {
>+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>+			break;
>+		}
>+		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, false);
>+		break;
>+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
>+		if (unlikely(fast || !rep_cnt || rep_idx)) {
>+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>+			break;
>+		}
>+		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, true);
>+		break;
>+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
>+		if (unlikely(fast || rep)) {
>+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>+			break;
>+		}
>+		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, true);
>+		break;
>+	case HVCALL_SEND_IPI:
>+		if (unlikely(rep)) {
>+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>+			break;
>+		}
>+		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, false, fast);
>+		break;
>+	case HVCALL_SEND_IPI_EX:
>+		if (unlikely(fast || rep)) {
>+			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>+			break;
>+		}
>+		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, true, false);
>+		break;
> 	case HVCALL_SIGNAL_EVENT:
> 		if (unlikely(rep)) {
> 			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>@@ -1653,12 +1695,8 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
> 		if (ret != HV_STATUS_INVALID_PORT_ID)
> 			break;
> 		/* fall through - maybe userspace knows this conn_id. */
>-	case HVCALL_POST_MESSAGE:
>-		/* don't bother userspace if it has no way to handle it */
>-		if (unlikely(rep || !vcpu_to_synic(vcpu)->active)) {
>-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>-			break;
>-		}
>+	default:
>+		/* forward unrecognized hypercalls to userspace */
> 		vcpu->run->exit_reason = KVM_EXIT_HYPERV;
> 		vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
> 		vcpu->run->hyperv.u.hcall.input = param;
>@@ -1667,51 +1705,6 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
> 		vcpu->arch.complete_userspace_io =
> 				kvm_hv_hypercall_complete_userspace;
> 		return 0;
>-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
>-		if (unlikely(fast || !rep_cnt || rep_idx)) {
>-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>-			break;
>-		}
>-		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, false);
>-		break;
>-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
>-		if (unlikely(fast || rep)) {
>-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>-			break;
>-		}
>-		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, false);
>-		break;
>-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
>-		if (unlikely(fast || !rep_cnt || rep_idx)) {
>-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>-			break;
>-		}
>-		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, true);
>-		break;
>-	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
>-		if (unlikely(fast || rep)) {
>-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>-			break;
>-		}
>-		ret = kvm_hv_flush_tlb(vcpu, ingpa, rep_cnt, true);
>-		break;
>-	case HVCALL_SEND_IPI:
>-		if (unlikely(rep)) {
>-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>-			break;
>-		}
>-		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, false, fast);
>-		break;
>-	case HVCALL_SEND_IPI_EX:
>-		if (unlikely(fast || rep)) {
>-			ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
>-			break;
>-		}
>-		ret = kvm_hv_send_ipi(vcpu, ingpa, outgpa, true, false);
>-		break;
>-	default:
>-		ret = HV_STATUS_INVALID_HYPERCALL_CODE;
>-		break;
> 	}
>
> 	return kvm_hv_hypercall_complete(vcpu, ret);
>
>(would also need a kvm cap for that)
>
>Roman.

This looks like a good idea, but I think it should be part of another 
patchset, I could revise one once this is in and expose a new CAP, and 
we need to make sure QEMU can handle this and wont just crash getting 
these additional exits.

-- Jon.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs
  2020-05-13  9:24   ` Roman Kagan
@ 2020-05-13 12:49     ` Jon Doron
  2020-05-29 11:13       ` Paolo Bonzini
  0 siblings, 1 reply; 22+ messages in thread
From: Jon Doron @ 2020-05-13 12:49 UTC (permalink / raw)
  To: Roman Kagan, kvm, linux-hyperv, vkuznets

On 13/05/2020, Roman Kagan wrote:
>On Fri, Apr 24, 2020 at 02:37:41PM +0300, Jon Doron wrote:
>> Simlify the code to define a new cpuid leaf group by enabled feature.
>>
>> This also fixes a bug in which the max cpuid leaf was always set to
>> HYPERV_CPUID_NESTED_FEATURES regardless if nesting is supported or not.
>
>I'm not sure the bug is there.  My understanding is that
>HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS is supposed to provide the range
>of leaves that return meaningful information.
>HYPERV_CPUID_NESTED_FEATURES *can* return meaningful information
>regardless of whether nested virt is active.
>
>So I'd rather skip reducing the returned set if !evmcs_ver.  The
>returned set is sparse in .function anyway; anything that isn't there
>will just return zeros to the guest.
>
>Changing the cpuid is also guest-visible so care must be taken with
>this.
>

To be honest from my understanding of the TLFS it states:
"The maximum input value for hypervisor CPUID information."

So we should not expose stuff we wont "answer" to, I agree you can 
always issue CPUID to any leaf and you will get zeroes but if we try to 
follow TLFS it sounds like this needs to be capped here.

>> Any new CPUID group needs to consider the max leaf and be added in the
>> correct order, in this method there are two rules:
>> 1. Each cpuid leaf group must be order in an ascending order
>> 2. The appending for the cpuid leafs by features also needs to happen by
>>    ascending order.
>
>It looks like unnecessary complication.  I think all you need to do to
>simplify adding new leaves is to add a macro to hyperv-tlfs.h, say,
>HYPERV_CPUID_MAX_PRESENT_LEAF, define it to
>HYPERV_CPUID_NESTED_FEATURES, and redefine once another leaf is added
>(compat may need to be taken care of).
>
>Thanks,
>Roman.
>

I suggest you will see the discussion around v8 of this patchset where I 
simply set HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS to be the maximum 
value, but then we noticed this issue and hence why this patch was 
revised to current form. (I agree it could be done under the TLFS header 
file but as I understand from other emails from Michal it's going to get 
re-worked a bit and splitted, still have not got into the details of 
that work).

Thanks,
-- Jon.

>> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Signed-off-by: Jon Doron <arilou@gmail.com>
>> ---
>>  arch/x86/kvm/hyperv.c | 46 ++++++++++++++++++++++++++++++-------------
>>  1 file changed, 32 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>> index bcefa9d4e57e..ab3e9dbcabbe 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -1785,27 +1785,45 @@ int kvm_vm_ioctl_hv_eventfd(struct kvm *kvm, struct kvm_hyperv_eventfd *args)
>>  	return kvm_hv_eventfd_assign(kvm, args->conn_id, args->fd);
>>  }
>>
>> +/* Must be sorted in ascending order by function */
>> +static struct kvm_cpuid_entry2 core_cpuid_entries[] = {
>> +	{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
>> +	{ .function = HYPERV_CPUID_INTERFACE },
>> +	{ .function = HYPERV_CPUID_VERSION },
>> +	{ .function = HYPERV_CPUID_FEATURES },
>> +	{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
>> +	{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
>> +};
>> +
>> +static struct kvm_cpuid_entry2 evmcs_cpuid_entries[] = {
>> +	{ .function = HYPERV_CPUID_NESTED_FEATURES },
>> +};
>> +
>> +#define HV_MAX_CPUID_ENTRIES \
>> +	(ARRAY_SIZE(core_cpuid_entries) +\
>> +	 ARRAY_SIZE(evmcs_cpuid_entries))
>> +
>>  int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>>  				struct kvm_cpuid_entry2 __user *entries)
>>  {
>>  	uint16_t evmcs_ver = 0;
>> -	struct kvm_cpuid_entry2 cpuid_entries[] = {
>> -		{ .function = HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS },
>> -		{ .function = HYPERV_CPUID_INTERFACE },
>> -		{ .function = HYPERV_CPUID_VERSION },
>> -		{ .function = HYPERV_CPUID_FEATURES },
>> -		{ .function = HYPERV_CPUID_ENLIGHTMENT_INFO },
>> -		{ .function = HYPERV_CPUID_IMPLEMENT_LIMITS },
>> -		{ .function = HYPERV_CPUID_NESTED_FEATURES },
>> -	};
>> -	int i, nent = ARRAY_SIZE(cpuid_entries);
>> +	struct kvm_cpuid_entry2 cpuid_entries[HV_MAX_CPUID_ENTRIES];
>> +	int i, nent = 0;
>> +
>> +	/* Set the core cpuid entries required for Hyper-V */
>> +	memcpy(&cpuid_entries[nent], &core_cpuid_entries,
>> +	       sizeof(core_cpuid_entries));
>> +	nent = ARRAY_SIZE(core_cpuid_entries);
>>
>>  	if (kvm_x86_ops.nested_get_evmcs_version)
>>  		evmcs_ver = kvm_x86_ops.nested_get_evmcs_version(vcpu);
>>
>> -	/* Skip NESTED_FEATURES if eVMCS is not supported */
>> -	if (!evmcs_ver)
>> -		--nent;
>> +	if (evmcs_ver) {
>> +		/* EVMCS is enabled, add the required EVMCS CPUID leafs */
>> +		memcpy(&cpuid_entries[nent], &evmcs_cpuid_entries,
>> +		       sizeof(evmcs_cpuid_entries));
>> +		nent += ARRAY_SIZE(evmcs_cpuid_entries);
>> +	}
>>
>>  	if (cpuid->nent < nent)
>>  		return -E2BIG;
>> @@ -1821,7 +1839,7 @@ int kvm_vcpu_ioctl_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
>>  		case HYPERV_CPUID_VENDOR_AND_MAX_FUNCTIONS:
>>  			memcpy(signature, "Linux KVM Hv", 12);
>>
>> -			ent->eax = HYPERV_CPUID_NESTED_FEATURES;
>> +			ent->eax = cpuid_entries[nent - 1].function;
>>  			ent->ebx = signature[0];
>>  			ent->ecx = signature[1];
>>  			ent->edx = signature[2];
>> --
>> 2.24.1
>>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability
  2020-04-24 11:37 ` [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability Jon Doron
@ 2020-05-29 10:46   ` Paolo Bonzini
  2020-05-29 12:08     ` Vitaly Kuznetsov
  0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2020-05-29 10:46 UTC (permalink / raw)
  To: Jon Doron, kvm, linux-hyperv; +Cc: vkuznets

On 24/04/20 13:37, Jon Doron wrote:
> +static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
> +{
> +	struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
> +
> +	if (!syndbg->active && !host)
> +		return 1;
> +

One small thing: is the ENABLE_CAP and active field needed?  Can you
just check if the guest has the syndbg CPUID bits set?

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg
  2020-04-24 11:37 ` [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg Jon Doron
  2020-05-13  9:57   ` Roman Kagan
@ 2020-05-29 10:48   ` Paolo Bonzini
  1 sibling, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2020-05-29 10:48 UTC (permalink / raw)
  To: Jon Doron, kvm, linux-hyperv; +Cc: vkuznets

On 24/04/20 13:37, Jon Doron wrote:
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 435516595090..524b5466a515 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1650,7 +1650,10 @@ static u64 kvm_hv_send_ipi(struct kvm_vcpu *current_vcpu, u64 ingpa, u64 outgpa,
>  
>  bool kvm_hv_hypercall_enabled(struct kvm *kvm)
>  {
> -	return READ_ONCE(kvm->arch.hyperv.hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE;
> +	struct kvm_hv *hv = &kvm->arch.hyperv;
> +
> +	return READ_ONCE(hv->hv_hypercall) & HV_X64_MSR_HYPERCALL_ENABLE ||
> +	       (hv->hv_syndbg.active && READ_ONCE(hv->hv_guest_os_id) != 0);
>  }

Here too we could just shrug and allow hypercalls if the guest OS is not
NULL.

Paolo


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs
  2020-05-13 12:49     ` Jon Doron
@ 2020-05-29 11:13       ` Paolo Bonzini
  0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2020-05-29 11:13 UTC (permalink / raw)
  To: Jon Doron, Roman Kagan, kvm, linux-hyperv, vkuznets

On 13/05/20 14:49, Jon Doron wrote:
>>
> 
> To be honest from my understanding of the TLFS it states:
> "The maximum input value for hypervisor CPUID information."
> 
> So we should not expose stuff we wont "answer" to, I agree you can
> always issue CPUID to any leaf and you will get zeroes but if we try to
> follow TLFS it sounds like this needs to be capped here.

I think Roman is right in the reading, but it's also nicer to have a
capped EAX because you can just look at EAX and guess what features are
there (it's simpler than looking for zeroes).

Paolo


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability
  2020-05-29 10:46   ` Paolo Bonzini
@ 2020-05-29 12:08     ` Vitaly Kuznetsov
  2020-05-29 12:17       ` Paolo Bonzini
  0 siblings, 1 reply; 22+ messages in thread
From: Vitaly Kuznetsov @ 2020-05-29 12:08 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Jon Doron, kvm, linux-hyperv

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 24/04/20 13:37, Jon Doron wrote:
>> +static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
>> +{
>> +	struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
>> +
>> +	if (!syndbg->active && !host)
>> +		return 1;
>> +
>
> One small thing: is the ENABLE_CAP and active field needed?  Can you
> just check if the guest has the syndbg CPUID bits set?
>

Yes, we can probably get away with a static capability (so userspace
knows that the interface is supported and CPUID bit can be set) and
check guest_cpuid_has() here but we don't have Hyper-V feature leaves
exposed as X86_FEATURE_* (yet). It is probably possible to implement an
interim solution by open coding the check with kvm_find_cpuid_entry() or
something like that.

-- 
Vitaly


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability
  2020-05-29 12:08     ` Vitaly Kuznetsov
@ 2020-05-29 12:17       ` Paolo Bonzini
  0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2020-05-29 12:17 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: Jon Doron, kvm, linux-hyperv

On 29/05/20 14:08, Vitaly Kuznetsov wrote:
>> On 24/04/20 13:37, Jon Doron wrote:
>>> +static int syndbg_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host)
>>> +{
>>> +	struct kvm_hv_syndbg *syndbg = vcpu_to_hv_syndbg(vcpu);
>>> +
>>> +	if (!syndbg->active && !host)
>>> +		return 1;
>>> +
>> One small thing: is the ENABLE_CAP and active field needed?  Can you
>> just check if the guest has the syndbg CPUID bits set?
>>
> Yes, we can probably get away with a static capability (so userspace
> knows that the interface is supported and CPUID bit can be set) and
> check guest_cpuid_has() here but we don't have Hyper-V feature leaves
> exposed as X86_FEATURE_* (yet). It is probably possible to implement an
> interim solution by open coding the check with kvm_find_cpuid_entry() or
> something like that.

Yes, that would be fine if you just abstract it in its own function.

Paolo


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-05-29 12:18 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-24 11:37 [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
2020-04-24 11:37 ` [PATCH v11 1/7] x86/kvm/hyper-v: Explicitly align hcall param for kvm_hyperv_exit Jon Doron
2020-05-13  8:42   ` Roman Kagan
2020-04-24 11:37 ` [PATCH v11 2/7] x86/kvm/hyper-v: Simplify addition for custom cpuid leafs Jon Doron
2020-05-13  9:24   ` Roman Kagan
2020-05-13 12:49     ` Jon Doron
2020-05-29 11:13       ` Paolo Bonzini
2020-04-24 11:37 ` [PATCH v11 3/7] x86/hyper-v: Add synthetic debugger definitions Jon Doron
2020-04-24 11:37 ` [PATCH v11 4/7] x86/kvm/hyper-v: Add support for synthetic debugger capability Jon Doron
2020-05-29 10:46   ` Paolo Bonzini
2020-05-29 12:08     ` Vitaly Kuznetsov
2020-05-29 12:17       ` Paolo Bonzini
2020-04-24 11:37 ` [PATCH v11 5/7] x86/kvm/hyper-v: enable hypercalls without hypercall page with syndbg Jon Doron
2020-05-13  9:57   ` Roman Kagan
2020-05-13 12:37     ` Jon Doron
2020-05-29 10:48   ` Paolo Bonzini
2020-04-24 11:37 ` [PATCH v11 6/7] x86/kvm/hyper-v: Add support for synthetic debugger via hypercalls Jon Doron
2020-05-12 15:33   ` Roman Kagan
2020-05-13 12:39     ` Jon Doron
2020-04-24 11:37 ` [PATCH v11 7/7] KVM: selftests: update hyperv_cpuid with SynDBG tests Jon Doron
2020-05-07  3:01 ` [PATCH v11 0/7] x86/kvm/hyper-v: add support for synthetic debugger Jon Doron
2020-05-07  7:57   ` Paolo Bonzini

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).