[PATCH V2 00/14] Drivers: hv: Some miscellaneous fixes and enhancements

[PATCH V2 00/14] Drivers: hv: Some miscellaneous fixes and enhancements

[kys]

From: K. Y. Srinivasan <kys@microsoft.com>

Some miscellaneous fixes and enhancements.

V2: Fixed a build issue reported by Greg. Only Patch # 13 is affected.

Alex Ng (6):
  Drivers: hv: utils: Fix the mapping between host version and protocol
    to use
  Drivers: hv: balloon: Disable hot add when CONFIG_MEMORY_HOTPLUG is
    not set
  Drivers: hv: balloon: Add logging for dynamic memory operations
  Drivers: hv: vss: Improve log messages.
  Drivers: hv: vss: Operation timeouts should match host expectation
  Drivers: hv: balloon: Fix info request to show max page count

K. Y. Srinivasan (3):
  Drivers: hv: vmbus: Base host signaling strictly on the ring state
  Drivers: hv: vmbus: On write cleanup the logic to interrupt the host
  Drivers: hv: vmbus: On the read path cleanup the logic to interrupt
    the host

Vitaly Kuznetsov (2):
  Drivers: hv: ring_buffer: count on wrap around mappings in
    get_next_pkt_raw() (v2)
  Drivers: hv: utils: reduce HV_UTIL_NEGO_TIMEOUT timeout

Weibing Zhang (3):
  tools: hv: remove unnecessary link flag
  tools: hv: fix a compile warning in snprintf
  tools: hv: remove unnecessary header files and netlink related code

 drivers/hv/channel.c       |   93 ++++++--------------------------------------
 drivers/hv/channel_mgmt.c  |    2 -
 drivers/hv/hv_balloon.c    |   44 ++++++++++++++++++--
 drivers/hv/hv_snapshot.c   |   33 ++++++++++++----
 drivers/hv/hv_util.c       |    9 +++-
 drivers/hv/hyperv_vmbus.h  |   12 +++---
 drivers/hv/ring_buffer.c   |   44 ++++++++++++---------
 include/linux/hyperv.h     |   45 ++++++++-------------
 tools/hv/Makefile          |    3 +-
 tools/hv/hv_fcopy_daemon.c |    7 ---
 tools/hv/hv_kvp_daemon.c   |    9 +----
 11 files changed, 133 insertions(+), 168 deletions(-)

-- 
1.7.4.1

[PATCH V2 01/14] Drivers: hv: ring_buffer: count on wrap around mappings in get_next_pkt_raw() (v2)

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

With wrap around mappings in place we can always provide drivers with
direct links to packets on the ring buffer, even when they wrap around.
Do the required updates to get_next_pkt_raw()/put_pkt_raw()

The first version of this commit was reverted (65a532f3d50a) to deal with
cross-tree merge issues which are (hopefully) resolved now.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Tested-by: Dexuan Cui <decui@microsoft.com>
---
 include/linux/hyperv.h |   32 +++++++++++---------------------
 1 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 6824556..42ae6a5 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1526,31 +1526,23 @@ static inline  bool hv_need_to_signal_on_read(struct hv_ring_buffer_info *rbi)
 get_next_pkt_raw(struct vmbus_channel *channel)
 {
 	struct hv_ring_buffer_info *ring_info = &channel->inbound;
-	u32 read_loc = ring_info->priv_read_index;
+	u32 priv_read_loc = ring_info->priv_read_index;
 	void *ring_buffer = hv_get_ring_buffer(ring_info);
-	struct vmpacket_descriptor *cur_desc;
-	u32 packetlen;
 	u32 dsize = ring_info->ring_datasize;
-	u32 delta = read_loc - ring_info->ring_buffer->read_index;
+	/*
+	 * delta is the difference between what is available to read and
+	 * what was already consumed in place. We commit read index after
+	 * the whole batch is processed.
+	 */
+	u32 delta = priv_read_loc >= ring_info->ring_buffer->read_index ?
+		priv_read_loc - ring_info->ring_buffer->read_index :
+		(dsize - ring_info->ring_buffer->read_index) + priv_read_loc;
 	u32 bytes_avail_toread = (hv_get_bytes_to_read(ring_info) - delta);
 
 	if (bytes_avail_toread < sizeof(struct vmpacket_descriptor))
 		return NULL;
 
-	if ((read_loc + sizeof(*cur_desc)) > dsize)
-		return NULL;
-
-	cur_desc = ring_buffer + read_loc;
-	packetlen = cur_desc->len8 << 3;
-
-	/*
-	 * If the packet under consideration is wrapping around,
-	 * return failure.
-	 */
-	if ((read_loc + packetlen + VMBUS_PKT_TRAILER) > (dsize - 1))
-		return NULL;
-
-	return cur_desc;
+	return ring_buffer + priv_read_loc;
 }
 
 /*
@@ -1562,16 +1554,14 @@ static inline void put_pkt_raw(struct vmbus_channel *channel,
 				struct vmpacket_descriptor *desc)
 {
 	struct hv_ring_buffer_info *ring_info = &channel->inbound;
-	u32 read_loc = ring_info->priv_read_index;
 	u32 packetlen = desc->len8 << 3;
 	u32 dsize = ring_info->ring_datasize;
 
-	if ((read_loc + packetlen + VMBUS_PKT_TRAILER) > dsize)
-		BUG();
 	/*
 	 * Include the packet trailer.
 	 */
 	ring_info->priv_read_index += packetlen + VMBUS_PKT_TRAILER;
+	ring_info->priv_read_index %= dsize;
 }
 
 /*
-- 
1.7.4.1

[PATCH V2 02/14] Drivers: hv: utils: reduce HV_UTIL_NEGO_TIMEOUT timeout

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

I discovered that at least WS2016TP5 host has 60 seconds timeout for the
ICMSGTYPE_NEGOTIATE message so we need to lower guest's timeout a little
bit to make sure we always respond in time. Let's make it 55 seconds.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hyperv_vmbus.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index a5b4442..bdab7e7 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -38,7 +38,7 @@
 /*
  * Timeout for guest-host handshake for services.
  */
