[PATCH 0000/0006] drivers: hv

[PATCH 0000/0006] drivers: hv

[K. Y. Srinivasan]

This patch-set does some cleanup of the KVP component. Also included is a
patch that will remove artificial limitation on the number of VCPUs that
can be assigned to the Linux guest on Hyper-V.

Regards,

K. Y

[PATCH 1/6] drivers: hv: kvp: Add/cleanup connector defines

[K. Y. Srinivasan]

The current KVP code carries some private connector related defines.
Update connector.h to have all the KVP defines. As part of this patch
get rid of some unused defines.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/hv/hv_kvp.h       |    3 ---
 include/linux/connector.h |    1 +
 tools/hv/hv_kvp_daemon.c  |    4 ----
 3 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/hv/hv_kvp.h b/drivers/hv/hv_kvp.h
index 9b765d7..c2c5bba 100644
--- a/drivers/hv/hv_kvp.h
+++ b/drivers/hv/hv_kvp.h
@@ -107,9 +107,6 @@
  * the KVP user-mode component.
  */
 
-#define CN_KVP_VAL             0x1 /* This supports queries from the kernel */
-#define CN_KVP_USER_VAL       0x2 /* This supports queries from the user */
-
 enum hv_ku_op {
 	KVP_REGISTER = 0, /* Register the user mode component */
 	KVP_KERNEL_GET, /* Kernel is requesting the value */
diff --git a/include/linux/connector.h b/include/linux/connector.h
index 3c9c54f..7638407 100644
--- a/include/linux/connector.h
+++ b/include/linux/connector.h
@@ -43,6 +43,7 @@
 #define CN_IDX_DRBD			0x8
 #define CN_VAL_DRBD			0x1
 #define CN_KVP_IDX			0x9	/* HyperV KVP */
+#define CN_KVP_VAL			0x1	/* queries from the kernel */
 
 #define CN_NETLINK_USERS		10	/* Highest index + 1 */
 
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 11224ed..2b6a2d9 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -40,15 +40,11 @@
 #include <syslog.h>
 
 /*
- * KYS: TODO. Need to register these in the kernel.
  *
  * The following definitions are shared with the in-kernel component; do not
  * change any of this without making the corresponding changes in
  * the KVP kernel component.
  */
-#define CN_KVP_IDX		0x9     /* MSFT KVP functionality */
-#define CN_KVP_VAL		0x1 /* This supports queries from the kernel */
-#define CN_KVP_USER_VAL		0x2 /* This supports queries from the user  */
 
 /*
  * KVP protocol: The user mode component first registers with the
-- 
1.7.4.1

[PATCH 2/6] drivers: hv: kvp: Move the contents of hv_kvp.h to hyperv.h

[K. Y. Srinivasan]

In preparation for consolidating all KVP related defines into a single header file
that both the kernel and user level components can use, move the contents of
hv_kvp.h into hyperv.h.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/hv/hv_kvp.c    |    2 -
 drivers/hv/hv_kvp.h    |  181 ------------------------------------------------
 drivers/hv/hv_util.c   |    3 -
 include/linux/hyperv.h |  165 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 165 insertions(+), 186 deletions(-)
 delete mode 100644 drivers/hv/hv_kvp.h

diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 0e8343f..4a6971e 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -28,8 +28,6 @@
 #include <linux/workqueue.h>
 #include <linux/hyperv.h>
 
-#include "hv_kvp.h"
-
 
 
 /*
diff --git a/drivers/hv/hv_kvp.h b/drivers/hv/hv_kvp.h
deleted file mode 100644
index c2c5bba..0000000
--- a/drivers/hv/hv_kvp.h
+++ /dev/null
@@ -1,181 +0,0 @@
-/*
- * An implementation of HyperV key value pair (KVP) functionality for Linux.
- *
- *
- * Copyright (C) 2010, Novell, Inc.
- * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published
- * by the Free Software Foundation.
- *
- * 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.
- *
- * 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.
- *
- */
-#ifndef	_KVP_H
-#define	_KVP_H_
-
-/*
- * Maximum value size - used for both key names and value data, and includes
- * any applicable NULL terminators.
- *
- * Note:  This limit is somewhat arbitrary, but falls easily within what is
- * supported for all native guests (back to Win 2000) and what is reasonable
- * for the IC KVP exchange functionality.  Note that Windows Me/98/95 are
- * limited to 255 character key names.
- *
- * MSDN recommends not storing data values larger than 2048 bytes in the
- * registry.
- *
- * Note:  This value is used in defining the KVP exchange message - this value
- * cannot be modified without affecting the message size and compatibility.
- */
-
-/*
- * bytes, including any null terminators
- */
-#define HV_KVP_EXCHANGE_MAX_VALUE_SIZE          (2048)
-
-
-/*
- * Maximum key size - the registry limit for the length of an entry name
- * is 256 characters, including the null terminator
- */
-
-#define HV_KVP_EXCHANGE_MAX_KEY_SIZE            (512)
-
-/*
- * In Linux, we implement the KVP functionality in two components:
- * 1) The kernel component which is packaged as part of the hv_utils driver
- * is responsible for communicating with the host and responsible for
- * implementing the host/guest protocol. 2) A user level daemon that is
- * responsible for data gathering.
- *
- * Host/Guest Protocol: The host iterates over an index and expects the guest
- * to assign a key name to the index and also return the value corresponding to
- * the key. The host will have atmost one KVP transaction outstanding at any
- * given point in time. The host side iteration stops when the guest returns
- * an error. Microsoft has specified the following mapping of key names to
- * host specified index:
- *
- *	Index		Key Name
- *	0		FullyQualifiedDomainName
- *	1		IntegrationServicesVersion
- *	2		NetworkAddressIPv4
- *	3		NetworkAddressIPv6
- *	4		OSBuildNumber
- *	5		OSName
- *	6		OSMajorVersion
- *	7		OSMinorVersion
- *	8		OSVersion
- *	9		ProcessorArchitecture
- *
- * The Windows host expects the Key Name and Key Value to be encoded in utf16.
- *
- * Guest Kernel/KVP Daemon Protocol: As noted earlier, we implement all of the
- * data gathering functionality in a user mode daemon. The user level daemon
- * is also responsible for binding the key name to the index as well. The
- * kernel and user-level daemon communicate using a connector channel.
- *
- * The user mode component first registers with the
- * the kernel component. Subsequently, the kernel component requests, data
- * for the specified keys. In response to this message the user mode component
- * fills in the value corresponding to the specified key. We overload the
- * sequence field in the cn_msg header to define our KVP message types.
- *
- *
- * The kernel component simply acts as a conduit for communication between the
- * Windows host and the user-level daemon. The kernel component passes up the
- * index received from the Host to the user-level daemon. If the index is
- * valid (supported), the corresponding key as well as its
- * value (both are strings) is returned. If the index is invalid
- * (not supported), a NULL key string is returned.
- */
-
-/*
- *
- * The following definitions are shared with the user-mode component; do not
- * change any of this without making the corresponding changes in
- * the KVP user-mode component.
- */
-
-enum hv_ku_op {
-	KVP_REGISTER = 0, /* Register the user mode component */
-	KVP_KERNEL_GET, /* Kernel is requesting the value */
-	KVP_KERNEL_SET, /* Kernel is providing the value */
-	KVP_USER_GET,  /* User is requesting the value */
-	KVP_USER_SET  /* User is providing the value */
-};
-
-struct hv_ku_msg {
-	__u32 kvp_index; /* Key index */
-	__u8  kvp_key[HV_KVP_EXCHANGE_MAX_KEY_SIZE]; /* Key name */
-	__u8  kvp_value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE]; /* Key  value */
-};
-
-
-
-
-#ifdef __KERNEL__
-
-/*
- * Registry value types.
- */
-
-#define REG_SZ 1
-
-enum hv_kvp_exchg_op {
-	KVP_OP_GET = 0,
-	KVP_OP_SET,
-	KVP_OP_DELETE,
-	KVP_OP_ENUMERATE,
-	KVP_OP_COUNT /* Number of operations, must be last. */
-};
-
-enum hv_kvp_exchg_pool {
-	KVP_POOL_EXTERNAL = 0,
-	KVP_POOL_GUEST,
-	KVP_POOL_AUTO,
-	KVP_POOL_AUTO_EXTERNAL,
-	KVP_POOL_AUTO_INTERNAL,
-	KVP_POOL_COUNT /* Number of pools, must be last. */
-};
-
-struct hv_kvp_hdr {
-	u8 operation;
-	u8 pool;
-};
-
-struct hv_kvp_exchg_msg_value {
-	u32 value_type;
-	u32 key_size;
-	u32 value_size;
-	u8 key[HV_KVP_EXCHANGE_MAX_KEY_SIZE];
-	u8 value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
-};
-
-struct hv_kvp_msg_enumerate {
-	u32 index;
-	struct hv_kvp_exchg_msg_value data;
-};
-
-struct hv_kvp_msg {
-	struct hv_kvp_hdr	kvp_hdr;
-	struct hv_kvp_msg_enumerate	kvp_data;
-};
-
-int hv_kvp_init(struct hv_util_service *);
-void hv_kvp_deinit(void);
-void hv_kvp_onchannelcallback(void *);
-
-#endif /* __KERNEL__ */
-#endif	/* _KVP_H */
-
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 55d58f2..dbb8b8e 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -28,9 +28,6 @@
 #include <linux/reboot.h>
 #include <linux/hyperv.h>
 
