[PATCH v2 0/4] Add support for updated vmware hypercall instruction

[PATCH v2 0/4] Add support for updated vmware hypercall instruction

[Thomas Hellström (VMware)]

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <x86@kernel.org>

VMware has started using "vmcall" / "vmmcall" instead of an inl instruction
for the "backdoor" interface. This series detects support for those
instructions.
Outside of the platform code we use the "ALTERNATIVES" self-patching
mechanism similarly to how this is done with KVM.
Unfortunately we need two new x86 CPU feature flags for this, since we need
the default instruction to be "inl". IIRC the vmmouse driver is used by
other virtualization solutions than VMware, and those might break if
they encounter any of the other instructions.

v2:
- Address various style review comments
- Use mnemonics instead of bytecode in the ALTERNATIVE_2 macros
- Use vmcall / vmmcall also for the High-Bandwidth port calls
- Change the %edx argument to what vmcall / vmmcall expect (flags instead of
  port number). The port number is added in the default ALTERNATIVE_2 path.
- Ack to merge the vmmouse patch from Dmitry.
- Drop license update for now. Will get back with a freestanding patch.

[PATCH v2 1/4] x86/vmware: Update platform detection code for VMCALL/VMMCALL hypercalls

[Thomas Hellström (VMware)]

From: Thomas Hellstrom <thellstrom@vmware.com>

Vmware has historically used an "inl" instruction for this, but recent
hardware versions support using VMCALL/VMMCALL instead, so use this method
if supported at platform detection time. We explicitly need to code
separate macro versions since the alternatives self-patching has not
been performed at platform detection time.

Also put tighter constraints on the assembly input parameters.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <x86@kernel.org>
Cc: <dri-devel@lists.freedekstop.org>
Co-developed-by: Doug Covelli <dcovelli@vmware.com>
Signed-off-by: Doug Covelli <dcovelli@vmware.com>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Doug Covelli <dcovelli@vmware.com>
---
 arch/x86/kernel/cpu/vmware.c | 88 +++++++++++++++++++++++++++++-------
 1 file changed, 71 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 3c648476d4fb..fcb84b1e099e 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -30,34 +30,70 @@
 #include <asm/hypervisor.h>
 #include <asm/timer.h>
 #include <asm/apic.h>
+#include <asm/svm.h>
 
 #undef pr_fmt
 #define pr_fmt(fmt)	"vmware: " fmt
 
-#define CPUID_VMWARE_INFO_LEAF	0x40000000
+#define CPUID_VMWARE_INFO_LEAF               0x40000000
+#define CPUID_VMWARE_FEATURES_LEAF           0x40000010
+#define CPUID_VMWARE_FEATURES_ECX_VMMCALL    BIT(0)
+#define CPUID_VMWARE_FEATURES_ECX_VMCALL     BIT(1)
+
 #define VMWARE_HYPERVISOR_MAGIC	0x564D5868
 #define VMWARE_HYPERVISOR_PORT	0x5658
 
-#define VMWARE_PORT_CMD_GETVERSION	10
-#define VMWARE_PORT_CMD_GETHZ		45
-#define VMWARE_PORT_CMD_GETVCPU_INFO	68
-#define VMWARE_PORT_CMD_LEGACY_X2APIC	3
-#define VMWARE_PORT_CMD_VCPU_RESERVED	31
+#define VMWARE_CMD_GETVERSION    10
+#define VMWARE_CMD_GETHZ         45
+#define VMWARE_CMD_GETVCPU_INFO  68
+#define VMWARE_CMD_LEGACY_X2APIC  3
+#define VMWARE_CMD_VCPU_RESERVED 31
 
 #define VMWARE_PORT(cmd, eax, ebx, ecx, edx)				\
 	__asm__("inl (%%dx)" :						\
-			"=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :	\
-			"0"(VMWARE_HYPERVISOR_MAGIC),			\
-			"1"(VMWARE_PORT_CMD_##cmd),			\
-			"2"(VMWARE_HYPERVISOR_PORT), "3"(UINT_MAX) :	\
-			"memory");
+		"=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :		\
+		"a"(VMWARE_HYPERVISOR_MAGIC),				\
+		"c"(VMWARE_CMD_##cmd),					\
+		"d"(VMWARE_HYPERVISOR_PORT), "b"(UINT_MAX) :		\
+		"memory")
+
+#define VMWARE_VMCALL(cmd, eax, ebx, ecx, edx)				\
+	__asm__("vmcall" :						\
+		"=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :		\
+		"a"(VMWARE_HYPERVISOR_MAGIC),				\
+		"c"(VMWARE_CMD_##cmd),					\
+		"d"(0), "b"(UINT_MAX) :					\
+		"memory")
+
+#define VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx)                         \
+	__asm__("vmmcall" :						\
+		"=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :		\
+		"a"(VMWARE_HYPERVISOR_MAGIC),				\
+		"c"(VMWARE_CMD_##cmd),					\
+		"d"(0), "b"(UINT_MAX) :					\
+		"memory")
+
+#define VMWARE_CMD(cmd, eax, ebx, ecx, edx) do {		\
+	switch (vmware_hypercall_mode) {			\
+	case CPUID_VMWARE_FEATURES_ECX_VMCALL:			\
+		VMWARE_VMCALL(cmd, eax, ebx, ecx, edx);		\
+		break;						\
+	case CPUID_VMWARE_FEATURES_ECX_VMMCALL:			\
+		VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx);	\
+		break;						\
+	default:						\
+		VMWARE_PORT(cmd, eax, ebx, ecx, edx);		\
+		break;						\
+	}							\
+	} while (0)
 
 static unsigned long vmware_tsc_khz __ro_after_init;
+static u8 vmware_hypercall_mode     __ro_after_init;
 
 static inline int __vmware_platform(void)
 {
 	uint32_t eax, ebx, ecx, edx;
-	VMWARE_PORT(GETVERSION, eax, ebx, ecx, edx);
+	VMWARE_CMD(GETVERSION, eax, ebx, ecx, edx);
 	return eax != (uint32_t)-1 && ebx == VMWARE_HYPERVISOR_MAGIC;
 }
 
@@ -136,7 +172,7 @@ static void __init vmware_platform_setup(void)
 	uint32_t eax, ebx, ecx, edx;
 	uint64_t lpj, tsc_khz;
 
-	VMWARE_PORT(GETHZ, eax, ebx, ecx, edx);
+	VMWARE_CMD(GETHZ, eax, ebx, ecx, edx);
 
 	if (ebx != UINT_MAX) {
 		lpj = tsc_khz = eax | (((uint64_t)ebx) << 32);
@@ -174,6 +210,16 @@ static void __init vmware_platform_setup(void)
 	vmware_set_capabilities();
 }
 
+static u8
+vmware_select_hypercall(void)
+{
+	int eax, ebx, ecx, edx;
+
+	cpuid(CPUID_VMWARE_FEATURES_LEAF, &eax, &ebx, &ecx, &edx);
+	return (ecx & (CPUID_VMWARE_FEATURES_ECX_VMMCALL |
+		       CPUID_VMWARE_FEATURES_ECX_VMCALL));
+}
+
 /*
  * While checking the dmi string information, just checking the product
  * serial key should be enough, as this will always have a VMware
@@ -187,8 +233,16 @@ static uint32_t __init vmware_platform(void)
 
 		cpuid(CPUID_VMWARE_INFO_LEAF, &eax, &hyper_vendor_id[0],
 		      &hyper_vendor_id[1], &hyper_vendor_id[2]);
-		if (!memcmp(hyper_vendor_id, "VMwareVMware", 12))
+		if (!memcmp(hyper_vendor_id, "VMwareVMware", 12)) {
+			if (eax >= CPUID_VMWARE_FEATURES_LEAF)
+				vmware_hypercall_mode =
+					vmware_select_hypercall();
+
+			pr_info("hypercall mode: 0x%02x\n",
+				(unsigned int) vmware_hypercall_mode);
+
 			return CPUID_VMWARE_INFO_LEAF;
+		}
 	} else if (dmi_available && dmi_name_in_serial("VMware") &&
 		   __vmware_platform())
 		return 1;
@@ -200,9 +254,9 @@ static uint32_t __init vmware_platform(void)
 static bool __init vmware_legacy_x2apic_available(void)
 {
 	uint32_t eax, ebx, ecx, edx;
-	VMWARE_PORT(GETVCPU_INFO, eax, ebx, ecx, edx);
-	return (eax & (1 << VMWARE_PORT_CMD_VCPU_RESERVED)) == 0 &&
-	       (eax & (1 << VMWARE_PORT_CMD_LEGACY_X2APIC)) != 0;
+	VMWARE_CMD(GETVCPU_INFO, eax, ebx, ecx, edx);
+	return (eax & (1 << VMWARE_CMD_VCPU_RESERVED)) == 0 &&
+	       (eax & (1 << VMWARE_CMD_LEGACY_X2APIC)) != 0;
 }
 
 const __initconst struct hypervisor_x86 x86_hyper_vmware = {
-- 
2.20.1

[PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions

[Thomas Hellström (VMware)]

From: Thomas Hellstrom <thellstrom@vmware.com>

The new header is intended to be used by drivers using the backdoor.
Follow the kvm example using alternatives self-patching to
choose between vmcall, vmmcall and io instructions.

Also define two new CPU feature flags to indicate hypervisor support
for vmcall- and vmmcall instructions.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <x86@kernel.org>
Cc: <dri-devel@lists.freedesktop.org>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Doug Covelli <dcovelli@vmware.com>
---
 MAINTAINERS                        |  1 +
 arch/x86/include/asm/cpufeatures.h |  2 ++
 arch/x86/include/asm/vmware.h      | 48 ++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/vmware.c       |  6 +++-
 4 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/include/asm/vmware.h

diff --git a/MAINTAINERS b/MAINTAINERS
index c2d975da561f..5bf65a49fa19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17203,6 +17203,7 @@ M:	"VMware, Inc." <pv-drivers@vmware.com>
 L:	virtualization@lists.linux-foundation.org
 S:	Supported
 F:	arch/x86/kernel/cpu/vmware.c
+F:	arch/x86/include/asm/vmware.h
 
 VMWARE PVRDMA DRIVER
 M:	Adit Ranadive <aditr@vmware.com>
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 998c2cc08363..55fa3b3f0bac 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -232,6 +232,8 @@
 #define X86_FEATURE_VMMCALL		( 8*32+15) /* Prefer VMMCALL to VMCALL */
 #define X86_FEATURE_XENPV		( 8*32+16) /* "" Xen paravirtual guest */
 #define X86_FEATURE_EPT_AD		( 8*32+17) /* Intel Extended Page Table access-dirty bit */
+#define X86_FEATURE_VMCALL		( 8*32+18) /* "" Hypervisor supports the VMCALL instruction */
+#define X86_FEATURE_VMW_VMMCALL		( 8*32+19) /* "" VMware prefers VMMCALL hypercall instruction */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (EBX), word 9 */
 #define X86_FEATURE_FSGSBASE		( 9*32+ 0) /* RDFSBASE, WRFSBASE, RDGSBASE, WRGSBASE instructions*/
diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
new file mode 100644
index 000000000000..4b220e2bb3e8
--- /dev/null
+++ b/arch/x86/include/asm/vmware.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 or MIT */
+#ifndef _ASM_X86_VMWARE_H
+#define _ASM_X86_VMWARE_H
+
+#include <asm/cpufeatures.h>
+#include <asm/alternative.h>
+
+/*
+ * The hypercall definitions differ in the low word of the edx argument in
+ * the following way: The old port base interface uses the port number to
+ * distinguish between high- and low bandwidth versions. The new vmcall
+ * interface instead uses a set of flags to select bandwidth mode and
+ * transfer direction. New driver code should strictly use the new
+ * definition of edx content.
+ */
+
+/* Old port-based version */
+#define VMWARE_HYPERVISOR_PORT    "0x5658"
+#define VMWARE_HYPERVISOR_PORT_HB "0x5659"
+
+/* Current vmcall / vmmcall version */
+#define VMWARE_HYPERVISOR_HB   BIT(0)
+#define VMWARE_HYPERVISOR_OUT  BIT(1)
+
+/* The low bandwidth call. The low word of edx is presumed clear. */
+#define VMWARE_HYPERCALL						\
+	ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT ", %%dx; inl (%%dx)", \
+		      "vmcall", X86_FEATURE_VMCALL,			\
+		      "vmmcall", X86_FEATURE_VMW_VMMCALL)
+
+/*
+ * The high bandwidth out call. The low word of edx is presumed to have the
+ * HB and OUT bits set.
+ */
+#define VMWARE_HYPERCALL_HB_OUT						\
+	ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT_HB ", %%dx; rep outsb", \
+		      "vmcall", X86_FEATURE_VMCALL,			\
+		      "vmmcall", X86_FEATURE_VMW_VMMCALL)
+
+/*
+ * The high bandwidth in call. The low word of edx is presumed to have the
+ * HB bit set.
+ */
+#define VMWARE_HYPERCALL_HB_IN						\
+	ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT_HB ", %%dx; rep insb", \
+		      "vmcall", X86_FEATURE_VMCALL,			\
+		      "vmmcall", X86_FEATURE_VMW_VMMCALL)
+#endif
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index fcb84b1e099e..abaa1b27353c 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -31,6 +31,7 @@
 #include <asm/timer.h>
 #include <asm/apic.h>
 #include <asm/svm.h>
+#include <asm/vmware.h>
 
 #undef pr_fmt
 #define pr_fmt(fmt)	"vmware: " fmt
@@ -41,7 +42,6 @@
 #define CPUID_VMWARE_FEATURES_ECX_VMCALL     BIT(1)
 
 #define VMWARE_HYPERVISOR_MAGIC	0x564D5868
-#define VMWARE_HYPERVISOR_PORT	0x5658
 
 #define VMWARE_CMD_GETVERSION    10
 #define VMWARE_CMD_GETHZ         45
@@ -165,6 +165,10 @@ static void __init vmware_set_capabilities(void)
 {
 	setup_force_cpu_cap(X86_FEATURE_CONSTANT_TSC);
 	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
+	if (vmware_hypercall_mode == CPUID_VMWARE_FEATURES_ECX_VMCALL)
+		setup_force_cpu_cap(X86_FEATURE_VMCALL);
+	else if (vmware_hypercall_mode == CPUID_VMWARE_FEATURES_ECX_VMMCALL)
+		setup_force_cpu_cap(X86_FEATURE_VMW_VMMCALL);
 }
 
 static void __init vmware_platform_setup(void)
-- 
2.20.1

[PATCH v2 3/4] drm/vmwgfx: Update the backdoor call with support for new instructions

[Thomas Hellström (VMware)]

From: Thomas Hellstrom <thellstrom@vmware.com>

Use the definition provided by include/asm/vmware.h

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <x86@kernel.org>
Cc: <dri-devel@lists.freedesktop.org>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Doug Covelli <dcovelli@vmware.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 21 +++++++++--------
 drivers/gpu/drm/vmwgfx/vmwgfx_msg.h | 35 +++++++++++++++--------------
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 81a86c3b77bc..1281e52898ee 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -45,8 +45,6 @@
 #define RETRIES                 3
 
 #define VMW_HYPERVISOR_MAGIC    0x564D5868
-#define VMW_HYPERVISOR_PORT     0x5658
-#define VMW_HYPERVISOR_HB_PORT  0x5659
 
 #define VMW_PORT_CMD_MSG        30
 #define VMW_PORT_CMD_HB_MSG     0
@@ -92,7 +90,7 @@ static int vmw_open_channel(struct rpc_channel *channel, unsigned int protocol)
 
 	VMW_PORT(VMW_PORT_CMD_OPEN_CHANNEL,
 		(protocol | GUESTMSG_FLAG_COOKIE), si, di,
-		VMW_HYPERVISOR_PORT,
+		0,
 		VMW_HYPERVISOR_MAGIC,
 		eax, ebx, ecx, edx, si, di);
 
@@ -125,7 +123,7 @@ static int vmw_close_channel(struct rpc_channel *channel)
 
 	VMW_PORT(VMW_PORT_CMD_CLOSE_CHANNEL,
 		0, si, di,
-		(VMW_HYPERVISOR_PORT | (channel->channel_id << 16)),
+		channel->channel_id << 16,
 		VMW_HYPERVISOR_MAGIC,
 		eax, ebx, ecx, edx, si, di);
 
@@ -159,7 +157,8 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
 		VMW_PORT_HB_OUT(
 			(MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG,
 			msg_len, si, di,
-			VMW_HYPERVISOR_HB_PORT | (channel->channel_id << 16),
+			VMWARE_HYPERVISOR_HB | (channel->channel_id << 16) |
+			VMWARE_HYPERVISOR_OUT,
 			VMW_HYPERVISOR_MAGIC, bp,
 			eax, ebx, ecx, edx, si, di);
 
@@ -180,7 +179,7 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
 
 		VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_SENDPAYLOAD << 16),
 			 word, si, di,
-			 VMW_HYPERVISOR_PORT | (channel->channel_id << 16),
+			 channel->channel_id << 16,
 			 VMW_HYPERVISOR_MAGIC,
 			 eax, ebx, ecx, edx, si, di);
 	}
@@ -212,7 +211,7 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
 		VMW_PORT_HB_IN(
 			(MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG,
 			reply_len, si, di,
-			VMW_HYPERVISOR_HB_PORT | (channel->channel_id << 16),
+			VMWARE_HYPERVISOR_HB | (channel->channel_id << 16),
 			VMW_HYPERVISOR_MAGIC, bp,
 			eax, ebx, ecx, edx, si, di);
 
@@ -229,7 +228,7 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
 
 		VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_RECVPAYLOAD << 16),
 			 MESSAGE_STATUS_SUCCESS, si, di,
-			 VMW_HYPERVISOR_PORT | (channel->channel_id << 16),
+			 channel->channel_id << 16,
 			 VMW_HYPERVISOR_MAGIC,
 			 eax, ebx, ecx, edx, si, di);
 
@@ -268,7 +267,7 @@ static int vmw_send_msg(struct rpc_channel *channel, const char *msg)
 
 		VMW_PORT(VMW_PORT_CMD_SENDSIZE,
 			msg_len, si, di,
-			VMW_HYPERVISOR_PORT | (channel->channel_id << 16),
+			channel->channel_id << 16,
 			VMW_HYPERVISOR_MAGIC,
 			eax, ebx, ecx, edx, si, di);
 
@@ -326,7 +325,7 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
 
 		VMW_PORT(VMW_PORT_CMD_RECVSIZE,
 			0, si, di,
-			(VMW_HYPERVISOR_PORT | (channel->channel_id << 16)),
+			channel->channel_id << 16,
 			VMW_HYPERVISOR_MAGIC,
 			eax, ebx, ecx, edx, si, di);
 
@@ -370,7 +369,7 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
 
 		VMW_PORT(VMW_PORT_CMD_RECVSTATUS,
 			MESSAGE_STATUS_SUCCESS, si, di,
-			(VMW_HYPERVISOR_PORT | (channel->channel_id << 16)),
+			channel->channel_id << 16,
 			VMW_HYPERVISOR_MAGIC,
 			eax, ebx, ecx, edx, si, di);
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h
index 4907e50fb20a..f685c7071dec 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h
@@ -32,6 +32,7 @@
 #ifndef _VMWGFX_MSG_H
 #define _VMWGFX_MSG_H
 
+#include <asm/vmware.h>
 
 /**
  * Hypervisor-specific bi-directional communication channel.  Should never
@@ -44,7 +45,7 @@
  * @in_ebx: [IN] Message Len, through EBX
  * @in_si: [IN] Input argument through SI, set to 0 if not used
  * @in_di: [IN] Input argument through DI, set ot 0 if not used
- * @port_num: [IN] port number + [channel id]
+ * @flags: [IN] hypercall flags + [channel id]
  * @magic: [IN] hypervisor magic value
  * @eax: [OUT] value of EAX register
  * @ebx: [OUT] e.g. status from an HB message status command
@@ -54,10 +55,10 @@
  * @di:  [OUT]
  */
 #define VMW_PORT(cmd, in_ebx, in_si, in_di,	\
-		 port_num, magic,		\
+		 flags, magic,		\
 		 eax, ebx, ecx, edx, si, di)	\
 ({						\
-	asm volatile ("inl %%dx, %%eax;" :	\
+	asm volatile (VMWARE_HYPERCALL :	\
 		"=a"(eax),			\
 		"=b"(ebx),			\
 		"=c"(ecx),			\
@@ -67,7 +68,7 @@
 		"a"(magic),			\
 		"b"(in_ebx),			\
 		"c"(cmd),			\
-		"d"(port_num),			\
+		"d"(flags),			\
 		"S"(in_si),			\
 		"D"(in_di) :			\
 		"memory");			\
@@ -85,7 +86,7 @@
  * @in_ecx: [IN] Message Len, through ECX
  * @in_si: [IN] Input argument through SI, set to 0 if not used
  * @in_di: [IN] Input argument through DI, set to 0 if not used
- * @port_num: [IN] port number + [channel id]
+ * @flags: [IN] hypercall flags + [channel id]
  * @magic: [IN] hypervisor magic value
  * @bp:  [IN]
  * @eax: [OUT] value of EAX register
@@ -98,12 +99,12 @@
 #ifdef __x86_64__
 
 #define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di,	\
-			port_num, magic, bp,		\
+			flags, magic, bp,		\
 			eax, ebx, ecx, edx, si, di)	\
 ({							\
 	asm volatile ("push %%rbp;"			\
 		"mov %12, %%rbp;"			\
-		"rep outsb;"				\
+		VMWARE_HYPERCALL_HB_OUT			\
 		"pop %%rbp;" :				\
 		"=a"(eax),				\
 		"=b"(ebx),				\
@@ -114,7 +115,7 @@
 		"a"(magic),				\
 		"b"(cmd),				\
 		"c"(in_ecx),				\
-		"d"(port_num),				\
+		"d"(flags),				\
 		"S"(in_si),				\
 		"D"(in_di),				\
 		"r"(bp) :				\
@@ -123,12 +124,12 @@
 
 
 #define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di,	\
-		       port_num, magic, bp,		\
+		       flags, magic, bp,		\
 		       eax, ebx, ecx, edx, si, di)	\
 ({							\
 	asm volatile ("push %%rbp;"			\
 		"mov %12, %%rbp;"			\
-		"rep insb;"				\
+		VMWARE_HYPERCALL_HB_IN			\
 		"pop %%rbp" :				\
 		"=a"(eax),				\
 		"=b"(ebx),				\
@@ -139,7 +140,7 @@
 		"a"(magic),				\
 		"b"(cmd),				\
 		"c"(in_ecx),				\
-		"d"(port_num),				\
+		"d"(flags),				\
 		"S"(in_si),				\
 		"D"(in_di),				\
 		"r"(bp) :				\
@@ -157,13 +158,13 @@
  * just pushed it.
  */
 #define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di,	\
-			port_num, magic, bp,		\
+			flags, magic, bp,		\
 			eax, ebx, ecx, edx, si, di)	\
 ({							\
 	asm volatile ("push %12;"			\
 		"push %%ebp;"				\
 		"mov 0x04(%%esp), %%ebp;"		\
-		"rep outsb;"				\
+		VMWARE_HYPERCALL_HB_OUT			\
 		"pop %%ebp;"				\
 		"add $0x04, %%esp;" :			\
 		"=a"(eax),				\
@@ -175,7 +176,7 @@
 		"a"(magic),				\
 		"b"(cmd),				\
 		"c"(in_ecx),				\
-		"d"(port_num),				\
+		"d"(flags),				\
 		"S"(in_si),				\
 		"D"(in_di),				\
 		"m"(bp) :				\
@@ -184,13 +185,13 @@
 
 
 #define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di,	\
-		       port_num, magic, bp,		\
+		       flags, magic, bp,		\
 		       eax, ebx, ecx, edx, si, di)	\
 ({							\
 	asm volatile ("push %12;"			\
 		"push %%ebp;"				\
 		"mov 0x04(%%esp), %%ebp;"		\
-		"rep insb;"				\
+		VMWARE_HYPERCALL_HB_IN			\
 		"pop %%ebp;"				\
 		"add $0x04, %%esp;" :			\
 		"=a"(eax),				\
@@ -202,7 +203,7 @@
 		"a"(magic),				\
 		"b"(cmd),				\
 		"c"(in_ecx),				\
-		"d"(port_num),				\
+		"d"(flags),				\
 		"S"(in_si),				\
 		"D"(in_di),				\
 		"m"(bp) :				\
-- 
2.20.1

[PATCH v2 4/4] input/vmmouse: Update the backdoor call with support for new instructions

[Thomas Hellström (VMware)]

From: Thomas Hellstrom <thellstrom@vmware.com>

Use the definition provided by include/asm/vmware.h

CC: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <x86@kernel.org>
Cc: <dri-devel@lists.freedesktop.org>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Doug Covelli <dcovelli@vmware.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/vmmouse.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index 871e5b5ab129..148245c69be7 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -16,12 +16,12 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <asm/hypervisor.h>
+#include <asm/vmware.h>
 
 #include "psmouse.h"
 #include "vmmouse.h"
 
 #define VMMOUSE_PROTO_MAGIC			0x564D5868U
-#define VMMOUSE_PROTO_PORT			0x5658
 
 /*
  * Main commands supported by the vmmouse hypervisor port.
@@ -84,7 +84,7 @@ struct vmmouse_data {
 #define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4)	\
 ({							\
 	unsigned long __dummy1, __dummy2;		\
-	__asm__ __volatile__ ("inl %%dx" :		\
+	__asm__ __volatile__ (VMWARE_HYPERCALL :	\
 		"=a"(out1),				\
 		"=b"(out2),				\
 		"=c"(out3),				\
@@ -94,7 +94,7 @@ struct vmmouse_data {
 		"a"(VMMOUSE_PROTO_MAGIC),		\
 		"b"(in1),				\
 		"c"(VMMOUSE_PROTO_CMD_##cmd),		\
-		"d"(VMMOUSE_PROTO_PORT) :		\
+		"d"(0) :			        \
 		"memory");		                \
 })
 
-- 
2.20.1

Re: [PATCH v2 3/4] drm/vmwgfx: Update the backdoor call with support for new instructions

[Dave Airlie]

Acked-by: Dave Airlie <airlied@redhat.com>

(for merging via x86 trees).

Dave.

On Fri, 23 Aug 2019 at 18:13, Thomas Hellström (VMware)
<thomas_os@shipmail.org> wrote:
>
> From: Thomas Hellstrom <thellstrom@vmware.com>
>
> Use the definition provided by include/asm/vmware.h
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: <x86@kernel.org>
> Cc: <dri-devel@lists.freedesktop.org>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> Reviewed-by: Doug Covelli <dcovelli@vmware.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 21 +++++++++--------
>  drivers/gpu/drm/vmwgfx/vmwgfx_msg.h | 35 +++++++++++++++--------------
>  2 files changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
> index 81a86c3b77bc..1281e52898ee 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
> @@ -45,8 +45,6 @@
>  #define RETRIES                 3
>
>  #define VMW_HYPERVISOR_MAGIC    0x564D5868
> -#define VMW_HYPERVISOR_PORT     0x5658
> -#define VMW_HYPERVISOR_HB_PORT  0x5659
>
>  #define VMW_PORT_CMD_MSG        30
>  #define VMW_PORT_CMD_HB_MSG     0
> @@ -92,7 +90,7 @@ static int vmw_open_channel(struct rpc_channel *channel, unsigned int protocol)
>
>         VMW_PORT(VMW_PORT_CMD_OPEN_CHANNEL,
>                 (protocol | GUESTMSG_FLAG_COOKIE), si, di,
> -               VMW_HYPERVISOR_PORT,
> +               0,
>                 VMW_HYPERVISOR_MAGIC,
>                 eax, ebx, ecx, edx, si, di);
>
> @@ -125,7 +123,7 @@ static int vmw_close_channel(struct rpc_channel *channel)
>
>         VMW_PORT(VMW_PORT_CMD_CLOSE_CHANNEL,
>                 0, si, di,
> -               (VMW_HYPERVISOR_PORT | (channel->channel_id << 16)),
> +               channel->channel_id << 16,
>                 VMW_HYPERVISOR_MAGIC,
>                 eax, ebx, ecx, edx, si, di);
>
> @@ -159,7 +157,8 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
>                 VMW_PORT_HB_OUT(
>                         (MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG,
>                         msg_len, si, di,
> -                       VMW_HYPERVISOR_HB_PORT | (channel->channel_id << 16),
> +                       VMWARE_HYPERVISOR_HB | (channel->channel_id << 16) |
> +                       VMWARE_HYPERVISOR_OUT,
>                         VMW_HYPERVISOR_MAGIC, bp,
>                         eax, ebx, ecx, edx, si, di);
>
> @@ -180,7 +179,7 @@ static unsigned long vmw_port_hb_out(struct rpc_channel *channel,
>
>                 VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_SENDPAYLOAD << 16),
>                          word, si, di,
> -                        VMW_HYPERVISOR_PORT | (channel->channel_id << 16),
> +                        channel->channel_id << 16,
>                          VMW_HYPERVISOR_MAGIC,
>                          eax, ebx, ecx, edx, si, di);
>         }
> @@ -212,7 +211,7 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
>                 VMW_PORT_HB_IN(
>                         (MESSAGE_STATUS_SUCCESS << 16) | VMW_PORT_CMD_HB_MSG,
>                         reply_len, si, di,
> -                       VMW_HYPERVISOR_HB_PORT | (channel->channel_id << 16),
> +                       VMWARE_HYPERVISOR_HB | (channel->channel_id << 16),
>                         VMW_HYPERVISOR_MAGIC, bp,
>                         eax, ebx, ecx, edx, si, di);
>
> @@ -229,7 +228,7 @@ static unsigned long vmw_port_hb_in(struct rpc_channel *channel, char *reply,
>
>                 VMW_PORT(VMW_PORT_CMD_MSG | (MSG_TYPE_RECVPAYLOAD << 16),
>                          MESSAGE_STATUS_SUCCESS, si, di,
> -                        VMW_HYPERVISOR_PORT | (channel->channel_id << 16),
> +                        channel->channel_id << 16,
>                          VMW_HYPERVISOR_MAGIC,
>                          eax, ebx, ecx, edx, si, di);
>
> @@ -268,7 +267,7 @@ static int vmw_send_msg(struct rpc_channel *channel, const char *msg)
>
>                 VMW_PORT(VMW_PORT_CMD_SENDSIZE,
>                         msg_len, si, di,
> -                       VMW_HYPERVISOR_PORT | (channel->channel_id << 16),
> +                       channel->channel_id << 16,
>                         VMW_HYPERVISOR_MAGIC,
>                         eax, ebx, ecx, edx, si, di);
>
> @@ -326,7 +325,7 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
>
>                 VMW_PORT(VMW_PORT_CMD_RECVSIZE,
>                         0, si, di,
> -                       (VMW_HYPERVISOR_PORT | (channel->channel_id << 16)),
> +                       channel->channel_id << 16,
>                         VMW_HYPERVISOR_MAGIC,
>                         eax, ebx, ecx, edx, si, di);
>
> @@ -370,7 +369,7 @@ static int vmw_recv_msg(struct rpc_channel *channel, void **msg,
>
>                 VMW_PORT(VMW_PORT_CMD_RECVSTATUS,
>                         MESSAGE_STATUS_SUCCESS, si, di,
> -                       (VMW_HYPERVISOR_PORT | (channel->channel_id << 16)),
> +                       channel->channel_id << 16,
>                         VMW_HYPERVISOR_MAGIC,
>                         eax, ebx, ecx, edx, si, di);
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h
> index 4907e50fb20a..f685c7071dec 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.h
> @@ -32,6 +32,7 @@
>  #ifndef _VMWGFX_MSG_H
>  #define _VMWGFX_MSG_H
>
> +#include <asm/vmware.h>
>
>  /**
>   * Hypervisor-specific bi-directional communication channel.  Should never
> @@ -44,7 +45,7 @@
>   * @in_ebx: [IN] Message Len, through EBX
>   * @in_si: [IN] Input argument through SI, set to 0 if not used
>   * @in_di: [IN] Input argument through DI, set ot 0 if not used
> - * @port_num: [IN] port number + [channel id]
> + * @flags: [IN] hypercall flags + [channel id]
>   * @magic: [IN] hypervisor magic value
>   * @eax: [OUT] value of EAX register
>   * @ebx: [OUT] e.g. status from an HB message status command
> @@ -54,10 +55,10 @@
>   * @di:  [OUT]
>   */
>  #define VMW_PORT(cmd, in_ebx, in_si, in_di,    \
> -                port_num, magic,               \
> +                flags, magic,          \
>                  eax, ebx, ecx, edx, si, di)    \
>  ({                                             \
> -       asm volatile ("inl %%dx, %%eax;" :      \
> +       asm volatile (VMWARE_HYPERCALL :        \
>                 "=a"(eax),                      \
>                 "=b"(ebx),                      \
>                 "=c"(ecx),                      \
> @@ -67,7 +68,7 @@
>                 "a"(magic),                     \
>                 "b"(in_ebx),                    \
>                 "c"(cmd),                       \
> -               "d"(port_num),                  \
> +               "d"(flags),                     \
>                 "S"(in_si),                     \
>                 "D"(in_di) :                    \
>                 "memory");                      \
> @@ -85,7 +86,7 @@
>   * @in_ecx: [IN] Message Len, through ECX
>   * @in_si: [IN] Input argument through SI, set to 0 if not used
>   * @in_di: [IN] Input argument through DI, set to 0 if not used
> - * @port_num: [IN] port number + [channel id]
> + * @flags: [IN] hypercall flags + [channel id]
>   * @magic: [IN] hypervisor magic value
>   * @bp:  [IN]
>   * @eax: [OUT] value of EAX register
> @@ -98,12 +99,12 @@
>  #ifdef __x86_64__
>
>  #define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di,     \
> -                       port_num, magic, bp,            \
> +                       flags, magic, bp,               \
>                         eax, ebx, ecx, edx, si, di)     \
>  ({                                                     \
>         asm volatile ("push %%rbp;"                     \
>                 "mov %12, %%rbp;"                       \
> -               "rep outsb;"                            \
> +               VMWARE_HYPERCALL_HB_OUT                 \
>                 "pop %%rbp;" :                          \
>                 "=a"(eax),                              \
>                 "=b"(ebx),                              \
> @@ -114,7 +115,7 @@
>                 "a"(magic),                             \
>                 "b"(cmd),                               \
>                 "c"(in_ecx),                            \
> -               "d"(port_num),                          \
> +               "d"(flags),                             \
>                 "S"(in_si),                             \
>                 "D"(in_di),                             \
>                 "r"(bp) :                               \
> @@ -123,12 +124,12 @@
>
>
>  #define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di,      \
> -                      port_num, magic, bp,             \
> +                      flags, magic, bp,                \
>                        eax, ebx, ecx, edx, si, di)      \
>  ({                                                     \
>         asm volatile ("push %%rbp;"                     \
>                 "mov %12, %%rbp;"                       \
> -               "rep insb;"                             \
> +               VMWARE_HYPERCALL_HB_IN                  \
>                 "pop %%rbp" :                           \
>                 "=a"(eax),                              \
>                 "=b"(ebx),                              \
> @@ -139,7 +140,7 @@
>                 "a"(magic),                             \
>                 "b"(cmd),                               \
>                 "c"(in_ecx),                            \
> -               "d"(port_num),                          \
> +               "d"(flags),                             \
>                 "S"(in_si),                             \
>                 "D"(in_di),                             \
>                 "r"(bp) :                               \
> @@ -157,13 +158,13 @@
>   * just pushed it.
>   */
>  #define VMW_PORT_HB_OUT(cmd, in_ecx, in_si, in_di,     \
> -                       port_num, magic, bp,            \
> +                       flags, magic, bp,               \
>                         eax, ebx, ecx, edx, si, di)     \
>  ({                                                     \
>         asm volatile ("push %12;"                       \
>                 "push %%ebp;"                           \
>                 "mov 0x04(%%esp), %%ebp;"               \
> -               "rep outsb;"                            \
> +               VMWARE_HYPERCALL_HB_OUT                 \
>                 "pop %%ebp;"                            \
>                 "add $0x04, %%esp;" :                   \
>                 "=a"(eax),                              \
> @@ -175,7 +176,7 @@
>                 "a"(magic),                             \
>                 "b"(cmd),                               \
>                 "c"(in_ecx),                            \
> -               "d"(port_num),                          \
> +               "d"(flags),                             \
>                 "S"(in_si),                             \
>                 "D"(in_di),                             \
>                 "m"(bp) :                               \
> @@ -184,13 +185,13 @@
>
>
>  #define VMW_PORT_HB_IN(cmd, in_ecx, in_si, in_di,      \
> -                      port_num, magic, bp,             \
> +                      flags, magic, bp,                \
>                        eax, ebx, ecx, edx, si, di)      \
>  ({                                                     \
>         asm volatile ("push %12;"                       \
>                 "push %%ebp;"                           \
>                 "mov 0x04(%%esp), %%ebp;"               \
> -               "rep insb;"                             \
> +               VMWARE_HYPERCALL_HB_IN                  \
>                 "pop %%ebp;"                            \
>                 "add $0x04, %%esp;" :                   \
>                 "=a"(eax),                              \
> @@ -202,7 +203,7 @@
>                 "a"(magic),                             \
>                 "b"(cmd),                               \
>                 "c"(in_ecx),                            \
> -               "d"(port_num),                          \
> +               "d"(flags),                             \
>                 "S"(in_si),                             \
>                 "D"(in_di),                             \
>                 "m"(bp) :                               \
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/4] Add support for updated vmware hypercall instruction

[Thomas Hellström (VMware)]

Thomas,

On 8/23/19 10:13 AM, Thomas Hellström (VMware) wrote:
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: <x86@kernel.org>
>
> VMware has started using "vmcall" / "vmmcall" instead of an inl instruction
> for the "backdoor" interface. This series detects support for those
> instructions.
> Outside of the platform code we use the "ALTERNATIVES" self-patching
> mechanism similarly to how this is done with KVM.
> Unfortunately we need two new x86 CPU feature flags for this, since we need
> the default instruction to be "inl". IIRC the vmmouse driver is used by
> other virtualization solutions than VMware, and those might break if
> they encounter any of the other instructions.
>
> v2:
> - Address various style review comments
> - Use mnemonics instead of bytecode in the ALTERNATIVE_2 macros
> - Use vmcall / vmmcall also for the High-Bandwidth port calls
> - Change the %edx argument to what vmcall / vmmcall expect (flags instead of
>    port number). The port number is added in the default ALTERNATIVE_2 path.
> - Ack to merge the vmmouse patch from Dmitry.
> - Drop license update for now. Will get back with a freestanding patch.
Are you OK taking the above series through the x86 tree? In addition to 
the ack from Dmitry to do this for the vmmouse patch, there's also an 
ack from Dave to do the same for vmwgfx.

Thanks,
Thomas

Re: [PATCH v2 1/4] x86/vmware: Update platform detection code for VMCALL/VMMCALL hypercalls

[Borislav Petkov]

On Fri, Aug 23, 2019 at 10:13:13AM +0200, Thomas Hellström (VMware) wrote:
> From: Thomas Hellstrom <thellstrom@vmware.com>
> 
> Vmware has historically used an "inl" instruction for this, but recent
> hardware versions support using VMCALL/VMMCALL instead, so use this method
> if supported at platform detection time. We explicitly need to code

Pls use passive voice in your commit message: no "we" or "I", etc, and
describe your changes in imperative mood.

> separate macro versions since the alternatives self-patching has not
> been performed at platform detection time.
> 
> Also put tighter constraints on the assembly input parameters.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: <x86@kernel.org>
> Cc: <dri-devel@lists.freedekstop.org>
> Co-developed-by: Doug Covelli <dcovelli@vmware.com>
> Signed-off-by: Doug Covelli <dcovelli@vmware.com>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> Reviewed-by: Doug Covelli <dcovelli@vmware.com>
> ---
>  arch/x86/kernel/cpu/vmware.c | 88 +++++++++++++++++++++++++++++-------
>  1 file changed, 71 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 3c648476d4fb..fcb84b1e099e 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -30,34 +30,70 @@
>  #include <asm/hypervisor.h>
>  #include <asm/timer.h>
>  #include <asm/apic.h>
> +#include <asm/svm.h>

That include is for?

>  #undef pr_fmt
>  #define pr_fmt(fmt)	"vmware: " fmt
>  
> -#define CPUID_VMWARE_INFO_LEAF	0x40000000
> +#define CPUID_VMWARE_INFO_LEAF               0x40000000
> +#define CPUID_VMWARE_FEATURES_LEAF           0x40000010
> +#define CPUID_VMWARE_FEATURES_ECX_VMMCALL    BIT(0)
> +#define CPUID_VMWARE_FEATURES_ECX_VMCALL     BIT(1)
> +
>  #define VMWARE_HYPERVISOR_MAGIC	0x564D5868
>  #define VMWARE_HYPERVISOR_PORT	0x5658
>  
> -#define VMWARE_PORT_CMD_GETVERSION	10
> -#define VMWARE_PORT_CMD_GETHZ		45
> -#define VMWARE_PORT_CMD_GETVCPU_INFO	68
> -#define VMWARE_PORT_CMD_LEGACY_X2APIC	3
> -#define VMWARE_PORT_CMD_VCPU_RESERVED	31
> +#define VMWARE_CMD_GETVERSION    10
> +#define VMWARE_CMD_GETHZ         45
> +#define VMWARE_CMD_GETVCPU_INFO  68
> +#define VMWARE_CMD_LEGACY_X2APIC  3
> +#define VMWARE_CMD_VCPU_RESERVED 31
>
>  #define VMWARE_PORT(cmd, eax, ebx, ecx, edx)				\
>  	__asm__("inl (%%dx)" :						\
> -			"=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :	\
> -			"0"(VMWARE_HYPERVISOR_MAGIC),			\
> -			"1"(VMWARE_PORT_CMD_##cmd),			\
> -			"2"(VMWARE_HYPERVISOR_PORT), "3"(UINT_MAX) :	\
> -			"memory");
> +		"=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :		\
> +		"a"(VMWARE_HYPERVISOR_MAGIC),				\
> +		"c"(VMWARE_CMD_##cmd),					\
> +		"d"(VMWARE_HYPERVISOR_PORT), "b"(UINT_MAX) :		\
> +		"memory")
> +
> +#define VMWARE_VMCALL(cmd, eax, ebx, ecx, edx)				\
> +	__asm__("vmcall" :						\
> +		"=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :		\
> +		"a"(VMWARE_HYPERVISOR_MAGIC),				\
> +		"c"(VMWARE_CMD_##cmd),					\
> +		"d"(0), "b"(UINT_MAX) :					\
> +		"memory")
> +
> +#define VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx)                         \
> +	__asm__("vmmcall" :						\
> +		"=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :		\
> +		"a"(VMWARE_HYPERVISOR_MAGIC),				\
> +		"c"(VMWARE_CMD_##cmd),					\
> +		"d"(0), "b"(UINT_MAX) :					\
> +		"memory")
> +
> +#define VMWARE_CMD(cmd, eax, ebx, ecx, edx) do {		\
> +	switch (vmware_hypercall_mode) {			\
> +	case CPUID_VMWARE_FEATURES_ECX_VMCALL:			\
> +		VMWARE_VMCALL(cmd, eax, ebx, ecx, edx);		\
> +		break;						\
> +	case CPUID_VMWARE_FEATURES_ECX_VMMCALL:			\
> +		VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx);	\
> +		break;						\
> +	default:						\

Please integrate scripts/checkpatch.pl into your patch creation
workflow. Some of the warnings/errors *actually* make sense:

WARNING: Possible switch case/default not preceded by break or fallthrough comment
#110: FILE: arch/x86/kernel/cpu/vmware.c:81:
+       case CPUID_VMWARE_FEATURES_ECX_VMMCALL:                 \

WARNING: Possible switch case/default not preceded by break or fallthrough comment
#113: FILE: arch/x86/kernel/cpu/vmware.c:84:
+       default:

In this case, we're going to enable -Wimplicit-fallthrough by default at
some point.

> +		VMWARE_PORT(cmd, eax, ebx, ecx, edx);		\
> +		break;						\
> +	}							\
> +	} while (0)
>  
>  static unsigned long vmware_tsc_khz __ro_after_init;
> +static u8 vmware_hypercall_mode     __ro_after_init;
>  
>  static inline int __vmware_platform(void)
>  {
>  	uint32_t eax, ebx, ecx, edx;
> -	VMWARE_PORT(GETVERSION, eax, ebx, ecx, edx);
> +	VMWARE_CMD(GETVERSION, eax, ebx, ecx, edx);
>  	return eax != (uint32_t)-1 && ebx == VMWARE_HYPERVISOR_MAGIC;
>  }
>  
> @@ -136,7 +172,7 @@ static void __init vmware_platform_setup(void)
>  	uint32_t eax, ebx, ecx, edx;
>  	uint64_t lpj, tsc_khz;
>  
> -	VMWARE_PORT(GETHZ, eax, ebx, ecx, edx);
> +	VMWARE_CMD(GETHZ, eax, ebx, ecx, edx);
>  
>  	if (ebx != UINT_MAX) {
>  		lpj = tsc_khz = eax | (((uint64_t)ebx) << 32);
> @@ -174,6 +210,16 @@ static void __init vmware_platform_setup(void)
>  	vmware_set_capabilities();
>  }
>  
> +static u8

No need for that linebreak here.

> +vmware_select_hypercall(void)
> +{
> +	int eax, ebx, ecx, edx;
> +
> +	cpuid(CPUID_VMWARE_FEATURES_LEAF, &eax, &ebx, &ecx, &edx);
> +	return (ecx & (CPUID_VMWARE_FEATURES_ECX_VMMCALL |
> +		       CPUID_VMWARE_FEATURES_ECX_VMCALL));
> +}
> +
>  /*
>   * While checking the dmi string information, just checking the product
>   * serial key should be enough, as this will always have a VMware
> @@ -187,8 +233,16 @@ static uint32_t __init vmware_platform(void)
>  
>  		cpuid(CPUID_VMWARE_INFO_LEAF, &eax, &hyper_vendor_id[0],
>  		      &hyper_vendor_id[1], &hyper_vendor_id[2]);
> -		if (!memcmp(hyper_vendor_id, "VMwareVMware", 12))
> +		if (!memcmp(hyper_vendor_id, "VMwareVMware", 12)) {
> +			if (eax >= CPUID_VMWARE_FEATURES_LEAF)
> +				vmware_hypercall_mode =
> +					vmware_select_hypercall();
> +
> +			pr_info("hypercall mode: 0x%02x\n",
> +				(unsigned int) vmware_hypercall_mode);

Is that supposed to be debug output? If so, pr_dbg().

> +
>  			return CPUID_VMWARE_INFO_LEAF;
> +		}
>  	} else if (dmi_available && dmi_name_in_serial("VMware") &&
>  		   __vmware_platform())

What sets vmware_hypercall_mode in this case? Or is the 0 magic to mean,
use the default: VMWARE_PORT inl call?

Also, you could restructure that function something like this to save yourself
an indentation level or two and make it more easily readable:

static uint32_t __init vmware_platform(void)
{
        unsigned int hyper_vendor_id[3];
        unsigned int eax;

        if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
                if (dmi_available && dmi_name_in_serial("VMware") && __vmware_platform())
                        return 1;
        }

        cpuid(CPUID_VMWARE_INFO_LEAF, &eax, &hyper_vendor_id[0],
              &hyper_vendor_id[1], &hyper_vendor_id[2]);

        if (!memcmp(hyper_vendor_id, "VMwareVMware", 12)) {
                if (eax >= CPUID_VMWARE_FEATURES_LEAF)
                        vmware_hypercall_mode = vmware_select_hypercall();

                pr_info("hypercall mode: 0x%02x\n", (unsigned int) vmware_hypercall_mode);

                return CPUID_VMWARE_INFO_LEAF;
        }
        return 0;
}

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions

[Borislav Petkov]

On Fri, Aug 23, 2019 at 10:13:14AM +0200, Thomas Hellström (VMware) wrote:
> From: Thomas Hellstrom <thellstrom@vmware.com>
> 
> The new header is intended to be used by drivers using the backdoor.
> Follow the kvm example using alternatives self-patching to
> choose between vmcall, vmmcall and io instructions.
> 
> Also define two new CPU feature flags to indicate hypervisor support
> for vmcall- and vmmcall instructions.

I could use some of the explanation why we need two feature flags added
here from:

https://lkml.kernel.org/r/970d2bb6-ab29-315f-f5d8-5d11095859af@shipmail.org

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions

[Borislav Petkov]

On Fri, Aug 23, 2019 at 10:13:14AM +0200, Thomas Hellström (VMware) wrote:
> +/*
> + * The high bandwidth out call. The low word of edx is presumed to have the
> + * HB and OUT bits set.
> + */
> +#define VMWARE_HYPERCALL_HB_OUT						\
> +	ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT_HB ", %%dx; rep outsb", \

Hmm, that looks fishy:

This call in vmw_port_hb_out(), for example, gets converted to the asm
below (I've left in the asm touching only rDX).

# drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:160:              VMW_PORT_HB_OUT(
#NO_APP
        movzwl  0(%rbp), %edx   # channel_20(D)->channel_id, channel_20(D)->channel_id

	...

        sall    $16, %edx       #, tmp172
        orl     $3, %edx        #, tmp173

this is adding channel_id and flags:

                        VMWARE_HYPERVISOR_HB | (channel->channel_id << 16) |
                        VMWARE_HYPERVISOR_OUT,

the $3 being (VMWARE_HYPERVISOR_HB | VMWARE_HYPERVISOR_OUT).

        movslq  %edx, %rdx      # tmp173, tmp174

Here it is sign-extending it.

#APP
# 160 "drivers/gpu/drm/vmwgfx/vmwgfx_msg.c" 1
        push %rbp;mov %r8, %rbp;# ALT: oldinstr2        # bp
661:
        movw $0x5659, %dx; rep outsb

And now here you're overwriting the low word of %edx. And now it
contains:

0x[channel_id]5659

and the low word doesn't contain the 3, i.e., (VMWARE_HYPERVISOR_HB |
VMWARE_HYPERVISOR_OUT) anymore. And that's before you do the hypercall
so I'm guessing that cannot be right.

Or?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions

[Thomas Hellström (VMware)]

On 8/27/19 5:44 PM, Borislav Petkov wrote:
> On Fri, Aug 23, 2019 at 10:13:14AM +0200, Thomas Hellström (VMware) wrote:
>> +/*
>> + * The high bandwidth out call. The low word of edx is presumed to have the
>> + * HB and OUT bits set.
>> + */
>> +#define VMWARE_HYPERCALL_HB_OUT						\
>> +	ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT_HB ", %%dx; rep outsb", \
> Hmm, that looks fishy:
>
> This call in vmw_port_hb_out(), for example, gets converted to the asm
> below (I've left in the asm touching only rDX).
>
> # drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:160:              VMW_PORT_HB_OUT(
> #NO_APP
>          movzwl  0(%rbp), %edx   # channel_20(D)->channel_id, channel_20(D)->channel_id
>
> 	...
>
>          sall    $16, %edx       #, tmp172
>          orl     $3, %edx        #, tmp173
>
> this is adding channel_id and flags:
>
>                          VMWARE_HYPERVISOR_HB | (channel->channel_id << 16) |
>                          VMWARE_HYPERVISOR_OUT,
>
> the $3 being (VMWARE_HYPERVISOR_HB | VMWARE_HYPERVISOR_OUT).
>
>          movslq  %edx, %rdx      # tmp173, tmp174
>
> Here it is sign-extending it.
>
> #APP
> # 160 "drivers/gpu/drm/vmwgfx/vmwgfx_msg.c" 1
>          push %rbp;mov %r8, %rbp;# ALT: oldinstr2        # bp
> 661:
>          movw $0x5659, %dx; rep outsb
>
> And now here you're overwriting the low word of %edx. And now it
> contains:
>
> 0x[channel_id]5659
>
> and the low word doesn't contain the 3, i.e., (VMWARE_HYPERVISOR_HB |
> VMWARE_HYPERVISOR_OUT) anymore. And that's before you do the hypercall
> so I'm guessing that cannot be right.
>
> Or?
>
It should be correct. The flags VMWARE_HYPERVISOR_HB and 
VMWARE_HYPERVISOR_OUT are only valid for the vmcall / vmmcall versions.

For the legacy version, the direction is toggled by the instruction (in 
vs out) and LB vs HB is toggled by the port number (0x5658 vs 0x5659)

So in essence the low word definition of %edx is different in the two 
versions. I've chosen to use the new vmcall/vmmcall definition in the 
driver code.

/Thomas

Re: [PATCH v2 1/4] x86/vmware: Update platform detection code for VMCALL/VMMCALL hypercalls

[Thomas Hellström (VMware)]

Thanks for reviewing, Borislav. Comments inline.

On 8/27/19 2:56 PM, Borislav Petkov wrote:

> On Fri, Aug 23, 2019 at 10:13:13AM +0200, Thomas Hellström (VMware) wrote:
>
>> +
>> +#define VMWARE_CMD(cmd, eax, ebx, ecx, edx) do {		\
>> +	switch (vmware_hypercall_mode) {			\
>> +	case CPUID_VMWARE_FEATURES_ECX_VMCALL:			\
>> +		VMWARE_VMCALL(cmd, eax, ebx, ecx, edx);		\
>> +		break;						\
>> +	case CPUID_VMWARE_FEATURES_ECX_VMMCALL:			\
>> +		VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx);	\
>> +		break;						\
>> +	default:						\
> Please integrate scripts/checkpatch.pl into your patch creation
> workflow. Some of the warnings/errors *actually* make sense:
>
> WARNING: Possible switch case/default not preceded by break or fallthrough comment
> #110: FILE: arch/x86/kernel/cpu/vmware.c:81:
> +       case CPUID_VMWARE_FEATURES_ECX_VMMCALL:                 \
>
> WARNING: Possible switch case/default not preceded by break or fallthrough comment
> #113: FILE: arch/x86/kernel/cpu/vmware.c:84:
> +       default:
>
> In this case, we're going to enable -Wimplicit-fallthrough by default at
> some point.

We *do* have checkpatch.pl in the workflow. In this case I figured the 
warnings actually didn't make sense. There are breaks present and 
-Wimplicit-fallthrough doesn't complain...


	(unsigned int) vmware_hypercall_mode);

> Is that supposed to be debug output? If so, pr_dbg().

This is intentionally intended to be part of the initial output.

>
>> +
>>   			return CPUID_VMWARE_INFO_LEAF;
>> +		}
>>   	} else if (dmi_available && dmi_name_in_serial("VMware") &&
>>   		   __vmware_platform())
> What sets vmware_hypercall_mode in this case? Or is the 0 magic to mean,
> use the default: VMWARE_PORT inl call?

Yes, Perhaps I should add a comment about that.

>
> Also, you could restructure that function something like this to save yourself
> an indentation level or two and make it more easily readable:
>
> static uint32_t __init vmware_platform(void)
> {
>          unsigned int hyper_vendor_id[3];
>          unsigned int eax;
>
>          if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
>                  if (dmi_available && dmi_name_in_serial("VMware") && __vmware_platform())
>                          return 1;
>          }
>
>          cpuid(CPUID_VMWARE_INFO_LEAF, &eax, &hyper_vendor_id[0],
>                &hyper_vendor_id[1], &hyper_vendor_id[2]);
>
>          if (!memcmp(hyper_vendor_id, "VMwareVMware", 12)) {
>                  if (eax >= CPUID_VMWARE_FEATURES_LEAF)
>                          vmware_hypercall_mode = vmware_select_hypercall();
>
>                  pr_info("hypercall mode: 0x%02x\n", (unsigned int) vmware_hypercall_mode);
>
>                  return CPUID_VMWARE_INFO_LEAF;
>          }
>          return 0;
> }
>
Sure, I'll add that as a separate patch.

For the other comments, I'll fix up and respin.

Thanks,

Thomas

Re: [PATCH v2 1/4] x86/vmware: Update platform detection code for VMCALL/VMMCALL hypercalls

[Borislav Petkov]

On Tue, Aug 27, 2019 at 09:19:54PM +0200, Thomas Hellström (VMware) wrote:
> We *do* have checkpatch.pl in the workflow. In this case I figured the
> warnings actually didn't make sense. There are breaks present and
> -Wimplicit-fallthrough doesn't complain...

Oh, we have enabled that by default already:

  a035d552a93b ("Makefile: Globally enable fall-through warning")

I guess checkpatch wrongly complains here and I did trust its output
blindly. That ain't happening again, sorry about that.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions

[Borislav Petkov]

On Tue, Aug 27, 2019 at 09:19:03PM +0200, Thomas Hellström (VMware) wrote:
> It should be correct. The flags VMWARE_HYPERVISOR_HB and
> VMWARE_HYPERVISOR_OUT are only valid for the vmcall / vmmcall versions.
> 
> For the legacy version, the direction is toggled by the instruction (in vs
> out) and LB vs HB is toggled by the port number (0x5658 vs 0x5659)
> 
> So in essence the low word definition of %edx is different in the two
> versions. I've chosen to use the new vmcall/vmmcall definition in the driver
> code.

Ah, ok, I see what you mean. The old method would overwrite the low word
of %edx but the new one would have the flags already prepared and *not*
overwrite them so all good.

Can you please document that more explicitly in the comment in
arch/x86/include/asm/vmware.h? Something like:

"... The new vmcall interface instead uses a set of flags to select
bandwidth mode and transfer direction. The set of flags is already
loaded into %edx by the macros which use VMWARE_HYPERCALL* and only when
the guest must use the old VMWARE_HYPERVISOR_PORT* method, the low word
is overwritten by the respective port number."

Anyway, something along those lines. We want to have the alternatives
code as clear and as transparent as possible because, well, of obvious
reasons. :-)

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v2 1/4] x86/vmware: Update platform detection code for VMCALL/VMMCALL hypercalls

[Thomas Hellström (VMware)]

On 8/27/19 2:56 PM, Borislav Petkov wrote:
>
> Also, you could restructure that function something like this to save yourself
> an indentation level or two and make it more easily readable:
>
> static uint32_t __init vmware_platform(void)
> {
>          unsigned int hyper_vendor_id[3];
>          unsigned int eax;
>
>          if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
>                  if (dmi_available && dmi_name_in_serial("VMware") && __vmware_platform())
>                          return 1;


Hmm, we're missing a return 0; here. The number of return points and the 
moved up variable declarations worries me a little. I'll see if I can 
clean this up a bit, but would prefer to make a follow-up patch in that 
case.

/Thomas


>          }
>
>          cpuid(CPUID_VMWARE_INFO_LEAF, &eax, &hyper_vendor_id[0],
>                &hyper_vendor_id[1], &hyper_vendor_id[2]);
>
>          if (!memcmp(hyper_vendor_id, "VMwareVMware", 12)) {
>                  if (eax >= CPUID_VMWARE_FEATURES_LEAF)
>                          vmware_hypercall_mode = vmware_select_hypercall();
>
>                  pr_info("hypercall mode: 0x%02x\n", (unsigned int) vmware_hypercall_mode);
>
>                  return CPUID_VMWARE_INFO_LEAF;
>          }
>          return 0;
> }
>

Re: [PATCH v2 1/4] x86/vmware: Update platform detection code for VMCALL/VMMCALL hypercalls

[Borislav Petkov]

On Wed, Aug 28, 2019 at 09:45:19AM +0200, Thomas Hellström (VMware) wrote:
> Hmm, we're missing a return 0; here. The number of return points and the
> moved up variable declarations worries me a little. I'll see if I can clean
> this up a bit, but would prefer to make a follow-up patch in that case.

By all means.

It was just a suggestion anyway - see if you prefer this better or would
like to keep the old style.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.