-#define HV_UTIL_NEGO_TIMEOUT 60
+#define HV_UTIL_NEGO_TIMEOUT 55
 
 /*
  * The below CPUID leaves are present if VersionAndFeatures.HypervisorPresent
-- 
1.7.4.1

[PATCH V2 03/14] Drivers: hv: utils: Fix the mapping between host version and protocol to use

[kys]

From: Alex Ng <alexng@messages.microsoft.com>

We should intentionally declare the protocols to use for every known host
and default to using the latest protocol if the host is unknown or new.

Signed-off-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_util.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index bcd0630..1b693bf 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -389,16 +389,19 @@ static int util_probe(struct hv_device *dev,
 		ts_srv_version = TS_VERSION_1;
 		hb_srv_version = HB_VERSION_1;
 		break;
-	case(VERSION_WIN10):
+	case (VERSION_WIN7):
+	case (VERSION_WIN8):
+	case (VERSION_WIN8_1):
 		util_fw_version = UTIL_FW_VERSION;
 		sd_srv_version = SD_VERSION;
-		ts_srv_version = TS_VERSION;
+		ts_srv_version = TS_VERSION_3;
 		hb_srv_version = HB_VERSION;
 		break;
+	case (VERSION_WIN10):
 	default:
 		util_fw_version = UTIL_FW_VERSION;
 		sd_srv_version = SD_VERSION;
-		ts_srv_version = TS_VERSION_3;
+		ts_srv_version = TS_VERSION;
 		hb_srv_version = HB_VERSION;
 	}
 
-- 
1.7.4.1

[PATCH V2 04/14] Drivers: hv: balloon: Disable hot add when CONFIG_MEMORY_HOTPLUG is not set

[kys]

From: Alex Ng <alexng@messages.microsoft.com>

If the guest does not support memory hotplugging, it should respond to
the host with zero pages added and successful result code. This signals
to the host that hotplugging is not supported and the host will avoid
sending future hot-add requests.

Signed-off-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_balloon.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index fdf8da9..8f932aa 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1501,7 +1501,11 @@ static int balloon_probe(struct hv_device *dev,
 	struct dm_version_request version_req;
 	struct dm_capabilities cap_msg;
 
+#ifdef CONFIG_MEMORY_HOTPLUG
 	do_hot_add = hot_add;
+#else
+	do_hot_add = false;
+#endif
 
 	/*
 	 * First allocate a send buffer.
-- 
1.7.4.1

[PATCH V2 05/14] Drivers: hv: balloon: Add logging for dynamic memory operations

[kys]

From: Alex Ng <alexng@messages.microsoft.com>

Added logging to help troubleshoot common ballooning, hot add,
and versioning issues.

Signed-off-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_balloon.c |   31 ++++++++++++++++++++++++++++---
 1 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 8f932aa..8cac29a 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -564,6 +564,11 @@ struct hv_dynmem_device {
 	 * next version to try.
 	 */
 	__u32 next_version;
+
+	/*
+	 * The negotiated version agreed by host.
+	 */
+	__u32 version;
 };
 
 static struct hv_dynmem_device dm_device;
@@ -645,6 +650,7 @@ static void hv_bring_pgs_online(struct hv_hotadd_state *has,
 {
 	int i;
 
+	pr_debug("Online %lu pages starting at pfn 0x%lx\n", size, start_pfn);
 	for (i = 0; i < size; i++)
 		hv_page_online_one(has, pfn_to_page(start_pfn + i));
 }
@@ -685,7 +691,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
 				(HA_CHUNK << PAGE_SHIFT));
 
 		if (ret) {
-			pr_info("hot_add memory failed error is %d\n", ret);
+			pr_warn("hot_add memory failed error is %d\n", ret);
 			if (ret == -EEXIST) {
 				/*
 				 * This error indicates that the error
@@ -814,6 +820,9 @@ static unsigned long handle_pg_range(unsigned long pg_start,
 	unsigned long old_covered_state;
 	unsigned long res = 0, flags;
 
+	pr_debug("Hot adding %lu pages starting at pfn 0x%lx.\n", pg_count,
+		pg_start);
+
 	spin_lock_irqsave(&dm_device.ha_lock, flags);
 	list_for_each_entry(has, &dm_device.ha_region_list, list) {
 		/*
@@ -1196,8 +1205,6 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
 	return num_pages;
 }
 
-
-
 static void balloon_up(struct work_struct *dummy)
 {
 	unsigned int num_pages = dm_device.balloon_wrk.num_pages;
@@ -1224,6 +1231,10 @@ static void balloon_up(struct work_struct *dummy)
 
 	/* Refuse to balloon below the floor, keep the 2M granularity. */
 	if (avail_pages < num_pages || avail_pages - num_pages < floor) {
+		pr_warn("Balloon request will be partially fulfilled. %s\n",
+			avail_pages < num_pages ? "Not enough memory." :
+			"Balloon floor reached.");
+
 		num_pages = avail_pages > floor ? (avail_pages - floor) : 0;
 		num_pages -= num_pages % PAGES_IN_2M;
 	}
@@ -1245,6 +1256,9 @@ static void balloon_up(struct work_struct *dummy)
 		}
 
 		if (num_ballooned == 0 || num_ballooned == num_pages) {
+			pr_debug("Ballooned %u out of %u requested pages.\n",
+				num_pages, dm_device.balloon_wrk.num_pages);
+
 			bl_resp->more_pages = 0;
 			done = true;
 			dm_device.state = DM_INITIALIZED;
@@ -1292,12 +1306,16 @@ static void balloon_down(struct hv_dynmem_device *dm,
 	int range_count = req->range_count;
 	struct dm_unballoon_response resp;
 	int i;
+	unsigned int prev_pages_ballooned = dm->num_pages_ballooned;
 
 	for (i = 0; i < range_count; i++) {
 		free_balloon_pages(dm, &range_array[i]);
 		complete(&dm_device.config_event);
 	}
 
+	pr_debug("Freed %u ballooned pages.\n",
+		prev_pages_ballooned - dm->num_pages_ballooned);
+
 	if (req->more_pages == 1)
 		return;
 
@@ -1365,6 +1383,7 @@ static void version_resp(struct hv_dynmem_device *dm,
 	version_req.hdr.size = sizeof(struct dm_version_request);
 	version_req.hdr.trans_id = atomic_inc_return(&trans_id);
 	version_req.version.version = dm->next_version;
+	dm->version = version_req.version.version;
 
 	/*
 	 * Set the next version to try in case current version fails.
@@ -1557,6 +1576,7 @@ static int balloon_probe(struct hv_device *dev,
 	version_req.hdr.trans_id = atomic_inc_return(&trans_id);
 	version_req.version.version = DYNMEM_PROTOCOL_VERSION_WIN10;
 	version_req.is_last_attempt = 0;
+	dm_device.version = version_req.version.version;
 
 	ret = vmbus_sendpacket(dev->channel, &version_req,
 				sizeof(struct dm_version_request),
@@ -1579,6 +1599,11 @@ static int balloon_probe(struct hv_device *dev,
 		ret = -ETIMEDOUT;
 		goto probe_error2;
 	}
+
+	pr_info("Using Dynamic Memory protocol version %u.%u\n",
+		DYNMEM_MAJOR_VERSION(dm_device.version),
+		DYNMEM_MINOR_VERSION(dm_device.version));
+
 	/*
 	 * Now submit our capabilities to the host.
 	 */
-- 
1.7.4.1

[PATCH V2 06/14] Drivers: hv: vss: Improve log messages.

[kys]

From: Alex Ng <alexng@messages.microsoft.com>

Adding log messages to help troubleshoot error cases and transaction
handling.

Signed-off-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_snapshot.c |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index a670713..5c95ba1 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -120,7 +120,7 @@ static int vss_handle_handshake(struct hv_vss_msg *vss_msg)
 	default:
 		return -EINVAL;
 	}
-	pr_debug("VSS: userspace daemon ver. %d connected\n", dm_reg_value);
+	pr_info("VSS: userspace daemon ver. %d connected\n", dm_reg_value);
 	return 0;
 }
 
@@ -128,8 +128,10 @@ static int vss_on_msg(void *msg, int len)
 {
 	struct hv_vss_msg *vss_msg = (struct hv_vss_msg *)msg;
 
-	if (len != sizeof(*vss_msg))
+	if (len != sizeof(*vss_msg)) {
+		pr_debug("VSS: Message size does not match length\n");
 		return -EINVAL;
+	}
 
 	if (vss_msg->vss_hdr.operation == VSS_OP_REGISTER ||
 	    vss_msg->vss_hdr.operation == VSS_OP_REGISTER1) {
@@ -137,8 +139,11 @@ static int vss_on_msg(void *msg, int len)
 		 * Don't process registration messages if we're in the middle
 		 * of a transaction processing.
 		 */
-		if (vss_transaction.state > HVUTIL_READY)
+		if (vss_transaction.state > HVUTIL_READY) {
+			pr_debug("VSS: Got unexpected registration request\n");
 			return -EINVAL;
+		}
+
 		return vss_handle_handshake(vss_msg);
 	} else if (vss_transaction.state == HVUTIL_USERSPACE_REQ) {
 		vss_transaction.state = HVUTIL_USERSPACE_RECV;
@@ -155,7 +160,7 @@ static int vss_on_msg(void *msg, int len)
 		}
 	} else {
 		/* This is a spurious call! */
-		pr_warn("VSS: Transaction not active\n");
+		pr_debug("VSS: Transaction not active\n");
 		return -EINVAL;
 	}
 	return 0;
@@ -168,8 +173,10 @@ static void vss_send_op(void)
 	struct hv_vss_msg *vss_msg;
 
 	/* The transaction state is wrong. */
-	if (vss_transaction.state != HVUTIL_HOSTMSG_RECEIVED)
+	if (vss_transaction.state != HVUTIL_HOSTMSG_RECEIVED) {
+		pr_debug("VSS: Unexpected attempt to send to daemon\n");
 		return;
+	}
 
 	vss_msg = kzalloc(sizeof(*vss_msg), GFP_KERNEL);
 	if (!vss_msg)
@@ -210,9 +217,13 @@ static void vss_handle_request(struct work_struct *dummy)
 	case VSS_OP_HOT_BACKUP:
 		if (vss_transaction.state < HVUTIL_READY) {
 			/* Userspace is not registered yet */
+			pr_debug("VSS: Not ready for request.\n");
 			vss_respond_to_host(HV_E_FAIL);
 			return;
 		}
+
+		pr_debug("VSS: Received request for op code: %d\n",
+			vss_transaction.msg->vss_hdr.operation);
 		vss_transaction.state = HVUTIL_HOSTMSG_RECEIVED;
 		vss_send_op();
 		return;
@@ -353,8 +364,10 @@ static void vss_on_reset(void)
 
 	hvt = hvutil_transport_init(vss_devname, CN_VSS_IDX, CN_VSS_VAL,
 				    vss_on_msg, vss_on_reset);
-	if (!hvt)
+	if (!hvt) {
+		pr_warn("VSS: Failed to initialize transport\n");
 		return -EFAULT;
+	}
 
 	return 0;
 }
-- 
1.7.4.1

[PATCH V2 07/14] Drivers: hv: vss: Operation timeouts should match host expectation

[kys]

From: Alex Ng <alexng@messages.microsoft.com>

Increase the timeout of backup operations. When system is under I/O load,
it needs more time to freeze. These timeout values should also match the
host timeout values more closely.

Signed-off-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_snapshot.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index 5c95ba1..eee238c 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -31,7 +31,10 @@
 #define VSS_MINOR  0
 #define VSS_VERSION    (VSS_MAJOR << 16 | VSS_MINOR)
 
-#define VSS_USERSPACE_TIMEOUT (msecs_to_jiffies(10 * 1000))
+/*
+ * Timeout values are based on expecations from host
+ */
+#define VSS_FREEZE_TIMEOUT (15 * 60)
 
 /*
  * Global state maintained for transaction that is being processed. For a class
@@ -186,7 +189,8 @@ static void vss_send_op(void)
 
 	vss_transaction.state = HVUTIL_USERSPACE_REQ;
 
-	schedule_delayed_work(&vss_timeout_work, VSS_USERSPACE_TIMEOUT);
+	schedule_delayed_work(&vss_timeout_work, op == VSS_OP_FREEZE ?
+			VSS_FREEZE_TIMEOUT * HZ : HV_UTIL_TIMEOUT * HZ);
 
 	rc = hvutil_transport_send(hvt, vss_msg, sizeof(*vss_msg), NULL);
 	if (rc) {
-- 
1.7.4.1

[PATCH V2 08/14] Drivers: hv: balloon: Fix info request to show max page count

[kys]

From: Alex Ng <alexng@messages.microsoft.com>

Balloon driver was only printing the size of the info blob and not the
actual content. This fixes it so that the info blob (max page count as
configured in Hyper-V) is printed out.

Signed-off-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_balloon.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 8cac29a..14c3dc4 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1034,8 +1034,13 @@ static void process_info(struct hv_dynmem_device *dm, struct dm_info_msg *msg)
 
 	switch (info_hdr->type) {
 	case INFO_TYPE_MAX_PAGE_CNT:
-		pr_info("Received INFO_TYPE_MAX_PAGE_CNT\n");
-		pr_info("Data Size is %d\n", info_hdr->data_size);
+		if (info_hdr->data_size == sizeof(__u64)) {
+			__u64 *max_page_count = (__u64 *)&info_hdr[1];
+
+			pr_info("INFO_TYPE_MAX_PAGE_CNT = %llu\n",
+				*max_page_count);
+		}
+
 		break;
 	default:
 		pr_info("Received Unknown type: %d\n", info_hdr->type);
-- 
1.7.4.1

Re: [PATCH V2 08/14] Drivers: hv: balloon: Fix info request to show max page count

[Dan Carpenter]

On Tue, Nov 01, 2016 at 01:07:28PM -0700, kys@exchange.microsoft.com wrote:
> From: Alex Ng <alexng@messages.microsoft.com>
> 
> Balloon driver was only printing the size of the info blob and not the
> actual content. This fixes it so that the info blob (max page count as
> configured in Hyper-V) is printed out.
> 
> Signed-off-by: Alex Ng <alexng@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/hv_balloon.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 8cac29a..14c3dc4 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -1034,8 +1034,13 @@ static void process_info(struct hv_dynmem_device *dm, struct dm_info_msg *msg)
>  
>  	switch (info_hdr->type) {
>  	case INFO_TYPE_MAX_PAGE_CNT:
> -		pr_info("Received INFO_TYPE_MAX_PAGE_CNT\n");
> -		pr_info("Data Size is %d\n", info_hdr->data_size);
> +		if (info_hdr->data_size == sizeof(__u64)) {
> +			__u64 *max_page_count = (__u64 *)&info_hdr[1];

Why __u64 instead of u64?  Is this code shared with user space?

regards,
dan carpenter

RE: [PATCH V2 08/14] Drivers: hv: balloon: Fix info request to show max page count

[Alex Ng (LIS)]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Thursday, November 10, 2016 11:09 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com;
> leann.ogasawara@canonical.com; Stephen Hemminger
> <sthemmin@microsoft.com>; Alex Ng (LIS) <alexng@microsoft.com>; Alex
> Ng (LIS) <alexng@microsoft.com>
> Subject: Re: [PATCH V2 08/14] Drivers: hv: balloon: Fix info request to show
> max page count
> 
> On Tue, Nov 01, 2016 at 01:07:28PM -0700, kys@exchange.microsoft.com
> wrote:
> > From: Alex Ng <alexng@messages.microsoft.com>
> >
> > Balloon driver was only printing the size of the info blob and not the
> > actual content. This fixes it so that the info blob (max page count as
> > configured in Hyper-V) is printed out.
> >
> > Signed-off-by: Alex Ng <alexng@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  drivers/hv/hv_balloon.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> > index 8cac29a..14c3dc4 100644
> > --- a/drivers/hv/hv_balloon.c
> > +++ b/drivers/hv/hv_balloon.c
> > @@ -1034,8 +1034,13 @@ static void process_info(struct
> hv_dynmem_device *dm, struct dm_info_msg *msg)
> >
> >  	switch (info_hdr->type) {
> >  	case INFO_TYPE_MAX_PAGE_CNT:
> > -		pr_info("Received INFO_TYPE_MAX_PAGE_CNT\n");
> > -		pr_info("Data Size is %d\n", info_hdr->data_size);
> > +		if (info_hdr->data_size == sizeof(__u64)) {
> > +			__u64 *max_page_count = (__u64 *)&info_hdr[1];
> 
> Why __u64 instead of u64?  Is this code shared with user space?

__u64 was being used in other parts of this code already and I decided to follow that.

I wasn't aware of this distinction between the two.
This code is not shared in user-space, so perhaps we should clean up this file in a future patch.

Thanks for bringing it up.

> 
> regards,
> dan carpenter
> 

[PATCH V2 09/14] tools: hv: remove unnecessary link flag

[kys]

From: Weibing Zhang <atheism.zhang@gmail.com>

The link flag pthread is not needed.

Signed-off-by: Weibing Zhang <atheism.zhang@gmail.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 tools/hv/Makefile |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/tools/hv/Makefile b/tools/hv/Makefile
index a8c4644..0d1e61b 100644
--- a/tools/hv/Makefile
+++ b/tools/hv/Makefile
@@ -1,9 +1,8 @@
 # Makefile for Hyper-V tools
 
 CC = $(CROSS_COMPILE)gcc
-PTHREAD_LIBS = -lpthread
 WARNINGS = -Wall -Wextra
-CFLAGS = $(WARNINGS) -g $(PTHREAD_LIBS) $(shell getconf LFS_CFLAGS)
+CFLAGS = $(WARNINGS) -g $(shell getconf LFS_CFLAGS)
 
 CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include
 
-- 
1.7.4.1

[PATCH V2 10/14] tools: hv: fix a compile warning in snprintf

[kys]

From: Weibing Zhang <atheism.zhang@gmail.com>

hv_kvp_daemon.c: In function .kvp_mac_to_if_name.:
hv_kvp_daemon.c:705:2: warning: format not a string literal and no format arguments [-Wformat-security]
  snprintf(dev_id, sizeof(dev_id), kvp_net_dir);
  ^
hv_kvp_daemon.c:705:2: warning: format not a string literal and no format arguments [-Wformat-security]

Signed-off-by: Weibing Zhang <atheism.zhang@gmail.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 tools/hv/hv_kvp_daemon.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index bc7adb8..bb0719b 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -702,7 +702,7 @@ void kvp_get_os_info(void)
 	if (dir == NULL)
 		return NULL;
 
-	snprintf(dev_id, sizeof(dev_id), kvp_net_dir);
+	snprintf(dev_id, sizeof(dev_id), "%s", kvp_net_dir);
 	q = dev_id + strlen(kvp_net_dir);
 
 	while ((entry = readdir(dir)) != NULL) {
-- 
1.7.4.1

[PATCH V2 11/14] tools: hv: remove unnecessary header files and netlink related code

[kys]

From: Weibing Zhang <atheism.zhang@gmail.com>

Remove unnecessary header files and netlink related code as the daemons
do not use netlink to communicate with the kernel now.

Signed-off-by: Weibing Zhang <atheism.zhang@gmail.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 tools/hv/hv_fcopy_daemon.c |    7 -------
 tools/hv/hv_kvp_daemon.c   |    7 -------
 2 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index fdc9ca4..26ae609 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -18,21 +18,14 @@
 
 
 #include <sys/types.h>
-#include <sys/socket.h>
-#include <sys/poll.h>
-#include <linux/types.h>
-#include <linux/kdev_t.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
-#include <string.h>
-#include <ctype.h>
 #include <errno.h>
 #include <linux/hyperv.h>
 #include <syslog.h>
 #include <sys/stat.h>
 #include <fcntl.h>
-#include <dirent.h>
 #include <getopt.h>
 
 static int target_fd;
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index bb0719b..d791dbf 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -22,8 +22,6 @@
  */
 
 
-#include <sys/types.h>
-#include <sys/socket.h>
 #include <sys/poll.h>
 #include <sys/utsname.h>
 #include <stdio.h>
@@ -34,7 +32,6 @@
 #include <errno.h>
 #include <arpa/inet.h>
 #include <linux/hyperv.h>
-#include <linux/netlink.h>
 #include <ifaddrs.h>
 #include <netdb.h>
 #include <syslog.h>
@@ -99,10 +96,6 @@ enum {
 #define MAX_FILE_NAME 100
 #define ENTRIES_PER_BLOCK 50
 
-#ifndef SOL_NETLINK
-#define SOL_NETLINK 270
-#endif
-
 struct kvp_record {
 	char key[HV_KVP_EXCHANGE_MAX_KEY_SIZE];
 	char value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
-- 
1.7.4.1

[PATCH V2 12/14] Drivers: hv: vmbus: Base host signaling strictly on the ring state

[kys]

From: K. Y. Srinivasan <kys@microsoft.com>

One of the factors that can result in the host concluding that a given
guest in mounting a DOS attack is if the guest generates interrupts
to the host when the host is not expecting it. If these "spurious"
interrupts reach a certain rate, the host can throttle the guest to
minimize the impact. The host computation of the "expected number
of interrupts" is strictly based on the ring transitions. Until
the host logic is fixed, base the guest logic to interrupt solely
on the ring state.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c      |   23 ++++++++++++++++++++---
 drivers/hv/channel_mgmt.c |    2 --
 drivers/hv/ring_buffer.c  |    7 -------
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 16f91c8..5e482d7 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -676,10 +676,18 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer,
 	 * NOTE: in this case, the hvsock channel is an exception, because
 	 * it looks the host side's hvsock implementation has a throttling
 	 * mechanism which can hurt the performance otherwise.
+	 *
+	 * KYS: Oct. 30, 2016:
+	 * It looks like Windows hosts have logic to deal with DOS attacks that
+	 * can be triggered if it receives interrupts when it is not expecting
+	 * the interrupt. The host expects interrupts only when the ring
+	 * transitions from empty to non-empty (or full to non full on the guest
+	 * to host ring).
+	 * So, base the signaling decision solely on the ring state until the
+	 * host logic is fixed.
 	 */
 
-	if (((ret == 0) && kick_q && signal) ||
-	    (ret && !is_hvsock_channel(channel)))
+	if (((ret == 0) && signal))
 		vmbus_setevent(channel);
 
 	return ret;
@@ -786,9 +794,18 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel,
 	 * If we cannot write to the ring-buffer; signal the host
 	 * even if we may not have written anything. This is a rare
 	 * enough condition that it should not matter.
+	 *
+	 * KYS: Oct. 30, 2016:
+	 * It looks like Windows hosts have logic to deal with DOS attacks that
+	 * can be triggered if it receives interrupts when it is not expecting
+	 * the interrupt. The host expects interrupts only when the ring
+	 * transitions from empty to non-empty (or full to non full on the guest
+	 * to host ring).
+	 * So, base the signaling decision solely on the ring state until the
+	 * host logic is fixed.
 	 */
 
-	if (((ret == 0) && kick_q && signal) || (ret))
+	if (((ret == 0) && signal))
 		vmbus_setevent(channel);
 
 	return ret;
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 96a85cd..cbb96f2 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -447,8 +447,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 	}
 
 	dev_type = hv_get_dev_type(newchannel);
-	if (dev_type == HV_NIC)
-		set_channel_signal_state(newchannel, HV_SIGNAL_POLICY_EXPLICIT);
 
 	init_vp_index(newchannel, dev_type);
 
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 08043da..5d11d93 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -75,13 +75,6 @@ static bool hv_need_to_signal(u32 old_write, struct hv_ring_buffer_info *rbi,
 	if (READ_ONCE(rbi->ring_buffer->interrupt_mask))
 		return false;
 
-	/*
-	 * When the client wants to control signaling,
-	 * we only honour the host interrupt mask.
-	 */
-	if (policy == HV_SIGNAL_POLICY_EXPLICIT)
-		return true;
-
 	/* check interrupt_mask before read_index */
 	virt_rmb();
 	/*
-- 
1.7.4.1

[PATCH V2 13/14] Drivers: hv: vmbus: On write cleanup the logic to interrupt the host

[kys]

From: K. Y. Srinivasan <kys@microsoft.com>

Signal the host when we determine the host is to be signaled.
The currrent code determines the need to signal in the ringbuffer
code and actually issues the signal elsewhere. This can result
in the host viewing this interrupt as spurious since the host may also
poll the channel. Make the necessary adjustments.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
	V2: Export vmbus_setevent() symbol; build issue reported by Greg

 drivers/hv/channel.c      |   99 +++++----------------------------------------
 drivers/hv/hyperv_vmbus.h |    6 +-
 drivers/hv/ring_buffer.c  |   30 +++++++++----
 include/linux/hyperv.h    |    1 +
 4 files changed, 35 insertions(+), 101 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 5e482d7..8a8148f 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -39,7 +39,7 @@
  * vmbus_setevent- Trigger an event notification on the specified
  * channel.
  */
-static void vmbus_setevent(struct vmbus_channel *channel)
+void vmbus_setevent(struct vmbus_channel *channel)
 {
 	struct hv_monitor_page *monitorpage;
 
@@ -65,6 +65,7 @@ static void vmbus_setevent(struct vmbus_channel *channel)
 		vmbus_set_event(channel);
 	}
 }
+EXPORT_SYMBOL_GPL(vmbus_setevent);
 
 /*
  * vmbus_open - Open the specified channel.
@@ -635,8 +636,6 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer,
 	u32 packetlen_aligned = ALIGN(packetlen, sizeof(u64));
 	struct kvec bufferlist[3];
 	u64 aligned_data = 0;
-	int ret;
-	bool signal = false;
 	bool lock = channel->acquire_ring_lock;
 	int num_vecs = ((bufferlen != 0) ? 3 : 1);
 
@@ -656,41 +655,9 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer,
 	bufferlist[2].iov_base = &aligned_data;
 	bufferlist[2].iov_len = (packetlen_aligned - packetlen);
 
-	ret = hv_ringbuffer_write(&channel->outbound, bufferlist, num_vecs,
-				  &signal, lock, channel->signal_policy);
-
-	/*
-	 * Signalling the host is conditional on many factors:
-	 * 1. The ring state changed from being empty to non-empty.
-	 *    This is tracked by the variable "signal".
-	 * 2. The variable kick_q tracks if more data will be placed
-	 *    on the ring. We will not signal if more data is
-	 *    to be placed.
-	 *
-	 * Based on the channel signal state, we will decide
-	 * which signaling policy will be applied.
-	 *
-	 * If we cannot write to the ring-buffer; signal the host
-	 * even if we may not have written anything. This is a rare
-	 * enough condition that it should not matter.
-	 * NOTE: in this case, the hvsock channel is an exception, because
-	 * it looks the host side's hvsock implementation has a throttling
-	 * mechanism which can hurt the performance otherwise.
-	 *
-	 * KYS: Oct. 30, 2016:
-	 * It looks like Windows hosts have logic to deal with DOS attacks that
-	 * can be triggered if it receives interrupts when it is not expecting
-	 * the interrupt. The host expects interrupts only when the ring
-	 * transitions from empty to non-empty (or full to non full on the guest
-	 * to host ring).
-	 * So, base the signaling decision solely on the ring state until the
-	 * host logic is fixed.
-	 */
-
-	if (((ret == 0) && signal))
-		vmbus_setevent(channel);
+	return hv_ringbuffer_write(channel, bufferlist, num_vecs,
+				   lock, kick_q);
 
-	return ret;
 }
 EXPORT_SYMBOL(vmbus_sendpacket_ctl);
 
@@ -731,7 +698,6 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel,
 				     u32 flags,
 				     bool kick_q)
 {
-	int ret;
 	int i;
 	struct vmbus_channel_packet_page_buffer desc;
 	u32 descsize;
@@ -739,7 +705,6 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel,
 	u32 packetlen_aligned;
 	struct kvec bufferlist[3];
 	u64 aligned_data = 0;
-	bool signal = false;
 	bool lock = channel->acquire_ring_lock;
 
 	if (pagecount > MAX_PAGE_BUFFER_COUNT)
@@ -777,38 +742,8 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel,
 	bufferlist[2].iov_base = &aligned_data;
 	bufferlist[2].iov_len = (packetlen_aligned - packetlen);
 
-	ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3,
-				  &signal, lock, channel->signal_policy);
-
-	/*
-	 * Signalling the host is conditional on many factors:
-	 * 1. The ring state changed from being empty to non-empty.
-	 *    This is tracked by the variable "signal".
-	 * 2. The variable kick_q tracks if more data will be placed
-	 *    on the ring. We will not signal if more data is
-	 *    to be placed.
-	 *
-	 * Based on the channel signal state, we will decide
-	 * which signaling policy will be applied.
-	 *
-	 * If we cannot write to the ring-buffer; signal the host
-	 * even if we may not have written anything. This is a rare
-	 * enough condition that it should not matter.
-	 *
-	 * KYS: Oct. 30, 2016:
-	 * It looks like Windows hosts have logic to deal with DOS attacks that
-	 * can be triggered if it receives interrupts when it is not expecting
-	 * the interrupt. The host expects interrupts only when the ring
-	 * transitions from empty to non-empty (or full to non full on the guest
-	 * to host ring).
-	 * So, base the signaling decision solely on the ring state until the
-	 * host logic is fixed.
-	 */
-
-	if (((ret == 0) && signal))
-		vmbus_setevent(channel);
-
-	return ret;
+	return hv_ringbuffer_write(channel, bufferlist, 3,
+				   lock, kick_q);
 }
 EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl);
 
@@ -839,12 +774,10 @@ int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
 			      u32 desc_size,
 			      void *buffer, u32 bufferlen, u64 requestid)
 {
-	int ret;
 	u32 packetlen;
 	u32 packetlen_aligned;
 	struct kvec bufferlist[3];
 	u64 aligned_data = 0;
-	bool signal = false;
 	bool lock = channel->acquire_ring_lock;
 
 	packetlen = desc_size + bufferlen;
@@ -865,13 +798,8 @@ int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
 	bufferlist[2].iov_base = &aligned_data;
 	bufferlist[2].iov_len = (packetlen_aligned - packetlen);
 
-	ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3,
-				  &signal, lock, channel->signal_policy);
-
-	if (ret == 0 && signal)
-		vmbus_setevent(channel);
-
-	return ret;
+	return hv_ringbuffer_write(channel, bufferlist, 3,
+				   lock, true);
 }
 EXPORT_SYMBOL_GPL(vmbus_sendpacket_mpb_desc);
 
@@ -883,14 +811,12 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel,
 				struct hv_multipage_buffer *multi_pagebuffer,
 				void *buffer, u32 bufferlen, u64 requestid)
 {
-	int ret;
 	struct vmbus_channel_packet_multipage_buffer desc;
 	u32 descsize;
 	u32 packetlen;
 	u32 packetlen_aligned;
 	struct kvec bufferlist[3];
 	u64 aligned_data = 0;
-	bool signal = false;
 	bool lock = channel->acquire_ring_lock;
 	u32 pfncount = NUM_PAGES_SPANNED(multi_pagebuffer->offset,
 					 multi_pagebuffer->len);
@@ -930,13 +856,8 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel,
 	bufferlist[2].iov_base = &aligned_data;
 	bufferlist[2].iov_len = (packetlen_aligned - packetlen);
 
-	ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3,
-				  &signal, lock, channel->signal_policy);
-
-	if (ret == 0 && signal)
-		vmbus_setevent(channel);
-
-	return ret;
+	return hv_ringbuffer_write(channel, bufferlist, 3,
+				   lock, true);
 }
 EXPORT_SYMBOL_GPL(vmbus_sendpacket_multipagebuffer);
 
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index bdab7e7..2d42ebe 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -527,10 +527,10 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
 
 void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info);
 