-#include "hv_kvp.h"
-
-
 static void shutdown_onchannelcallback(void *context);
 static struct hv_util_service util_shutdown = {
 	.util_cb = shutdown_onchannelcallback,
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 62b908e..7332b3f 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -25,6 +25,166 @@
 #ifndef _HYPERV_H
 #define _HYPERV_H
 
+#include <linux/types.h>
+
+/*
+ * An implementation of HyperV key value pair (KVP) functionality for Linux.
+ *
+ *
+ * Copyright (C) 2010, Novell, Inc.
+ * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
+ *
+ */
+
+/*
+ * Maximum value size - used for both key names and value data, and includes
+ * any applicable NULL terminators.
+ *
+ * Note:  This limit is somewhat arbitrary, but falls easily within what is
+ * supported for all native guests (back to Win 2000) and what is reasonable
+ * for the IC KVP exchange functionality.  Note that Windows Me/98/95 are
+ * limited to 255 character key names.
+ *
+ * MSDN recommends not storing data values larger than 2048 bytes in the
+ * registry.
+ *
+ * Note:  This value is used in defining the KVP exchange message - this value
+ * cannot be modified without affecting the message size and compatibility.
+ */
+
+/*
+ * bytes, including any null terminators
+ */
+#define HV_KVP_EXCHANGE_MAX_VALUE_SIZE          (2048)
+
+
+/*
+ * Maximum key size - the registry limit for the length of an entry name
+ * is 256 characters, including the null terminator
+ */
+
+#define HV_KVP_EXCHANGE_MAX_KEY_SIZE            (512)
+
+/*
+ * In Linux, we implement the KVP functionality in two components:
+ * 1) The kernel component which is packaged as part of the hv_utils driver
+ * is responsible for communicating with the host and responsible for
+ * implementing the host/guest protocol. 2) A user level daemon that is
+ * responsible for data gathering.
+ *
+ * Host/Guest Protocol: The host iterates over an index and expects the guest
+ * to assign a key name to the index and also return the value corresponding to
+ * the key. The host will have atmost one KVP transaction outstanding at any
+ * given point in time. The host side iteration stops when the guest returns
+ * an error. Microsoft has specified the following mapping of key names to
+ * host specified index:
+ *
+ *	Index		Key Name
+ *	0		FullyQualifiedDomainName
+ *	1		IntegrationServicesVersion
+ *	2		NetworkAddressIPv4
+ *	3		NetworkAddressIPv6
+ *	4		OSBuildNumber
+ *	5		OSName
+ *	6		OSMajorVersion
+ *	7		OSMinorVersion
+ *	8		OSVersion
+ *	9		ProcessorArchitecture
+ *
+ * The Windows host expects the Key Name and Key Value to be encoded in utf16.
+ *
+ * Guest Kernel/KVP Daemon Protocol: As noted earlier, we implement all of the
+ * data gathering functionality in a user mode daemon. The user level daemon
+ * is also responsible for binding the key name to the index as well. The
+ * kernel and user-level daemon communicate using a connector channel.
+ *
+ * The user mode component first registers with the
+ * the kernel component. Subsequently, the kernel component requests, data
+ * for the specified keys. In response to this message the user mode component
+ * fills in the value corresponding to the specified key. We overload the
+ * sequence field in the cn_msg header to define our KVP message types.
+ *
+ *
+ * The kernel component simply acts as a conduit for communication between the
+ * Windows host and the user-level daemon. The kernel component passes up the
+ * index received from the Host to the user-level daemon. If the index is
+ * valid (supported), the corresponding key as well as its
+ * value (both are strings) is returned. If the index is invalid
+ * (not supported), a NULL key string is returned.
+ */
+
+/*
+ *
+ * The following definitions are shared with the user-mode component; do not
+ * change any of this without making the corresponding changes in
+ * the KVP user-mode component.
+ */
+
+enum hv_ku_op {
+	KVP_REGISTER = 0, /* Register the user mode component */
+	KVP_KERNEL_GET, /* Kernel is requesting the value */
+	KVP_KERNEL_SET, /* Kernel is providing the value */
+	KVP_USER_GET,  /* User is requesting the value */
+	KVP_USER_SET  /* User is providing the value */
+};
+
+struct hv_ku_msg {
+	__u32 kvp_index; /* Key index */
+	__u8  kvp_key[HV_KVP_EXCHANGE_MAX_KEY_SIZE]; /* Key name */
+	__u8  kvp_value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE]; /* Key  value */
+};
+
+
+
+
+#ifdef __KERNEL__
+
+/*
+ * Registry value types.
+ */
+
+#define REG_SZ 1
+
+enum hv_kvp_exchg_op {
+	KVP_OP_GET = 0,
+	KVP_OP_SET,
+	KVP_OP_DELETE,
+	KVP_OP_ENUMERATE,
+	KVP_OP_COUNT /* Number of operations, must be last. */
+};
+
+enum hv_kvp_exchg_pool {
+	KVP_POOL_EXTERNAL = 0,
+	KVP_POOL_GUEST,
+	KVP_POOL_AUTO,
+	KVP_POOL_AUTO_EXTERNAL,
+	KVP_POOL_AUTO_INTERNAL,
+	KVP_POOL_COUNT /* Number of pools, must be last. */
+};
+
+struct hv_kvp_hdr {
+	u8 operation;
+	u8 pool;
+};
+
+struct hv_kvp_exchg_msg_value {
+	u32 value_type;
+	u32 key_size;
+	u32 value_size;
+	u8 key[HV_KVP_EXCHANGE_MAX_KEY_SIZE];
+	u8 value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
+};
+
+struct hv_kvp_msg_enumerate {
+	u32 index;
+	struct hv_kvp_exchg_msg_value data;
+};
+
+struct hv_kvp_msg {
+	struct hv_kvp_hdr	kvp_hdr;
+	struct hv_kvp_msg_enumerate	kvp_data;
+};
+
 #include <linux/scatterlist.h>
 #include <linux/list.h>
 #include <linux/uuid.h>
@@ -870,4 +1030,9 @@ struct hyperv_service_callback {
 extern void vmbus_prep_negotiate_resp(struct icmsg_hdr *,
 				      struct icmsg_negotiate *, u8 *);
 
+int hv_kvp_init(struct hv_util_service *);
+void hv_kvp_deinit(void);
+void hv_kvp_onchannelcallback(void *);
+
+#endif /* __KERNEL__ */
 #endif /* _HYPERV_H */
-- 
1.7.4.1

[PATCH 3/6] drivers: hv: Cleanup the kvp related state in hyperv.h

[K. Y. Srinivasan]

Now cleanup the hyperv.h with regards to KVP definitions.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
 include/linux/hyperv.h |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 7332b3f..802ece8 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -137,7 +137,6 @@ struct hv_ku_msg {
 
 
 
-#ifdef __KERNEL__
 
 /*
  * Registry value types.
@@ -162,29 +161,36 @@ enum hv_kvp_exchg_pool {
 	KVP_POOL_COUNT /* Number of pools, must be last. */
 };
 
+#ifndef __packed
+#define __packed __attribute__((packed))
+#endif
+
+
 struct hv_kvp_hdr {
-	u8 operation;
-	u8 pool;
-};
+	__u8 operation;
+	__u8 pool;
+	__u16 pad;
+} __packed;
 
 struct hv_kvp_exchg_msg_value {
-	u32 value_type;
-	u32 key_size;
-	u32 value_size;
-	u8 key[HV_KVP_EXCHANGE_MAX_KEY_SIZE];
-	u8 value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
-};
+	__u32 value_type;
+	__u32 key_size;
+	__u32 value_size;
+	__u8 key[HV_KVP_EXCHANGE_MAX_KEY_SIZE];
+	__u8 value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
+} __packed;
 
 struct hv_kvp_msg_enumerate {
-	u32 index;
+	__u32 index;
 	struct hv_kvp_exchg_msg_value data;
-};
+} __packed;
 
 struct hv_kvp_msg {
 	struct hv_kvp_hdr	kvp_hdr;
 	struct hv_kvp_msg_enumerate	kvp_data;
-};
+} __packed;
 
+#ifdef __KERNEL__
 #include <linux/scatterlist.h>
 #include <linux/list.h>
 #include <linux/uuid.h>
-- 
1.7.4.1

[PATCH 4/6] tools: hv: Use hyperv.h to get the KVP definitions

[K. Y. Srinivasan]

Now use hyperv.h to get the KVP defines in the KVP user-mode code.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 tools/hv/hv_kvp_daemon.c |   28 +---------------------------
 1 files changed, 1 insertions(+), 27 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index 2b6a2d9..b75523c 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -34,17 +34,12 @@
 #include <errno.h>
 #include <arpa/inet.h>
 #include <linux/connector.h>
+#include <linux/hyperv.h>
 #include <linux/netlink.h>
 #include <ifaddrs.h>
 #include <netdb.h>
 #include <syslog.h>
 
-/*
- *
- * The following definitions are shared with the in-kernel component; do not
- * change any of this without making the corresponding changes in
- * the KVP kernel component.
- */
 
 /*
  * KVP protocol: The user mode component first registers with the
@@ -56,25 +51,8 @@
  * We use this infrastructure for also supporting queries from user mode
  * application for state that may be maintained in the KVP kernel component.
  *
- * XXXKYS: Have a shared header file between the user and kernel (TODO)
  */
 
