* [PATCH 1/6] x86: Add VMWare Host Communication Macros
@ 2015-12-01 22:18 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, Xavier Deguillard, linux-kernel, virtualization,
Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Alok Kataria
These macros will be used by multiple VMWare modules for handling
host communication.
v2:
* Keeping only the minimal common platform defines
* added vmware_platform() check function
v3:
* Added new field to handle different hypervisor magic values
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Alok Kataria <akataria@vmware.com>
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: Xavier Deguillard <xdeguillard@vmware.com>
---
arch/x86/include/asm/vmware.h | 110 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 110 insertions(+)
create mode 100644 arch/x86/include/asm/vmware.h
diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
new file mode 100644
index 0000000..32bf99b
--- /dev/null
+++ b/arch/x86/include/asm/vmware.h
@@ -0,0 +1,110 @@
+/*
+ * Copyright (C) 2015, VMware, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT. See the GNU General Public License for more
+ * details.
+ *
+ * Based on code from vmware.c and vmmouse.c.
+ * Author:
+ * Sinclair Yeh <syeh@vmware.com>
+ */
+#ifndef _ASM_X86_VMWARE_H
+#define _ASM_X86_VMWARE_H
+
+
+/**
+ * Hypervisor-specific bi-directional communication channel. Should never
+ * execute on bare metal hardware. The caller must make sure to check for
+ * supported hypervisor before using these macros.
+ *
+ * Several of the parameters are both input and output and must be initialized.
+ *
+ * @in1: [IN] Message Len or Message Cmd (HB)
+ * @in2: [IN] Message Len (HB) or Message Cmd
+ * @port_num: [IN] port number + [channel id]
+ * @magic: [IN] hypervisor magic value
+ * @eax: [OUT] value of EAX register
+ * @ebx: [OUT] e.g. status from an HB message status command
+ * @ecx: [OUT] e.g. status from a non-HB message status command
+ * @edx: [OUT] e.g. channel id
+ * @si: [INOUT] set to 0 if not used
+ * @di: [INOUT] set to 0 if not used
+ * @bp: [INOUT] set to 0 if not used
+ */
+#define VMW_PORT(in1, in2, port_num, magic, eax, ebx, ecx, edx, si, di) \
+({ \
+ __asm__ __volatile__ ("inl %%dx" : \
+ "=a"(eax), \
+ "=b"(ebx), \
+ "=c"(ecx), \
+ "=d"(edx), \
+ "=S"(si), \
+ "=D"(di) : \
+ "a"(magic), \
+ "b"(in1), \
+ "c"(in2), \
+ "d"(port_num), \
+ "S"(si), \
+ "D"(di) : \
+ "memory"); \
+})
+
+
+#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \
+ eax, ebx, ecx, edx, si, di, bp) \
+({ \
+ __asm__ __volatile__ ("movq %13, %%rbp;" \
+ "cld; rep outsb; " \
+ "movq %%rbp, %6" : \
+ "=a"(eax), \
+ "=b"(ebx), \
+ "=c"(ecx), \
+ "=d"(edx), \
+ "=S"(si), \
+ "=D"(di), \
+ "=r"(bp) : \
+ "a"(magic), \
+ "b"(in1), \
+ "c"(in2), \
+ "d"(port_num), \
+ "S"(si), \
+ "D"(di), \
+ "r"(bp) : \
+ "memory", "cc"); \
+})
+
+
+#define VMW_PORT_HB_IN(in1, in2, port_num, magic, \
+ eax, ebx, ecx, edx, si, di, bp) \
+({ \
+ __asm__ __volatile__ ("push %%rbp; movq %13, %%rbp;" \
+ "cld; rep insb; " \
+ "movq %%rbp, %6;pop %%rbp" : \
+ "=a"(eax), \
+ "=b"(ebx), \
+ "=c"(ecx), \
+ "=d"(edx), \
+ "=S"(si), \
+ "=D"(di), \
+ "=r"(bp) : \
+ "a"(magic), \
+ "b"(in1), \
+ "c"(in2), \
+ "d"(port_num), \
+ "S"(si), \
+ "D"(di), \
+ "r"(bp) : \
+ "memory", "cc"); \
+})
+
+
+#endif
+
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* [PATCH 2/6] x86: Update vmware.c to use the common VMW_PORT macros
2015-12-01 22:18 ` Sinclair Yeh
@ 2015-12-01 22:18 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
pv-drivers, virtualization, linux-kernel
v2:
Instead of replacing all existing instances of VMWARE_PORT
with VMW_PORT, update VMWARE_PORT to use the new VMW_PORT.
v3:
Using updated VMWARE_PORT() macro, which needs hypervisor magic in the
parameter
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: pv-drivers@vmware.com
Cc: virtualization@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
---
arch/x86/kernel/cpu/vmware.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 628a059..1837f66 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -26,6 +26,7 @@
#include <asm/div64.h>
#include <asm/x86_init.h>
#include <asm/hypervisor.h>
+#include <asm/vmware.h>
#define CPUID_VMWARE_INFO_LEAF 0x40000000
#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
@@ -37,13 +38,14 @@
#define VMWARE_PORT_CMD_LEGACY_X2APIC 3
#define VMWARE_PORT_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");
+#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
+({ \
+ unsigned long __si = 0, __di = 0; \
+ VMW_PORT(UINT_MAX, VMWARE_PORT_CMD_##cmd, VMWARE_HYPERVISOR_PORT, \
+ VMWARE_HYPERVISOR_MAGIC, \
+ eax, ebx, ecx, edx, __si, __di); \
+})
+
static inline int __vmware_platform(void)
{
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* [PATCH 2/6] x86: Update vmware.c to use the common VMW_PORT macros
@ 2015-12-01 22:18 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, pv-drivers, linux-kernel, virtualization,
Ingo Molnar, H. Peter Anvin, Thomas Gleixner
v2:
Instead of replacing all existing instances of VMWARE_PORT
with VMW_PORT, update VMWARE_PORT to use the new VMW_PORT.
v3:
Using updated VMWARE_PORT() macro, which needs hypervisor magic in the
parameter
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: pv-drivers@vmware.com
Cc: virtualization@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
---
arch/x86/kernel/cpu/vmware.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 628a059..1837f66 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -26,6 +26,7 @@
#include <asm/div64.h>
#include <asm/x86_init.h>
#include <asm/hypervisor.h>
+#include <asm/vmware.h>
#define CPUID_VMWARE_INFO_LEAF 0x40000000
#define VMWARE_HYPERVISOR_MAGIC 0x564D5868
@@ -37,13 +38,14 @@
#define VMWARE_PORT_CMD_LEGACY_X2APIC 3
#define VMWARE_PORT_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");
+#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
+({ \
+ unsigned long __si = 0, __di = 0; \
+ VMW_PORT(UINT_MAX, VMWARE_PORT_CMD_##cmd, VMWARE_HYPERVISOR_PORT, \
+ VMWARE_HYPERVISOR_MAGIC, \
+ eax, ebx, ecx, edx, __si, __di); \
+})
+
static inline int __vmware_platform(void)
{
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:18 ` Sinclair Yeh
@ 2015-12-01 22:18 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, pv-drivers, linux-graphics-maintainer,
Dmitry Torokhov, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
virtualization, linux-input
v2:
Instead of replacing existing VMMOUSE defines, only modify enough
to use the new VMW_PORT define.
v3:
Use updated VMWARE_PORT() which requires hypervisor magic as an added
parameter
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: pv-drivers@vmware.com
Cc: linux-graphics-maintainer@vmware.com
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: linux-input@vger.kernel.org
---
drivers/input/mouse/vmmouse.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index e272f06..d34e3e4 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -19,6 +19,7 @@
#include <linux/slab.h>
#include <linux/module.h>
#include <asm/hypervisor.h>
+#include <asm/vmware.h>
#include "psmouse.h"
#include "vmmouse.h"
@@ -84,21 +85,12 @@ struct vmmouse_data {
* implementing the vmmouse protocol. Should never execute on
* bare metal hardware.
*/
-#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
-({ \
- unsigned long __dummy1, __dummy2; \
- __asm__ __volatile__ ("inl %%dx" : \
- "=a"(out1), \
- "=b"(out2), \
- "=c"(out3), \
- "=d"(out4), \
- "=S"(__dummy1), \
- "=D"(__dummy2) : \
- "a"(VMMOUSE_PROTO_MAGIC), \
- "b"(in1), \
- "c"(VMMOUSE_PROTO_CMD_##cmd), \
- "d"(VMMOUSE_PROTO_PORT) : \
- "memory"); \
+#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
+({ \
+ unsigned long __dummy1 = 0, __dummy2 = 0; \
+ VMW_PORT(in1, VMMOUSE_PROTO_CMD_##cmd, VMMOUSE_PROTO_PORT, \
+ VMMOUSE_PROTO_MAGIC, \
+ out1, out2, out3, out4, __dummy1, __dummy2); \
})
/**
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-01 22:18 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: Arnd Bergmann, Sinclair Yeh, pv-drivers, Greg Kroah-Hartman,
Dmitry Torokhov, linux-kernel, virtualization,
linux-graphics-maintainer, linux-input
v2:
Instead of replacing existing VMMOUSE defines, only modify enough
to use the new VMW_PORT define.
v3:
Use updated VMWARE_PORT() which requires hypervisor magic as an added
parameter
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: pv-drivers@vmware.com
Cc: linux-graphics-maintainer@vmware.com
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: linux-input@vger.kernel.org
---
drivers/input/mouse/vmmouse.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index e272f06..d34e3e4 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -19,6 +19,7 @@
#include <linux/slab.h>
#include <linux/module.h>
#include <asm/hypervisor.h>
+#include <asm/vmware.h>
#include "psmouse.h"
#include "vmmouse.h"
@@ -84,21 +85,12 @@ struct vmmouse_data {
* implementing the vmmouse protocol. Should never execute on
* bare metal hardware.
*/
-#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
-({ \
- unsigned long __dummy1, __dummy2; \
- __asm__ __volatile__ ("inl %%dx" : \
- "=a"(out1), \
- "=b"(out2), \
- "=c"(out3), \
- "=d"(out4), \
- "=S"(__dummy1), \
- "=D"(__dummy2) : \
- "a"(VMMOUSE_PROTO_MAGIC), \
- "b"(in1), \
- "c"(VMMOUSE_PROTO_CMD_##cmd), \
- "d"(VMMOUSE_PROTO_PORT) : \
- "memory"); \
+#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
+({ \
+ unsigned long __dummy1 = 0, __dummy2 = 0; \
+ VMW_PORT(in1, VMMOUSE_PROTO_CMD_##cmd, VMMOUSE_PROTO_PORT, \
+ VMMOUSE_PROTO_MAGIC, \
+ out1, out2, out3, out4, __dummy1, __dummy2); \
})
/**
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:18 ` Sinclair Yeh
(?)
@ 2015-12-01 22:24 ` Dmitry Torokhov
2015-12-01 22:32 ` Sinclair Yeh
2015-12-01 22:32 ` Sinclair Yeh
-1 siblings, 2 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 22:24 UTC (permalink / raw)
To: Sinclair Yeh
Cc: x86, pv-drivers, linux-graphics-maintainer, Arnd Bergmann,
Greg Kroah-Hartman, linux-kernel, virtualization, linux-input
On Tue, Dec 01, 2015 at 02:18:49PM -0800, Sinclair Yeh wrote:
> v2:
> Instead of replacing existing VMMOUSE defines, only modify enough
> to use the new VMW_PORT define.
>
> v3:
> Use updated VMWARE_PORT() which requires hypervisor magic as an added
> parameter
>
> Signed-off-by: Sinclair Yeh <syeh@vmware.com>
> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> Reviewed-by: Alok N Kataria <akataria@vmware.com>
> Cc: pv-drivers@vmware.com
> Cc: linux-graphics-maintainer@vmware.com
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: linux-input@vger.kernel.org
> ---
> drivers/input/mouse/vmmouse.c | 22 +++++++---------------
> 1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
> index e272f06..d34e3e4 100644
> --- a/drivers/input/mouse/vmmouse.c
> +++ b/drivers/input/mouse/vmmouse.c
> @@ -19,6 +19,7 @@
> #include <linux/slab.h>
> #include <linux/module.h>
> #include <asm/hypervisor.h>
> +#include <asm/vmware.h>
>
> #include "psmouse.h"
> #include "vmmouse.h"
> @@ -84,21 +85,12 @@ struct vmmouse_data {
> * implementing the vmmouse protocol. Should never execute on
> * bare metal hardware.
> */
> -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> -({ \
> - unsigned long __dummy1, __dummy2; \
> - __asm__ __volatile__ ("inl %%dx" : \
> - "=a"(out1), \
> - "=b"(out2), \
> - "=c"(out3), \
> - "=d"(out4), \
> - "=S"(__dummy1), \
> - "=D"(__dummy2) : \
> - "a"(VMMOUSE_PROTO_MAGIC), \
> - "b"(in1), \
> - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> - "d"(VMMOUSE_PROTO_PORT) : \
> - "memory"); \
> +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> +({ \
> + unsigned long __dummy1 = 0, __dummy2 = 0; \
Why do we need to initialize dummies?
> + VMW_PORT(in1, VMMOUSE_PROTO_CMD_##cmd, VMMOUSE_PROTO_PORT, \
> + VMMOUSE_PROTO_MAGIC, \
> + out1, out2, out3, out4, __dummy1, __dummy2); \
> })
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:24 ` Dmitry Torokhov
@ 2015-12-01 22:32 ` Sinclair Yeh
2015-12-01 22:45 ` Dmitry Torokhov
2015-12-01 22:45 ` Dmitry Torokhov
2015-12-01 22:32 ` Sinclair Yeh
1 sibling, 2 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: x86, pv-drivers, linux-graphics-maintainer, Arnd Bergmann,
Greg Kroah-Hartman, linux-kernel, virtualization, linux-input
Hi,
On Tue, Dec 01, 2015 at 02:24:14PM -0800, Dmitry Torokhov wrote:
> On Tue, Dec 01, 2015 at 02:18:49PM -0800, Sinclair Yeh wrote:
> > v2:
> > Instead of replacing existing VMMOUSE defines, only modify enough
> > to use the new VMW_PORT define.
> >
> > v3:
> > Use updated VMWARE_PORT() which requires hypervisor magic as an added
> > parameter
> >
> > Signed-off-by: Sinclair Yeh <syeh@vmware.com>
> > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> > Reviewed-by: Alok N Kataria <akataria@vmware.com>
> > Cc: pv-drivers@vmware.com
> > Cc: linux-graphics-maintainer@vmware.com
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: linux-kernel@vger.kernel.org
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: linux-input@vger.kernel.org
> > ---
> > drivers/input/mouse/vmmouse.c | 22 +++++++---------------
> > 1 file changed, 7 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
> > index e272f06..d34e3e4 100644
> > --- a/drivers/input/mouse/vmmouse.c
> > +++ b/drivers/input/mouse/vmmouse.c
> > @@ -19,6 +19,7 @@
> > #include <linux/slab.h>
> > #include <linux/module.h>
> > #include <asm/hypervisor.h>
> > +#include <asm/vmware.h>
> >
> > #include "psmouse.h"
> > #include "vmmouse.h"
> > @@ -84,21 +85,12 @@ struct vmmouse_data {
> > * implementing the vmmouse protocol. Should never execute on
> > * bare metal hardware.
> > */
> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > -({ \
> > - unsigned long __dummy1, __dummy2; \
> > - __asm__ __volatile__ ("inl %%dx" : \
> > - "=a"(out1), \
> > - "=b"(out2), \
> > - "=c"(out3), \
> > - "=d"(out4), \
> > - "=S"(__dummy1), \
> > - "=D"(__dummy2) : \
> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > - "b"(in1), \
> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > - "d"(VMMOUSE_PROTO_PORT) : \
> > - "memory"); \
> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > +({ \
> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
>
> Why do we need to initialize dummies?
Because for some commands those parameters to VMW_PORT() can be both
input and outout. So it's safer to initialize them to 0. Since
they can potentially be an output, we can't make them a constant.
>
> > + VMW_PORT(in1, VMMOUSE_PROTO_CMD_##cmd, VMMOUSE_PROTO_PORT, \
> > + VMMOUSE_PROTO_MAGIC, \
> > + out1, out2, out3, out4, __dummy1, __dummy2); \
> > })
> >
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:32 ` Sinclair Yeh
@ 2015-12-01 22:45 ` Dmitry Torokhov
2015-12-01 22:45 ` Dmitry Torokhov
1 sibling, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 22:45 UTC (permalink / raw)
To: Sinclair Yeh
Cc: Arnd Bergmann, pv-drivers, Greg Kroah-Hartman, X86 ML, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:24:14PM -0800, Dmitry Torokhov wrote:
>> On Tue, Dec 01, 2015 at 02:18:49PM -0800, Sinclair Yeh wrote:
>> > v2:
>> > Instead of replacing existing VMMOUSE defines, only modify enough
>> > to use the new VMW_PORT define.
>> >
>> > v3:
>> > Use updated VMWARE_PORT() which requires hypervisor magic as an added
>> > parameter
>> >
>> > Signed-off-by: Sinclair Yeh <syeh@vmware.com>
>> > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
>> > Reviewed-by: Alok N Kataria <akataria@vmware.com>
>> > Cc: pv-drivers@vmware.com
>> > Cc: linux-graphics-maintainer@vmware.com
>> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > Cc: Arnd Bergmann <arnd@arndb.de>
>> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > Cc: linux-kernel@vger.kernel.org
>> > Cc: virtualization@lists.linux-foundation.org
>> > Cc: linux-input@vger.kernel.org
>> > ---
>> > drivers/input/mouse/vmmouse.c | 22 +++++++---------------
>> > 1 file changed, 7 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
>> > index e272f06..d34e3e4 100644
>> > --- a/drivers/input/mouse/vmmouse.c
>> > +++ b/drivers/input/mouse/vmmouse.c
>> > @@ -19,6 +19,7 @@
>> > #include <linux/slab.h>
>> > #include <linux/module.h>
>> > #include <asm/hypervisor.h>
>> > +#include <asm/vmware.h>
>> >
>> > #include "psmouse.h"
>> > #include "vmmouse.h"
>> > @@ -84,21 +85,12 @@ struct vmmouse_data {
>> > * implementing the vmmouse protocol. Should never execute on
>> > * bare metal hardware.
>> > */
>> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> > -({ \
>> > - unsigned long __dummy1, __dummy2; \
>> > - __asm__ __volatile__ ("inl %%dx" : \
>> > - "=a"(out1), \
>> > - "=b"(out2), \
>> > - "=c"(out3), \
>> > - "=d"(out4), \
>> > - "=S"(__dummy1), \
>> > - "=D"(__dummy2) : \
>> > - "a"(VMMOUSE_PROTO_MAGIC), \
>> > - "b"(in1), \
>> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>> > - "d"(VMMOUSE_PROTO_PORT) : \
>> > - "memory"); \
>> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> > +({ \
>> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
>>
>> Why do we need to initialize dummies?
>
> Because for some commands those parameters to VMW_PORT() can be both
> input and outout.
The vmmouse commands do not use them as input though, so it seems we
are simply wasting CPU cycles setting them to 0 just because we are
using the new VMW_PORT here. Why do we need to switch? What is the
benefit of doing this?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:32 ` Sinclair Yeh
@ 2015-12-01 22:45 ` Dmitry Torokhov
2015-12-01 22:45 ` Dmitry Torokhov
1 sibling, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 22:45 UTC (permalink / raw)
To: Sinclair Yeh
Cc: X86 ML, pv-drivers, linux-graphics-maintainer, Arnd Bergmann,
Greg Kroah-Hartman, lkml, virtualization, linux-input
On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:24:14PM -0800, Dmitry Torokhov wrote:
>> On Tue, Dec 01, 2015 at 02:18:49PM -0800, Sinclair Yeh wrote:
>> > v2:
>> > Instead of replacing existing VMMOUSE defines, only modify enough
>> > to use the new VMW_PORT define.
>> >
>> > v3:
>> > Use updated VMWARE_PORT() which requires hypervisor magic as an added
>> > parameter
>> >
>> > Signed-off-by: Sinclair Yeh <syeh@vmware.com>
>> > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
>> > Reviewed-by: Alok N Kataria <akataria@vmware.com>
>> > Cc: pv-drivers@vmware.com
>> > Cc: linux-graphics-maintainer@vmware.com
>> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > Cc: Arnd Bergmann <arnd@arndb.de>
>> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > Cc: linux-kernel@vger.kernel.org
>> > Cc: virtualization@lists.linux-foundation.org
>> > Cc: linux-input@vger.kernel.org
>> > ---
>> > drivers/input/mouse/vmmouse.c | 22 +++++++---------------
>> > 1 file changed, 7 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
>> > index e272f06..d34e3e4 100644
>> > --- a/drivers/input/mouse/vmmouse.c
>> > +++ b/drivers/input/mouse/vmmouse.c
>> > @@ -19,6 +19,7 @@
>> > #include <linux/slab.h>
>> > #include <linux/module.h>
>> > #include <asm/hypervisor.h>
>> > +#include <asm/vmware.h>
>> >
>> > #include "psmouse.h"
>> > #include "vmmouse.h"
>> > @@ -84,21 +85,12 @@ struct vmmouse_data {
>> > * implementing the vmmouse protocol. Should never execute on
>> > * bare metal hardware.
>> > */
>> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> > -({ \
>> > - unsigned long __dummy1, __dummy2; \
>> > - __asm__ __volatile__ ("inl %%dx" : \
>> > - "=a"(out1), \
>> > - "=b"(out2), \
>> > - "=c"(out3), \
>> > - "=d"(out4), \
>> > - "=S"(__dummy1), \
>> > - "=D"(__dummy2) : \
>> > - "a"(VMMOUSE_PROTO_MAGIC), \
>> > - "b"(in1), \
>> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>> > - "d"(VMMOUSE_PROTO_PORT) : \
>> > - "memory"); \
>> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> > +({ \
>> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
>>
>> Why do we need to initialize dummies?
>
> Because for some commands those parameters to VMW_PORT() can be both
> input and outout.
The vmmouse commands do not use them as input though, so it seems we
are simply wasting CPU cycles setting them to 0 just because we are
using the new VMW_PORT here. Why do we need to switch? What is the
benefit of doing this?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-01 22:45 ` Dmitry Torokhov
0 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 22:45 UTC (permalink / raw)
To: Sinclair Yeh
Cc: X86 ML, pv-drivers, linux-graphics-maintainer, Arnd Bergmann,
Greg Kroah-Hartman, lkml, virtualization, linux-input
On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:24:14PM -0800, Dmitry Torokhov wrote:
>> On Tue, Dec 01, 2015 at 02:18:49PM -0800, Sinclair Yeh wrote:
>> > v2:
>> > Instead of replacing existing VMMOUSE defines, only modify enough
>> > to use the new VMW_PORT define.
>> >
>> > v3:
>> > Use updated VMWARE_PORT() which requires hypervisor magic as an added
>> > parameter
>> >
>> > Signed-off-by: Sinclair Yeh <syeh@vmware.com>
>> > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
>> > Reviewed-by: Alok N Kataria <akataria@vmware.com>
>> > Cc: pv-drivers@vmware.com
>> > Cc: linux-graphics-maintainer@vmware.com
>> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > Cc: Arnd Bergmann <arnd@arndb.de>
>> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > Cc: linux-kernel@vger.kernel.org
>> > Cc: virtualization@lists.linux-foundation.org
>> > Cc: linux-input@vger.kernel.org
>> > ---
>> > drivers/input/mouse/vmmouse.c | 22 +++++++---------------
>> > 1 file changed, 7 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
>> > index e272f06..d34e3e4 100644
>> > --- a/drivers/input/mouse/vmmouse.c
>> > +++ b/drivers/input/mouse/vmmouse.c
>> > @@ -19,6 +19,7 @@
>> > #include <linux/slab.h>
>> > #include <linux/module.h>
>> > #include <asm/hypervisor.h>
>> > +#include <asm/vmware.h>
>> >
>> > #include "psmouse.h"
>> > #include "vmmouse.h"
>> > @@ -84,21 +85,12 @@ struct vmmouse_data {
>> > * implementing the vmmouse protocol. Should never execute on
>> > * bare metal hardware.
>> > */
>> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> > -({ \
>> > - unsigned long __dummy1, __dummy2; \
>> > - __asm__ __volatile__ ("inl %%dx" : \
>> > - "=a"(out1), \
>> > - "=b"(out2), \
>> > - "=c"(out3), \
>> > - "=d"(out4), \
>> > - "=S"(__dummy1), \
>> > - "=D"(__dummy2) : \
>> > - "a"(VMMOUSE_PROTO_MAGIC), \
>> > - "b"(in1), \
>> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>> > - "d"(VMMOUSE_PROTO_PORT) : \
>> > - "memory"); \
>> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> > +({ \
>> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
>>
>> Why do we need to initialize dummies?
>
> Because for some commands those parameters to VMW_PORT() can be both
> input and outout.
The vmmouse commands do not use them as input though, so it seems we
are simply wasting CPU cycles setting them to 0 just because we are
using the new VMW_PORT here. Why do we need to switch? What is the
benefit of doing this?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:45 ` Dmitry Torokhov
@ 2015-12-01 22:54 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: X86 ML, pv-drivers, linux-graphics-maintainer, Arnd Bergmann,
Greg Kroah-Hartman, lkml, virtualization, linux-input
Hi,
On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > Hi,
> >
<snip>
> >> > */
> >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> >> > -({ \
> >> > - unsigned long __dummy1, __dummy2; \
> >> > - __asm__ __volatile__ ("inl %%dx" : \
> >> > - "=a"(out1), \
> >> > - "=b"(out2), \
> >> > - "=c"(out3), \
> >> > - "=d"(out4), \
> >> > - "=S"(__dummy1), \
> >> > - "=D"(__dummy2) : \
> >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> >> > - "b"(in1), \
> >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> >> > - "d"(VMMOUSE_PROTO_PORT) : \
> >> > - "memory"); \
> >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> >> > +({ \
> >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> >>
> >> Why do we need to initialize dummies?
> >
> > Because for some commands those parameters to VMW_PORT() can be both
> > input and outout.
>
> The vmmouse commands do not use them as input though, so it seems we
> are simply wasting CPU cycles setting them to 0 just because we are
> using the new VMW_PORT here. Why do we need to switch? What is the
> benefit of doing this?
There are two reasons. One is to make the code more readable and
maintainable. Rather than having mostly similar inline assembly
code sprinkled across multiple modules, we can just use the macros
and document that.
The second reason is this organization makes some on-going future
development easier.
Hope this helps.
Sinclair
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-01 22:54 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Arnd Bergmann, pv-drivers, Greg Kroah-Hartman, X86 ML, lkml,
virtualization, linux-graphics-maintainer, linux-input
Hi,
On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > Hi,
> >
<snip>
> >> > */
> >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> >> > -({ \
> >> > - unsigned long __dummy1, __dummy2; \
> >> > - __asm__ __volatile__ ("inl %%dx" : \
> >> > - "=a"(out1), \
> >> > - "=b"(out2), \
> >> > - "=c"(out3), \
> >> > - "=d"(out4), \
> >> > - "=S"(__dummy1), \
> >> > - "=D"(__dummy2) : \
> >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> >> > - "b"(in1), \
> >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> >> > - "d"(VMMOUSE_PROTO_PORT) : \
> >> > - "memory"); \
> >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> >> > +({ \
> >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> >>
> >> Why do we need to initialize dummies?
> >
> > Because for some commands those parameters to VMW_PORT() can be both
> > input and outout.
>
> The vmmouse commands do not use them as input though, so it seems we
> are simply wasting CPU cycles setting them to 0 just because we are
> using the new VMW_PORT here. Why do we need to switch? What is the
> benefit of doing this?
There are two reasons. One is to make the code more readable and
maintainable. Rather than having mostly similar inline assembly
code sprinkled across multiple modules, we can just use the macros
and document that.
The second reason is this organization makes some on-going future
development easier.
Hope this helps.
Sinclair
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:54 ` Sinclair Yeh
(?)
@ 2015-12-01 23:56 ` Dmitry Torokhov
-1 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 23:56 UTC (permalink / raw)
To: Sinclair Yeh
Cc: Arnd Bergmann, pv-drivers, Greg Kroah-Hartman, X86 ML, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Tue, Dec 1, 2015 at 2:54 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>> > Hi,
>> >
>
> <snip>
>
>> >> > */
>> >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> >> > -({ \
>> >> > - unsigned long __dummy1, __dummy2; \
>> >> > - __asm__ __volatile__ ("inl %%dx" : \
>> >> > - "=a"(out1), \
>> >> > - "=b"(out2), \
>> >> > - "=c"(out3), \
>> >> > - "=d"(out4), \
>> >> > - "=S"(__dummy1), \
>> >> > - "=D"(__dummy2) : \
>> >> > - "a"(VMMOUSE_PROTO_MAGIC), \
>> >> > - "b"(in1), \
>> >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>> >> > - "d"(VMMOUSE_PROTO_PORT) : \
>> >> > - "memory"); \
>> >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> >> > +({ \
>> >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
>> >>
>> >> Why do we need to initialize dummies?
>> >
>> > Because for some commands those parameters to VMW_PORT() can be both
>> > input and outout.
>>
>> The vmmouse commands do not use them as input though, so it seems we
>> are simply wasting CPU cycles setting them to 0 just because we are
>> using the new VMW_PORT here. Why do we need to switch? What is the
>> benefit of doing this?
>
> There are two reasons. One is to make the code more readable and
> maintainable. Rather than having mostly similar inline assembly
> code sprinkled across multiple modules, we can just use the macros
> and document that.
At the cost of wasting cycles though :(.
Oh well, it is not like we are polling the backdoor here, so if you do
not care about a few wasted cycles I don't have to either ;)
>
> The second reason is this organization makes some on-going future
> development easier.
>
> Hope this helps.
>
> Sinclair
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:54 ` Sinclair Yeh
@ 2015-12-01 23:56 ` Dmitry Torokhov
-1 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 23:56 UTC (permalink / raw)
To: Sinclair Yeh
Cc: X86 ML, pv-drivers, linux-graphics-maintainer, Arnd Bergmann,
Greg Kroah-Hartman, lkml, virtualization, linux-input
On Tue, Dec 1, 2015 at 2:54 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>> > Hi,
>> >
>
> <snip>
>
>> >> > */
>> >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> >> > -({ \
>> >> > - unsigned long __dummy1, __dummy2; \
>> >> > - __asm__ __volatile__ ("inl %%dx" : \
>> >> > - "=a"(out1), \
>> >> > - "=b"(out2), \
>> >> > - "=c"(out3), \
>> >> > - "=d"(out4), \
>> >> > - "=S"(__dummy1), \
>> >> > - "=D"(__dummy2) : \
>> >> > - "a"(VMMOUSE_PROTO_MAGIC), \
>> >> > - "b"(in1), \
>> >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>> >> > - "d"(VMMOUSE_PROTO_PORT) : \
>> >> > - "memory"); \
>> >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> >> > +({ \
>> >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
>> >>
>> >> Why do we need to initialize dummies?
>> >
>> > Because for some commands those parameters to VMW_PORT() can be both
>> > input and outout.
>>
>> The vmmouse commands do not use them as input though, so it seems we
>> are simply wasting CPU cycles setting them to 0 just because we are
>> using the new VMW_PORT here. Why do we need to switch? What is the
>> benefit of doing this?
>
> There are two reasons. One is to make the code more readable and
> maintainable. Rather than having mostly similar inline assembly
> code sprinkled across multiple modules, we can just use the macros
> and document that.
At the cost of wasting cycles though :(.
Oh well, it is not like we are polling the backdoor here, so if you do
not care about a few wasted cycles I don't have to either ;)
>
> The second reason is this organization makes some on-going future
> development easier.
>
> Hope this helps.
>
> Sinclair
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-01 23:56 ` Dmitry Torokhov
0 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 23:56 UTC (permalink / raw)
To: Sinclair Yeh
Cc: X86 ML, pv-drivers, linux-graphics-maintainer, Arnd Bergmann,
Greg Kroah-Hartman, lkml, virtualization, linux-input
On Tue, Dec 1, 2015 at 2:54 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>> > Hi,
>> >
>
> <snip>
>
>> >> > */
>> >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> >> > -({ \
>> >> > - unsigned long __dummy1, __dummy2; \
>> >> > - __asm__ __volatile__ ("inl %%dx" : \
>> >> > - "=a"(out1), \
>> >> > - "=b"(out2), \
>> >> > - "=c"(out3), \
>> >> > - "=d"(out4), \
>> >> > - "=S"(__dummy1), \
>> >> > - "=D"(__dummy2) : \
>> >> > - "a"(VMMOUSE_PROTO_MAGIC), \
>> >> > - "b"(in1), \
>> >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>> >> > - "d"(VMMOUSE_PROTO_PORT) : \
>> >> > - "memory"); \
>> >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> >> > +({ \
>> >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
>> >>
>> >> Why do we need to initialize dummies?
>> >
>> > Because for some commands those parameters to VMW_PORT() can be both
>> > input and outout.
>>
>> The vmmouse commands do not use them as input though, so it seems we
>> are simply wasting CPU cycles setting them to 0 just because we are
>> using the new VMW_PORT here. Why do we need to switch? What is the
>> benefit of doing this?
>
> There are two reasons. One is to make the code more readable and
> maintainable. Rather than having mostly similar inline assembly
> code sprinkled across multiple modules, we can just use the macros
> and document that.
At the cost of wasting cycles though :(.
Oh well, it is not like we are polling the backdoor here, so if you do
not care about a few wasted cycles I don't have to either ;)
>
> The second reason is this organization makes some on-going future
> development easier.
>
> Hope this helps.
>
> Sinclair
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:54 ` Sinclair Yeh
@ 2015-12-02 0:04 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 96+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-02 0:04 UTC (permalink / raw)
To: Sinclair Yeh
Cc: Dmitry Torokhov, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > Hi,
> > >
>
> <snip>
>
> > >> > */
> > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > >> > -({ \
> > >> > - unsigned long __dummy1, __dummy2; \
> > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > >> > - "=a"(out1), \
> > >> > - "=b"(out2), \
> > >> > - "=c"(out3), \
> > >> > - "=d"(out4), \
> > >> > - "=S"(__dummy1), \
> > >> > - "=D"(__dummy2) : \
> > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > >> > - "b"(in1), \
> > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > >> > - "memory"); \
> > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > >> > +({ \
> > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > >>
> > >> Why do we need to initialize dummies?
> > >
> > > Because for some commands those parameters to VMW_PORT() can be both
> > > input and outout.
> >
> > The vmmouse commands do not use them as input though, so it seems we
> > are simply wasting CPU cycles setting them to 0 just because we are
> > using the new VMW_PORT here. Why do we need to switch? What is the
> > benefit of doing this?
>
> There are two reasons. One is to make the code more readable and
> maintainable. Rather than having mostly similar inline assembly
> code sprinkled across multiple modules, we can just use the macros
> and document that.
But the macro is only used here, and the variables aren't used at all,
so it makes no sense in this file.
> The second reason is this organization makes some on-going future
> development easier.
We don't plan for "future" development other than a single patch series,
as we have no idea what that development is, nor if it will really
happen. You can always change this file later if you need to, nothing
is keeping that from happening.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 0:04 ` Greg Kroah-Hartman
0 siblings, 0 replies; 96+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-02 0:04 UTC (permalink / raw)
To: Sinclair Yeh
Cc: X86 ML, Arnd Bergmann, pv-drivers, Dmitry Torokhov, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > Hi,
> > >
>
> <snip>
>
> > >> > */
> > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > >> > -({ \
> > >> > - unsigned long __dummy1, __dummy2; \
> > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > >> > - "=a"(out1), \
> > >> > - "=b"(out2), \
> > >> > - "=c"(out3), \
> > >> > - "=d"(out4), \
> > >> > - "=S"(__dummy1), \
> > >> > - "=D"(__dummy2) : \
> > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > >> > - "b"(in1), \
> > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > >> > - "memory"); \
> > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > >> > +({ \
> > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > >>
> > >> Why do we need to initialize dummies?
> > >
> > > Because for some commands those parameters to VMW_PORT() can be both
> > > input and outout.
> >
> > The vmmouse commands do not use them as input though, so it seems we
> > are simply wasting CPU cycles setting them to 0 just because we are
> > using the new VMW_PORT here. Why do we need to switch? What is the
> > benefit of doing this?
>
> There are two reasons. One is to make the code more readable and
> maintainable. Rather than having mostly similar inline assembly
> code sprinkled across multiple modules, we can just use the macros
> and document that.
But the macro is only used here, and the variables aren't used at all,
so it makes no sense in this file.
> The second reason is this organization makes some on-going future
> development easier.
We don't plan for "future" development other than a single patch series,
as we have no idea what that development is, nor if it will really
happen. You can always change this file later if you need to, nothing
is keeping that from happening.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 0:04 ` Greg Kroah-Hartman
@ 2015-12-02 2:21 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-02 2:21 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dmitry Torokhov, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > Hi,
> >
> > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > Hi,
> > > >
> >
> > <snip>
> >
> > > >> > */
> > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > >> > -({ \
> > > >> > - unsigned long __dummy1, __dummy2; \
> > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > >> > - "=a"(out1), \
> > > >> > - "=b"(out2), \
> > > >> > - "=c"(out3), \
> > > >> > - "=d"(out4), \
> > > >> > - "=S"(__dummy1), \
> > > >> > - "=D"(__dummy2) : \
> > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > >> > - "b"(in1), \
> > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > >> > - "memory"); \
> > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > >> > +({ \
> > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > >>
> > > >> Why do we need to initialize dummies?
> > > >
> > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > input and outout.
> > >
> > > The vmmouse commands do not use them as input though, so it seems we
> > > are simply wasting CPU cycles setting them to 0 just because we are
> > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > benefit of doing this?
> >
> > There are two reasons. One is to make the code more readable and
> > maintainable. Rather than having mostly similar inline assembly
> > code sprinkled across multiple modules, we can just use the macros
> > and document that.
>
> But the macro is only used here, and the variables aren't used at all,
> so it makes no sense in this file.
Maybe it's because I didn't CC you on the rest of the series. I wasn't
sure what the proper distribution list is for each part.
This new macro is also used in arch/x86/kernel/cpu/vmware.c and
vmw_balloon.c
>
> > The second reason is this organization makes some on-going future
> > development easier.
>
> We don't plan for "future" development other than a single patch series,
> as we have no idea what that development is, nor if it will really
> happen. You can always change this file later if you need to, nothing
> is keeping that from happening.
So the intent of this series is to centralize similar lines of inline
assembly code that are currently used by 3 different kernel modules
to a central place. The new vmware.h [patch 0/6] becomes the one header
to include for common guest-host communication needs.
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 2:21 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-02 2:21 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dmitry Torokhov, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > Hi,
> >
> > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > Hi,
> > > >
> >
> > <snip>
> >
> > > >> > */
> > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > >> > -({ \
> > > >> > - unsigned long __dummy1, __dummy2; \
> > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > >> > - "=a"(out1), \
> > > >> > - "=b"(out2), \
> > > >> > - "=c"(out3), \
> > > >> > - "=d"(out4), \
> > > >> > - "=S"(__dummy1), \
> > > >> > - "=D"(__dummy2) : \
> > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > >> > - "b"(in1), \
> > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > >> > - "memory"); \
> > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > >> > +({ \
> > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > >>
> > > >> Why do we need to initialize dummies?
> > > >
> > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > input and outout.
> > >
> > > The vmmouse commands do not use them as input though, so it seems we
> > > are simply wasting CPU cycles setting them to 0 just because we are
> > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > benefit of doing this?
> >
> > There are two reasons. One is to make the code more readable and
> > maintainable. Rather than having mostly similar inline assembly
> > code sprinkled across multiple modules, we can just use the macros
> > and document that.
>
> But the macro is only used here, and the variables aren't used at all,
> so it makes no sense in this file.
Maybe it's because I didn't CC you on the rest of the series. I wasn't
sure what the proper distribution list is for each part.
This new macro is also used in arch/x86/kernel/cpu/vmware.c and
vmw_balloon.c
>
> > The second reason is this organization makes some on-going future
> > development easier.
>
> We don't plan for "future" development other than a single patch series,
> as we have no idea what that development is, nor if it will really
> happen. You can always change this file later if you need to, nothing
> is keeping that from happening.
So the intent of this series is to centralize similar lines of inline
assembly code that are currently used by 3 different kernel modules
to a central place. The new vmware.h [patch 0/6] becomes the one header
to include for common guest-host communication needs.
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 2:21 ` Sinclair Yeh
@ 2015-12-02 15:31 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 96+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-02 15:31 UTC (permalink / raw)
To: Sinclair Yeh
Cc: Dmitry Torokhov, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > Hi,
> > >
> > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > Hi,
> > > > >
> > >
> > > <snip>
> > >
> > > > >> > */
> > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > >> > -({ \
> > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > >> > - "=a"(out1), \
> > > > >> > - "=b"(out2), \
> > > > >> > - "=c"(out3), \
> > > > >> > - "=d"(out4), \
> > > > >> > - "=S"(__dummy1), \
> > > > >> > - "=D"(__dummy2) : \
> > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > >> > - "b"(in1), \
> > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > >> > - "memory"); \
> > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > >> > +({ \
> > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > >>
> > > > >> Why do we need to initialize dummies?
> > > > >
> > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > input and outout.
> > > >
> > > > The vmmouse commands do not use them as input though, so it seems we
> > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > benefit of doing this?
> > >
> > > There are two reasons. One is to make the code more readable and
> > > maintainable. Rather than having mostly similar inline assembly
> > > code sprinkled across multiple modules, we can just use the macros
> > > and document that.
> >
> > But the macro is only used here, and the variables aren't used at all,
> > so it makes no sense in this file.
>
> Maybe it's because I didn't CC you on the rest of the series. I wasn't
> sure what the proper distribution list is for each part.
Use scripts/get_maintainer.pl, that's what it is there for. A number of
those patches should go through me, if not all of them, if you want them
merged...
>
> This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> vmw_balloon.c
And it's used inconsistantly in those patches (you don't set the dummy
variables to 0 in all of them...) Now maybe that's just how the asm
functions work, but it's not very obvious as to why this is at all.
> > > The second reason is this organization makes some on-going future
> > > development easier.
> >
> > We don't plan for "future" development other than a single patch series,
> > as we have no idea what that development is, nor if it will really
> > happen. You can always change this file later if you need to, nothing
> > is keeping that from happening.
>
> So the intent of this series is to centralize similar lines of inline
> assembly code that are currently used by 3 different kernel modules
> to a central place. The new vmware.h [patch 0/6] becomes the one header
> to include for common guest-host communication needs.
Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
create yet-another-.h-file for your bus? You already have 2, this would
make it 3, which seems like a lot...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 15:31 ` Greg Kroah-Hartman
0 siblings, 0 replies; 96+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-02 15:31 UTC (permalink / raw)
To: Sinclair Yeh
Cc: X86 ML, Arnd Bergmann, pv-drivers, Dmitry Torokhov, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > Hi,
> > >
> > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > Hi,
> > > > >
> > >
> > > <snip>
> > >
> > > > >> > */
> > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > >> > -({ \
> > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > >> > - "=a"(out1), \
> > > > >> > - "=b"(out2), \
> > > > >> > - "=c"(out3), \
> > > > >> > - "=d"(out4), \
> > > > >> > - "=S"(__dummy1), \
> > > > >> > - "=D"(__dummy2) : \
> > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > >> > - "b"(in1), \
> > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > >> > - "memory"); \
> > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > >> > +({ \
> > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > >>
> > > > >> Why do we need to initialize dummies?
> > > > >
> > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > input and outout.
> > > >
> > > > The vmmouse commands do not use them as input though, so it seems we
> > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > benefit of doing this?
> > >
> > > There are two reasons. One is to make the code more readable and
> > > maintainable. Rather than having mostly similar inline assembly
> > > code sprinkled across multiple modules, we can just use the macros
> > > and document that.
> >
> > But the macro is only used here, and the variables aren't used at all,
> > so it makes no sense in this file.
>
> Maybe it's because I didn't CC you on the rest of the series. I wasn't
> sure what the proper distribution list is for each part.
Use scripts/get_maintainer.pl, that's what it is there for. A number of
those patches should go through me, if not all of them, if you want them
merged...
>
> This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> vmw_balloon.c
And it's used inconsistantly in those patches (you don't set the dummy
variables to 0 in all of them...) Now maybe that's just how the asm
functions work, but it's not very obvious as to why this is at all.
> > > The second reason is this organization makes some on-going future
> > > development easier.
> >
> > We don't plan for "future" development other than a single patch series,
> > as we have no idea what that development is, nor if it will really
> > happen. You can always change this file later if you need to, nothing
> > is keeping that from happening.
>
> So the intent of this series is to centralize similar lines of inline
> assembly code that are currently used by 3 different kernel modules
> to a central place. The new vmware.h [patch 0/6] becomes the one header
> to include for common guest-host communication needs.
Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
create yet-another-.h-file for your bus? You already have 2, this would
make it 3, which seems like a lot...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 15:31 ` Greg Kroah-Hartman
@ 2015-12-02 15:57 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-02 15:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dmitry Torokhov, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > Hi,
> > > >
> > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > Hi,
> > > > > >
> > > >
> > > > <snip>
> > > >
> > > > > >> > */
> > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > -({ \
> > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > >> > - "=a"(out1), \
> > > > > >> > - "=b"(out2), \
> > > > > >> > - "=c"(out3), \
> > > > > >> > - "=d"(out4), \
> > > > > >> > - "=S"(__dummy1), \
> > > > > >> > - "=D"(__dummy2) : \
> > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > >> > - "b"(in1), \
> > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > >> > - "memory"); \
> > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > +({ \
> > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > >>
> > > > > >> Why do we need to initialize dummies?
> > > > > >
> > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > input and outout.
> > > > >
> > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > benefit of doing this?
> > > >
> > > > There are two reasons. One is to make the code more readable and
> > > > maintainable. Rather than having mostly similar inline assembly
> > > > code sprinkled across multiple modules, we can just use the macros
> > > > and document that.
> > >
> > > But the macro is only used here, and the variables aren't used at all,
> > > so it makes no sense in this file.
> >
> > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > sure what the proper distribution list is for each part.
>
> Use scripts/get_maintainer.pl, that's what it is there for. A number of
> those patches should go through me, if not all of them, if you want them
> merged...
>
> >
> > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > vmw_balloon.c
>
> And it's used inconsistantly in those patches (you don't set the dummy
> variables to 0 in all of them...) Now maybe that's just how the asm
> functions work, but it's not very obvious as to why this is at all.
>
> > > > The second reason is this organization makes some on-going future
> > > > development easier.
> > >
> > > We don't plan for "future" development other than a single patch series,
> > > as we have no idea what that development is, nor if it will really
> > > happen. You can always change this file later if you need to, nothing
> > > is keeping that from happening.
> >
> > So the intent of this series is to centralize similar lines of inline
> > assembly code that are currently used by 3 different kernel modules
> > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > to include for common guest-host communication needs.
>
> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> create yet-another-.h-file for your bus? You already have 2, this would
> make it 3, which seems like a lot...
Ok, thanks. Let me see if it make sense to use one of the existing 2
files. Either way, I'll respin this series to include all the comments
so far.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 15:57 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-02 15:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dmitry Torokhov, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > Hi,
> > > >
> > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > Hi,
> > > > > >
> > > >
> > > > <snip>
> > > >
> > > > > >> > */
> > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > -({ \
> > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > >> > - "=a"(out1), \
> > > > > >> > - "=b"(out2), \
> > > > > >> > - "=c"(out3), \
> > > > > >> > - "=d"(out4), \
> > > > > >> > - "=S"(__dummy1), \
> > > > > >> > - "=D"(__dummy2) : \
> > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > >> > - "b"(in1), \
> > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > >> > - "memory"); \
> > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > +({ \
> > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > >>
> > > > > >> Why do we need to initialize dummies?
> > > > > >
> > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > input and outout.
> > > > >
> > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > benefit of doing this?
> > > >
> > > > There are two reasons. One is to make the code more readable and
> > > > maintainable. Rather than having mostly similar inline assembly
> > > > code sprinkled across multiple modules, we can just use the macros
> > > > and document that.
> > >
> > > But the macro is only used here, and the variables aren't used at all,
> > > so it makes no sense in this file.
> >
> > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > sure what the proper distribution list is for each part.
>
> Use scripts/get_maintainer.pl, that's what it is there for. A number of
> those patches should go through me, if not all of them, if you want them
> merged...
>
> >
> > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > vmw_balloon.c
>
> And it's used inconsistantly in those patches (you don't set the dummy
> variables to 0 in all of them...) Now maybe that's just how the asm
> functions work, but it's not very obvious as to why this is at all.
>
> > > > The second reason is this organization makes some on-going future
> > > > development easier.
> > >
> > > We don't plan for "future" development other than a single patch series,
> > > as we have no idea what that development is, nor if it will really
> > > happen. You can always change this file later if you need to, nothing
> > > is keeping that from happening.
> >
> > So the intent of this series is to centralize similar lines of inline
> > assembly code that are currently used by 3 different kernel modules
> > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > to include for common guest-host communication needs.
>
> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> create yet-another-.h-file for your bus? You already have 2, this would
> make it 3, which seems like a lot...
Ok, thanks. Let me see if it make sense to use one of the existing 2
files. Either way, I'll respin this series to include all the comments
so far.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 15:31 ` Greg Kroah-Hartman
(?)
(?)
@ 2015-12-02 15:57 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-02 15:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: X86 ML, Arnd Bergmann, pv-drivers, Dmitry Torokhov, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > Hi,
> > > >
> > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > Hi,
> > > > > >
> > > >
> > > > <snip>
> > > >
> > > > > >> > */
> > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > -({ \
> > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > >> > - "=a"(out1), \
> > > > > >> > - "=b"(out2), \
> > > > > >> > - "=c"(out3), \
> > > > > >> > - "=d"(out4), \
> > > > > >> > - "=S"(__dummy1), \
> > > > > >> > - "=D"(__dummy2) : \
> > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > >> > - "b"(in1), \
> > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > >> > - "memory"); \
> > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > +({ \
> > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > >>
> > > > > >> Why do we need to initialize dummies?
> > > > > >
> > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > input and outout.
> > > > >
> > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > benefit of doing this?
> > > >
> > > > There are two reasons. One is to make the code more readable and
> > > > maintainable. Rather than having mostly similar inline assembly
> > > > code sprinkled across multiple modules, we can just use the macros
> > > > and document that.
> > >
> > > But the macro is only used here, and the variables aren't used at all,
> > > so it makes no sense in this file.
> >
> > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > sure what the proper distribution list is for each part.
>
> Use scripts/get_maintainer.pl, that's what it is there for. A number of
> those patches should go through me, if not all of them, if you want them
> merged...
>
> >
> > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > vmw_balloon.c
>
> And it's used inconsistantly in those patches (you don't set the dummy
> variables to 0 in all of them...) Now maybe that's just how the asm
> functions work, but it's not very obvious as to why this is at all.
>
> > > > The second reason is this organization makes some on-going future
> > > > development easier.
> > >
> > > We don't plan for "future" development other than a single patch series,
> > > as we have no idea what that development is, nor if it will really
> > > happen. You can always change this file later if you need to, nothing
> > > is keeping that from happening.
> >
> > So the intent of this series is to centralize similar lines of inline
> > assembly code that are currently used by 3 different kernel modules
> > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > to include for common guest-host communication needs.
>
> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> create yet-another-.h-file for your bus? You already have 2, this would
> make it 3, which seems like a lot...
Ok, thanks. Let me see if it make sense to use one of the existing 2
files. Either way, I'll respin this series to include all the comments
so far.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 15:31 ` Greg Kroah-Hartman
@ 2015-12-02 17:26 ` Dmitry Torokhov
-1 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-02 17:26 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sinclair Yeh, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > Hi,
> > > >
> > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > Hi,
> > > > > >
> > > >
> > > > <snip>
> > > >
> > > > > >> > */
> > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > -({ \
> > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > >> > - "=a"(out1), \
> > > > > >> > - "=b"(out2), \
> > > > > >> > - "=c"(out3), \
> > > > > >> > - "=d"(out4), \
> > > > > >> > - "=S"(__dummy1), \
> > > > > >> > - "=D"(__dummy2) : \
> > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > >> > - "b"(in1), \
> > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > >> > - "memory"); \
> > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > +({ \
> > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > >>
> > > > > >> Why do we need to initialize dummies?
> > > > > >
> > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > input and outout.
> > > > >
> > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > benefit of doing this?
> > > >
> > > > There are two reasons. One is to make the code more readable and
> > > > maintainable. Rather than having mostly similar inline assembly
> > > > code sprinkled across multiple modules, we can just use the macros
> > > > and document that.
> > >
> > > But the macro is only used here, and the variables aren't used at all,
> > > so it makes no sense in this file.
> >
> > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > sure what the proper distribution list is for each part.
>
> Use scripts/get_maintainer.pl, that's what it is there for. A number of
> those patches should go through me, if not all of them, if you want them
> merged...
>
> >
> > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > vmw_balloon.c
>
> And it's used inconsistantly in those patches (you don't set the dummy
> variables to 0 in all of them...) Now maybe that's just how the asm
> functions work, but it's not very obvious as to why this is at all.
>
> > > > The second reason is this organization makes some on-going future
> > > > development easier.
> > >
> > > We don't plan for "future" development other than a single patch series,
> > > as we have no idea what that development is, nor if it will really
> > > happen. You can always change this file later if you need to, nothing
> > > is keeping that from happening.
> >
> > So the intent of this series is to centralize similar lines of inline
> > assembly code that are currently used by 3 different kernel modules
> > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > to include for common guest-host communication needs.
>
> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> create yet-another-.h-file for your bus? You already have 2, this would
> make it 3, which seems like a lot...
Umm, you are not saying that vmmouse should include vmci header file(s),
are you? Because the 2 are unrelated and vmci does not use the
hypervisor port to communicate with host IIRC.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 17:26 ` Dmitry Torokhov
0 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-02 17:26 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arnd Bergmann, Sinclair Yeh, pv-drivers, X86 ML, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > Hi,
> > > >
> > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > Hi,
> > > > > >
> > > >
> > > > <snip>
> > > >
> > > > > >> > */
> > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > -({ \
> > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > >> > - "=a"(out1), \
> > > > > >> > - "=b"(out2), \
> > > > > >> > - "=c"(out3), \
> > > > > >> > - "=d"(out4), \
> > > > > >> > - "=S"(__dummy1), \
> > > > > >> > - "=D"(__dummy2) : \
> > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > >> > - "b"(in1), \
> > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > >> > - "memory"); \
> > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > +({ \
> > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > >>
> > > > > >> Why do we need to initialize dummies?
> > > > > >
> > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > input and outout.
> > > > >
> > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > benefit of doing this?
> > > >
> > > > There are two reasons. One is to make the code more readable and
> > > > maintainable. Rather than having mostly similar inline assembly
> > > > code sprinkled across multiple modules, we can just use the macros
> > > > and document that.
> > >
> > > But the macro is only used here, and the variables aren't used at all,
> > > so it makes no sense in this file.
> >
> > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > sure what the proper distribution list is for each part.
>
> Use scripts/get_maintainer.pl, that's what it is there for. A number of
> those patches should go through me, if not all of them, if you want them
> merged...
>
> >
> > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > vmw_balloon.c
>
> And it's used inconsistantly in those patches (you don't set the dummy
> variables to 0 in all of them...) Now maybe that's just how the asm
> functions work, but it's not very obvious as to why this is at all.
>
> > > > The second reason is this organization makes some on-going future
> > > > development easier.
> > >
> > > We don't plan for "future" development other than a single patch series,
> > > as we have no idea what that development is, nor if it will really
> > > happen. You can always change this file later if you need to, nothing
> > > is keeping that from happening.
> >
> > So the intent of this series is to centralize similar lines of inline
> > assembly code that are currently used by 3 different kernel modules
> > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > to include for common guest-host communication needs.
>
> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> create yet-another-.h-file for your bus? You already have 2, this would
> make it 3, which seems like a lot...
Umm, you are not saying that vmmouse should include vmci header file(s),
are you? Because the 2 are unrelated and vmci does not use the
hypervisor port to communicate with host IIRC.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [Linux-graphics-maintainer] [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 17:26 ` Dmitry Torokhov
@ 2015-12-02 17:29 ` Thomas Hellstrom
-1 siblings, 0 replies; 96+ messages in thread
From: Thomas Hellstrom @ 2015-12-02 17:29 UTC (permalink / raw)
To: Dmitry Torokhov, Greg Kroah-Hartman
Cc: Arnd Bergmann, pv-drivers, X86 ML, lkml, virtualization,
linux-graphics-maintainer, linux-input
On 12/02/2015 06:26 PM, Dmitry Torokhov wrote:
> On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
>> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
>>> On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
>>>> On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
>>>>> Hi,
>>>>>
>>>>> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>>>>>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>> <snip>
>>>>>
>>>>>>>>> */
>>>>>>>>> -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>>>>> -({ \
>>>>>>>>> - unsigned long __dummy1, __dummy2; \
>>>>>>>>> - __asm__ __volatile__ ("inl %%dx" : \
>>>>>>>>> - "=a"(out1), \
>>>>>>>>> - "=b"(out2), \
>>>>>>>>> - "=c"(out3), \
>>>>>>>>> - "=d"(out4), \
>>>>>>>>> - "=S"(__dummy1), \
>>>>>>>>> - "=D"(__dummy2) : \
>>>>>>>>> - "a"(VMMOUSE_PROTO_MAGIC), \
>>>>>>>>> - "b"(in1), \
>>>>>>>>> - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>>>>>>>>> - "d"(VMMOUSE_PROTO_PORT) : \
>>>>>>>>> - "memory"); \
>>>>>>>>> +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>>>>> +({ \
>>>>>>>>> + unsigned long __dummy1 = 0, __dummy2 = 0; \
>>>>>>>> Why do we need to initialize dummies?
>>>>>>> Because for some commands those parameters to VMW_PORT() can be both
>>>>>>> input and outout.
>>>>>> The vmmouse commands do not use them as input though, so it seems we
>>>>>> are simply wasting CPU cycles setting them to 0 just because we are
>>>>>> using the new VMW_PORT here. Why do we need to switch? What is the
>>>>>> benefit of doing this?
>>>>> There are two reasons. One is to make the code more readable and
>>>>> maintainable. Rather than having mostly similar inline assembly
>>>>> code sprinkled across multiple modules, we can just use the macros
>>>>> and document that.
>>>> But the macro is only used here, and the variables aren't used at all,
>>>> so it makes no sense in this file.
>>> Maybe it's because I didn't CC you on the rest of the series. I wasn't
>>> sure what the proper distribution list is for each part.
>> Use scripts/get_maintainer.pl, that's what it is there for. A number of
>> those patches should go through me, if not all of them, if you want them
>> merged...
>>
>>> This new macro is also used in arch/x86/kernel/cpu/vmware.c and
>>> vmw_balloon.c
>> And it's used inconsistantly in those patches (you don't set the dummy
>> variables to 0 in all of them...) Now maybe that's just how the asm
>> functions work, but it's not very obvious as to why this is at all.
>>
>>>>> The second reason is this organization makes some on-going future
>>>>> development easier.
>>>> We don't plan for "future" development other than a single patch series,
>>>> as we have no idea what that development is, nor if it will really
>>>> happen. You can always change this file later if you need to, nothing
>>>> is keeping that from happening.
>>> So the intent of this series is to centralize similar lines of inline
>>> assembly code that are currently used by 3 different kernel modules
>>> to a central place. The new vmware.h [patch 0/6] becomes the one header
>>> to include for common guest-host communication needs.
>> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
>> create yet-another-.h-file for your bus? You already have 2, this would
>> make it 3, which seems like a lot...
> Umm, you are not saying that vmmouse should include vmci header file(s),
> are you? Because the 2 are unrelated and vmci does not use the
> hypervisor port to communicate with host IIRC.
Also the platform setup code uses the hypervisor port, so it's a natural
place for the macro defines.
/Thomas
>
> Thanks.
>
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [Linux-graphics-maintainer] [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 17:29 ` Thomas Hellstrom
0 siblings, 0 replies; 96+ messages in thread
From: Thomas Hellstrom @ 2015-12-02 17:29 UTC (permalink / raw)
To: Dmitry Torokhov, Greg Kroah-Hartman
Cc: Arnd Bergmann, pv-drivers, X86 ML, lkml, virtualization,
linux-graphics-maintainer, linux-input
On 12/02/2015 06:26 PM, Dmitry Torokhov wrote:
> On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
>> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
>>> On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
>>>> On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
>>>>> Hi,
>>>>>
>>>>> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>>>>>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>> <snip>
>>>>>
>>>>>>>>> */
>>>>>>>>> -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>>>>> -({ \
>>>>>>>>> - unsigned long __dummy1, __dummy2; \
>>>>>>>>> - __asm__ __volatile__ ("inl %%dx" : \
>>>>>>>>> - "=a"(out1), \
>>>>>>>>> - "=b"(out2), \
>>>>>>>>> - "=c"(out3), \
>>>>>>>>> - "=d"(out4), \
>>>>>>>>> - "=S"(__dummy1), \
>>>>>>>>> - "=D"(__dummy2) : \
>>>>>>>>> - "a"(VMMOUSE_PROTO_MAGIC), \
>>>>>>>>> - "b"(in1), \
>>>>>>>>> - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>>>>>>>>> - "d"(VMMOUSE_PROTO_PORT) : \
>>>>>>>>> - "memory"); \
>>>>>>>>> +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>>>>> +({ \
>>>>>>>>> + unsigned long __dummy1 = 0, __dummy2 = 0; \
>>>>>>>> Why do we need to initialize dummies?
>>>>>>> Because for some commands those parameters to VMW_PORT() can be both
>>>>>>> input and outout.
>>>>>> The vmmouse commands do not use them as input though, so it seems we
>>>>>> are simply wasting CPU cycles setting them to 0 just because we are
>>>>>> using the new VMW_PORT here. Why do we need to switch? What is the
>>>>>> benefit of doing this?
>>>>> There are two reasons. One is to make the code more readable and
>>>>> maintainable. Rather than having mostly similar inline assembly
>>>>> code sprinkled across multiple modules, we can just use the macros
>>>>> and document that.
>>>> But the macro is only used here, and the variables aren't used at all,
>>>> so it makes no sense in this file.
>>> Maybe it's because I didn't CC you on the rest of the series. I wasn't
>>> sure what the proper distribution list is for each part.
>> Use scripts/get_maintainer.pl, that's what it is there for. A number of
>> those patches should go through me, if not all of them, if you want them
>> merged...
>>
>>> This new macro is also used in arch/x86/kernel/cpu/vmware.c and
>>> vmw_balloon.c
>> And it's used inconsistantly in those patches (you don't set the dummy
>> variables to 0 in all of them...) Now maybe that's just how the asm
>> functions work, but it's not very obvious as to why this is at all.
>>
>>>>> The second reason is this organization makes some on-going future
>>>>> development easier.
>>>> We don't plan for "future" development other than a single patch series,
>>>> as we have no idea what that development is, nor if it will really
>>>> happen. You can always change this file later if you need to, nothing
>>>> is keeping that from happening.
>>> So the intent of this series is to centralize similar lines of inline
>>> assembly code that are currently used by 3 different kernel modules
>>> to a central place. The new vmware.h [patch 0/6] becomes the one header
>>> to include for common guest-host communication needs.
>> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
>> create yet-another-.h-file for your bus? You already have 2, this would
>> make it 3, which seems like a lot...
> Umm, you are not saying that vmmouse should include vmci header file(s),
> are you? Because the 2 are unrelated and vmci does not use the
> hypervisor port to communicate with host IIRC.
Also the platform setup code uses the hypervisor port, so it's a natural
place for the macro defines.
/Thomas
>
> Thanks.
>
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 17:26 ` Dmitry Torokhov
@ 2015-12-02 18:45 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 96+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-02 18:45 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Sinclair Yeh, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > Hi,
> > > > >
> > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > > Hi,
> > > > > > >
> > > > >
> > > > > <snip>
> > > > >
> > > > > > >> > */
> > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > >> > -({ \
> > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > >> > - "=a"(out1), \
> > > > > > >> > - "=b"(out2), \
> > > > > > >> > - "=c"(out3), \
> > > > > > >> > - "=d"(out4), \
> > > > > > >> > - "=S"(__dummy1), \
> > > > > > >> > - "=D"(__dummy2) : \
> > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > >> > - "b"(in1), \
> > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > >> > - "memory"); \
> > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > >> > +({ \
> > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > > >>
> > > > > > >> Why do we need to initialize dummies?
> > > > > > >
> > > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > > input and outout.
> > > > > >
> > > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > benefit of doing this?
> > > > >
> > > > > There are two reasons. One is to make the code more readable and
> > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > and document that.
> > > >
> > > > But the macro is only used here, and the variables aren't used at all,
> > > > so it makes no sense in this file.
> > >
> > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > sure what the proper distribution list is for each part.
> >
> > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > those patches should go through me, if not all of them, if you want them
> > merged...
> >
> > >
> > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > vmw_balloon.c
> >
> > And it's used inconsistantly in those patches (you don't set the dummy
> > variables to 0 in all of them...) Now maybe that's just how the asm
> > functions work, but it's not very obvious as to why this is at all.
> >
> > > > > The second reason is this organization makes some on-going future
> > > > > development easier.
> > > >
> > > > We don't plan for "future" development other than a single patch series,
> > > > as we have no idea what that development is, nor if it will really
> > > > happen. You can always change this file later if you need to, nothing
> > > > is keeping that from happening.
> > >
> > > So the intent of this series is to centralize similar lines of inline
> > > assembly code that are currently used by 3 different kernel modules
> > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > to include for common guest-host communication needs.
> >
> > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > create yet-another-.h-file for your bus? You already have 2, this would
> > make it 3, which seems like a lot...
>
> Umm, you are not saying that vmmouse should include vmci header file(s),
> are you? Because the 2 are unrelated and vmci does not use the
> hypervisor port to communicate with host IIRC.
vmmouse should include some type of "vmware bus" .h file, if it's not
the vmw_* files, what are they for? My point being, I didn't see the
need to add another .h file when we should probably already have one for
this bus, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 18:45 ` Greg Kroah-Hartman
0 siblings, 0 replies; 96+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-02 18:45 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Arnd Bergmann, Sinclair Yeh, pv-drivers, X86 ML, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > Hi,
> > > > >
> > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > > Hi,
> > > > > > >
> > > > >
> > > > > <snip>
> > > > >
> > > > > > >> > */
> > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > >> > -({ \
> > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > >> > - "=a"(out1), \
> > > > > > >> > - "=b"(out2), \
> > > > > > >> > - "=c"(out3), \
> > > > > > >> > - "=d"(out4), \
> > > > > > >> > - "=S"(__dummy1), \
> > > > > > >> > - "=D"(__dummy2) : \
> > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > >> > - "b"(in1), \
> > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > >> > - "memory"); \
> > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > >> > +({ \
> > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > > >>
> > > > > > >> Why do we need to initialize dummies?
> > > > > > >
> > > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > > input and outout.
> > > > > >
> > > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > benefit of doing this?
> > > > >
> > > > > There are two reasons. One is to make the code more readable and
> > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > and document that.
> > > >
> > > > But the macro is only used here, and the variables aren't used at all,
> > > > so it makes no sense in this file.
> > >
> > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > sure what the proper distribution list is for each part.
> >
> > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > those patches should go through me, if not all of them, if you want them
> > merged...
> >
> > >
> > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > vmw_balloon.c
> >
> > And it's used inconsistantly in those patches (you don't set the dummy
> > variables to 0 in all of them...) Now maybe that's just how the asm
> > functions work, but it's not very obvious as to why this is at all.
> >
> > > > > The second reason is this organization makes some on-going future
> > > > > development easier.
> > > >
> > > > We don't plan for "future" development other than a single patch series,
> > > > as we have no idea what that development is, nor if it will really
> > > > happen. You can always change this file later if you need to, nothing
> > > > is keeping that from happening.
> > >
> > > So the intent of this series is to centralize similar lines of inline
> > > assembly code that are currently used by 3 different kernel modules
> > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > to include for common guest-host communication needs.
> >
> > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > create yet-another-.h-file for your bus? You already have 2, this would
> > make it 3, which seems like a lot...
>
> Umm, you are not saying that vmmouse should include vmci header file(s),
> are you? Because the 2 are unrelated and vmci does not use the
> hypervisor port to communicate with host IIRC.
vmmouse should include some type of "vmware bus" .h file, if it's not
the vmw_* files, what are they for? My point being, I didn't see the
need to add another .h file when we should probably already have one for
this bus, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 18:45 ` Greg Kroah-Hartman
@ 2015-12-02 18:58 ` Dmitry Torokhov
-1 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-02 18:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sinclair Yeh, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Wed, Dec 02, 2015 at 10:45:28AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> > On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > >
> > > > > > <snip>
> > > > > >
> > > > > > > >> > */
> > > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > -({ \
> > > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > > >> > - "=a"(out1), \
> > > > > > > >> > - "=b"(out2), \
> > > > > > > >> > - "=c"(out3), \
> > > > > > > >> > - "=d"(out4), \
> > > > > > > >> > - "=S"(__dummy1), \
> > > > > > > >> > - "=D"(__dummy2) : \
> > > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > > >> > - "b"(in1), \
> > > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > > >> > - "memory"); \
> > > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > +({ \
> > > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > > > >>
> > > > > > > >> Why do we need to initialize dummies?
> > > > > > > >
> > > > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > > > input and outout.
> > > > > > >
> > > > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > > benefit of doing this?
> > > > > >
> > > > > > There are two reasons. One is to make the code more readable and
> > > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > > and document that.
> > > > >
> > > > > But the macro is only used here, and the variables aren't used at all,
> > > > > so it makes no sense in this file.
> > > >
> > > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > > sure what the proper distribution list is for each part.
> > >
> > > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > > those patches should go through me, if not all of them, if you want them
> > > merged...
> > >
> > > >
> > > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > > vmw_balloon.c
> > >
> > > And it's used inconsistantly in those patches (you don't set the dummy
> > > variables to 0 in all of them...) Now maybe that's just how the asm
> > > functions work, but it's not very obvious as to why this is at all.
> > >
> > > > > > The second reason is this organization makes some on-going future
> > > > > > development easier.
> > > > >
> > > > > We don't plan for "future" development other than a single patch series,
> > > > > as we have no idea what that development is, nor if it will really
> > > > > happen. You can always change this file later if you need to, nothing
> > > > > is keeping that from happening.
> > > >
> > > > So the intent of this series is to centralize similar lines of inline
> > > > assembly code that are currently used by 3 different kernel modules
> > > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > > to include for common guest-host communication needs.
> > >
> > > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > > create yet-another-.h-file for your bus? You already have 2, this would
> > > make it 3, which seems like a lot...
> >
> > Umm, you are not saying that vmmouse should include vmci header file(s),
> > are you? Because the 2 are unrelated and vmci does not use the
> > hypervisor port to communicate with host IIRC.
>
> vmmouse should include some type of "vmware bus" .h file, if it's not
> the vmw_* files, what are they for?
I see:
dtor@dtor-ws:~/kernel/work$ ls include/linux/vmw*
include/linux/vmw_vmci_api.h include/linux/vmw_vmci_defs.h
so they are for VMCI.
> My point being, I didn't see the
> need to add another .h file when we should probably already have one for
> this bus, right?
You keep saying "bus" and I am confused. There is not really a bus there
as (unfortunately) VMware did not come with a unified interface for
host/guest communication, but rather let each group come up with their
own way of doing things. So we have vmmouse and, as I was reminded,
userspace agent using one virtual IO port to communicate, vmballoon is
using another IO port, pvscsi, vmxnet3 and vmci are virtual PCI devices
and Thomas can tell more about vmwgfx as I never looked at it closely.
So if we want to consolidate the hypervisor port accessors (which I am
not entirely convinced is worthwhile as the set of arguments/returned
values/registers used depends on command sent through the port and these
unified macros now require unneeded initializations of dummy variables)
then we indeed need a new header.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 18:58 ` Dmitry Torokhov
0 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-02 18:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sinclair Yeh, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Wed, Dec 02, 2015 at 10:45:28AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> > On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > >
> > > > > > <snip>
> > > > > >
> > > > > > > >> > */
> > > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > -({ \
> > > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > > >> > - "=a"(out1), \
> > > > > > > >> > - "=b"(out2), \
> > > > > > > >> > - "=c"(out3), \
> > > > > > > >> > - "=d"(out4), \
> > > > > > > >> > - "=S"(__dummy1), \
> > > > > > > >> > - "=D"(__dummy2) : \
> > > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > > >> > - "b"(in1), \
> > > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > > >> > - "memory"); \
> > > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > +({ \
> > > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > > > >>
> > > > > > > >> Why do we need to initialize dummies?
> > > > > > > >
> > > > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > > > input and outout.
> > > > > > >
> > > > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > > benefit of doing this?
> > > > > >
> > > > > > There are two reasons. One is to make the code more readable and
> > > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > > and document that.
> > > > >
> > > > > But the macro is only used here, and the variables aren't used at all,
> > > > > so it makes no sense in this file.
> > > >
> > > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > > sure what the proper distribution list is for each part.
> > >
> > > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > > those patches should go through me, if not all of them, if you want them
> > > merged...
> > >
> > > >
> > > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > > vmw_balloon.c
> > >
> > > And it's used inconsistantly in those patches (you don't set the dummy
> > > variables to 0 in all of them...) Now maybe that's just how the asm
> > > functions work, but it's not very obvious as to why this is at all.
> > >
> > > > > > The second reason is this organization makes some on-going future
> > > > > > development easier.
> > > > >
> > > > > We don't plan for "future" development other than a single patch series,
> > > > > as we have no idea what that development is, nor if it will really
> > > > > happen. You can always change this file later if you need to, nothing
> > > > > is keeping that from happening.
> > > >
> > > > So the intent of this series is to centralize similar lines of inline
> > > > assembly code that are currently used by 3 different kernel modules
> > > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > > to include for common guest-host communication needs.
> > >
> > > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > > create yet-another-.h-file for your bus? You already have 2, this would
> > > make it 3, which seems like a lot...
> >
> > Umm, you are not saying that vmmouse should include vmci header file(s),
> > are you? Because the 2 are unrelated and vmci does not use the
> > hypervisor port to communicate with host IIRC.
>
> vmmouse should include some type of "vmware bus" .h file, if it's not
> the vmw_* files, what are they for?
I see:
dtor@dtor-ws:~/kernel/work$ ls include/linux/vmw*
include/linux/vmw_vmci_api.h include/linux/vmw_vmci_defs.h
so they are for VMCI.
> My point being, I didn't see the
> need to add another .h file when we should probably already have one for
> this bus, right?
You keep saying "bus" and I am confused. There is not really a bus there
as (unfortunately) VMware did not come with a unified interface for
host/guest communication, but rather let each group come up with their
own way of doing things. So we have vmmouse and, as I was reminded,
userspace agent using one virtual IO port to communicate, vmballoon is
using another IO port, pvscsi, vmxnet3 and vmci are virtual PCI devices
and Thomas can tell more about vmwgfx as I never looked at it closely.
So if we want to consolidate the hypervisor port accessors (which I am
not entirely convinced is worthwhile as the set of arguments/returned
values/registers used depends on command sent through the port and these
unified macros now require unneeded initializations of dummy variables)
then we indeed need a new header.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 18:45 ` Greg Kroah-Hartman
(?)
(?)
@ 2015-12-02 18:58 ` Dmitry Torokhov
-1 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-02 18:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arnd Bergmann, Sinclair Yeh, pv-drivers, X86 ML, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Wed, Dec 02, 2015 at 10:45:28AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> > On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > >
> > > > > > <snip>
> > > > > >
> > > > > > > >> > */
> > > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > -({ \
> > > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > > >> > - "=a"(out1), \
> > > > > > > >> > - "=b"(out2), \
> > > > > > > >> > - "=c"(out3), \
> > > > > > > >> > - "=d"(out4), \
> > > > > > > >> > - "=S"(__dummy1), \
> > > > > > > >> > - "=D"(__dummy2) : \
> > > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > > >> > - "b"(in1), \
> > > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > > >> > - "memory"); \
> > > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > +({ \
> > > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > > > >>
> > > > > > > >> Why do we need to initialize dummies?
> > > > > > > >
> > > > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > > > input and outout.
> > > > > > >
> > > > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > > benefit of doing this?
> > > > > >
> > > > > > There are two reasons. One is to make the code more readable and
> > > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > > and document that.
> > > > >
> > > > > But the macro is only used here, and the variables aren't used at all,
> > > > > so it makes no sense in this file.
> > > >
> > > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > > sure what the proper distribution list is for each part.
> > >
> > > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > > those patches should go through me, if not all of them, if you want them
> > > merged...
> > >
> > > >
> > > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > > vmw_balloon.c
> > >
> > > And it's used inconsistantly in those patches (you don't set the dummy
> > > variables to 0 in all of them...) Now maybe that's just how the asm
> > > functions work, but it's not very obvious as to why this is at all.
> > >
> > > > > > The second reason is this organization makes some on-going future
> > > > > > development easier.
> > > > >
> > > > > We don't plan for "future" development other than a single patch series,
> > > > > as we have no idea what that development is, nor if it will really
> > > > > happen. You can always change this file later if you need to, nothing
> > > > > is keeping that from happening.
> > > >
> > > > So the intent of this series is to centralize similar lines of inline
> > > > assembly code that are currently used by 3 different kernel modules
> > > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > > to include for common guest-host communication needs.
> > >
> > > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > > create yet-another-.h-file for your bus? You already have 2, this would
> > > make it 3, which seems like a lot...
> >
> > Umm, you are not saying that vmmouse should include vmci header file(s),
> > are you? Because the 2 are unrelated and vmci does not use the
> > hypervisor port to communicate with host IIRC.
>
> vmmouse should include some type of "vmware bus" .h file, if it's not
> the vmw_* files, what are they for?
I see:
dtor@dtor-ws:~/kernel/work$ ls include/linux/vmw*
include/linux/vmw_vmci_api.h include/linux/vmw_vmci_defs.h
so they are for VMCI.
> My point being, I didn't see the
> need to add another .h file when we should probably already have one for
> this bus, right?
You keep saying "bus" and I am confused. There is not really a bus there
as (unfortunately) VMware did not come with a unified interface for
host/guest communication, but rather let each group come up with their
own way of doing things. So we have vmmouse and, as I was reminded,
userspace agent using one virtual IO port to communicate, vmballoon is
using another IO port, pvscsi, vmxnet3 and vmci are virtual PCI devices
and Thomas can tell more about vmwgfx as I never looked at it closely.
So if we want to consolidate the hypervisor port accessors (which I am
not entirely convinced is worthwhile as the set of arguments/returned
values/registers used depends on command sent through the port and these
unified macros now require unneeded initializations of dummy variables)
then we indeed need a new header.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 18:45 ` Greg Kroah-Hartman
@ 2015-12-02 19:02 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-02 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dmitry Torokhov, X86 ML, pv-drivers, linux-graphics-maintainer,
Arnd Bergmann, lkml, virtualization, linux-input
On Wed, Dec 02, 2015 at 10:45:28AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> > On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > >
> > > > > > <snip>
> > > > > >
> > > > > > > >> > */
> > > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > -({ \
> > > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > > >> > - "=a"(out1), \
> > > > > > > >> > - "=b"(out2), \
> > > > > > > >> > - "=c"(out3), \
> > > > > > > >> > - "=d"(out4), \
> > > > > > > >> > - "=S"(__dummy1), \
> > > > > > > >> > - "=D"(__dummy2) : \
> > > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > > >> > - "b"(in1), \
> > > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > > >> > - "memory"); \
> > > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > +({ \
> > > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > > > >>
> > > > > > > >> Why do we need to initialize dummies?
> > > > > > > >
> > > > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > > > input and outout.
> > > > > > >
> > > > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > > benefit of doing this?
> > > > > >
> > > > > > There are two reasons. One is to make the code more readable and
> > > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > > and document that.
> > > > >
> > > > > But the macro is only used here, and the variables aren't used at all,
> > > > > so it makes no sense in this file.
> > > >
> > > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > > sure what the proper distribution list is for each part.
> > >
> > > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > > those patches should go through me, if not all of them, if you want them
> > > merged...
> > >
> > > >
> > > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > > vmw_balloon.c
> > >
> > > And it's used inconsistantly in those patches (you don't set the dummy
> > > variables to 0 in all of them...) Now maybe that's just how the asm
> > > functions work, but it's not very obvious as to why this is at all.
> > >
> > > > > > The second reason is this organization makes some on-going future
> > > > > > development easier.
> > > > >
> > > > > We don't plan for "future" development other than a single patch series,
> > > > > as we have no idea what that development is, nor if it will really
> > > > > happen. You can always change this file later if you need to, nothing
> > > > > is keeping that from happening.
> > > >
> > > > So the intent of this series is to centralize similar lines of inline
> > > > assembly code that are currently used by 3 different kernel modules
> > > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > > to include for common guest-host communication needs.
> > >
> > > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > > create yet-another-.h-file for your bus? You already have 2, this would
> > > make it 3, which seems like a lot...
> >
> > Umm, you are not saying that vmmouse should include vmci header file(s),
> > are you? Because the 2 are unrelated and vmci does not use the
> > hypervisor port to communicate with host IIRC.
>
> vmmouse should include some type of "vmware bus" .h file, if it's not
> the vmw_* files, what are they for? My point being, I didn't see the
> need to add another .h file when we should probably already have one for
> this bus, right?
I had a brief chat with Thomas on this, and he added to Dmitry's reply
earlier.
The two existing *.h files are for the VMCI driver, and the new macros
are used by the platform code and also other drivers like vmw_balloon and
vmmouse.
It makes more sense to create a new vmware.h file in arch/x86/include/asm
Sinclair
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 19:02 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-02 19:02 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: X86 ML, Arnd Bergmann, pv-drivers, Dmitry Torokhov, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Wed, Dec 02, 2015 at 10:45:28AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> > On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > >
> > > > > > <snip>
> > > > > >
> > > > > > > >> > */
> > > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > -({ \
> > > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > > >> > - "=a"(out1), \
> > > > > > > >> > - "=b"(out2), \
> > > > > > > >> > - "=c"(out3), \
> > > > > > > >> > - "=d"(out4), \
> > > > > > > >> > - "=S"(__dummy1), \
> > > > > > > >> > - "=D"(__dummy2) : \
> > > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > > >> > - "b"(in1), \
> > > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > > >> > - "memory"); \
> > > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > +({ \
> > > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > > > >>
> > > > > > > >> Why do we need to initialize dummies?
> > > > > > > >
> > > > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > > > input and outout.
> > > > > > >
> > > > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > > benefit of doing this?
> > > > > >
> > > > > > There are two reasons. One is to make the code more readable and
> > > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > > and document that.
> > > > >
> > > > > But the macro is only used here, and the variables aren't used at all,
> > > > > so it makes no sense in this file.
> > > >
> > > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > > sure what the proper distribution list is for each part.
> > >
> > > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > > those patches should go through me, if not all of them, if you want them
> > > merged...
> > >
> > > >
> > > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > > vmw_balloon.c
> > >
> > > And it's used inconsistantly in those patches (you don't set the dummy
> > > variables to 0 in all of them...) Now maybe that's just how the asm
> > > functions work, but it's not very obvious as to why this is at all.
> > >
> > > > > > The second reason is this organization makes some on-going future
> > > > > > development easier.
> > > > >
> > > > > We don't plan for "future" development other than a single patch series,
> > > > > as we have no idea what that development is, nor if it will really
> > > > > happen. You can always change this file later if you need to, nothing
> > > > > is keeping that from happening.
> > > >
> > > > So the intent of this series is to centralize similar lines of inline
> > > > assembly code that are currently used by 3 different kernel modules
> > > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > > to include for common guest-host communication needs.
> > >
> > > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > > create yet-another-.h-file for your bus? You already have 2, this would
> > > make it 3, which seems like a lot...
> >
> > Umm, you are not saying that vmmouse should include vmci header file(s),
> > are you? Because the 2 are unrelated and vmci does not use the
> > hypervisor port to communicate with host IIRC.
>
> vmmouse should include some type of "vmware bus" .h file, if it's not
> the vmw_* files, what are they for? My point being, I didn't see the
> need to add another .h file when we should probably already have one for
> this bus, right?
I had a brief chat with Thomas on this, and he added to Dmitry's reply
earlier.
The two existing *.h files are for the VMCI driver, and the new macros
are used by the platform code and also other drivers like vmw_balloon and
vmmouse.
It makes more sense to create a new vmware.h file in arch/x86/include/asm
Sinclair
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 0:04 ` Greg Kroah-Hartman
(?)
(?)
@ 2015-12-02 2:21 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-02 2:21 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: X86 ML, Arnd Bergmann, pv-drivers, Dmitry Torokhov, lkml,
virtualization, linux-graphics-maintainer, linux-input
On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > Hi,
> >
> > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > > > Hi,
> > > >
> >
> > <snip>
> >
> > > >> > */
> > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > >> > -({ \
> > > >> > - unsigned long __dummy1, __dummy2; \
> > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > >> > - "=a"(out1), \
> > > >> > - "=b"(out2), \
> > > >> > - "=c"(out3), \
> > > >> > - "=d"(out4), \
> > > >> > - "=S"(__dummy1), \
> > > >> > - "=D"(__dummy2) : \
> > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > >> > - "b"(in1), \
> > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > >> > - "memory"); \
> > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > >> > +({ \
> > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > >>
> > > >> Why do we need to initialize dummies?
> > > >
> > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > input and outout.
> > >
> > > The vmmouse commands do not use them as input though, so it seems we
> > > are simply wasting CPU cycles setting them to 0 just because we are
> > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > benefit of doing this?
> >
> > There are two reasons. One is to make the code more readable and
> > maintainable. Rather than having mostly similar inline assembly
> > code sprinkled across multiple modules, we can just use the macros
> > and document that.
>
> But the macro is only used here, and the variables aren't used at all,
> so it makes no sense in this file.
Maybe it's because I didn't CC you on the rest of the series. I wasn't
sure what the proper distribution list is for each part.
This new macro is also used in arch/x86/kernel/cpu/vmware.c and
vmw_balloon.c
>
> > The second reason is this organization makes some on-going future
> > development easier.
>
> We don't plan for "future" development other than a single patch series,
> as we have no idea what that development is, nor if it will really
> happen. You can always change this file later if you need to, nothing
> is keeping that from happening.
So the intent of this series is to centralize similar lines of inline
assembly code that are currently used by 3 different kernel modules
to a central place. The new vmware.h [patch 0/6] becomes the one header
to include for common guest-host communication needs.
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [Linux-graphics-maintainer] [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 0:04 ` Greg Kroah-Hartman
` (2 preceding siblings ...)
(?)
@ 2015-12-02 7:07 ` Thomas Hellstrom
-1 siblings, 0 replies; 96+ messages in thread
From: Thomas Hellstrom @ 2015-12-02 7:07 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sinclair Yeh
Cc: Dmitry Torokhov, Arnd Bergmann, pv-drivers, X86 ML, lkml,
virtualization, linux-graphics-maintainer, linux-input
On 12/02/2015 01:04 AM, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
>> Hi,
>>
>> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>>>> Hi,
>>>>
>> <snip>
>>
>>>>>> */
>>>>>> -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>> -({ \
>>>>>> - unsigned long __dummy1, __dummy2; \
>>>>>> - __asm__ __volatile__ ("inl %%dx" : \
>>>>>> - "=a"(out1), \
>>>>>> - "=b"(out2), \
>>>>>> - "=c"(out3), \
>>>>>> - "=d"(out4), \
>>>>>> - "=S"(__dummy1), \
>>>>>> - "=D"(__dummy2) : \
>>>>>> - "a"(VMMOUSE_PROTO_MAGIC), \
>>>>>> - "b"(in1), \
>>>>>> - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>>>>>> - "d"(VMMOUSE_PROTO_PORT) : \
>>>>>> - "memory"); \
>>>>>> +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>> +({ \
>>>>>> + unsigned long __dummy1 = 0, __dummy2 = 0; \
>>>>> Why do we need to initialize dummies?
>>>> Because for some commands those parameters to VMW_PORT() can be both
>>>> input and outout.
>>> The vmmouse commands do not use them as input though, so it seems we
>>> are simply wasting CPU cycles setting them to 0 just because we are
>>> using the new VMW_PORT here. Why do we need to switch? What is the
>>> benefit of doing this?
>> There are two reasons. One is to make the code more readable and
>> maintainable. Rather than having mostly similar inline assembly
>> code sprinkled across multiple modules, we can just use the macros
>> and document that.
> But the macro is only used here, and the variables aren't used at all,
> so it makes no sense in this file.
>
IMO, this makes a lot of sense because we now get a single definition of
VMW_PORT in the platform code that a developer can refer to to
understand things like 32-64 bit compatibilty, and usage conditions and
it also forces the developer to adopt the good practice of clearing
currently unused input variables rather than to leave them undefined. In
addition, if something needs to be changed we have one single place to
change rather than a lot of places scattered all over various kernel
modules.
Things that we (I) previously, for example, didn't get quite right in
the vmmouse module despite spending a considerable amount of time on the
subject.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [Linux-graphics-maintainer] [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-02 0:04 ` Greg Kroah-Hartman
@ 2015-12-02 7:07 ` Thomas Hellstrom
-1 siblings, 0 replies; 96+ messages in thread
From: Thomas Hellstrom @ 2015-12-02 7:07 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sinclair Yeh
Cc: X86 ML, Arnd Bergmann, pv-drivers, Dmitry Torokhov, lkml,
virtualization, linux-graphics-maintainer, linux-input
On 12/02/2015 01:04 AM, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
>> Hi,
>>
>> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>>>> Hi,
>>>>
>> <snip>
>>
>>>>>> */
>>>>>> -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>> -({ \
>>>>>> - unsigned long __dummy1, __dummy2; \
>>>>>> - __asm__ __volatile__ ("inl %%dx" : \
>>>>>> - "=a"(out1), \
>>>>>> - "=b"(out2), \
>>>>>> - "=c"(out3), \
>>>>>> - "=d"(out4), \
>>>>>> - "=S"(__dummy1), \
>>>>>> - "=D"(__dummy2) : \
>>>>>> - "a"(VMMOUSE_PROTO_MAGIC), \
>>>>>> - "b"(in1), \
>>>>>> - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>>>>>> - "d"(VMMOUSE_PROTO_PORT) : \
>>>>>> - "memory"); \
>>>>>> +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>> +({ \
>>>>>> + unsigned long __dummy1 = 0, __dummy2 = 0; \
>>>>> Why do we need to initialize dummies?
>>>> Because for some commands those parameters to VMW_PORT() can be both
>>>> input and outout.
>>> The vmmouse commands do not use them as input though, so it seems we
>>> are simply wasting CPU cycles setting them to 0 just because we are
>>> using the new VMW_PORT here. Why do we need to switch? What is the
>>> benefit of doing this?
>> There are two reasons. One is to make the code more readable and
>> maintainable. Rather than having mostly similar inline assembly
>> code sprinkled across multiple modules, we can just use the macros
>> and document that.
> But the macro is only used here, and the variables aren't used at all,
> so it makes no sense in this file.
>
IMO, this makes a lot of sense because we now get a single definition of
VMW_PORT in the platform code that a developer can refer to to
understand things like 32-64 bit compatibilty, and usage conditions and
it also forces the developer to adopt the good practice of clearing
currently unused input variables rather than to leave them undefined. In
addition, if something needs to be changed we have one single place to
change rather than a lot of places scattered all over various kernel
modules.
Things that we (I) previously, for example, didn't get quite right in
the vmmouse module despite spending a considerable amount of time on the
subject.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [Linux-graphics-maintainer] [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
@ 2015-12-02 7:07 ` Thomas Hellstrom
0 siblings, 0 replies; 96+ messages in thread
From: Thomas Hellstrom @ 2015-12-02 7:07 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sinclair Yeh
Cc: X86 ML, Arnd Bergmann, pv-drivers, Dmitry Torokhov, lkml,
virtualization, linux-graphics-maintainer, linux-input
On 12/02/2015 01:04 AM, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
>> Hi,
>>
>> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>>>> Hi,
>>>>
>> <snip>
>>
>>>>>> */
>>>>>> -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>> -({ \
>>>>>> - unsigned long __dummy1, __dummy2; \
>>>>>> - __asm__ __volatile__ ("inl %%dx" : \
>>>>>> - "=a"(out1), \
>>>>>> - "=b"(out2), \
>>>>>> - "=c"(out3), \
>>>>>> - "=d"(out4), \
>>>>>> - "=S"(__dummy1), \
>>>>>> - "=D"(__dummy2) : \
>>>>>> - "a"(VMMOUSE_PROTO_MAGIC), \
>>>>>> - "b"(in1), \
>>>>>> - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>>>>>> - "d"(VMMOUSE_PROTO_PORT) : \
>>>>>> - "memory"); \
>>>>>> +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>>>>>> +({ \
>>>>>> + unsigned long __dummy1 = 0, __dummy2 = 0; \
>>>>> Why do we need to initialize dummies?
>>>> Because for some commands those parameters to VMW_PORT() can be both
>>>> input and outout.
>>> The vmmouse commands do not use them as input though, so it seems we
>>> are simply wasting CPU cycles setting them to 0 just because we are
>>> using the new VMW_PORT here. Why do we need to switch? What is the
>>> benefit of doing this?
>> There are two reasons. One is to make the code more readable and
>> maintainable. Rather than having mostly similar inline assembly
>> code sprinkled across multiple modules, we can just use the macros
>> and document that.
> But the macro is only used here, and the variables aren't used at all,
> so it makes no sense in this file.
>
IMO, this makes a lot of sense because we now get a single definition of
VMW_PORT in the platform code that a developer can refer to to
understand things like 32-64 bit compatibilty, and usage conditions and
it also forces the developer to adopt the good practice of clearing
currently unused input variables rather than to leave them undefined. In
addition, if something needs to be changed we have one single place to
change rather than a lot of places scattered all over various kernel
modules.
Things that we (I) previously, for example, didn't get quite right in
the vmmouse module despite spending a considerable amount of time on the
subject.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:24 ` Dmitry Torokhov
2015-12-01 22:32 ` Sinclair Yeh
@ 2015-12-01 22:32 ` Sinclair Yeh
1 sibling, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Arnd Bergmann, pv-drivers, Greg Kroah-Hartman, x86, linux-kernel,
virtualization, linux-graphics-maintainer, linux-input
Hi,
On Tue, Dec 01, 2015 at 02:24:14PM -0800, Dmitry Torokhov wrote:
> On Tue, Dec 01, 2015 at 02:18:49PM -0800, Sinclair Yeh wrote:
> > v2:
> > Instead of replacing existing VMMOUSE defines, only modify enough
> > to use the new VMW_PORT define.
> >
> > v3:
> > Use updated VMWARE_PORT() which requires hypervisor magic as an added
> > parameter
> >
> > Signed-off-by: Sinclair Yeh <syeh@vmware.com>
> > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> > Reviewed-by: Alok N Kataria <akataria@vmware.com>
> > Cc: pv-drivers@vmware.com
> > Cc: linux-graphics-maintainer@vmware.com
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: linux-kernel@vger.kernel.org
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: linux-input@vger.kernel.org
> > ---
> > drivers/input/mouse/vmmouse.c | 22 +++++++---------------
> > 1 file changed, 7 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
> > index e272f06..d34e3e4 100644
> > --- a/drivers/input/mouse/vmmouse.c
> > +++ b/drivers/input/mouse/vmmouse.c
> > @@ -19,6 +19,7 @@
> > #include <linux/slab.h>
> > #include <linux/module.h>
> > #include <asm/hypervisor.h>
> > +#include <asm/vmware.h>
> >
> > #include "psmouse.h"
> > #include "vmmouse.h"
> > @@ -84,21 +85,12 @@ struct vmmouse_data {
> > * implementing the vmmouse protocol. Should never execute on
> > * bare metal hardware.
> > */
> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > -({ \
> > - unsigned long __dummy1, __dummy2; \
> > - __asm__ __volatile__ ("inl %%dx" : \
> > - "=a"(out1), \
> > - "=b"(out2), \
> > - "=c"(out3), \
> > - "=d"(out4), \
> > - "=S"(__dummy1), \
> > - "=D"(__dummy2) : \
> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > - "b"(in1), \
> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > - "d"(VMMOUSE_PROTO_PORT) : \
> > - "memory"); \
> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > +({ \
> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
>
> Why do we need to initialize dummies?
Because for some commands those parameters to VMW_PORT() can be both
input and outout. So it's safer to initialize them to 0. Since
they can potentially be an output, we can't make them a constant.
>
> > + VMW_PORT(in1, VMMOUSE_PROTO_CMD_##cmd, VMMOUSE_PROTO_PORT, \
> > + VMMOUSE_PROTO_MAGIC, \
> > + out1, out2, out3, out4, __dummy1, __dummy2); \
> > })
> >
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:18 ` Sinclair Yeh
(?)
(?)
@ 2015-12-01 22:24 ` Dmitry Torokhov
-1 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 22:24 UTC (permalink / raw)
To: Sinclair Yeh
Cc: Arnd Bergmann, pv-drivers, Greg Kroah-Hartman, x86, linux-kernel,
virtualization, linux-graphics-maintainer, linux-input
On Tue, Dec 01, 2015 at 02:18:49PM -0800, Sinclair Yeh wrote:
> v2:
> Instead of replacing existing VMMOUSE defines, only modify enough
> to use the new VMW_PORT define.
>
> v3:
> Use updated VMWARE_PORT() which requires hypervisor magic as an added
> parameter
>
> Signed-off-by: Sinclair Yeh <syeh@vmware.com>
> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> Reviewed-by: Alok N Kataria <akataria@vmware.com>
> Cc: pv-drivers@vmware.com
> Cc: linux-graphics-maintainer@vmware.com
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: linux-input@vger.kernel.org
> ---
> drivers/input/mouse/vmmouse.c | 22 +++++++---------------
> 1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
> index e272f06..d34e3e4 100644
> --- a/drivers/input/mouse/vmmouse.c
> +++ b/drivers/input/mouse/vmmouse.c
> @@ -19,6 +19,7 @@
> #include <linux/slab.h>
> #include <linux/module.h>
> #include <asm/hypervisor.h>
> +#include <asm/vmware.h>
>
> #include "psmouse.h"
> #include "vmmouse.h"
> @@ -84,21 +85,12 @@ struct vmmouse_data {
> * implementing the vmmouse protocol. Should never execute on
> * bare metal hardware.
> */
> -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> -({ \
> - unsigned long __dummy1, __dummy2; \
> - __asm__ __volatile__ ("inl %%dx" : \
> - "=a"(out1), \
> - "=b"(out2), \
> - "=c"(out3), \
> - "=d"(out4), \
> - "=S"(__dummy1), \
> - "=D"(__dummy2) : \
> - "a"(VMMOUSE_PROTO_MAGIC), \
> - "b"(in1), \
> - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> - "d"(VMMOUSE_PROTO_PORT) : \
> - "memory"); \
> +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> +({ \
> + unsigned long __dummy1 = 0, __dummy2 = 0; \
Why do we need to initialize dummies?
> + VMW_PORT(in1, VMMOUSE_PROTO_CMD_##cmd, VMMOUSE_PROTO_PORT, \
> + VMMOUSE_PROTO_MAGIC, \
> + out1, out2, out3, out4, __dummy1, __dummy2); \
> })
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:18 ` Sinclair Yeh
` (2 preceding siblings ...)
(?)
@ 2015-12-02 0:01 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 96+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-02 0:01 UTC (permalink / raw)
To: Sinclair Yeh
Cc: Dmitry Torokhov, Arnd Bergmann, pv-drivers, x86, linux-kernel,
virtualization, linux-graphics-maintainer, linux-input
On Tue, Dec 01, 2015 at 02:18:49PM -0800, Sinclair Yeh wrote:
> v2:
> Instead of replacing existing VMMOUSE defines, only modify enough
> to use the new VMW_PORT define.
>
> v3:
> Use updated VMWARE_PORT() which requires hypervisor magic as an added
> parameter
Why are these here and not below the --- line?
And no changelog text at all? Not acceptable :(
greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
2015-12-01 22:18 ` Sinclair Yeh
` (3 preceding siblings ...)
(?)
@ 2015-12-02 0:01 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 96+ messages in thread
From: Greg Kroah-Hartman @ 2015-12-02 0:01 UTC (permalink / raw)
To: Sinclair Yeh
Cc: x86, pv-drivers, linux-graphics-maintainer, Dmitry Torokhov,
Arnd Bergmann, linux-kernel, virtualization, linux-input
On Tue, Dec 01, 2015 at 02:18:49PM -0800, Sinclair Yeh wrote:
> v2:
> Instead of replacing existing VMMOUSE defines, only modify enough
> to use the new VMW_PORT define.
>
> v3:
> Use updated VMWARE_PORT() which requires hypervisor magic as an added
> parameter
Why are these here and not below the --- line?
And no changelog text at all? Not acceptable :(
greg k-h
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH 4/6] Input: Remove vmmouse port reservation
2015-12-01 22:18 ` Sinclair Yeh
` (2 preceding siblings ...)
(?)
@ 2015-12-01 22:18 ` Sinclair Yeh
2015-12-01 22:30 ` Dmitry Torokhov
-1 siblings, 1 reply; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, pv-drivers, linux-graphics-maintainer,
virtualization, linux-kernel
Port reservation is not required. Furthermore, this port is shared
by other VMware services for host-side communication.
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: pv-drivers@vmware.com
Cc: linux-graphics-maintainer@vmware.com
Cc: virtualization@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
---
drivers/input/mouse/vmmouse.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index d34e3e4..9109d54 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -347,16 +347,10 @@ int vmmouse_detect(struct psmouse *psmouse, bool set_properties)
return -ENXIO;
}
- if (!request_region(VMMOUSE_PROTO_PORT, 4, "vmmouse")) {
- psmouse_dbg(psmouse, "VMMouse port in use.\n");
- return -EBUSY;
- }
-
/* Check if the device is present */
response = ~VMMOUSE_PROTO_MAGIC;
VMMOUSE_CMD(GETVERSION, 0, version, response, dummy1, dummy2);
if (response != VMMOUSE_PROTO_MAGIC || version == 0xffffffffU) {
- release_region(VMMOUSE_PROTO_PORT, 4);
return -ENXIO;
}
@@ -366,8 +360,6 @@ int vmmouse_detect(struct psmouse *psmouse, bool set_properties)
psmouse->model = version;
}
- release_region(VMMOUSE_PROTO_PORT, 4);
-
return 0;
}
@@ -386,7 +378,6 @@ static void vmmouse_disconnect(struct psmouse *psmouse)
psmouse_reset(psmouse);
input_unregister_device(priv->abs_dev);
kfree(priv);
- release_region(VMMOUSE_PROTO_PORT, 4);
}
/**
@@ -430,15 +421,10 @@ int vmmouse_init(struct psmouse *psmouse)
struct input_dev *rel_dev = psmouse->dev, *abs_dev;
int error;
- if (!request_region(VMMOUSE_PROTO_PORT, 4, "vmmouse")) {
- psmouse_dbg(psmouse, "VMMouse port in use.\n");
- return -EBUSY;
- }
-
psmouse_reset(psmouse);
error = vmmouse_enable(psmouse);
if (error)
- goto release_region;
+ return error;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
abs_dev = input_allocate_device();
@@ -493,8 +479,5 @@ init_fail:
kfree(priv);
psmouse->private = NULL;
-release_region:
- release_region(VMMOUSE_PROTO_PORT, 4);
-
return error;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* Re: [PATCH 4/6] Input: Remove vmmouse port reservation
2015-12-01 22:18 ` [PATCH 4/6] Input: Remove vmmouse port reservation Sinclair Yeh
@ 2015-12-01 22:30 ` Dmitry Torokhov
0 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 22:30 UTC (permalink / raw)
To: Sinclair Yeh
Cc: X86 ML, pv-drivers, linux-graphics-maintainer, virtualization, lkml
Hi Sinclair,
On Tue, Dec 1, 2015 at 2:18 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Port reservation is not required.
You need to expand on why we do not need to reserve port.
> Furthermore, this port is shared
> by other VMware services for host-side communication.
What services would that be? Do they reserve the port?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 4/6] Input: Remove vmmouse port reservation
@ 2015-12-01 22:30 ` Dmitry Torokhov
0 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 22:30 UTC (permalink / raw)
To: Sinclair Yeh
Cc: pv-drivers, X86 ML, linux-graphics-maintainer, lkml, virtualization
Hi Sinclair,
On Tue, Dec 1, 2015 at 2:18 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Port reservation is not required.
You need to expand on why we do not need to reserve port.
> Furthermore, this port is shared
> by other VMware services for host-side communication.
What services would that be? Do they reserve the port?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 4/6] Input: Remove vmmouse port reservation
2015-12-01 22:30 ` Dmitry Torokhov
@ 2015-12-01 23:04 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 23:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: X86 ML, pv-drivers, linux-graphics-maintainer, virtualization, lkml
Hi,
On Tue, Dec 01, 2015 at 02:30:05PM -0800, Dmitry Torokhov wrote:
> Hi Sinclair,
>
> On Tue, Dec 1, 2015 at 2:18 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > Port reservation is not required.
>
> You need to expand on why we do not need to reserve port.
Thomas gave me this input earlier, too, so I added the one liner.
There was a long discussion on accessing the port a few years ago:
https://lkml.org/lkml/2008/9/24/512
>
> > Furthermore, this port is shared
> > by other VMware services for host-side communication.
>
> What services would that be? Do they reserve the port?
This port is used by quite a few guest-to-host communication capabilities,
e.g. getting configuration, logging, etc. Currently multiple kernel
modules, and one or more priviledged guest user mode app, e.g.
open-vmware-tools, use this port without reservation.
After some internal discussions, it was determined that no reservation
is required when accessing the port in this manner.
Do you want me to put the above in the commit message?
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 4/6] Input: Remove vmmouse port reservation
@ 2015-12-01 23:04 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 23:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: pv-drivers, X86 ML, linux-graphics-maintainer, lkml, virtualization
Hi,
On Tue, Dec 01, 2015 at 02:30:05PM -0800, Dmitry Torokhov wrote:
> Hi Sinclair,
>
> On Tue, Dec 1, 2015 at 2:18 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> > Port reservation is not required.
>
> You need to expand on why we do not need to reserve port.
Thomas gave me this input earlier, too, so I added the one liner.
There was a long discussion on accessing the port a few years ago:
https://lkml.org/lkml/2008/9/24/512
>
> > Furthermore, this port is shared
> > by other VMware services for host-side communication.
>
> What services would that be? Do they reserve the port?
This port is used by quite a few guest-to-host communication capabilities,
e.g. getting configuration, logging, etc. Currently multiple kernel
modules, and one or more priviledged guest user mode app, e.g.
open-vmware-tools, use this port without reservation.
After some internal discussions, it was determined that no reservation
is required when accessing the port in this manner.
Do you want me to put the above in the commit message?
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 4/6] Input: Remove vmmouse port reservation
2015-12-01 23:04 ` Sinclair Yeh
@ 2015-12-01 23:52 ` Dmitry Torokhov
-1 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 23:52 UTC (permalink / raw)
To: Sinclair Yeh
Cc: X86 ML, pv-drivers, linux-graphics-maintainer, virtualization, lkml
On Tue, Dec 1, 2015 at 3:04 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:30:05PM -0800, Dmitry Torokhov wrote:
>> Hi Sinclair,
>>
>> On Tue, Dec 1, 2015 at 2:18 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>> > Port reservation is not required.
>>
>> You need to expand on why we do not need to reserve port.
>
> Thomas gave me this input earlier, too, so I added the one liner.
>
> There was a long discussion on accessing the port a few years ago:
> https://lkml.org/lkml/2008/9/24/512
>
>>
>> > Furthermore, this port is shared
>> > by other VMware services for host-side communication.
>>
>> What services would that be? Do they reserve the port?
>
> This port is used by quite a few guest-to-host communication capabilities,
> e.g. getting configuration, logging, etc. Currently multiple kernel
> modules, and one or more priviledged guest user mode app, e.g.
> open-vmware-tools, use this port without reservation.
Ah, I forgot that vmmouse does not have a dedicated port...
>
> After some internal discussions, it was determined that no reservation
> is required when accessing the port in this manner.
>
> Do you want me to put the above in the commit message?
Not about the bit about "internal discussions", but the rest - yes please.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 4/6] Input: Remove vmmouse port reservation
@ 2015-12-01 23:52 ` Dmitry Torokhov
0 siblings, 0 replies; 96+ messages in thread
From: Dmitry Torokhov @ 2015-12-01 23:52 UTC (permalink / raw)
To: Sinclair Yeh
Cc: pv-drivers, X86 ML, linux-graphics-maintainer, lkml, virtualization
On Tue, Dec 1, 2015 at 3:04 PM, Sinclair Yeh <syeh@vmware.com> wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:30:05PM -0800, Dmitry Torokhov wrote:
>> Hi Sinclair,
>>
>> On Tue, Dec 1, 2015 at 2:18 PM, Sinclair Yeh <syeh@vmware.com> wrote:
>> > Port reservation is not required.
>>
>> You need to expand on why we do not need to reserve port.
>
> Thomas gave me this input earlier, too, so I added the one liner.
>
> There was a long discussion on accessing the port a few years ago:
> https://lkml.org/lkml/2008/9/24/512
>
>>
>> > Furthermore, this port is shared
>> > by other VMware services for host-side communication.
>>
>> What services would that be? Do they reserve the port?
>
> This port is used by quite a few guest-to-host communication capabilities,
> e.g. getting configuration, logging, etc. Currently multiple kernel
> modules, and one or more priviledged guest user mode app, e.g.
> open-vmware-tools, use this port without reservation.
Ah, I forgot that vmmouse does not have a dedicated port...
>
> After some internal discussions, it was determined that no reservation
> is required when accessing the port in this manner.
>
> Do you want me to put the above in the commit message?
Not about the bit about "internal discussions", but the rest - yes please.
--
Dmitry
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH 4/6] Input: Remove vmmouse port reservation
2015-12-01 22:18 ` Sinclair Yeh
` (3 preceding siblings ...)
(?)
@ 2015-12-01 22:18 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: pv-drivers, virtualization, linux-graphics-maintainer,
linux-kernel, Sinclair Yeh
Port reservation is not required. Furthermore, this port is shared
by other VMware services for host-side communication.
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: pv-drivers@vmware.com
Cc: linux-graphics-maintainer@vmware.com
Cc: virtualization@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
---
drivers/input/mouse/vmmouse.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
index d34e3e4..9109d54 100644
--- a/drivers/input/mouse/vmmouse.c
+++ b/drivers/input/mouse/vmmouse.c
@@ -347,16 +347,10 @@ int vmmouse_detect(struct psmouse *psmouse, bool set_properties)
return -ENXIO;
}
- if (!request_region(VMMOUSE_PROTO_PORT, 4, "vmmouse")) {
- psmouse_dbg(psmouse, "VMMouse port in use.\n");
- return -EBUSY;
- }
-
/* Check if the device is present */
response = ~VMMOUSE_PROTO_MAGIC;
VMMOUSE_CMD(GETVERSION, 0, version, response, dummy1, dummy2);
if (response != VMMOUSE_PROTO_MAGIC || version == 0xffffffffU) {
- release_region(VMMOUSE_PROTO_PORT, 4);
return -ENXIO;
}
@@ -366,8 +360,6 @@ int vmmouse_detect(struct psmouse *psmouse, bool set_properties)
psmouse->model = version;
}
- release_region(VMMOUSE_PROTO_PORT, 4);
-
return 0;
}
@@ -386,7 +378,6 @@ static void vmmouse_disconnect(struct psmouse *psmouse)
psmouse_reset(psmouse);
input_unregister_device(priv->abs_dev);
kfree(priv);
- release_region(VMMOUSE_PROTO_PORT, 4);
}
/**
@@ -430,15 +421,10 @@ int vmmouse_init(struct psmouse *psmouse)
struct input_dev *rel_dev = psmouse->dev, *abs_dev;
int error;
- if (!request_region(VMMOUSE_PROTO_PORT, 4, "vmmouse")) {
- psmouse_dbg(psmouse, "VMMouse port in use.\n");
- return -EBUSY;
- }
-
psmouse_reset(psmouse);
error = vmmouse_enable(psmouse);
if (error)
- goto release_region;
+ return error;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
abs_dev = input_allocate_device();
@@ -493,8 +479,5 @@ init_fail:
kfree(priv);
psmouse->private = NULL;
-release_region:
- release_region(VMMOUSE_PROTO_PORT, 4);
-
return error;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* [PATCH 5/6] x86: Remove address from the vmware.c header
2015-12-01 22:18 ` Sinclair Yeh
` (4 preceding siblings ...)
(?)
@ 2015-12-01 22:18 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86; +Cc: Sinclair Yeh, pv-drivers, Ingo Molnar, Thomas Gleixner, linux-kernel
We should no longer have mailing address of Free Software
Foundation in the copyright header. Since this patch series
touches this file, taking this opportunity to fix the header.
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: pv-drivers@vmware.com
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
---
arch/x86/kernel/cpu/vmware.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 1837f66..850a940 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -15,10 +15,6 @@
* NON INFRINGEMENT. See the GNU General Public License for more
* details.
*
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
*/
#include <linux/dmi.h>
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
2015-12-01 22:18 ` Sinclair Yeh
` (5 preceding siblings ...)
(?)
@ 2015-12-01 22:18 ` Sinclair Yeh
2015-12-01 22:38 ` Xavier Deguillard
2015-12-01 22:38 ` Xavier Deguillard
-1 siblings, 2 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: Sinclair Yeh, pv-drivers, Xavier Deguillard, linux-kernel,
virtualization
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: pv-drivers@vmware.com
Cc: Xavier Deguillard <xdeguillard@vmware.com>
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
---
drivers/misc/vmw_balloon.c | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index ffb5634..90a0d07 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -43,6 +43,7 @@
#include <linux/debugfs.h>
#include <linux/seq_file.h>
#include <asm/hypervisor.h>
+#include <asm/vmware.h>
MODULE_AUTHOR("VMware, Inc.");
MODULE_DESCRIPTION("VMware Memory Control (Balloon) Driver");
@@ -142,23 +143,17 @@ enum vmwballoon_capabilities {
#define VMW_BALLOON_SUCCESS_WITH_CAPABILITIES (0x03000000)
-#define VMWARE_BALLOON_CMD(cmd, data, result) \
-({ \
- unsigned long __status, __dummy1, __dummy2; \
- __asm__ __volatile__ ("inl %%dx" : \
- "=a"(__status), \
- "=c"(__dummy1), \
- "=d"(__dummy2), \
- "=b"(result) : \
- "0"(VMW_BALLOON_HV_MAGIC), \
- "1"(VMW_BALLOON_CMD_##cmd), \
- "2"(VMW_BALLOON_HV_PORT), \
- "3"(data) : \
- "memory"); \
- if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
- result = __dummy1; \
- result &= -1UL; \
- __status & -1UL; \
+#define VMWARE_BALLOON_CMD(cmd, data, result) \
+({ \
+ unsigned long __status, __dummy1, __dummy2; \
+ unsigned long __si = 0, __di = 0; \
+ VMW_PORT(data, VMW_BALLOON_CMD_##cmd, VMW_BALLOON_HV_PORT, \
+ VMW_BALLOON_HV_MAGIC, \
+ __status, result, __dummy1, __dummy2, __si, __di);\
+ if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
+ result = __dummy1; \
+ result &= -1UL; \
+ __status & -1UL; \
})
#ifdef CONFIG_DEBUG_FS
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* Re: [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
2015-12-01 22:18 ` [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro Sinclair Yeh
@ 2015-12-01 22:38 ` Xavier Deguillard
2015-12-01 23:17 ` Sinclair Yeh
2015-12-01 23:17 ` Sinclair Yeh
2015-12-01 22:38 ` Xavier Deguillard
1 sibling, 2 replies; 96+ messages in thread
From: Xavier Deguillard @ 2015-12-01 22:38 UTC (permalink / raw)
To: Sinclair Yeh; +Cc: x86, pv-drivers, linux-kernel, virtualization
Hey Sinclair,
On Tue, Dec 01, 2015 at 02:18:52PM -0800, Sinclair Yeh wrote:
> +#define VMWARE_BALLOON_CMD(cmd, data, result) \
> +({ \
> + unsigned long __status, __dummy1, __dummy2; \
> + unsigned long __si = 0, __di = 0; \
> + VMW_PORT(data, VMW_BALLOON_CMD_##cmd, VMW_BALLOON_HV_PORT, \
> + VMW_BALLOON_HV_MAGIC, \
> + __status, result, __dummy1, __dummy2, __si, __di);\
> + if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
> + result = __dummy1; \
> + result &= -1UL; \
> + __status & -1UL; \
> })
You need to indent the '\' with tabs only, and it looks like spaces are
present here (which is also why they don't look aligned).
Other than that I'm good with this:
Acked-by: Xavier Deguillard <xdeguillard@vmware.com>
Xavier
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
2015-12-01 22:38 ` Xavier Deguillard
@ 2015-12-01 23:17 ` Sinclair Yeh
2015-12-01 23:17 ` Sinclair Yeh
1 sibling, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 23:17 UTC (permalink / raw)
To: Xavier Deguillard; +Cc: pv-drivers, x86, linux-kernel, virtualization
Thanks! Done.
On Tue, Dec 01, 2015 at 02:38:01PM -0800, Xavier Deguillard wrote:
> Hey Sinclair,
>
> On Tue, Dec 01, 2015 at 02:18:52PM -0800, Sinclair Yeh wrote:
> > +#define VMWARE_BALLOON_CMD(cmd, data, result) \
> > +({ \
> > + unsigned long __status, __dummy1, __dummy2; \
> > + unsigned long __si = 0, __di = 0; \
> > + VMW_PORT(data, VMW_BALLOON_CMD_##cmd, VMW_BALLOON_HV_PORT, \
> > + VMW_BALLOON_HV_MAGIC, \
> > + __status, result, __dummy1, __dummy2, __si, __di);\
> > + if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
> > + result = __dummy1; \
> > + result &= -1UL; \
> > + __status & -1UL; \
> > })
>
> You need to indent the '\' with tabs only, and it looks like spaces are
> present here (which is also why they don't look aligned).
>
> Other than that I'm good with this:
>
> Acked-by: Xavier Deguillard <xdeguillard@vmware.com>
>
> Xavier
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
2015-12-01 22:38 ` Xavier Deguillard
2015-12-01 23:17 ` Sinclair Yeh
@ 2015-12-01 23:17 ` Sinclair Yeh
1 sibling, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 23:17 UTC (permalink / raw)
To: Xavier Deguillard; +Cc: x86, pv-drivers, linux-kernel, virtualization
Thanks! Done.
On Tue, Dec 01, 2015 at 02:38:01PM -0800, Xavier Deguillard wrote:
> Hey Sinclair,
>
> On Tue, Dec 01, 2015 at 02:18:52PM -0800, Sinclair Yeh wrote:
> > +#define VMWARE_BALLOON_CMD(cmd, data, result) \
> > +({ \
> > + unsigned long __status, __dummy1, __dummy2; \
> > + unsigned long __si = 0, __di = 0; \
> > + VMW_PORT(data, VMW_BALLOON_CMD_##cmd, VMW_BALLOON_HV_PORT, \
> > + VMW_BALLOON_HV_MAGIC, \
> > + __status, result, __dummy1, __dummy2, __si, __di);\
> > + if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
> > + result = __dummy1; \
> > + result &= -1UL; \
> > + __status & -1UL; \
> > })
>
> You need to indent the '\' with tabs only, and it looks like spaces are
> present here (which is also why they don't look aligned).
>
> Other than that I'm good with this:
>
> Acked-by: Xavier Deguillard <xdeguillard@vmware.com>
>
> Xavier
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
2015-12-01 22:18 ` [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro Sinclair Yeh
2015-12-01 22:38 ` Xavier Deguillard
@ 2015-12-01 22:38 ` Xavier Deguillard
1 sibling, 0 replies; 96+ messages in thread
From: Xavier Deguillard @ 2015-12-01 22:38 UTC (permalink / raw)
To: Sinclair Yeh; +Cc: pv-drivers, x86, linux-kernel, virtualization
Hey Sinclair,
On Tue, Dec 01, 2015 at 02:18:52PM -0800, Sinclair Yeh wrote:
> +#define VMWARE_BALLOON_CMD(cmd, data, result) \
> +({ \
> + unsigned long __status, __dummy1, __dummy2; \
> + unsigned long __si = 0, __di = 0; \
> + VMW_PORT(data, VMW_BALLOON_CMD_##cmd, VMW_BALLOON_HV_PORT, \
> + VMW_BALLOON_HV_MAGIC, \
> + __status, result, __dummy1, __dummy2, __si, __di);\
> + if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
> + result = __dummy1; \
> + result &= -1UL; \
> + __status & -1UL; \
> })
You need to indent the '\' with tabs only, and it looks like spaces are
present here (which is also why they don't look aligned).
Other than that I'm good with this:
Acked-by: Xavier Deguillard <xdeguillard@vmware.com>
Xavier
^ permalink raw reply [flat|nested] 96+ messages in thread
* [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
2015-12-01 22:18 ` Sinclair Yeh
` (6 preceding siblings ...)
(?)
@ 2015-12-01 22:18 ` Sinclair Yeh
-1 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: pv-drivers, Xavier Deguillard, virtualization, linux-kernel,
Sinclair Yeh
Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Alok N Kataria <akataria@vmware.com>
Cc: pv-drivers@vmware.com
Cc: Xavier Deguillard <xdeguillard@vmware.com>
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
---
drivers/misc/vmw_balloon.c | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index ffb5634..90a0d07 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -43,6 +43,7 @@
#include <linux/debugfs.h>
#include <linux/seq_file.h>
#include <asm/hypervisor.h>
+#include <asm/vmware.h>
MODULE_AUTHOR("VMware, Inc.");
MODULE_DESCRIPTION("VMware Memory Control (Balloon) Driver");
@@ -142,23 +143,17 @@ enum vmwballoon_capabilities {
#define VMW_BALLOON_SUCCESS_WITH_CAPABILITIES (0x03000000)
-#define VMWARE_BALLOON_CMD(cmd, data, result) \
-({ \
- unsigned long __status, __dummy1, __dummy2; \
- __asm__ __volatile__ ("inl %%dx" : \
- "=a"(__status), \
- "=c"(__dummy1), \
- "=d"(__dummy2), \
- "=b"(result) : \
- "0"(VMW_BALLOON_HV_MAGIC), \
- "1"(VMW_BALLOON_CMD_##cmd), \
- "2"(VMW_BALLOON_HV_PORT), \
- "3"(data) : \
- "memory"); \
- if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
- result = __dummy1; \
- result &= -1UL; \
- __status & -1UL; \
+#define VMWARE_BALLOON_CMD(cmd, data, result) \
+({ \
+ unsigned long __status, __dummy1, __dummy2; \
+ unsigned long __si = 0, __di = 0; \
+ VMW_PORT(data, VMW_BALLOON_CMD_##cmd, VMW_BALLOON_HV_PORT, \
+ VMW_BALLOON_HV_MAGIC, \
+ __status, result, __dummy1, __dummy2, __si, __di);\
+ if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START) \
+ result = __dummy1; \
+ result &= -1UL; \
+ __status & -1UL; \
})
#ifdef CONFIG_DEBUG_FS
--
1.9.1
^ permalink raw reply related [flat|nested] 96+ messages in thread
* Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros
2015-12-01 22:18 ` Sinclair Yeh
` (7 preceding siblings ...)
(?)
@ 2015-12-01 22:32 ` Xavier Deguillard
-1 siblings, 0 replies; 96+ messages in thread
From: Xavier Deguillard @ 2015-12-01 22:32 UTC (permalink / raw)
To: Sinclair Yeh
Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Alok Kataria,
linux-kernel, virtualization
Hey Sinclair,
On Tue, Dec 01, 2015 at 02:18:47PM -0800, Sinclair Yeh wrote:
> +/**
> + * Hypervisor-specific bi-directional communication channel. Should never
> + * execute on bare metal hardware. The caller must make sure to check for
> + * supported hypervisor before using these macros.
> + *
> + * Several of the parameters are both input and output and must be initialized.
> + *
> + * @in1: [IN] Message Len or Message Cmd (HB)
> + * @in2: [IN] Message Len (HB) or Message Cmd
Can you make in1 always be the "Message Cmd" and in2 always be the
"Message len"?
> + * @port_num: [IN] port number + [channel id]
> + * @magic: [IN] hypervisor magic value
> + * @eax: [OUT] value of EAX register
> + * @ebx: [OUT] e.g. status from an HB message status command
> + * @ecx: [OUT] e.g. status from a non-HB message status command
> + * @edx: [OUT] e.g. channel id
> + * @si: [INOUT] set to 0 if not used
> + * @di: [INOUT] set to 0 if not used
> + * @bp: [INOUT] set to 0 if not used
> + */
> +#define VMW_PORT(in1, in2, port_num, magic, eax, ebx, ecx, edx, si, di) \
> +({ \
> + __asm__ __volatile__ ("inl %%dx" : \
Are those '\' aligned in the code?
> +
> +#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \
> + eax, ebx, ecx, edx, si, di, bp) \
> +({ \
> + __asm__ __volatile__ ("movq %13, %%rbp;" \
Same here.
> +
> +#define VMW_PORT_HB_IN(in1, in2, port_num, magic, \
> + eax, ebx, ecx, edx, si, di, bp) \
> +({ \
> + __asm__ __volatile__ ("push %%rbp; movq %13, %%rbp;" \
Same.
Xavier
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros
2015-12-01 22:18 ` Sinclair Yeh
` (8 preceding siblings ...)
(?)
@ 2015-12-01 22:32 ` Xavier Deguillard
-1 siblings, 0 replies; 96+ messages in thread
From: Xavier Deguillard @ 2015-12-01 22:32 UTC (permalink / raw)
To: Sinclair Yeh
Cc: x86, linux-kernel, virtualization, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner, Alok Kataria
Hey Sinclair,
On Tue, Dec 01, 2015 at 02:18:47PM -0800, Sinclair Yeh wrote:
> +/**
> + * Hypervisor-specific bi-directional communication channel. Should never
> + * execute on bare metal hardware. The caller must make sure to check for
> + * supported hypervisor before using these macros.
> + *
> + * Several of the parameters are both input and output and must be initialized.
> + *
> + * @in1: [IN] Message Len or Message Cmd (HB)
> + * @in2: [IN] Message Len (HB) or Message Cmd
Can you make in1 always be the "Message Cmd" and in2 always be the
"Message len"?
> + * @port_num: [IN] port number + [channel id]
> + * @magic: [IN] hypervisor magic value
> + * @eax: [OUT] value of EAX register
> + * @ebx: [OUT] e.g. status from an HB message status command
> + * @ecx: [OUT] e.g. status from a non-HB message status command
> + * @edx: [OUT] e.g. channel id
> + * @si: [INOUT] set to 0 if not used
> + * @di: [INOUT] set to 0 if not used
> + * @bp: [INOUT] set to 0 if not used
> + */
> +#define VMW_PORT(in1, in2, port_num, magic, eax, ebx, ecx, edx, si, di) \
> +({ \
> + __asm__ __volatile__ ("inl %%dx" : \
Are those '\' aligned in the code?
> +
> +#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \
> + eax, ebx, ecx, edx, si, di, bp) \
> +({ \
> + __asm__ __volatile__ ("movq %13, %%rbp;" \
Same here.
> +
> +#define VMW_PORT_HB_IN(in1, in2, port_num, magic, \
> + eax, ebx, ecx, edx, si, di, bp) \
> +({ \
> + __asm__ __volatile__ ("push %%rbp; movq %13, %%rbp;" \
Same.
Xavier
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros
2015-12-01 22:18 ` Sinclair Yeh
` (9 preceding siblings ...)
(?)
@ 2015-12-01 22:49 ` H. Peter Anvin
2015-12-04 22:33 ` Sinclair Yeh
-1 siblings, 1 reply; 96+ messages in thread
From: H. Peter Anvin @ 2015-12-01 22:49 UTC (permalink / raw)
To: Sinclair Yeh, x86
Cc: Thomas Gleixner, Ingo Molnar, Alok Kataria, linux-kernel,
virtualization, Xavier Deguillard
On 12/01/15 14:18, Sinclair Yeh wrote:
> These macros will be used by multiple VMWare modules for handling
> host communication.
> + __asm__ __volatile__ ("inl %%dx" : \
This is odd at best; the standard assembly form of this instruction is:
inl (%dx),%eax
Also, we don't need the underscored forms of asm and volatile for kernel
code.
> + __asm__ __volatile__ ("movq %13, %%rbp;" \
> + "cld; rep outsb; " \
> + "movq %%rbp, %6" : \
cld shouldn't be necessary here, DF=0 is part of the normal ABI environment.
You also don't save/restore %rbp here, but you do below? Seems very odd.
It might be better do so something like:
+#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \
+ eax, ebx, ecx, edx, si, di, bp) \
+({ \
+ __asm__ __volatile__ ("xchgq %6, %%rbp;" \
+ "cld; rep outsb; " \
+ "xchgq %%rbp, %6" : \
+ "=a"(eax), \
+ "=b"(ebx), \
+ "=c"(ecx), \
+ "=d"(edx), \
+ "+S"(si), \
+ "+D"(di), \
+ "+r"(bp) : \
+ "a"(magic), \
+ "b"(in1), \
+ "c"(in2), \
+ "d"(port_num) : \
+ "memory", "cc"); \
+})
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros
2015-12-01 22:49 ` H. Peter Anvin
@ 2015-12-04 22:33 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-04 22:33 UTC (permalink / raw)
To: H. Peter Anvin
Cc: x86, Thomas Gleixner, Ingo Molnar, Alok Kataria, linux-kernel,
virtualization, Xavier Deguillard
Thanks Peter.
On Tue, Dec 01, 2015 at 02:49:11PM -0800, H. Peter Anvin wrote:
> On 12/01/15 14:18, Sinclair Yeh wrote:
> > These macros will be used by multiple VMWare modules for handling
> > host communication.
>
> > + __asm__ __volatile__ ("inl %%dx" : \
>
> This is odd at best; the standard assembly form of this instruction is:
>
> inl (%dx),%eax
Ok, I'm not sure why this worked and compiled before. Fixed.
>
> Also, we don't need the underscored forms of asm and volatile for kernel
> code.
>
> > + __asm__ __volatile__ ("movq %13, %%rbp;" \
> > + "cld; rep outsb; " \
> > + "movq %%rbp, %6" : \
>
> cld shouldn't be necessary here, DF=0 is part of the normal ABI environment.
>
> You also don't save/restore %rbp here, but you do below? Seems very odd.
Good catch. Fixed.
>
> It might be better do so something like:
>
> +#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \
> + eax, ebx, ecx, edx, si, di, bp) \
> +({ \
> + __asm__ __volatile__ ("xchgq %6, %%rbp;" \
> + "cld; rep outsb; " \
> + "xchgq %%rbp, %6" : \
> + "=a"(eax), \
> + "=b"(ebx), \
> + "=c"(ecx), \
> + "=d"(edx), \
> + "+S"(si), \
> + "+D"(di), \
> + "+r"(bp) : \
> + "a"(magic), \
> + "b"(in1), \
> + "c"(in2), \
> + "d"(port_num) : \
> + "memory", "cc"); \
> +})
This is great. Changed.
Updated patch set incoming.
Sinclair
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros
@ 2015-12-04 22:33 ` Sinclair Yeh
0 siblings, 0 replies; 96+ messages in thread
From: Sinclair Yeh @ 2015-12-04 22:33 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Xavier Deguillard, x86, linux-kernel, virtualization,
Ingo Molnar, Thomas Gleixner, Alok Kataria
Thanks Peter.
On Tue, Dec 01, 2015 at 02:49:11PM -0800, H. Peter Anvin wrote:
> On 12/01/15 14:18, Sinclair Yeh wrote:
> > These macros will be used by multiple VMWare modules for handling
> > host communication.
>
> > + __asm__ __volatile__ ("inl %%dx" : \
>
> This is odd at best; the standard assembly form of this instruction is:
>
> inl (%dx),%eax
Ok, I'm not sure why this worked and compiled before. Fixed.
>
> Also, we don't need the underscored forms of asm and volatile for kernel
> code.
>
> > + __asm__ __volatile__ ("movq %13, %%rbp;" \
> > + "cld; rep outsb; " \
> > + "movq %%rbp, %6" : \
>
> cld shouldn't be necessary here, DF=0 is part of the normal ABI environment.
>
> You also don't save/restore %rbp here, but you do below? Seems very odd.
Good catch. Fixed.
>
> It might be better do so something like:
>
> +#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \
> + eax, ebx, ecx, edx, si, di, bp) \
> +({ \
> + __asm__ __volatile__ ("xchgq %6, %%rbp;" \
> + "cld; rep outsb; " \
> + "xchgq %%rbp, %6" : \
> + "=a"(eax), \
> + "=b"(ebx), \
> + "=c"(ecx), \
> + "=d"(edx), \
> + "+S"(si), \
> + "+D"(di), \
> + "+r"(bp) : \
> + "a"(magic), \
> + "b"(in1), \
> + "c"(in2), \
> + "d"(port_num) : \
> + "memory", "cc"); \
> +})
This is great. Changed.
Updated patch set incoming.
Sinclair
^ permalink raw reply [flat|nested] 96+ messages in thread
* Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros
2015-12-01 22:18 ` Sinclair Yeh
` (10 preceding siblings ...)
(?)
@ 2015-12-01 22:49 ` H. Peter Anvin
-1 siblings, 0 replies; 96+ messages in thread
From: H. Peter Anvin @ 2015-12-01 22:49 UTC (permalink / raw)
To: Sinclair Yeh, x86
Cc: Xavier Deguillard, linux-kernel, virtualization, Ingo Molnar,
Thomas Gleixner, Alok Kataria
On 12/01/15 14:18, Sinclair Yeh wrote:
> These macros will be used by multiple VMWare modules for handling
> host communication.
> + __asm__ __volatile__ ("inl %%dx" : \
This is odd at best; the standard assembly form of this instruction is:
inl (%dx),%eax
Also, we don't need the underscored forms of asm and volatile for kernel
code.
> + __asm__ __volatile__ ("movq %13, %%rbp;" \
> + "cld; rep outsb; " \
> + "movq %%rbp, %6" : \
cld shouldn't be necessary here, DF=0 is part of the normal ABI environment.
You also don't save/restore %rbp here, but you do below? Seems very odd.
It might be better do so something like:
+#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \
+ eax, ebx, ecx, edx, si, di, bp) \
+({ \
+ __asm__ __volatile__ ("xchgq %6, %%rbp;" \
+ "cld; rep outsb; " \
+ "xchgq %%rbp, %6" : \
+ "=a"(eax), \
+ "=b"(ebx), \
+ "=c"(ecx), \
+ "=d"(edx), \
+ "+S"(si), \
+ "+D"(di), \
+ "+r"(bp) : \
+ "a"(magic), \
+ "b"(in1), \
+ "c"(in2), \
+ "d"(port_num) : \
+ "memory", "cc"); \
+})
^ permalink raw reply [flat|nested] 96+ messages in thread