-int hv_ringbuffer_write(struct hv_ring_buffer_info *ring_info,
+int hv_ringbuffer_write(struct vmbus_channel *channel,
 		    struct kvec *kv_list,
-		    u32 kv_count, bool *signal, bool lock,
-		    enum hv_signal_policy policy);
+		    u32 kv_count, bool lock,
+		    bool kick_q);
 
 int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
 		       void *buffer, u32 buflen, u32 *buffer_actual_len,
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 5d11d93..4af7130 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -66,14 +66,25 @@ u32 hv_end_read(struct hv_ring_buffer_info *rbi)
  *	   once the ring buffer is empty, it will clear the
  *	   interrupt_mask and re-check to see if new data has
  *	   arrived.
+ *
+ * KYS: Oct. 30, 2016:
+ * It looks like Windows hosts have logic to deal with DOS attacks that
+ * can be triggered if it receives interrupts when it is not expecting
+ * the interrupt. The host expects interrupts only when the ring
+ * transitions from empty to non-empty (or full to non full on the guest
+ * to host ring).
+ * So, base the signaling decision solely on the ring state until the
+ * host logic is fixed.
  */
 
-static bool hv_need_to_signal(u32 old_write, struct hv_ring_buffer_info *rbi,
-			      enum hv_signal_policy policy)
+static void hv_signal_on_write(u32 old_write, struct vmbus_channel *channel,
+			       bool kick_q)
 {
+	struct hv_ring_buffer_info *rbi = &channel->outbound;
+
 	virt_mb();
 	if (READ_ONCE(rbi->ring_buffer->interrupt_mask))
-		return false;
+		return;
 
 	/* check interrupt_mask before read_index */
 	virt_rmb();
@@ -82,9 +93,9 @@ static bool hv_need_to_signal(u32 old_write, struct hv_ring_buffer_info *rbi,
 	 * ring transitions from being empty to non-empty.
 	 */
 	if (old_write == READ_ONCE(rbi->ring_buffer->read_index))
-		return true;
+		vmbus_setevent(channel);
 
-	return false;
+	return;
 }
 
 /* Get the next write location for the specified ring buffer. */
@@ -273,9 +284,9 @@ void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info)
 }
 
 /* Write to the ring buffer. */
