* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
From: Dmitry Torokhov @ 2015-12-01 23:56 UTC (permalink / raw)
To: Sinclair Yeh
Cc: Arnd Bergmann, pv-drivers@vmware.com, Greg Kroah-Hartman, X86 ML,
lkml, virtualization, linux-graphics-maintainer@vmware.com,
linux-input@vger.kernel.org
In-Reply-To: <20151201225420.GA11210@syeh-linux>
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
* Re: [PATCH 4/6] Input: Remove vmmouse port reservation
From: Dmitry Torokhov @ 2015-12-01 23:52 UTC (permalink / raw)
To: Sinclair Yeh
Cc: pv-drivers@vmware.com, X86 ML,
linux-graphics-maintainer@vmware.com, lkml, virtualization
In-Reply-To: <20151201230422.GA11239@syeh-linux>
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
* Re: [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
From: Sinclair Yeh @ 2015-12-01 23:17 UTC (permalink / raw)
To: Xavier Deguillard; +Cc: pv-drivers, x86, linux-kernel, virtualization
In-Reply-To: <20151201223801.GN24642@xdeguillard-ws.eng.vmware.com>
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
* Re: [PATCH 4/6] Input: Remove vmmouse port reservation
From: Sinclair Yeh @ 2015-12-01 23:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: pv-drivers@vmware.com, X86 ML,
linux-graphics-maintainer@vmware.com, lkml, virtualization
In-Reply-To: <CAKdAkRTHPSKOePts8cKAHc+PLAgN83skhfPqoDZUvoPXae5-pQ@mail.gmail.com>
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
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
From: Sinclair Yeh @ 2015-12-01 22:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Arnd Bergmann, pv-drivers@vmware.com, Greg Kroah-Hartman, X86 ML,
lkml, virtualization, linux-graphics-maintainer@vmware.com,
linux-input@vger.kernel.org
In-Reply-To: <CAKdAkRTVJ_DWD6uYhp+qd1uYwe3rPguDFAMXpa5=8CwVuO16NQ@mail.gmail.com>
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
* Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros
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
In-Reply-To: <1449008332-9394-1-git-send-email-syeh@vmware.com>
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
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
From: Dmitry Torokhov @ 2015-12-01 22:45 UTC (permalink / raw)
To: Sinclair Yeh
Cc: Arnd Bergmann, pv-drivers@vmware.com, Greg Kroah-Hartman, X86 ML,
lkml, virtualization, linux-graphics-maintainer@vmware.com,
linux-input@vger.kernel.org
In-Reply-To: <20151201223255.GA10753@syeh-linux>
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
* Re: [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
From: Xavier Deguillard @ 2015-12-01 22:38 UTC (permalink / raw)
To: Sinclair Yeh; +Cc: pv-drivers, x86, linux-kernel, virtualization
In-Reply-To: <1449008332-9394-6-git-send-email-syeh@vmware.com>
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
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
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
In-Reply-To: <20151201222414.GH3740@dtor-ws>
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
* Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros
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
In-Reply-To: <1449008332-9394-1-git-send-email-syeh@vmware.com>
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
* Re: [PATCH 4/6] Input: Remove vmmouse port reservation
From: Dmitry Torokhov @ 2015-12-01 22:30 UTC (permalink / raw)
To: Sinclair Yeh
Cc: pv-drivers@vmware.com, X86 ML,
linux-graphics-maintainer@vmware.com, lkml, virtualization
In-Reply-To: <1449008332-9394-4-git-send-email-syeh@vmware.com>
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
* Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
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
In-Reply-To: <1449008332-9394-3-git-send-email-syeh@vmware.com>
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
* [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: pv-drivers, Xavier Deguillard, virtualization, linux-kernel,
Sinclair Yeh
In-Reply-To: <1449008332-9394-1-git-send-email-syeh@vmware.com>
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
* [PATCH 4/6] Input: Remove vmmouse port reservation
From: Sinclair Yeh @ 2015-12-01 22:18 UTC (permalink / raw)
To: x86
Cc: pv-drivers, virtualization, linux-graphics-maintainer,
linux-kernel, Sinclair Yeh
In-Reply-To: <1449008332-9394-1-git-send-email-syeh@vmware.com>
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
* [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
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
In-Reply-To: <1449008332-9394-1-git-send-email-syeh@vmware.com>
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
* [PATCH 2/6] x86: Update vmware.c to use the common VMW_PORT macros
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
In-Reply-To: <1449008332-9394-1-git-send-email-syeh@vmware.com>
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
* [PATCH 1/6] x86: Add VMWare Host Communication Macros
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
In-Reply-To: <1449008047-8252-1-git-send-email-syeh@vmware.com>
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
* Re: [RFC PATCH 0/9] vhost-nvme: new qemu nvme backend using nvme target
From: Paolo Bonzini @ 2015-12-01 16:59 UTC (permalink / raw)
To: Ming Lin; +Cc: qemu-devel, Christoph Hellwig, linux-nvme, virtualization
In-Reply-To: <1448987171.3041.2.camel@hasee>
> What do you think about virtio-nvme+vhost-nvme?
What would be the advantage over virtio-blk? Multiqueue is not supported
by QEMU but it's already supported by Linux (commit 6a27b656fc).
To me, the advantage of nvme is that it provides more than decent performance on
unmodified Windows guests, and thanks to your vendor extension can be used
on Linux as well with speeds comparable to virtio-blk. So it's potentially
a very good choice for a cloud provider that wants to support Windows guests
(together with e.g. a fast SAS emulated controller to replace virtio-scsi,
and emulated igb or ixgbe to replace virtio-net).
Which features are supported by NVMe and not virtio-blk?
Paolo
> I also have patch for vritio-nvme:
> https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-split/virtio
>
> Just need to change vhost-nvme to work with it.
>
> >
> > Paolo
> >
> > > Still tuning.
>
>
>
^ permalink raw reply
* Re: [RFC PATCH 0/9] vhost-nvme: new qemu nvme backend using nvme target
From: Ming Lin @ 2015-12-01 16:26 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Christoph Hellwig, linux-nvme, virtualization
In-Reply-To: <565DC48B.6030903@redhat.com>
On Tue, 2015-12-01 at 17:02 +0100, Paolo Bonzini wrote:
>
> On 01/12/2015 00:20, Ming Lin wrote:
> > qemu-nvme: 148MB/s
> > vhost-nvme + google-ext: 230MB/s
> > qemu-nvme + google-ext + eventfd: 294MB/s
> > virtio-scsi: 296MB/s
> > virtio-blk: 344MB/s
> >
> > "vhost-nvme + google-ext" didn't get good enough performance.
>
> I'd expect it to be on par of qemu-nvme with ioeventfd but the question
> is: why should it be better? For vhost-net, the answer is that more
> zerocopy can be done if you put the data path in the kernel.
>
> But qemu-nvme is already using io_submit for the data path, perhaps
> there's not much to gain from vhost-nvme...
What do you think about virtio-nvme+vhost-nvme?
I also have patch for vritio-nvme:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-split/virtio
Just need to change vhost-nvme to work with it.
>
> Paolo
>
> > Still tuning.
^ permalink raw reply
* Re: [PATCH] virtio: Do not drop __GFP_HIGH in alloc_indirect
From: Will Deacon @ 2015-12-01 16:17 UTC (permalink / raw)
To: Michal Hocko; +Cc: virtualization, Michal Hocko, LKML, Michael S. Tsirkin
In-Reply-To: <1448980369-27130-1-git-send-email-mhocko@kernel.org>
On Tue, Dec 01, 2015 at 03:32:49PM +0100, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> b92b1b89a33c ("virtio: force vring descriptors to be allocated from
> lowmem") tried to exclude highmem pages for descriptors so it cleared
> __GFP_HIGHMEM from a given gfp mask. The patch also cleared __GFP_HIGH
> which doesn't make much sense for this fix because __GFP_HIGH only
> controls access to memory reserves and it doesn't have any influence
> on the zone selection. Some of the call paths use GFP_ATOMIC and
> dropping __GFP_HIGH will reduce their changes for success because the
> lack of access to memory reserves.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
> Hi,
> I have stumbled over this code while looking at other issue [1]. I think
> that using __GFP_HIGH simply got there because of its confusing name. It
> doesn't have anything to do with the highmem zone.
>
> The patch is based on the current linux-next.
>
> I think that clearing __GFP_HIGHMEM is bogus in the current code because
> all code paths either use GFP_KERNEL or GFP_ATOMIC and those do not fall
> back to the highmem zone but I have kept it because clearing the flag
> cannot be harmful.
>
> [1] http://lkml.kernel.org/r/87h9k4kzcv.fsf%40yhuang-dev.intel.com
>
> drivers/virtio/virtio_ring.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks for cleaning this up:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 14e7ce9b3e96..734de927c89d 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -110,7 +110,7 @@ static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
> * otherwise virt_to_phys will give us bogus addresses in the
> * virtqueue.
> */
> - gfp &= ~(__GFP_HIGHMEM | __GFP_HIGH);
> + gfp &= ~__GFP_HIGHMEM;
>
> desc = kmalloc(total_sg * sizeof(struct vring_desc), gfp);
> if (!desc)
> --
> 2.6.2
>
^ permalink raw reply
* Re: [RFC PATCH 0/9] vhost-nvme: new qemu nvme backend using nvme target
From: Paolo Bonzini @ 2015-12-01 16:02 UTC (permalink / raw)
To: Ming Lin; +Cc: qemu-devel, Christoph Hellwig, linux-nvme, virtualization
In-Reply-To: <1448925639.27669.7.camel@ssi>
On 01/12/2015 00:20, Ming Lin wrote:
> qemu-nvme: 148MB/s
> vhost-nvme + google-ext: 230MB/s
> qemu-nvme + google-ext + eventfd: 294MB/s
> virtio-scsi: 296MB/s
> virtio-blk: 344MB/s
>
> "vhost-nvme + google-ext" didn't get good enough performance.
I'd expect it to be on par of qemu-nvme with ioeventfd but the question
is: why should it be better? For vhost-net, the answer is that more
zerocopy can be done if you put the data path in the kernel.
But qemu-nvme is already using io_submit for the data path, perhaps
there's not much to gain from vhost-nvme...
Paolo
> Still tuning.
^ permalink raw reply
* Re: [PATCH net-next 3/3] vhost_net: basic polling support
From: Michael S. Tsirkin @ 2015-12-01 14:43 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <565D2D7D.4010903@redhat.com>
On Tue, Dec 01, 2015 at 01:17:49PM +0800, Jason Wang wrote:
>
>
> On 11/30/2015 06:44 PM, Michael S. Tsirkin wrote:
> > On Wed, Nov 25, 2015 at 03:11:29PM +0800, Jason Wang wrote:
> >> > This patch tries to poll for new added tx buffer or socket receive
> >> > queue for a while at the end of tx/rx processing. The maximum time
> >> > spent on polling were specified through a new kind of vring ioctl.
> >> >
> >> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > One further enhancement would be to actually poll
> > the underlying device. This should be reasonably
> > straight-forward with macvtap (especially in the
> > passthrough mode).
> >
> >
>
> Yes, it is. I have some patches to do this by replacing
> skb_queue_empty() with sk_busy_loop() but for tap.
We probably don't want to do this unconditionally, though.
> Tests does not show
> any improvement but some regression.
Did you add code to call sk_mark_napi_id on tap then?
sk_busy_loop won't do anything useful without.
> Maybe it's better to test macvtap.
Same thing ...
--
MST
^ permalink raw reply
* [PATCH] virtio: Do not drop __GFP_HIGH in alloc_indirect
From: Michal Hocko @ 2015-12-01 14:32 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Michal Hocko, Will Deacon, LKML, virtualization
From: Michal Hocko <mhocko@suse.com>
b92b1b89a33c ("virtio: force vring descriptors to be allocated from
lowmem") tried to exclude highmem pages for descriptors so it cleared
__GFP_HIGHMEM from a given gfp mask. The patch also cleared __GFP_HIGH
which doesn't make much sense for this fix because __GFP_HIGH only
controls access to memory reserves and it doesn't have any influence
on the zone selection. Some of the call paths use GFP_ATOMIC and
dropping __GFP_HIGH will reduce their changes for success because the
lack of access to memory reserves.
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
Hi,
I have stumbled over this code while looking at other issue [1]. I think
that using __GFP_HIGH simply got there because of its confusing name. It
doesn't have anything to do with the highmem zone.
The patch is based on the current linux-next.
I think that clearing __GFP_HIGHMEM is bogus in the current code because
all code paths either use GFP_KERNEL or GFP_ATOMIC and those do not fall
back to the highmem zone but I have kept it because clearing the flag
cannot be harmful.
[1] http://lkml.kernel.org/r/87h9k4kzcv.fsf%40yhuang-dev.intel.com
drivers/virtio/virtio_ring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 14e7ce9b3e96..734de927c89d 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -110,7 +110,7 @@ static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
* otherwise virt_to_phys will give us bogus addresses in the
* virtqueue.
*/
- gfp &= ~(__GFP_HIGHMEM | __GFP_HIGH);
+ gfp &= ~__GFP_HIGHMEM;
desc = kmalloc(total_sg * sizeof(struct vring_desc), gfp);
if (!desc)
--
2.6.2
^ permalink raw reply related
* [PATCH V2 3/3] vhost_net: basic polling support
From: Jason Wang @ 2015-12-01 6:39 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1448951985-12385-1-git-send-email-jasowang@redhat.com>
This patch tries to poll for new added tx buffer or socket receive
queue for a while at the end of tx/rx processing. The maximum time
spent on polling were specified through a new kind of vring ioctl.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 72 ++++++++++++++++++++++++++++++++++++++++++----
drivers/vhost/vhost.c | 15 ++++++++++
drivers/vhost/vhost.h | 1 +
include/uapi/linux/vhost.h | 11 +++++++
4 files changed, 94 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9eda69e..ce6da77 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -287,6 +287,41 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
rcu_read_unlock_bh();
}
+static inline unsigned long busy_clock(void)
+{
+ return local_clock() >> 10;
+}
+
+static bool vhost_can_busy_poll(struct vhost_dev *dev,
+ unsigned long endtime)
+{
+ return likely(!need_resched()) &&
+ likely(!time_after(busy_clock(), endtime)) &&
+ likely(!signal_pending(current)) &&
+ !vhost_has_work(dev) &&
+ single_task_running();
+}
+
+static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
+ struct vhost_virtqueue *vq,
+ struct iovec iov[], unsigned int iov_size,
+ unsigned int *out_num, unsigned int *in_num)
+{
+ unsigned long uninitialized_var(endtime);
+
+ if (vq->busyloop_timeout) {
+ preempt_disable();
+ endtime = busy_clock() + vq->busyloop_timeout;
+ while (vhost_can_busy_poll(vq->dev, endtime) &&
+ !vhost_vq_more_avail(vq->dev, vq))
+ cpu_relax();
+ preempt_enable();
+ }
+
+ return vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
+ out_num, in_num, NULL, NULL);
+}
+
/* Expects to be always run from workqueue - which acts as
* read-size critical section for our kind of RCU. */
static void handle_tx(struct vhost_net *net)
@@ -331,10 +366,9 @@ static void handle_tx(struct vhost_net *net)
% UIO_MAXIOV == nvq->done_idx))
break;
- head = vhost_get_vq_desc(vq, vq->iov,
- ARRAY_SIZE(vq->iov),
- &out, &in,
- NULL, NULL);
+ head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
+ ARRAY_SIZE(vq->iov),
+ &out, &in);
/* On error, stop handling until the next kick. */
if (unlikely(head < 0))
break;
@@ -435,6 +469,34 @@ static int peek_head_len(struct sock *sk)
return len;
}
+static int vhost_net_peek_head_len(struct vhost_net *net, struct sock *sk)
+{
+ struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
+ struct vhost_virtqueue *vq = &nvq->vq;
+ unsigned long uninitialized_var(endtime);
+
+ if (vq->busyloop_timeout) {
+ mutex_lock(&vq->mutex);
+ vhost_disable_notify(&net->dev, vq);
+
+ preempt_disable();
+ endtime = busy_clock() + vq->busyloop_timeout;
+
+ while (vhost_can_busy_poll(&net->dev, endtime) &&
+ skb_queue_empty(&sk->sk_receive_queue) &&
+ !vhost_vq_more_avail(&net->dev, vq))
+ cpu_relax();
+
+ preempt_enable();
+
+ if (vhost_enable_notify(&net->dev, vq))
+ vhost_poll_queue(&vq->poll);
+ mutex_unlock(&vq->mutex);
+ }
+
+ return peek_head_len(sk);
+}
+
/* This is a multi-buffer version of vhost_get_desc, that works if
* vq has read descriptors only.
* @vq - the relevant virtqueue
@@ -553,7 +615,7 @@ static void handle_rx(struct vhost_net *net)
vq->log : NULL;
mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
- while ((sock_len = peek_head_len(sock->sk))) {
+ while ((sock_len = vhost_net_peek_head_len(net, sock->sk))) {
sock_len += sock_hlen;
vhost_len = sock_len + vhost_hlen;
headcount = get_rx_bufs(vq, vq->heads, vhost_len,
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 4f45a03..b8ca873 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -285,6 +285,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->memory = NULL;
vq->is_le = virtio_legacy_is_little_endian();
vhost_vq_reset_user_be(vq);
+ vq->busyloop_timeout = 0;
}
static int vhost_worker(void *data)
@@ -747,6 +748,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
struct vhost_vring_state s;
struct vhost_vring_file f;
struct vhost_vring_addr a;
+ struct vhost_vring_busyloop_timeout t;
u32 idx;
long r;
@@ -919,6 +921,19 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
case VHOST_GET_VRING_ENDIAN:
r = vhost_get_vring_endian(vq, idx, argp);
break;
+ case VHOST_SET_VRING_BUSYLOOP_TIMEOUT:
+ if (copy_from_user(&t, argp, sizeof(t))) {
+ r = -EFAULT;
+ break;
+ }
+ vq->busyloop_timeout = t.timeout;
+ break;
+ case VHOST_GET_VRING_BUSYLOOP_TIMEOUT:
+ t.index = idx;
+ t.timeout = vq->busyloop_timeout;
+ if (copy_to_user(argp, &t, sizeof(t)))
+ r = -EFAULT;
+ break;
default:
r = -ENOIOCTLCMD;
}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 2f3c57c..4b7d4fa 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -115,6 +115,7 @@ struct vhost_virtqueue {
/* Ring endianness requested by userspace for cross-endian support. */
bool user_be;
#endif
+ u32 busyloop_timeout;
};
struct vhost_dev {
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index ab373191..eaf6c33 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -27,6 +27,11 @@ struct vhost_vring_file {
};
+struct vhost_vring_busyloop_timeout {
+ unsigned int index;
+ unsigned int timeout;
+};
+
struct vhost_vring_addr {
unsigned int index;
/* Option flags. */
@@ -126,6 +131,12 @@ struct vhost_memory {
#define VHOST_SET_VRING_CALL _IOW(VHOST_VIRTIO, 0x21, struct vhost_vring_file)
/* Set eventfd to signal an error */
#define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
+/* Set busy loop timeout */
+#define VHOST_SET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x23, \
+ struct vhost_vring_busyloop_timeout)
+/* Get busy loop timeout */
+#define VHOST_GET_VRING_BUSYLOOP_TIMEOUT _IOW(VHOST_VIRTIO, 0x24, \
+ struct vhost_vring_busyloop_timeout)
/* VHOST_NET specific defines */
--
2.5.0
^ permalink raw reply related
* [PATCH V2 2/3] vhost: introduce vhost_vq_more_avail()
From: Jason Wang @ 2015-12-01 6:39 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1448951985-12385-1-git-send-email-jasowang@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 13 +++++++++++++
drivers/vhost/vhost.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 163b365..4f45a03 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1633,6 +1633,19 @@ void vhost_add_used_and_signal_n(struct vhost_dev *dev,
}
EXPORT_SYMBOL_GPL(vhost_add_used_and_signal_n);
+bool vhost_vq_more_avail(struct vhost_dev *dev, struct vhost_virtqueue *vq)
+{
+ __virtio16 avail_idx;
+ int r;
+
+ r = __get_user(avail_idx, &vq->avail->idx);
+ if (r)
+ return false;
+
+ return vhost16_to_cpu(vq, avail_idx) != vq->avail_idx;
+}
+EXPORT_SYMBOL_GPL(vhost_vq_more_avail);
+
/* OK, now we need to know about added descriptors. */
bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
{
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 43284ad..2f3c57c 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -159,6 +159,7 @@ void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
struct vring_used_elem *heads, unsigned count);
void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
void vhost_disable_notify(struct vhost_dev *, struct vhost_virtqueue *);
+bool vhost_vq_more_avail(struct vhost_dev *, struct vhost_virtqueue *);
bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
--
2.5.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox