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

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

[Dexuan Cui]

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 <akataria@vmware.com>
Cc: Andy King <acking@vmware.com>
Cc: Adit Ranadive <aditr@vmware.com>
Cc: George Zhang <georgezhang@vmware.com>
Cc: Jorgen Hansen <jhansen@vmware.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
---
 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

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

[David Miller]

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

> +	/*
> +	 * 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.

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

[Dexuan Cui]

> 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
> 
> > +	/*
> > +	 * 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

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

[David Miller]

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.

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

[Jorgen S. Hansen]

> On Aug 16, 2017, at 12:13 AM, Dexuan Cui <decui@microsoft.com> 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

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

[Dexuan Cui]

> From: Jorgen S. Hansen [mailto:jhansen@vmware.com]
> > 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

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

[Dexuan Cui]

> From: Dexuan Cui
> Sent: Wednesday, August 16, 2017 15:34
> > From: Jorgen S. Hansen [mailto:jhansen@vmware.com]
> > > 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

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