[PATCH net-next 1/3] VMCI: only load on VMware hypervisor

Without the patch, vmw_vsock_vmci_transport.ko and vmw_vmci.ko can
automatically load when an application creates an AF_VSOCK socket.

This is the expected good behavior on VMware hypervisor, but as we
are going to add hv_sock.ko (i.e. Hyper-V transport for AF_VSOCK), we
should make sure vmw_vsock_vmci_transport.ko doesn't load on Hyper-V,
otherwise there is a -EBUSY conflict when both vmw_vsock_vmci_transport.ko
and hv_sock.ko try to call vsock_core_init() on Hyper-V.

On the other hand, hv_sock.ko can only load on Hyper-V, because it
depends on hv_vmbus.ko, which detects Hyper-V in hv_acpi_init().

KVM's vsock_virtio_transport doesn't have the issue because it doesn't
define MODULE_ALIAS_NETPROTO(PF_VSOCK).

Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: Alok Kataria <redacted>
Cc: Andy King <redacted>
Cc: Adit Ranadive <redacted>
Cc: George Zhang <redacted>
Cc: Jorgen Hansen <redacted>
Cc: K. Y. Srinivasan <redacted>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <redacted>
---
 drivers/misc/vmw_vmci/vmci_driver.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/misc/vmw_vmci/vmci_driver.c b/drivers/misc/vmw_vmci/vmci_driver.c
index d7eaf1e..1789ea7 100644
--- a/drivers/misc/vmw_vmci/vmci_driver.c
+++ b/drivers/misc/vmw_vmci/vmci_driver.c

@@ -19,6 +19,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/hypervisor.h>
 
 #include "vmci_driver.h"
 #include "vmci_event.h"

@@ -58,6 +59,13 @@ static int __init vmci_drv_init(void)
 	int vmci_err;
 	int error;
 