-enum kvp_op {
-	KVP_REGISTER = 0, /* Register the user mode component*/
-	KVP_KERNEL_GET, /*Kernel is requesting the value for the specified key*/
-	KVP_KERNEL_SET, /*Kernel is providing the value for the specified key*/
-	KVP_USER_GET, /*User is requesting the value for the specified key*/
-	KVP_USER_SET /*User is providing the value for the specified key*/
-};
-
-#define HV_KVP_EXCHANGE_MAX_KEY_SIZE	512
-#define HV_KVP_EXCHANGE_MAX_VALUE_SIZE	2048
-
-struct hv_ku_msg {
-	__u32	kvp_index;
-	__u8  kvp_key[HV_KVP_EXCHANGE_MAX_KEY_SIZE]; /* Key name */
-	__u8  kvp_value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE]; /* Key  value */
-};
 
 enum key_index {
 	FullyQualifiedDomainName = 0,
@@ -89,10 +67,6 @@ enum key_index {
 	ProcessorArchitecture
 };
 
-/*
- * End of shared definitions.
- */
-
 static char kvp_send_buffer[4096];
 static char kvp_recv_buffer[4096];
 static struct sockaddr_nl addr;
-- 
1.7.4.1

[PATCH 5/6] drivers: hv: kvp: Cleanup the kernel/user protocol

[K. Y. Srinivasan]

Now, cleanup the user/kernel KVP protocol by using the same structure
definition that is used for host/guest KVP protocol. This simplifies the code. 

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/hv/hv_kvp.c      |   41 +++++++++++++++++++++++++----------------
 include/linux/hyperv.h   |   30 +++++-------------------------
 tools/hv/hv_kvp_daemon.c |   30 +++++++++++++++---------------
 3 files changed, 45 insertions(+), 56 deletions(-)

diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 4a6971e..0ef4c1f 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -71,15 +71,20 @@ kvp_register(void)
 {
 
 	struct cn_msg *msg;
+	struct hv_kvp_msg *kvp_msg;
+	char *version;
 
-	msg = kzalloc(sizeof(*msg) + strlen(HV_DRV_VERSION) + 1 , GFP_ATOMIC);
+	msg = kzalloc(sizeof(*msg) + sizeof(struct hv_kvp_msg), GFP_ATOMIC);
 
 	if (msg) {
+		kvp_msg = (struct hv_kvp_msg *)msg->data;
+		version = kvp_msg->body.kvp_version;
 		msg->id.idx =  CN_KVP_IDX;
 		msg->id.val = CN_KVP_VAL;
-		msg->seq = KVP_REGISTER;
-		strcpy(msg->data, HV_DRV_VERSION);
-		msg->len = strlen(HV_DRV_VERSION) + 1;
+
+		kvp_msg->kvp_hdr.operation = KVP_OP_REGISTER;
+		strcpy(version, HV_DRV_VERSION);
+		msg->len = sizeof(struct hv_kvp_msg);
 		cn_netlink_send(msg, 0, GFP_ATOMIC);
 		kfree(msg);
 	}
@@ -101,23 +106,24 @@ kvp_work_func(struct work_struct *dummy)
 static void
 kvp_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp)
 {
-	struct hv_ku_msg *message;
+	struct hv_kvp_msg *message;
+	struct hv_kvp_msg_enumerate *data;
 
-	message = (struct hv_ku_msg *)msg->data;
-	if (msg->seq == KVP_REGISTER) {
+	message = (struct hv_kvp_msg *)msg->data;
+	if (message->kvp_hdr.operation == KVP_OP_REGISTER) {
 		pr_info("KVP: user-mode registering done.\n");
 		kvp_register();
 	}
 
-	if (msg->seq == KVP_USER_SET) {
+	if (message->kvp_hdr.operation == KVP_OP_ENUMERATE) {
+		data = &message->body.kvp_enum_data;
 		/*
 		 * Complete the transaction by forwarding the key value
 		 * to the host. But first, cancel the timeout.
 		 */
 		if (cancel_delayed_work_sync(&kvp_work))
-			kvp_respond_to_host(message->kvp_key,
-						message->kvp_value,
-						!strlen(message->kvp_key));
+			kvp_respond_to_host(data->data.key, data->data.value,
+					!strlen(data->data.key));
 	}
 }
 
@@ -125,6 +131,7 @@ static void
 kvp_send_key(struct work_struct *dummy)
 {
 	struct cn_msg *msg;
+	struct hv_kvp_msg *message;
 	int index = kvp_transaction.index;
 
 	msg = kzalloc(sizeof(*msg) + sizeof(struct hv_kvp_msg) , GFP_ATOMIC);
@@ -132,9 +139,11 @@ kvp_send_key(struct work_struct *dummy)
 	if (msg) {
 		msg->id.idx =  CN_KVP_IDX;
 		msg->id.val = CN_KVP_VAL;
-		msg->seq = KVP_KERNEL_GET;
-		((struct hv_ku_msg *)msg->data)->kvp_index = index;
-		msg->len = sizeof(struct hv_ku_msg);
+
+		message = (struct hv_kvp_msg *)msg->data;
+		message->kvp_hdr.operation = KVP_OP_ENUMERATE;
+		message->body.kvp_enum_data.index = index;
+		msg->len = sizeof(struct hv_kvp_msg);
 		cn_netlink_send(msg, 0, GFP_ATOMIC);
 		kfree(msg);
 	}
@@ -191,7 +200,7 @@ kvp_respond_to_host(char *key, char *value, int error)
 	kvp_msg = (struct hv_kvp_msg *)
 			&recv_buffer[sizeof(struct vmbuspipe_hdr) +
 			sizeof(struct icmsg_hdr)];
-	kvp_data = &kvp_msg->kvp_data;
+	kvp_data = &kvp_msg->body.kvp_enum_data;
 	key_name = key;
 
 	/*
@@ -266,7 +275,7 @@ void hv_kvp_onchannelcallback(void *context)
 				sizeof(struct vmbuspipe_hdr) +
 				sizeof(struct icmsg_hdr)];
 
-			kvp_data = &kvp_msg->kvp_data;
+			kvp_data = &kvp_msg->body.kvp_enum_data;
 
 			/*
 			 * We only support the "get" operation on
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 802ece8..1029515 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -113,30 +113,6 @@
  * (not supported), a NULL key string is returned.
  */
 
-/*
- *
- * The following definitions are shared with the user-mode component; do not
- * change any of this without making the corresponding changes in
- * the KVP user-mode component.
- */
-
-enum hv_ku_op {
-	KVP_REGISTER = 0, /* Register the user mode component */
-	KVP_KERNEL_GET, /* Kernel is requesting the value */
-	KVP_KERNEL_SET, /* Kernel is providing the value */
-	KVP_USER_GET,  /* User is requesting the value */
-	KVP_USER_SET  /* User is providing the value */
-};
-
-struct hv_ku_msg {
-	__u32 kvp_index; /* Key index */
-	__u8  kvp_key[HV_KVP_EXCHANGE_MAX_KEY_SIZE]; /* Key name */
-	__u8  kvp_value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE]; /* Key  value */
-};
-
-
-
-
 
 /*
  * Registry value types.
@@ -149,6 +125,7 @@ enum hv_kvp_exchg_op {
 	KVP_OP_SET,
 	KVP_OP_DELETE,
 	KVP_OP_ENUMERATE,
+	KVP_OP_REGISTER,
 	KVP_OP_COUNT /* Number of operations, must be last. */
 };
 
@@ -187,7 +164,10 @@ struct hv_kvp_msg_enumerate {
 
 struct hv_kvp_msg {
 	struct hv_kvp_hdr	kvp_hdr;
-	struct hv_kvp_msg_enumerate	kvp_data;
+	union {
+		struct hv_kvp_msg_enumerate     kvp_enum_data;
+		char    kvp_version[HV_KVP_EXCHANGE_MAX_KEY_SIZE];
+	} body;
 } __packed;
 
 #ifdef __KERNEL__
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index b75523c..4ebf703 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -302,7 +302,7 @@ int main(void)
 	struct pollfd pfd;
 	struct nlmsghdr *incoming_msg;
 	struct cn_msg	*incoming_cn_msg;
-	struct hv_ku_msg *hv_msg;
+	struct hv_kvp_msg *hv_msg;
 	char	*p;
 	char	*key_value;
 	char	*key_name;
@@ -340,9 +340,11 @@ int main(void)
 	message = (struct cn_msg *)kvp_send_buffer;
 	message->id.idx = CN_KVP_IDX;
 	message->id.val = CN_KVP_VAL;
-	message->seq = KVP_REGISTER;
+
+	hv_msg = (struct hv_kvp_msg *)message->data;
+	hv_msg->kvp_hdr.operation = KVP_OP_REGISTER;
 	message->ack = 0;
-	message->len = 0;
+	message->len = sizeof(struct hv_kvp_msg);
 
 	len = netlink_send(fd, message);
 	if (len < 0) {
@@ -368,14 +370,15 @@ int main(void)
 
 		incoming_msg = (struct nlmsghdr *)kvp_recv_buffer;
 		incoming_cn_msg = (struct cn_msg *)NLMSG_DATA(incoming_msg);
+		hv_msg = (struct hv_kvp_msg *)incoming_cn_msg->data;
 
-		switch (incoming_cn_msg->seq) {
-		case KVP_REGISTER:
+		switch (hv_msg->kvp_hdr.operation) {
+		case KVP_OP_REGISTER:
 			/*
 			 * Driver is registering with us; stash away the version
 			 * information.
 			 */
-			p = (char *)incoming_cn_msg->data;
+			p = (char *)hv_msg->body.kvp_version;
 			lic_version = malloc(strlen(p) + 1);
 			if (lic_version) {
 				strcpy(lic_version, p);
@@ -386,17 +389,15 @@ int main(void)
 			}
 			continue;
 
-		case KVP_KERNEL_GET:
-			break;
 		default:
-			continue;
+			break;
 		}
 
-		hv_msg = (struct hv_ku_msg *)incoming_cn_msg->data;
-		key_name = (char *)hv_msg->kvp_key;
-		key_value = (char *)hv_msg->kvp_value;
+		hv_msg = (struct hv_kvp_msg *)incoming_cn_msg->data;
+		key_name = (char *)hv_msg->body.kvp_enum_data.data.key;
+		key_value = (char *)hv_msg->body.kvp_enum_data.data.value;
 
-		switch (hv_msg->kvp_index) {
+		switch (hv_msg->body.kvp_enum_data.index) {
 		case FullyQualifiedDomainName:
 			kvp_get_domain_name(key_value,
 					HV_KVP_EXCHANGE_MAX_VALUE_SIZE);
@@ -456,9 +457,8 @@ int main(void)
 
 		incoming_cn_msg->id.idx = CN_KVP_IDX;
 		incoming_cn_msg->id.val = CN_KVP_VAL;
-		incoming_cn_msg->seq = KVP_USER_SET;
 		incoming_cn_msg->ack = 0;
-		incoming_cn_msg->len = sizeof(struct hv_ku_msg);
+		incoming_cn_msg->len = sizeof(struct hv_kvp_msg);
 
 		len = netlink_send(fd, incoming_cn_msg);
 		if (len < 0) {
-- 
1.7.4.1

[PATCH 6/6] drivers: hv: Increase the number of VCPUs supported in the guest

[K. Y. Srinivasan]

The current code arbirarily limited the number of CPUs the guest could have.
Change that so that we can support the maximum number of CPUs the guest can
support. While we use NR_CPUS to size the per-cpu state all we are allocating
based on NR_CPUS are the  pointers to per-cpu state that will be allocatted in
the context of the initializing CPU. This patch triggers a checkpatch warning
for the usage of NR_CPU and since all we are allocating a couple of pointers 
per CPU, it should be ok..


Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/hv/hv.c           |    4 ++--
 drivers/hv/hyperv_vmbus.h |    5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 12aa97f..15956bd 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -155,9 +155,9 @@ int hv_init(void)
 	union hv_x64_msr_hypercall_contents hypercall_msr;
 	void *virtaddr = NULL;
 
-	memset(hv_context.synic_event_page, 0, sizeof(void *) * MAX_NUM_CPUS);
+	memset(hv_context.synic_event_page, 0, sizeof(void *) * NR_CPUS);
 	memset(hv_context.synic_message_page, 0,
-	       sizeof(void *) * MAX_NUM_CPUS);
+	       sizeof(void *) * NR_CPUS);
 
 	if (!query_hypervisor_presence())
 		goto cleanup;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 6d7d286..699f0d8 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -457,7 +457,6 @@ static const uuid_le VMBUS_SERVICE_ID = {
 	},
 };
 
-#define MAX_NUM_CPUS	32
 
 
 struct hv_input_signal_event_buffer {
@@ -483,8 +482,8 @@ struct hv_context {
 	/* 8-bytes aligned of the buffer above */
 	struct hv_input_signal_event *signal_event_param;
 
-	void *synic_message_page[MAX_NUM_CPUS];
-	void *synic_event_page[MAX_NUM_CPUS];
+	void *synic_message_page[NR_CPUS];
+	void *synic_event_page[NR_CPUS];
 };
 
 extern struct hv_context hv_context;
-- 
1.7.4.1

Re: [PATCH 3/6] drivers: hv: Cleanup the kvp related state in hyperv.h

[Greg KH]

On Fri, Jan 27, 2012 at 03:55:59PM -0800, K. Y. Srinivasan wrote:
> Now cleanup the hyperv.h with regards to KVP definitions.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
>  include/linux/hyperv.h |   32 +++++++++++++++++++-------------
>  1 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 7332b3f..802ece8 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -137,7 +137,6 @@ struct hv_ku_msg {
>  
>  
>  
> -#ifdef __KERNEL__
>  
>  /*
>   * Registry value types.
> @@ -162,29 +161,36 @@ enum hv_kvp_exchg_pool {
>  	KVP_POOL_COUNT /* Number of pools, must be last. */
>  };
>  
> +#ifndef __packed
> +#define __packed __attribute__((packed))
> +#endif

Why do this?

If you are so worried about this in userspace, then just change the
values below to __attribute__((packed)), like all of the other public .h
files do.

thanks,

greg k-h

Re: [PATCH 0000/0006] drivers: hv

[Greg KH]

On Fri, Jan 27, 2012 at 03:55:22PM -0800, K. Y. Srinivasan wrote:
> 
> This patch-set does some cleanup of the KVP component. Also included is a
> patch that will remove artificial limitation on the number of VCPUs that
> can be assigned to the Linux guest on Hyper-V.

I have applied the first 2, please fix patch 3, and resend the rest.

thanks,

greg k-h

RE: [PATCH 3/6] drivers: hv: Cleanup the kvp related state in hyperv.h

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, February 02, 2012 6:30 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; ohering@suse.com;
> zbr@ioremap.net; Haiyang Zhang
> Subject: Re: [PATCH 3/6] drivers: hv: Cleanup the kvp related state in hyperv.h
> 
> On Fri, Jan 27, 2012 at 03:55:59PM -0800, K. Y. Srinivasan wrote:
> > Now cleanup the hyperv.h with regards to KVP definitions.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> >  include/linux/hyperv.h |   32 +++++++++++++++++++-------------
> >  1 files changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> > index 7332b3f..802ece8 100644
> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -137,7 +137,6 @@ struct hv_ku_msg {
> >
> >
> >
> > -#ifdef __KERNEL__
> >
> >  /*
> >   * Registry value types.
> > @@ -162,29 +161,36 @@ enum hv_kvp_exchg_pool {
> >  	KVP_POOL_COUNT /* Number of pools, must be last. */
> >  };
> >
> > +#ifndef __packed
> > +#define __packed __attribute__((packed))
> > +#endif
> 
> Why do this?
> 
> If you are so worried about this in userspace, then just change the
> values below to __attribute__((packed)), like all of the other public .h
> files do.

Greg,

When I ran the checkpatch script against these patches I got a warning that the
preferred directive was to use "__packed".  So, for the header file that will be
consumed in the kernel, I chose to go with the __packed. For inclusion of this
header file in the user space daemon, I put in this definition here. So rather
than having numerous warnings, I now have a single warning. If you prefer,
I can move this definition to the daemon code where it is really needed.

Regards,

K. Y 

Re: [PATCH 3/6] drivers: hv: Cleanup the kvp related state in hyperv.h

[Greg KH]

On Thu, Feb 02, 2012 at 11:41:29PM +0000, KY Srinivasan wrote:
> > > +#ifndef __packed
> > > +#define __packed __attribute__((packed))
> > > +#endif
> > 
> > Why do this?
> > 
> > If you are so worried about this in userspace, then just change the
> > values below to __attribute__((packed)), like all of the other public .h
> > files do.
> 
> Greg,
> 
> When I ran the checkpatch script against these patches I got a warning that the
> preferred directive was to use "__packed".

Well, checkpatch is stupid sometimes.

> So, for the header file that will be
> consumed in the kernel, I chose to go with the __packed. For inclusion of this
> header file in the user space daemon, I put in this definition here. So rather
> than having numerous warnings, I now have a single warning. If you prefer,
> I can move this definition to the daemon code where it is really needed.

Please do it like all other public kernel header files do.

greg k-h

Re: [PATCH 3/6] drivers: hv: Cleanup the kvp related state in hyperv.h

[Joe Perches]

On Thu, 2012-02-02 at 15:48 -0800, Greg KH wrote:
> On Thu, Feb 02, 2012 at 11:41:29PM +0000, KY Srinivasan wrote:
> > > > +#define __packed __attribute__((packed))
[]
> > When I ran the checkpatch script against these patches I got a warning that the
> > preferred directive was to use "__packed".
> Well, checkpatch is stupid sometimes.

True.

I suppose checkpatch warning could be improved to
say that it's preferred for "kernel only" files
and should be ignored for public/user cases.