-int hv_ringbuffer_write(struct hv_ring_buffer_info *outring_info,
-		    struct kvec *kv_list, u32 kv_count, bool *signal, bool lock,
-		    enum hv_signal_policy policy)
+int hv_ringbuffer_write(struct vmbus_channel *channel,
+		    struct kvec *kv_list, u32 kv_count, bool lock,
+		    bool kick_q)
 {
 	int i = 0;
 	u32 bytes_avail_towrite;
@@ -285,6 +296,7 @@ int hv_ringbuffer_write(struct hv_ring_buffer_info *outring_info,
 	u32 old_write;
 	u64 prev_indices = 0;
 	unsigned long flags = 0;
+	struct hv_ring_buffer_info *outring_info = &channel->outbound;
 
 	for (i = 0; i < kv_count; i++)
 		totalbytes_towrite += kv_list[i].iov_len;
@@ -337,7 +349,7 @@ int hv_ringbuffer_write(struct hv_ring_buffer_info *outring_info,
 	if (lock)
 		spin_unlock_irqrestore(&outring_info->ring_lock, flags);
 
-	*signal = hv_need_to_signal(old_write, outring_info, policy);
+	hv_signal_on_write(old_write, channel, kick_q);
 	return 0;
 }
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 42ae6a5..8cf78ed 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1454,6 +1454,7 @@ extern bool vmbus_prep_negotiate_resp(struct icmsg_hdr *,
 
 void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid);
 
+void vmbus_setevent(struct vmbus_channel *channel);
 /*
  * Negotiated version with the Host.
  */
-- 
1.7.4.1

[PATCH V2 14/14] Drivers: hv: vmbus: On the read path cleanup the logic to interrupt the host

[kys]

From: K. Y. Srinivasan <kys@microsoft.com>

Signal the host when we determine the host is to be signaled -
on th read path. The currrent code determines the need to signal in the
ringbuffer code and actually issues the signal elsewhere. This can result
in the host viewing this interrupt as spurious since the host may also
poll the channel. Make the necessary adjustments.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c      |   11 ++---------
 drivers/hv/hyperv_vmbus.h |    4 ++--
 drivers/hv/ring_buffer.c  |    7 ++++---
 include/linux/hyperv.h    |   12 ++++++------
 4 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 8a8148f..5fb4c6d 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -879,16 +879,9 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel,
 		   u32 bufferlen, u32 *buffer_actual_len, u64 *requestid,
 		   bool raw)
 {
-	int ret;
-	bool signal = false;
-
-	ret = hv_ringbuffer_read(&channel->inbound, buffer, bufferlen,
-				 buffer_actual_len, requestid, &signal, raw);
+	return hv_ringbuffer_read(channel, buffer, bufferlen,
+				  buffer_actual_len, requestid, raw);
 
-	if (signal)
-		vmbus_setevent(channel);
-
-	return ret;
 }
 
 int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer,
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 2d42ebe..0675b39 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -532,9 +532,9 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
 		    u32 kv_count, bool lock,
 		    bool kick_q);
 
-int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
+int hv_ringbuffer_read(struct vmbus_channel *channel,
 		       void *buffer, u32 buflen, u32 *buffer_actual_len,
-		       u64 *requestid, bool *signal, bool raw);
+		       u64 *requestid, bool raw);
 
 void hv_ringbuffer_get_debuginfo(struct hv_ring_buffer_info *ring_info,
 			    struct hv_ring_buffer_debug_info *debug_info);
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 4af7130..cd49cb1 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -353,9 +353,9 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
 	return 0;
 }
 
-int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
+int hv_ringbuffer_read(struct vmbus_channel *channel,
 		       void *buffer, u32 buflen, u32 *buffer_actual_len,
-		       u64 *requestid, bool *signal, bool raw)
+		       u64 *requestid, bool raw)
 {
 	u32 bytes_avail_toread;
 	u32 next_read_location = 0;
@@ -364,6 +364,7 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
 	u32 offset;
 	u32 packetlen;
 	int ret = 0;
+	struct hv_ring_buffer_info *inring_info = &channel->inbound;
 
 	if (buflen <= 0)
 		return -EINVAL;
@@ -421,7 +422,7 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
 	/* Update the read index */
 	hv_set_next_read_location(inring_info, next_read_location);
 
-	*signal = hv_need_to_signal_on_read(inring_info);
+	hv_signal_on_read(channel);
 
 	return ret;
 }
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 8cf78ed..fdb0a87 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1487,10 +1487,11 @@ int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id,
  *    there is room for the producer to send the pending packet.
  */
 