+	/*
+	 * Check if we are running on VMware's hypervisor and bail out
+	 * if we are not.
+	 */
+	if (x86_hyper != &x86_hyper_vmware)
+		return -ENODEV;
+
 	vmci_err = vmci_event_init();
 	if (vmci_err < VMCI_SUCCESS) {
 		pr_err("Failed to initialize VMCIEvent (result=%d)\n",

-- 
2.7.4

From: Dexuan Cui <decui@microsoft.com>
Date: Tue, 15 Aug 2017 22:13:29 +0000

This symbol is only available when CONFIG_HYPERVISOR_GUEST is defined.
But this driver does not have a Kconfig dependency on that symbol so
the build can fail in some configurations.

From: David Miller [mailto:davem@davemloft.net]
Sent: Wednesday, August 16, 2017 11:07

From: Dexuan Cui <decui@microsoft.com>
Date: Tue, 15 Aug 2017 22:13:29 +0000

quoted

+	/*
+	 * Check if we are running on VMware's hypervisor and bail out
+	 * if we are not.
+	 */
+	if (x86_hyper != &x86_hyper_vmware)
+		return -ENODEV;

This symbol is only available when CONFIG_HYPERVISOR_GUEST is defined.
But this driver does not have a Kconfig dependency on that symbol so
the build can fail in some configurations.

Hi David,
It looks typically modern Linux distros have CONFIG_HYPERVISOR_GUEST=y
by default, but I agree here we should make the dependency explicit:

--- a/drivers/misc/vmw_vmci/Kconfig
+++ b/drivers/misc/vmw_vmci/Kconfig

@@ -4,7 +4,7 @@

 config VMWARE_VMCI
        tristate "VMware VMCI Driver"
-       depends on X86 && PCI
+       depends on X86 && PCI && HYPERVISOR_GUEST
        help
          This is VMware's Virtual Machine Communication Interface.  It enables
          high-speed communication between host and guest in a virtual

And it looks it's not a bad thing to add the dependency, because some
existing VMWare drivers have had the dependency on CONFIG_HYPERVISOR_GUEST=y:
drivers/input/mouse/vmmouse.c (MOUSE_PS2_VMMOUSE)
drivers/misc/vmw_balloon.c (VMWARE_BALLOON)

Do you want me to submit a v2 for this patch with the Kconfig change?

-- Dexuan

From: Dexuan Cui <decui@microsoft.com>
Date: Wed, 16 Aug 2017 18:51:36 +0000

It looks typically modern Linux distros have CONFIG_HYPERVISOR_GUEST=y
by default

It doesn't matter what any distribution does or does not do.

People are going to do 'randconfig' builds over thousands and
thousands of configuration combinations to test your changes,
and those tests will fail.

Do you want me to submit a v2 for this patch with the Kconfig change?

Of course, there is no way I can apply your series as-is.

On Aug 16, 2017, at 12:13 AM, Dexuan Cui [off-list ref] wrote:


Without the patch, vmw_vsock_vmci_transport.ko and vmw_vmci.ko can
automatically load when an application creates an AF_VSOCK socket.

This is the expected good behavior on VMware hypervisor, but as we
are going to add hv_sock.ko (i.e. Hyper-V transport for AF_VSOCK), we
should make sure vmw_vsock_vmci_transport.ko doesn't load on Hyper-V,
otherwise there is a -EBUSY conflict when both vmw_vsock_vmci_transport.ko
and hv_sock.ko try to call vsock_core_init() on Hyper-V.

The VMCI driver (vmw_vmci.ko) is used both by the VMware guest support (VMware Tools primarily) and by our Workstation product. Always disabling the VMCI driver on Hyper-V means that user won’t be able to run Workstation nested in Linux VMs on Hyper-V. Since the VMCI driver itself isn’t the problem here, maybe we could move the check to vmw_vsock_vmci_transport.ko? Ideally, there should be some way for a user to have access to both protocols, but for now disabling the VMCI socket transport for Hyper-V (possibly with a module option to skip that check and always load it) but leaving the VMCI driver functional would be better,

Thanks,
Jorgen

From: Jorgen S. Hansen [mailto:jhansen@vmware.com]

quoted

Without the patch, vmw_vsock_vmci_transport.ko and vmw_vmci.ko can
automatically load when an application creates an AF_VSOCK socket.

This is the expected good behavior on VMware hypervisor, but as we
are going to add hv_sock.ko (i.e. Hyper-V transport for AF_VSOCK), we
should make sure vmw_vsock_vmci_transport.ko doesn't load on Hyper-V,
otherwise there is a -EBUSY conflict when both vmw_vsock_vmci_transport.ko
and hv_sock.ko try to call vsock_core_init() on Hyper-V.

The VMCI driver (vmw_vmci.ko) is used both by the VMware guest support
(VMware Tools primarily) and by our Workstation product. Always disabling the
VMCI driver on Hyper-V means that user won’t be able to run Workstation
nested in Linux VMs on Hyper-V. Since the VMCI driver itself isn’t the problem
here, maybe we could move the check to vmw_vsock_vmci_transport.ko?
Ideally, there should be some way for a user to have access to both protocols,
but for now disabling the VMCI socket transport for Hyper-V (possibly with a
module option to skip that check and always load it) but leaving the VMCI driver
functional would be better,

Jorgen

Thank you for explaining the background!
Then I'll make a new patch, following your suggestion.

-- Dexuan

From: Dexuan Cui
Sent: Wednesday, August 16, 2017 15:34

quoted

From: Jorgen S. Hansen [mailto:jhansen@vmware.com]

quoted

Without the patch, vmw_vsock_vmci_transport.ko and vmw_vmci.ko can
automatically load when an application creates an AF_VSOCK socket.

This is the expected good behavior on VMware hypervisor, but as we
are going to add hv_sock.ko (i.e. Hyper-V transport for AF_VSOCK), we
should make sure vmw_vsock_vmci_transport.ko doesn't load on Hyper-

V,

quoted

quoted

otherwise there is a -EBUSY conflict when both

vmw_vsock_vmci_transport.ko

quoted

quoted

and hv_sock.ko try to call vsock_core_init() on Hyper-V.

The VMCI driver (vmw_vmci.ko) is used both by the VMware guest support
(VMware Tools primarily) and by our Workstation product. Always

disabling the

quoted

VMCI driver on Hyper-V means that user won’t be able to run Workstation
nested in Linux VMs on Hyper-V. Since the VMCI driver itself isn’t the

problem

quoted

here, maybe we could move the check to vmw_vsock_vmci_transport.ko?
Ideally, there should be some way for a user to have access to both

protocols,

quoted

but for now disabling the VMCI socket transport for Hyper-V (possibly with

a

quoted

module option to skip that check and always load it) but leaving the VMCI

driver

quoted

functional would be better,

Jorgen

Thank you for explaining the background!
Then I'll make a new patch, following your suggestion.

-- Dexuan

Hi Jorgen, David,

Just now I posted a new patch
 "[PATCH] vsock: only load vmci transport on VMware hypervisor by default"
to replace this patch.

@Jorgen: 
FWIW, with the new patch, when I create an AF_VSOCK sockets on Hyper-V,
vmw_vmci.ko is also automatically loaded and 3 lines of kernel messages are
printed, but I think I'm OK with this, since it's harmless.

-- Dexuan
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel