On 12/02/2015 01:04 AM, Greg Kroah-Hartman wrote:

On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:

quoted

Hi,

On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:

quoted

On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh [off-list ref] wrote:

quoted

Hi,

<snip>

quoted

quoted

quoted

quoted

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