-static inline  bool hv_need_to_signal_on_read(struct hv_ring_buffer_info *rbi)
+static inline  void hv_signal_on_read(struct vmbus_channel *channel)
 {
 	u32 cur_write_sz;
 	u32 pending_sz;
+	struct hv_ring_buffer_info *rbi = &channel->inbound;
 
 	/*
 	 * Issue a full memory barrier before making the signaling decision.
@@ -1508,14 +1509,14 @@ static inline  bool hv_need_to_signal_on_read(struct hv_ring_buffer_info *rbi)
 	pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz);
 	/* If the other end is not blocked on write don't bother. */
 	if (pending_sz == 0)
-		return false;
+		return;
 
 	cur_write_sz = hv_get_bytes_to_write(rbi);
 
 	if (cur_write_sz >= pending_sz)
-		return true;
+		vmbus_setevent(channel);
 
-	return false;
+	return;
 }
 
 /*
@@ -1587,8 +1588,7 @@ static inline void commit_rd_index(struct vmbus_channel *channel)
 	virt_rmb();
 	ring_info->ring_buffer->read_index = ring_info->priv_read_index;
 
-	if (hv_need_to_signal_on_read(ring_info))
-		vmbus_set_event(channel);
+	hv_signal_on_read(channel);
 }
 
 
-- 
1.7.4.1