[PATCH 00/15] Drivers: hv: CPU management fixes and a new uio driver

[PATCH 00/15] Drivers: hv: CPU management fixes and a new uio driver

[kys]

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

Fixes to handle CPU online/offline. Also included is a new uio
driver for Hyper-V.

Alex Fluter (1):
  Tools: hv: kvp: configurable external scripts path

Haiyang Zhang (2):
  hyperv: Add a function to detect hv_device
  hyperv: Fix spelling of HV_UNKOWN

K. Y. Srinivasan (2):
  Drivers: hv: vmbus: Prevent sending data on a rescinded channel
  Drivers: hv: vmbus: Enhance the rescind callback functionality

Stephen Hemminger (2):
  vmbus: add support for dynamic device id's
  uio-hv-generic: new userspace i/o driver for VMBus

Vitaly Kuznetsov (8):
  Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg()
  hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels()
  hv: allocate synic pages for all present CPUs
  hv: init percpu_list in hv_synic_alloc()
  hv: change clockevents unbind tactics
  hv: switch to cpuhp state machine for synic init/cleanup
  hv: make CPU offlining prevention fine-grained
  hv: don't reset hv_context.tsc_page on crash

 MAINTAINERS                  |    1 +
 drivers/hv/channel.c         |   17 ++--
 drivers/hv/channel_mgmt.c    |   29 ++++--
 drivers/hv/connection.c      |   18 +++-
 drivers/hv/hv.c              |   65 ++++++++++---
 drivers/hv/hyperv_vmbus.h    |    6 +-
 drivers/hv/ring_buffer.c     |    3 +
 drivers/hv/vmbus_drv.c       |  208 ++++++++++++++++++++++++++++++++++++----
 drivers/uio/Kconfig          |    9 ++
 drivers/uio/Makefile         |    1 +
 drivers/uio/uio_hv_generic.c |  218 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/hyperv.h       |   17 +++-
 tools/hv/hv_kvp_daemon.c     |   11 ++-
 13 files changed, 532 insertions(+), 71 deletions(-)
 create mode 100644 drivers/uio/uio_hv_generic.c

-- 
1.7.4.1

[PATCH 01/15] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

DoS protection conditions were altered in WS2016 and now it's easy to get
-EAGAIN returned from vmbus_post_msg() (e.g. when we try changing MTU on a
netvsc device in a loop). All vmbus_post_msg() callers don't retry the
operation and we usually end up with a non-functional device or crash.

While host's DoS protection conditions are unknown to me my tests show that
it can take up to 10 seconds before the message is sent so doing udelay()
is not an option, we really need to sleep. Almost all vmbus_post_msg()
callers are ready to sleep but there is one special case:
vmbus_initiate_unload() which can be called from interrupt/NMI context and
we can't sleep there. I'm also not sure about the lonely
vmbus_send_tl_connect_request() which has no in-tree users but its external
users are most likely waiting for the host to reply so sleeping there is
also appropriate.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c      |   17 +++++++++--------
 drivers/hv/channel_mgmt.c |   10 ++++++----
 drivers/hv/connection.c   |   17 ++++++++++++-----
 drivers/hv/hyperv_vmbus.h |    2 +-
 4 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 5fb4c6d..d5b8d9f 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -181,7 +181,7 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
 	ret = vmbus_post_msg(open_msg,
-			       sizeof(struct vmbus_channel_open_channel));
+			     sizeof(struct vmbus_channel_open_channel), true);
 
 	if (ret != 0) {
 		err = ret;
@@ -233,7 +233,7 @@ int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id,
 	conn_msg.guest_endpoint_id = *shv_guest_servie_id;
 	conn_msg.host_service_id = *shv_host_servie_id;
 
-	return vmbus_post_msg(&conn_msg, sizeof(conn_msg));
+	return vmbus_post_msg(&conn_msg, sizeof(conn_msg), true);
 }
 EXPORT_SYMBOL_GPL(vmbus_send_tl_connect_request);
 
@@ -419,7 +419,7 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
 	ret = vmbus_post_msg(gpadlmsg, msginfo->msgsize -
-			       sizeof(*msginfo));
+			     sizeof(*msginfo), true);
 	if (ret != 0)
 		goto cleanup;
 
@@ -433,8 +433,8 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 		gpadl_body->gpadl = next_gpadl_handle;
 
 		ret = vmbus_post_msg(gpadl_body,
-				     submsginfo->msgsize -
-				     sizeof(*submsginfo));
+				     submsginfo->msgsize - sizeof(*submsginfo),
+				     true);
 		if (ret != 0)
 			goto cleanup;
 
@@ -485,8 +485,8 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
 	list_add_tail(&info->msglistentry,
 		      &vmbus_connection.chn_msg_list);
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
-	ret = vmbus_post_msg(msg,
-			       sizeof(struct vmbus_channel_gpadl_teardown));
+	ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_gpadl_teardown),
+			     true);
 
 	if (ret)
 		goto post_msg_err;
@@ -557,7 +557,8 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
 	msg->header.msgtype = CHANNELMSG_CLOSECHANNEL;
 	msg->child_relid = channel->offermsg.child_relid;
 
-	ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_close_channel));
+	ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_close_channel),
+			     true);
 
 	if (ret) {
 		pr_err("Close failed: close post msg return is %d\n", ret);
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index cbb96f2..dc6b675 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -321,7 +321,8 @@ static void vmbus_release_relid(u32 relid)
 	memset(&msg, 0, sizeof(struct vmbus_channel_relid_released));
 	msg.child_relid = relid;
 	msg.header.msgtype = CHANNELMSG_RELID_RELEASED;
-	vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released));
+	vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released),
+		       true);
 }
 
 void hv_event_tasklet_disable(struct vmbus_channel *channel)
@@ -726,7 +727,8 @@ void vmbus_initiate_unload(bool crash)
 	init_completion(&vmbus_connection.unload_event);
 	memset(&hdr, 0, sizeof(struct vmbus_channel_message_header));
 	hdr.msgtype = CHANNELMSG_UNLOAD;
-	vmbus_post_msg(&hdr, sizeof(struct vmbus_channel_message_header));
+	vmbus_post_msg(&hdr, sizeof(struct vmbus_channel_message_header),
+		       !crash);
 
 	/*
 	 * vmbus_initiate_unload() is also called on crash and the crash can be
@@ -1114,8 +1116,8 @@ int vmbus_request_offers(void)
 	msg->msgtype = CHANNELMSG_REQUESTOFFERS;
 
 
-	ret = vmbus_post_msg(msg,
-			       sizeof(struct vmbus_channel_message_header));
+	ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_message_header),
+			     true);
 	if (ret != 0) {
 		pr_err("Unable to request offers - %d\n", ret);
 
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 78e6368..840b6db 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -110,7 +110,8 @@ static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo,
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
 	ret = vmbus_post_msg(msg,
-			       sizeof(struct vmbus_channel_initiate_contact));
+			     sizeof(struct vmbus_channel_initiate_contact),
+			     true);
 	if (ret != 0) {
 		spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
 		list_del(&msginfo->msglistentry);
@@ -434,7 +435,7 @@ void vmbus_on_event(unsigned long data)
 /*
  * vmbus_post_msg - Send a msg on the vmbus's message connection
  */
-int vmbus_post_msg(void *buffer, size_t buflen)
+int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep)
 {
 	union hv_connection_id conn_id;
 	int ret = 0;
@@ -449,7 +450,7 @@ int vmbus_post_msg(void *buffer, size_t buflen)
 	 * insufficient resources. Retry the operation a couple of
 	 * times before giving up.
 	 */
-	while (retries < 20) {
+	while (retries < 100) {
 		ret = hv_post_message(conn_id, 1, buffer, buflen);
 
 		switch (ret) {
@@ -472,8 +473,14 @@ int vmbus_post_msg(void *buffer, size_t buflen)
 		}
 
 		retries++;
-		udelay(usec);
-		if (usec < 2048)
+		if (can_sleep && usec > 1000)
+			msleep(usec / 1000);
+		else if (usec < MAX_UDELAY_MS * 1000)
+			udelay(usec);
+		else
+			mdelay(usec / 1000);
+
+		if (usec < 256000)
 			usec *= 2;
 	}
 	return ret;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 0675b39..27982df 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -683,7 +683,7 @@ struct hv_device *vmbus_device_create(const uuid_le *type,
 int vmbus_connect(void);
 void vmbus_disconnect(void);
 
-int vmbus_post_msg(void *buffer, size_t buflen);
+int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep);
 
 void vmbus_on_event(unsigned long data);
 void vmbus_on_msg_dpc(unsigned long data);
-- 
1.7.4.1

[PATCH 02/15] hyperv: Add a function to detect hv_device

[kys]

From: Haiyang Zhang <haiyangz@microsoft.com>

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/vmbus_drv.c |    6 ++++++
 include/linux/hyperv.h |    2 ++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 0276d2e..1730ac0 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -692,6 +692,12 @@ struct onmessage_work_context {
 	struct hv_message msg;
 };
 
+bool device_is_hyperv(struct device *dev)
+{
+	return dev->release == vmbus_device_release;
+}
+EXPORT_SYMBOL_GPL(device_is_hyperv);
+
 static void vmbus_onmessage_work(struct work_struct *work)
 {
 	struct onmessage_work_context *ctx;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 2a52d9a..fdd541c 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -946,6 +946,8 @@ static inline void clear_low_latency_mode(struct vmbus_channel *c)
 	c->low_latency = false;
 }
 
+bool device_is_hyperv(struct device *dev);
+
 void vmbus_onmessage(void *context);
 
 int vmbus_request_offers(void);
-- 
1.7.4.1

[PATCH 03/15] hyperv: Fix spelling of HV_UNKOWN

[kys]

From: Haiyang Zhang <haiyangz@microsoft.com>

Changed it to HV_UNKNOWN

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

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index dc6b675..f9c5827 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -134,7 +134,7 @@
 	},
 
 	/* Unknown GUID */
-	{ .dev_type = HV_UNKOWN,
+	{ .dev_type = HV_UNKNOWN,
 	  .perf_device = false,
 	},
 };
@@ -163,9 +163,9 @@ static u16 hv_get_dev_type(const struct vmbus_channel *channel)
 	u16 i;
 
 	if (is_hvsock_channel(channel) || is_unsupported_vmbus_devs(guid))
-		return HV_UNKOWN;
+		return HV_UNKNOWN;
 
-	for (i = HV_IDE; i < HV_UNKOWN; i++) {
+	for (i = HV_IDE; i < HV_UNKNOWN; i++) {
 		if (!uuid_le_cmp(*guid, vmbus_devs[i].guid))
 			return i;
 	}
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index fdd541c..5a2d2dd 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -696,7 +696,7 @@ enum vmbus_device_type {
 	HV_FCOPY,
 	HV_BACKUP,
 	HV_DM,
-	HV_UNKOWN,
+	HV_UNKNOWN,
 };
 
 struct vmbus_device {
-- 
1.7.4.1

[PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel

[kys]

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

After the channel is rescinded, the host does not read from the rescinded channel.
Fail writes to a channel that has already been rescinded
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/ring_buffer.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index cd49cb1..72f649d 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -298,6 +298,9 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
 	unsigned long flags = 0;
 	struct hv_ring_buffer_info *outring_info = &channel->outbound;
 
+	if (channel->rescind)
+		return -ENODEV;
+
 	for (i = 0; i < kv_count; i++)
 		totalbytes_towrite += kv_list[i].iov_len;
 
-- 
1.7.4.1

[PATCH 05/15] Drivers: hv: vmbus: Enhance the rescind callback functionality

[kys]

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

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |   11 +++++++----
 include/linux/hyperv.h    |    7 ++++---
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index f9c5827..d83c1ac 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -825,8 +825,10 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 
 	if (channel->device_obj) {
 		if (channel->chn_rescind_callback) {
-			channel->chn_rescind_callback(channel);
-			goto out;
+			channel->chn_rescind_callback(channel,
+						      channel->rescind_arg);
+			if (is_hvsock_channel(channel))
+				goto out;
 		}
 		/*
 		 * We will have to unregister this device from the
@@ -1215,9 +1217,10 @@ bool vmbus_are_subchannels_present(struct vmbus_channel *primary)
 }
 EXPORT_SYMBOL_GPL(vmbus_are_subchannels_present);
 
-void vmbus_set_chn_rescind_callback(struct vmbus_channel *channel,
-		void (*chn_rescind_cb)(struct vmbus_channel *))
+void vmbus_set_chn_rescind_callback(struct vmbus_channel *channel, void *arg,
+		void (*chn_rescind_cb)(struct vmbus_channel *, void *))
 {
 	channel->chn_rescind_callback = chn_rescind_cb;
+	channel->rescind_arg = arg;
 }
 EXPORT_SYMBOL_GPL(vmbus_set_chn_rescind_callback);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 5a2d2dd..5051eca 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -800,7 +800,8 @@ struct vmbus_channel {
 	 * Channel rescind callback. Some channels (the hvsock ones), need to
 	 * register a callback which is invoked in vmbus_onoffer_rescind().
 	 */
-	void (*chn_rescind_callback)(struct vmbus_channel *channel);
+	void (*chn_rescind_callback)(struct vmbus_channel *channel, void *arg);
+	void *rescind_arg;
 
 	/*
 	 * The spinlock to protect the structure. It is being used to protect
@@ -959,8 +960,8 @@ static inline void clear_low_latency_mode(struct vmbus_channel *c)
 void vmbus_set_sc_create_callback(struct vmbus_channel *primary_channel,
 			void (*sc_cr_cb)(struct vmbus_channel *new_sc));
 
-void vmbus_set_chn_rescind_callback(struct vmbus_channel *channel,
-		void (*chn_rescind_cb)(struct vmbus_channel *));
+void vmbus_set_chn_rescind_callback(struct vmbus_channel *channel, void *arg,
+		void (*chn_rescind_cb)(struct vmbus_channel *, void *));
 
 /*
  * Retrieve the (sub) channel on which to send an outgoing request.
-- 
1.7.4.1

[PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

"kernel BUG at drivers/hv/channel_mgmt.c:350!" is observed when hv_vmbus
module is unloaded. BUG_ON() was introduced in commit 85d9aa705184
("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()") as
vmbus_free_channels() codepath was apparently forgotten.

Fixes: 85d9aa705184 ("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index d83c1ac..e68f06c 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -390,6 +390,7 @@ void vmbus_free_channels(void)
 {
 	struct vmbus_channel *channel, *tmp;
 
+	mutex_lock(&vmbus_connection.channel_mutex);
 	list_for_each_entry_safe(channel, tmp, &vmbus_connection.chn_list,
 		listentry) {
 		/* hv_process_channel_removal() needs this */
@@ -397,6 +398,7 @@ void vmbus_free_channels(void)
 
 		vmbus_device_unregister(channel->device_obj);
 	}
+	mutex_unlock(&vmbus_connection.channel_mutex);
 }
 
 /*
-- 
1.7.4.1

[PATCH 07/15] hv: allocate synic pages for all present CPUs

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

It may happen that not all CPUs are online when we do hv_synic_alloc() and
in case more CPUs come online later we may try accessing these allocated
structures.

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

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 60dbd6c..e4bb498 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -411,7 +411,7 @@ int hv_synic_alloc(void)
 		goto err;
 	}
 
-	for_each_online_cpu(cpu) {
+	for_each_present_cpu(cpu) {
 		hv_context.event_dpc[cpu] = kmalloc(size, GFP_ATOMIC);
 		if (hv_context.event_dpc[cpu] == NULL) {
 			pr_err("Unable to allocate event dpc\n");
@@ -482,7 +482,7 @@ void hv_synic_free(void)
 	int cpu;
 
 	kfree(hv_context.hv_numa_map);
-	for_each_online_cpu(cpu)
+	for_each_present_cpu(cpu)
 		hv_synic_free_cpu(cpu);
 }
 
-- 
1.7.4.1

[PATCH 08/15] hv: init percpu_list in hv_synic_alloc()

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Initializing hv_context.percpu_list in hv_synic_alloc() helps to prevent a
crash in percpu_channel_enq() when not all CPUs were online during
initialization and it naturally belongs there.

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

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index e4bb498..a2567a4 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -457,6 +457,8 @@ int hv_synic_alloc(void)
 			pr_err("Unable to allocate post msg page\n");
 			goto err;
 		}
+
+		INIT_LIST_HEAD(&hv_context.percpu_list[cpu]);
 	}
 
 	return 0;
@@ -552,8 +554,6 @@ void hv_synic_init(void *arg)
 	rdmsrl(HV_X64_MSR_VP_INDEX, vp_index);
 	hv_context.vp_index[cpu] = (u32)vp_index;
 
-	INIT_LIST_HEAD(&hv_context.percpu_list[cpu]);
-
 	/*
 	 * Register the per-cpu clockevent source.
 	 */
-- 
1.7.4.1

[PATCH 09/15] hv: change clockevents unbind tactics

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

To get prepared to CPU offlining support we need co change the way how we
unbind clockevent devices. As one CPU may go online/offline multiple times
we need to bind it in hv_synic_init() and unbind it in hv_synic_cleanup().
There is an additional corner case: when we unload the module completely we
need to switch to some other clockevent mechanism before stopping VMBus or
we will hang. We can't call hv_synic_cleanup() before unloading VMBus as
we won't be able to send UNLOAD request and get a response so
hv_synic_clockevents_cleanup() has to live. Luckily, we can always call
clockevents_unbind_device(), even if it wasn't bound before and there is
no issue if we call it twice.

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

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index a2567a4..c11393c 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -575,7 +575,7 @@ void hv_synic_clockevents_cleanup(void)
 	if (!(ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE))
 		return;
 
-	for_each_online_cpu(cpu)
+	for_each_present_cpu(cpu)
 		clockevents_unbind_device(hv_context.clk_evt[cpu], cpu);
 }
 
@@ -594,8 +594,10 @@ void hv_synic_cleanup(void *arg)
 		return;
 
 	/* Turn off clockevent device */
-	if (ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE)
+	if (ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE) {
+		clockevents_unbind_device(hv_context.clk_evt[cpu], cpu);
 		hv_ce_shutdown(hv_context.clk_evt[cpu]);
+	}
 
 	rdmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
 
-- 
1.7.4.1

[PATCH 10/15] hv: switch to cpuhp state machine for synic init/cleanup

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

To make it possible to online/offline CPUs switch to cpuhp infrastructure
for doing hv_synic_init()/hv_synic_cleanup().

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv.c           |   15 +++++++--------
 drivers/hv/hyperv_vmbus.h |    4 ++--
 drivers/hv/vmbus_drv.c    |   19 +++++++++++--------
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index c11393c..b3f6a1b 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -495,7 +495,7 @@ void hv_synic_free(void)
  * retrieve the initialized message and event pages.  Otherwise, we create and
  * initialize the message and event pages.
  */
-void hv_synic_init(void *arg)
+int hv_synic_init(unsigned int cpu)
 {
 	u64 version;
 	union hv_synic_simp simp;
@@ -504,10 +504,8 @@ void hv_synic_init(void *arg)
 	union hv_synic_scontrol sctrl;
 	u64 vp_index;
 
-	int cpu = smp_processor_id();
-
 	if (!hv_context.hypercall_page)
-		return;
+		return -EFAULT;
 
 	/* Check the version */
 	rdmsrl(HV_X64_MSR_SVERSION, version);
@@ -562,7 +560,7 @@ void hv_synic_init(void *arg)
 						HV_TIMER_FREQUENCY,
 						HV_MIN_DELTA_TICKS,
 						HV_MAX_MAX_DELTA_TICKS);
-	return;
+	return 0;
 }
 
 /*
@@ -582,16 +580,15 @@ void hv_synic_clockevents_cleanup(void)
 /*
  * hv_synic_cleanup - Cleanup routine for hv_synic_init().
  */
-void hv_synic_cleanup(void *arg)
+int hv_synic_cleanup(unsigned int cpu)
 {
 	union hv_synic_sint shared_sint;
 	union hv_synic_simp simp;
 	union hv_synic_siefp siefp;
 	union hv_synic_scontrol sctrl;
-	int cpu = smp_processor_id();
 
 	if (!hv_context.synic_initialized)
-		return;
+		return -EFAULT;
 
 	/* Turn off clockevent device */
 	if (ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE) {
@@ -623,4 +620,6 @@ void hv_synic_cleanup(void *arg)
 	rdmsrl(HV_X64_MSR_SCONTROL, sctrl.as_uint64);
 	sctrl.enable = 0;
 	wrmsrl(HV_X64_MSR_SCONTROL, sctrl.as_uint64);
+
+	return 0;
 }
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 27982df..83beea7 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -505,9 +505,9 @@ extern int hv_post_message(union hv_connection_id connection_id,
 
 extern void hv_synic_free(void);
 
-extern void hv_synic_init(void *irqarg);
+extern int hv_synic_init(unsigned int cpu);
 
-extern void hv_synic_cleanup(void *arg);
+extern int hv_synic_cleanup(unsigned int cpu);
 
 extern void hv_synic_clockevents_cleanup(void);
 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 1730ac0..cafd273 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -49,6 +49,7 @@
 
 static struct completion probe_event;
 
+static int hyperv_cpuhp_online;
 
 static void hyperv_report_panic(struct pt_regs *regs)
 {
@@ -850,7 +851,12 @@ static int vmbus_bus_init(void)
 	 * Initialize the per-cpu interrupt state and
 	 * connect to the host.
 	 */
-	on_each_cpu(hv_synic_init, NULL, 1);
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/hyperv:online",
+				hv_synic_init, hv_synic_cleanup);
+	if (ret < 0)
+		goto err_alloc;
+	hyperv_cpuhp_online = ret;
+
 	ret = vmbus_connect();
 	if (ret)
 		goto err_connect;
@@ -872,7 +878,7 @@ static int vmbus_bus_init(void)
 	return 0;
 
 err_connect:
-	on_each_cpu(hv_synic_cleanup, NULL, 1);
+	cpuhp_remove_state(hyperv_cpuhp_online);
 err_alloc:
 	hv_synic_free();
 	hv_remove_vmbus_irq();
@@ -1326,12 +1332,9 @@ static int vmbus_acpi_add(struct acpi_device *device)
 
 static void hv_kexec_handler(void)
 {
-	int cpu;
-
 	hv_synic_clockevents_cleanup();
 	vmbus_initiate_unload(false);
-	for_each_online_cpu(cpu)
-		smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
+	cpuhp_remove_state(hyperv_cpuhp_online);
 	hv_cleanup(false);
 };
 
@@ -1343,7 +1346,7 @@ static void hv_crash_handler(struct pt_regs *regs)
 	 * doing the cleanup for current CPU only. This should be sufficient
 	 * for kdump.
 	 */
-	hv_synic_cleanup(NULL);
+	hv_synic_cleanup(smp_processor_id());
 	hv_cleanup(true);
 };
 
@@ -1407,8 +1410,8 @@ static void __exit vmbus_exit(void)
 	hv_cleanup(false);
 	for_each_online_cpu(cpu) {
 		tasklet_kill(hv_context.event_dpc[cpu]);
-		smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
 	}
+	cpuhp_remove_state(hyperv_cpuhp_online);
 	hv_synic_free();
 	acpi_bus_unregister_driver(&vmbus_acpi_driver);
 	if (vmbus_proto_version > VERSION_WIN7)
-- 
1.7.4.1

[PATCH 11/15] hv: make CPU offlining prevention fine-grained

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Since commit e513229b4c38 ("Drivers: hv: vmbus: prevent cpu offlining on
newer hypervisors") cpu offlining was disabled. It is still true that we
can't offline CPUs which have VMBus channels bound to them but we may have
'free' CPUs (e.v. we booted with maxcpus= parameter and onlined CPUs after
VMBus was initialized), these CPUs may be disabled without issues.

In future, we may even allow closing CPUs which have only sub-channels
assinged to them by closing these sub-channels. All devices will continue
to work.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv.c        |   31 +++++++++++++++++++++++++++++++
 drivers/hv/vmbus_drv.c |    9 ++++-----
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index b3f6a1b..4ece040 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -586,10 +586,41 @@ int hv_synic_cleanup(unsigned int cpu)
 	union hv_synic_simp simp;
 	union hv_synic_siefp siefp;
 	union hv_synic_scontrol sctrl;
+	struct vmbus_channel *channel, *sc;
+	bool channel_found = false;
+	unsigned long flags;
 
 	if (!hv_context.synic_initialized)
 		return -EFAULT;
 
+	/*
+	 * Search for channels which are bound to the CPU we're about to
+	 * cleanup. In case we find one and vmbus is still connected we need to
+	 * fail, this will effectively prevent CPU offlining. There is no way
+	 * we can re-bind channels to different CPUs for now.
+	 */
+	mutex_lock(&vmbus_connection.channel_mutex);
+	list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
+		if (channel->target_cpu == cpu) {
+			channel_found = true;
+			break;
+		}
+		spin_lock_irqsave(&channel->lock, flags);
+		list_for_each_entry(sc, &channel->sc_list, sc_list) {
+			if (sc->target_cpu == cpu) {
+				channel_found = true;
+				break;
+			}
+		}
+		spin_unlock_irqrestore(&channel->lock, flags);
+		if (channel_found)
+			break;
+	}
+	mutex_unlock(&vmbus_connection.channel_mutex);
+
+	if (channel_found && vmbus_connection.conn_state == CONNECTED)
+		return -EBUSY;
+
 	/* Turn off clockevent device */
 	if (ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE) {
 		clockevents_unbind_device(hv_context.clk_evt[cpu], cpu);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index cafd273..1e2a413 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -861,9 +861,6 @@ static int vmbus_bus_init(void)
 	if (ret)
 		goto err_connect;
 
-	if (vmbus_proto_version > VERSION_WIN7)
-		cpu_hotplug_disable();
-
 	/*
 	 * Only register if the crash MSRs are available
 	 */
@@ -1334,6 +1331,9 @@ static void hv_kexec_handler(void)
 {
 	hv_synic_clockevents_cleanup();
 	vmbus_initiate_unload(false);
+	vmbus_connection.conn_state = DISCONNECTED;
+	/* Make sure conn_state is set as hv_synic_cleanup checks for it */
+	mb();
 	cpuhp_remove_state(hyperv_cpuhp_online);
 	hv_cleanup(false);
 };
@@ -1346,6 +1346,7 @@ static void hv_crash_handler(struct pt_regs *regs)
 	 * doing the cleanup for current CPU only. This should be sufficient
 	 * for kdump.
 	 */
+	vmbus_connection.conn_state = DISCONNECTED;
 	hv_synic_cleanup(smp_processor_id());
 	hv_cleanup(true);
 };
@@ -1414,8 +1415,6 @@ static void __exit vmbus_exit(void)
 	cpuhp_remove_state(hyperv_cpuhp_online);
 	hv_synic_free();
 	acpi_bus_unregister_driver(&vmbus_acpi_driver);
-	if (vmbus_proto_version > VERSION_WIN7)
-		cpu_hotplug_enable();
 }
 
 
-- 
1.7.4.1

[PATCH 12/15] hv: don't reset hv_context.tsc_page on crash

[kys]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

It may happen that secondary CPUs are still alive and resetting
hv_context.tsc_page will cause a consequent crash in read_hv_clock_tsc()
as we don't check for it being not NULL there. It is safe as we're not
freeing this page anyways.

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

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 4ece040..04a0fd8 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -309,9 +309,10 @@ void hv_cleanup(bool crash)
 
 		hypercall_msr.as_uint64 = 0;
 		wrmsrl(HV_X64_MSR_REFERENCE_TSC, hypercall_msr.as_uint64);
-		if (!crash)
+		if (!crash) {
 			vfree(hv_context.tsc_page);
-		hv_context.tsc_page = NULL;
+			hv_context.tsc_page = NULL;
+		}
 	}
 #endif
 }
-- 
1.7.4.1

[PATCH 13/15] vmbus: add support for dynamic device id's

[kys]

From: Stephen Hemminger <sthemmin@microsoft.com>

This patch adds sysfs interface to dynamically bind new UUID values
to existing VMBus device. This is useful for generic UIO driver to
act similar to uio_pci_generic.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/vmbus_drv.c |  174 +++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/hyperv.h |    6 ++
 2 files changed, 172 insertions(+), 8 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 1e2a413..fbf1422 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -45,6 +45,11 @@
 #include <linux/random.h>
 #include "hyperv_vmbus.h"
 
+struct vmbus_dynid {
+	struct list_head node;
+	struct hv_vmbus_device_id id;
+};
+
 static struct acpi_device  *hv_acpi_dev;
 
 static struct completion probe_event;
@@ -501,7 +506,7 @@ static ssize_t device_show(struct device *dev,
 static DEVICE_ATTR_RO(device);
 
 /* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
-static struct attribute *vmbus_attrs[] = {
+static struct attribute *vmbus_dev_attrs[] = {
 	&dev_attr_id.attr,
 	&dev_attr_state.attr,
 	&dev_attr_monitor_id.attr,
@@ -529,7 +534,7 @@ static ssize_t device_show(struct device *dev,
 	&dev_attr_device.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(vmbus);
+ATTRIBUTE_GROUPS(vmbus_dev);
 
 /*
  * vmbus_uevent - add uevent for our device
@@ -566,10 +571,29 @@ static inline bool is_null_guid(const uuid_le *guid)
  * Return a matching hv_vmbus_device_id pointer.
  * If there is no match, return NULL.
  */
-static const struct hv_vmbus_device_id *hv_vmbus_get_id(
-					const struct hv_vmbus_device_id *id,
+static const struct hv_vmbus_device_id *hv_vmbus_get_id(struct hv_driver *drv,
 					const uuid_le *guid)
 {
+	const struct hv_vmbus_device_id *id = NULL;
+	struct vmbus_dynid *dynid;
+
+	/* Look at the dynamic ids first, before the static ones */
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry(dynid, &drv->dynids.list, node) {
+		if (!uuid_le_cmp(dynid->id.guid, *guid)) {
+			id = &dynid->id;
+			break;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+
+	if (id)
+		return id;
+
+	id = drv->id_table;
+	if (id == NULL)
+		return NULL; /* empty device table */
+
 	for (; !is_null_guid(&id->guid); id++)
 		if (!uuid_le_cmp(id->guid, *guid))
 			return id;
@@ -577,6 +601,134 @@ static inline bool is_null_guid(const uuid_le *guid)
 	return NULL;
 }
 
+/* vmbus_add_dynid - add a new device ID to this driver and re-probe devices */
+static int vmbus_add_dynid(struct hv_driver *drv, uuid_le *guid)
+{
+	struct vmbus_dynid *dynid;
+
+	dynid = kzalloc(sizeof(*dynid), GFP_KERNEL);
+	if (!dynid)
+		return -ENOMEM;
+
+	dynid->id.guid = *guid;
+
+	spin_lock(&drv->dynids.lock);
+	list_add_tail(&dynid->node, &drv->dynids.list);
+	spin_unlock(&drv->dynids.lock);
+
+	return driver_attach(&drv->driver);
+}
+
+static void vmbus_free_dynids(struct hv_driver *drv)
+{
+	struct vmbus_dynid *dynid, *n;
+
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) {
+		list_del(&dynid->node);
+		kfree(dynid);
+	}
+	spin_unlock(&drv->dynids.lock);
+}
+
+/* Parse string of form: 1b4e28ba-2fa1-11d2-883f-b9a761bde3f */
+static int get_uuid_le(const char *str, uuid_le *uu)
+{
+	unsigned int b[16];
+	int i;
+
+	if (strlen(str) < 37)
+		return -1;
+
+	for (i = 0; i < 36; i++) {
+		switch (i) {
+		case 8: case 13: case 18: case 23:
+			if (str[i] != '-')
+				return -1;
+			break;
+		default:
+			if (!isxdigit(str[i]))
+				return -1;
+		}
+	}
+
+	/* unparse little endian output byte order */
+	if (sscanf(str,
+		   "%2x%2x%2x%2x-%2x%2x-%2x%2x-%2x%2x-%2x%2x%2x%2x%2x%2x",
+		   &b[3], &b[2], &b[1], &b[0],
+		   &b[5], &b[4], &b[7], &b[6], &b[8], &b[9],
+		   &b[10], &b[11], &b[12], &b[13], &b[14], &b[15]) != 16)
+		return -1;
+
+	for (i = 0; i < 16; i++)
+		uu->b[i] = b[i];
+	return 0;
+}
+
+/*
+ * store_new_id - sysfs frontend to vmbus_add_dynid()
+ *
+ * Allow GUIDs to be added to an existing driver via sysfs.
+ */
+static ssize_t new_id_store(struct device_driver *driver, const char *buf,
+			    size_t count)
+{
+	struct hv_driver *drv = drv_to_hv_drv(driver);
+	uuid_le guid = NULL_UUID_LE;
+	ssize_t retval;
+
+	if (get_uuid_le(buf, &guid) != 0)
+		return -EINVAL;
+
+	if (hv_vmbus_get_id(drv, &guid))
+		return -EEXIST;
+
+	retval = vmbus_add_dynid(drv, &guid);
+	if (retval)
+		return retval;
+	return count;
+}
+static DRIVER_ATTR_WO(new_id);
+
+/*
+ * store_remove_id - remove a PCI device ID from this driver
+ *
+ * Removes a dynamic pci device ID to this driver.
+ */
+static ssize_t remove_id_store(struct device_driver *driver, const char *buf,
+			       size_t count)
+{
+	struct hv_driver *drv = drv_to_hv_drv(driver);
+	struct vmbus_dynid *dynid, *n;
+	uuid_le guid = NULL_UUID_LE;
+	size_t retval = -ENODEV;
+
+	if (get_uuid_le(buf, &guid))
+		return -EINVAL;
+
+	spin_lock(&drv->dynids.lock);
+	list_for_each_entry_safe(dynid, n, &drv->dynids.list, node) {
+		struct hv_vmbus_device_id *id = &dynid->id;
+
+		if (!uuid_le_cmp(id->guid, guid)) {
+			list_del(&dynid->node);
+			kfree(dynid);
+			retval = count;
+			break;
+		}
+	}
+	spin_unlock(&drv->dynids.lock);
+
+	return retval;
+}
+static DRIVER_ATTR_WO(remove_id);
+
+static struct attribute *vmbus_drv_attrs[] = {
+	&driver_attr_new_id.attr,
+	&driver_attr_remove_id.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(vmbus_drv);
 
 
 /*
@@ -591,7 +743,7 @@ static int vmbus_match(struct device *device, struct device_driver *driver)
 	if (is_hvsock_channel(hv_dev->channel))
 		return drv->hvsock;
 
-	if (hv_vmbus_get_id(drv->id_table, &hv_dev->dev_type))
+	if (hv_vmbus_get_id(drv, &hv_dev->dev_type))
 		return 1;
 
 	return 0;
@@ -608,7 +760,7 @@ static int vmbus_probe(struct device *child_device)
 	struct hv_device *dev = device_to_hv_device(child_device);
 	const struct hv_vmbus_device_id *dev_id;
 
-	dev_id = hv_vmbus_get_id(drv->id_table, &dev->dev_type);
+	dev_id = hv_vmbus_get_id(drv, &dev->dev_type);
 	if (drv->probe) {
 		ret = drv->probe(dev, dev_id);
 		if (ret != 0)
@@ -685,7 +837,8 @@ static void vmbus_device_release(struct device *device)
 	.remove =		vmbus_remove,
 	.probe =		vmbus_probe,
 	.uevent =		vmbus_uevent,
-	.dev_groups =		vmbus_groups,
+	.dev_groups =		vmbus_dev_groups,
+	.drv_groups =		vmbus_drv_groups,
 };
 
 struct onmessage_work_context {
@@ -914,6 +1067,9 @@ int __vmbus_driver_register(struct hv_driver *hv_driver, struct module *owner, c
 	hv_driver->driver.mod_name = mod_name;
 	hv_driver->driver.bus = &hv_bus;
 
+	spin_lock_init(&hv_driver->dynids.lock);
+	INIT_LIST_HEAD(&hv_driver->dynids.list);
+
 	ret = driver_register(&hv_driver->driver);
 
 	return ret;
@@ -932,8 +1088,10 @@ void vmbus_driver_unregister(struct hv_driver *hv_driver)
 {
 	pr_info("unregistering driver %s\n", hv_driver->name);
 
-	if (!vmbus_exists())
+	if (!vmbus_exists()) {
 		driver_unregister(&hv_driver->driver);
+		vmbus_free_dynids(hv_driver);
+	}
 }
 EXPORT_SYMBOL_GPL(vmbus_driver_unregister);
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 5051eca..f4c0788 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1122,6 +1122,12 @@ struct hv_driver {
 
 	struct device_driver driver;
 
+	/* dynamic device GUID's */
+	struct  {
+		spinlock_t lock;
+		struct list_head list;
+	} dynids;
+
 	int (*probe)(struct hv_device *, const struct hv_vmbus_device_id *);
 	int (*remove)(struct hv_device *);
 	void (*shutdown)(struct hv_device *);
-- 
1.7.4.1

[PATCH 14/15] uio-hv-generic: new userspace i/o driver for VMBus

[kys]

From: Stephen Hemminger <sthemmin@microsoft.com>

This is a new driver to enable userspace networking on VMBus.
It is based largely on the similar driver that already exists
for PCI, and earlier work done by Brocade to support DPDK.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 MAINTAINERS                  |    1 +
 drivers/hv/connection.c      |    1 +
 drivers/uio/Kconfig          |    9 ++
 drivers/uio/Makefile         |    1 +
 drivers/uio/uio_hv_generic.c |  218 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 230 insertions(+), 0 deletions(-)
 create mode 100644 drivers/uio/uio_hv_generic.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2f89c3b..6d6bea9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5851,6 +5851,7 @@ F:	drivers/input/serio/hyperv-keyboard.c
 F:	drivers/pci/host/pci-hyperv.c
 F:	drivers/net/hyperv/
 F:	drivers/scsi/storvsc_drv.c
+F:	drivers/uio/uio_hv_generic.c
 F:	drivers/video/fbdev/hyperv_fb.c
 F:	include/linux/hyperv.h
 F:	tools/hv/
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 840b6db..9b72ebc 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -39,6 +39,7 @@ struct vmbus_connection vmbus_connection = {
 	.conn_state		= DISCONNECTED,
 	.next_gpadl_handle	= ATOMIC_INIT(0xE1E10),
 };
+EXPORT_SYMBOL_GPL(vmbus_connection);
 
 /*
  * Negotiated protocol version with the host.
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 52c98ce..7e8dc78 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -155,4 +155,13 @@ config UIO_MF624
 
 	  If you compile this as a module, it will be called uio_mf624.
 
+config UIO_HV_GENERIC
+	tristate "Generic driver for Hyper-V VMBus"
+	depends on HYPERV
+	help
+	  Generic driver that you can bind, dynamically, to any
+	  Hyper-V VMBus device. It is useful to provide direct access
+	  to network and storage devices from userspace.
+
+	  If you compile this as a module, it will be called uio_hv_generic.
 endif
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index 8560dad..e9663bb 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_UIO_NETX)	+= uio_netx.o
 obj-$(CONFIG_UIO_PRUSS)         += uio_pruss.o
 obj-$(CONFIG_UIO_MF624)         += uio_mf624.o
 obj-$(CONFIG_UIO_FSL_ELBC_GPCM)	+= uio_fsl_elbc_gpcm.o
+obj-$(CONFIG_UIO_HV_GENERIC)	+= uio_hv_generic.o
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
new file mode 100644
index 0000000..ad3ab58
--- /dev/null
+++ b/drivers/uio/uio_hv_generic.c
@@ -0,0 +1,218 @@
+/*
+ * uio_hv_generic - generic UIO driver for VMBus
+ *
+ * Copyright (c) 2013-2016 Brocade Communications Systems, Inc.
+ * Copyright (c) 2016, Microsoft Corporation.
+ *
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ *
+ * Since the driver does not declare any device ids, you must allocate
+ * id and bind the device to the driver yourself.  For example:
+ *
+ * # echo "f8615163-df3e-46c5-913f-f2d2f965ed0e" \
+ *    > /sys/bus/vmbus/drivers/uio_hv_generic
+ * # echo -n vmbus-ed963694-e847-4b2a-85af-bc9cfc11d6f3 \
+ *    > /sys/bus/vmbus/drivers/hv_netvsc/unbind
+ * # echo -n vmbus-ed963694-e847-4b2a-85af-bc9cfc11d6f3 \
+ *    > /sys/bus/vmbus/drivers/uio_hv_generic/bind
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/uio_driver.h>
+#include <linux/netdevice.h>
+#include <linux/if_ether.h>
+#include <linux/skbuff.h>
+#include <linux/hyperv.h>
+#include <linux/vmalloc.h>
+#include <linux/slab.h>
+
+#include "../hv/hyperv_vmbus.h"
+
+#define DRIVER_VERSION	"0.02.0"
+#define DRIVER_AUTHOR	"Stephen Hemminger <sthemmin at microsoft.com>"
+#define DRIVER_DESC	"Generic UIO driver for VMBus devices"
+
+/*
+ * List of resources to be mapped to user space
+ * can be extended up to MAX_UIO_MAPS(5) items
+ */
+enum hv_uio_map {
+	TXRX_RING_MAP = 0,
+	INT_PAGE_MAP,
+	MON_PAGE_MAP,
+};
+
+#define HV_RING_SIZE	512
+
+struct hv_uio_private_data {
+	struct uio_info info;
+	struct hv_device *device;
+};
+
+static int
+hv_uio_mmap(struct uio_info *info, struct vm_area_struct *vma)
+{
+	int mi;
+
+	if (vma->vm_pgoff >= MAX_UIO_MAPS)
+		return -EINVAL;
+
+	if (info->mem[vma->vm_pgoff].size == 0)
+		return -EINVAL;
+
+	mi = (int)vma->vm_pgoff;
+
+	return remap_pfn_range(vma, vma->vm_start,
+			virt_to_phys((void *)info->mem[mi].addr) >> PAGE_SHIFT,
+			vma->vm_end - vma->vm_start, vma->vm_page_prot);
+}
+
+/*
+ * This is the irqcontrol callback to be registered to uio_info.
+ * It can be used to disable/enable interrupt from user space processes.
+ *
+ * @param info
+ *  pointer to uio_info.
+ * @param irq_state
+ *  state value. 1 to enable interrupt, 0 to disable interrupt.
+ */
+static int
+hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)
+{
+	struct hv_uio_private_data *pdata = info->priv;
+	struct hv_device *dev = pdata->device;
+
+	dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state;
+	virt_mb();
+
+	return 0;
+}
+
+/*
+ * Callback from vmbus_event when something is in inbound ring.
+ */
+static void hv_uio_channel_cb(void *context)
+{
+	struct hv_uio_private_data *pdata = context;
+	struct hv_device *dev = pdata->device;
+
+	dev->channel->inbound.ring_buffer->interrupt_mask = 1;
+	virt_mb();
+
+	uio_event_notify(&pdata->info);
+}
+
+static int
+hv_uio_probe(struct hv_device *dev,
+	     const struct hv_vmbus_device_id *dev_id)
+{
+	struct hv_uio_private_data *pdata;
+	int ret;
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	ret = vmbus_open(dev->channel, HV_RING_SIZE * PAGE_SIZE,
+			 HV_RING_SIZE * PAGE_SIZE, NULL, 0,
+			 hv_uio_channel_cb, pdata);
+	if (ret)
+		goto fail;
+
+	dev->channel->inbound.ring_buffer->interrupt_mask = 1;
+	dev->channel->batched_reading = false;
+
+	/* Fill general uio info */
+	pdata->info.name = "uio_hv_generic";
+	pdata->info.version = DRIVER_VERSION;
+	pdata->info.irqcontrol = hv_uio_irqcontrol;
+	pdata->info.mmap = hv_uio_mmap;
+	pdata->info.irq = UIO_IRQ_CUSTOM;
+
+	/* mem resources */
+	pdata->info.mem[TXRX_RING_MAP].name = "txrx_rings";
+	pdata->info.mem[TXRX_RING_MAP].addr
+		= (phys_addr_t)dev->channel->ringbuffer_pages;
+	pdata->info.mem[TXRX_RING_MAP].size
+		= dev->channel->ringbuffer_pagecount * PAGE_SIZE;
+	pdata->info.mem[TXRX_RING_MAP].memtype = UIO_MEM_LOGICAL;
+
+	pdata->info.mem[INT_PAGE_MAP].name = "int_page";
+	pdata->info.mem[INT_PAGE_MAP].addr =
+		(phys_addr_t)vmbus_connection.int_page;
+	pdata->info.mem[INT_PAGE_MAP].size = PAGE_SIZE;
+	pdata->info.mem[INT_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
+
+	pdata->info.mem[MON_PAGE_MAP].name = "monitor_pages";
+	pdata->info.mem[MON_PAGE_MAP].addr =
+		(phys_addr_t)vmbus_connection.monitor_pages[1];
+	pdata->info.mem[MON_PAGE_MAP].size = PAGE_SIZE;
+	pdata->info.mem[MON_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
+
+	pdata->info.priv = pdata;
+	pdata->device = dev;
+
+	ret = uio_register_device(&dev->device, &pdata->info);
+	if (ret) {
+		dev_err(&dev->device, "hv_uio register failed\n");
+		goto fail_close;
+	}
+
+	hv_set_drvdata(dev, pdata);
+
+	return 0;
+
+fail_close:
+	vmbus_close(dev->channel);
+fail:
+	kfree(pdata);
+
+	return ret;
+}
+
+static int
+hv_uio_remove(struct hv_device *dev)
+{
+	struct hv_uio_private_data *pdata = hv_get_drvdata(dev);
+
+	if (!pdata)
+		return 0;
+
+	uio_unregister_device(&pdata->info);
+	hv_set_drvdata(dev, NULL);
+	vmbus_close(dev->channel);
+	kfree(pdata);
+	return 0;
+}
+
+static struct hv_driver hv_uio_drv = {
+	.name = "uio_hv_generic",
+	.id_table = NULL, /* only dynamic id's */
+	.probe = hv_uio_probe,
+	.remove = hv_uio_remove,
+};
+
+static int __init
+hyperv_module_init(void)
+{
+	return vmbus_driver_register(&hv_uio_drv);
+}
+
+static void __exit
+hyperv_module_exit(void)
+{
+	vmbus_driver_unregister(&hv_uio_drv);
+}
+
+module_init(hyperv_module_init);
+module_exit(hyperv_module_exit);
+
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
-- 
1.7.4.1

[PATCH 15/15] Tools: hv: kvp: configurable external scripts path

[kys]

From: Alex Fluter <afluter@yandex.com>

error when running hypervkvpd:
$ sudo ./hv_kvp_daemon -n

sh: hv_get_dns_info: command not found
sh: hv_get_dhcp_info: command not found
sh: hv_get_dns_info: command not found
sh: hv_get_dhcp_info: command not found

The external scripts are not installed in system path,
adding a configurable macro.

Signed-off-by: Alex Fluter <afluter@yandex.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 tools/hv/hv_kvp_daemon.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index d791dbf..f1758fc 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -93,6 +93,10 @@ enum {
 
 #define KVP_CONFIG_LOC	"/var/lib/hyperv"
 
+#ifndef KVP_SCRIPTS_PATH
+#define KVP_SCRIPTS_PATH "/usr/libexec/hypervkvpd/"
+#endif
+
 #define MAX_FILE_NAME 100
 #define ENTRIES_PER_BLOCK 50
 
@@ -818,7 +822,7 @@ static void kvp_get_ipconfig_info(char *if_name,
 	 * .
 	 */
 
-	sprintf(cmd, "%s",  "hv_get_dns_info");
+	sprintf(cmd, KVP_SCRIPTS_PATH "%s",  "hv_get_dns_info");
 
 	/*
 	 * Execute the command to gather DNS info.
@@ -835,7 +839,7 @@ static void kvp_get_ipconfig_info(char *if_name,
 	 * Enabled: DHCP enabled.
 	 */
 
-	sprintf(cmd, "%s %s", "hv_get_dhcp_info", if_name);
+	sprintf(cmd, KVP_SCRIPTS_PATH "%s %s", "hv_get_dhcp_info", if_name);
 
 	file = popen(cmd, "r");
 	if (file == NULL)
@@ -1341,7 +1345,8 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
 	 * invoke the external script to do its magic.
 	 */
 
-	snprintf(cmd, sizeof(cmd), "%s %s", "hv_set_ifconfig", if_file);
+	snprintf(cmd, sizeof(cmd), KVP_SCRIPTS_PATH "%s %s",
+		 "hv_set_ifconfig", if_file);
 	if (system(cmd)) {
 		syslog(LOG_ERR, "Failed to execute cmd '%s'; error: %d %s",
 				cmd, errno, strerror(errno));
-- 
1.7.4.1

Re: [PATCH 02/15] hyperv: Add a function to detect hv_device

[Greg KH]

On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/vmbus_drv.c |    6 ++++++
>  include/linux/hyperv.h |    2 ++
>  2 files changed, 8 insertions(+), 0 deletions(-)

I can't, and you shouldn't, take patches with no changelog comments at
all.

sorry.

greg k-h

Re: [PATCH 02/15] hyperv: Add a function to detect hv_device

[Greg KH]

On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
> 
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/vmbus_drv.c |    6 ++++++
>  include/linux/hyperv.h |    2 ++
>  2 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 0276d2e..1730ac0 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -692,6 +692,12 @@ struct onmessage_work_context {
>  	struct hv_message msg;
>  };
>  
> +bool device_is_hyperv(struct device *dev)
> +{
> +	return dev->release == vmbus_device_release;
> +}
> +EXPORT_SYMBOL_GPL(device_is_hyperv);

Wait, eek, no!  That's NOT how you determine a device type, if you
really even ever need to do that.

Why are you needing this?  You should always "just know" what type of
device a struct device * is, that's what we rely on in the driver model.
Otherwise things get messy very very quickly.

Sorry, I can't take this without a ton of justification, and even then,
you need to do this correctly (and no, I'm not going to tell you how to
do that as I don't like it being done...)

greg k-h

Re: [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel

[Greg KH]

On Thu, Dec 01, 2016 at 09:28:41AM -0800, kys@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
> 
> After the channel is rescinded, the host does not read from the rescinded channel.
> Fail writes to a channel that has already been rescinded
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

{sigh}

What's wrong with this picture...

Re: [PATCH 05/15] Drivers: hv: vmbus: Enhance the rescind callback functionality

[Greg KH]

On Thu, Dec 01, 2016 at 09:28:42AM -0800, kys@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/channel_mgmt.c |   11 +++++++----
>  include/linux/hyperv.h    |    7 ++++---
>  2 files changed, 11 insertions(+), 7 deletions(-)

no commit log == will not be committed

Re: [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels()

[Greg KH]

On Thu, Dec 01, 2016 at 09:28:43AM -0800, kys@exchange.microsoft.com wrote:
> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> "kernel BUG at drivers/hv/channel_mgmt.c:350!" is observed when hv_vmbus
> module is unloaded. BUG_ON() was introduced in commit 85d9aa705184
> ("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()") as
> vmbus_free_channels() codepath was apparently forgotten.
> 
> Fixes: 85d9aa705184 ("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/channel_mgmt.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

Shouldn't this go to stable kernels?

Re: [PATCH 02/15] hyperv: Add a function to detect hv_device

[Greg KH]

On Thu, Dec 01, 2016 at 09:35:47PM +0100, Greg KH wrote:
> On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> > 
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  drivers/hv/vmbus_drv.c |    6 ++++++
> >  include/linux/hyperv.h |    2 ++
> >  2 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index 0276d2e..1730ac0 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -692,6 +692,12 @@ struct onmessage_work_context {
> >  	struct hv_message msg;
> >  };
> >  
> > +bool device_is_hyperv(struct device *dev)
> > +{
> > +	return dev->release == vmbus_device_release;
> > +}
> > +EXPORT_SYMBOL_GPL(device_is_hyperv);
> 
> Wait, eek, no!  That's NOT how you determine a device type, if you
> really even ever need to do that.
> 
> Why are you needing this?  You should always "just know" what type of
> device a struct device * is, that's what we rely on in the driver model.
> Otherwise things get messy very very quickly.
> 
> Sorry, I can't take this without a ton of justification, and even then,
> you need to do this correctly (and no, I'm not going to tell you how to
> do that as I don't like it being done...)

And, to make this even worse, you never use this function in this
series, making this something that no one even needs!!!

ugh.

I'm dropping this whole series, sorry, get it together, this was a
mess...

greg k-h

RE: [PATCH 02/15] hyperv: Add a function to detect hv_device

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 12:34 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> 
> On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com
> wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  drivers/hv/vmbus_drv.c |    6 ++++++
> >  include/linux/hyperv.h |    2 ++
> >  2 files changed, 8 insertions(+), 0 deletions(-)
> 
> I can't, and you shouldn't, take patches with no changelog comments at
> all.
> 
> sorry.

Sorry about this; this will be fixed.

K. Y
> 
> greg k-h

RE: [PATCH 02/15] hyperv: Add a function to detect hv_device

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 12:36 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> 
> On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com
> wrote:
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  drivers/hv/vmbus_drv.c |    6 ++++++
> >  include/linux/hyperv.h |    2 ++
> >  2 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index 0276d2e..1730ac0 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -692,6 +692,12 @@ struct onmessage_work_context {
> >  	struct hv_message msg;
> >  };
> >
> > +bool device_is_hyperv(struct device *dev)
> > +{
> > +	return dev->release == vmbus_device_release;
> > +}
> > +EXPORT_SYMBOL_GPL(device_is_hyperv);
> 
> Wait, eek, no!  That's NOT how you determine a device type, if you
> really even ever need to do that.
> 
> Why are you needing this?  You should always "just know" what type of
> device a struct device * is, that's what we rely on in the driver model.
> Otherwise things get messy very very quickly.
> 
> Sorry, I can't take this without a ton of justification, and even then,
> you need to do this correctly (and no, I'm not going to tell you how to
> do that as I don't like it being done...)
> 
Greg,

To support SR-IOV, netvsc registers for all netdev events. For netdev events related to the
VF interface, we need to do some special processing. And so, we need to determine
if the device that is generating the netdev event is Hyper-V device or not
(passed through the PCI pass through driver). Is this justification sufficient.
As you have observed, currently there is no user of this API and that is because netvsc
will be the user. To avoid cross-tree dependency, we wanted to get this functionality in first
before submitting the netvsc patch.

Regards,

K. Y  

> greg k-h

RE: [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in vmbus_free_channels()

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 12:37 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com
> Subject: Re: [PATCH 06/15] hv: acquire vmbus_connection.channel_mutex in
> vmbus_free_channels()
> 
> On Thu, Dec 01, 2016 at 09:28:43AM -0800, kys@exchange.microsoft.com
> wrote:
> > From: Vitaly Kuznetsov <vkuznets@redhat.com>
> >
> > "kernel BUG at drivers/hv/channel_mgmt.c:350!" is observed when
> hv_vmbus
> > module is unloaded. BUG_ON() was introduced in commit 85d9aa705184
> > ("Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister()") as
> > vmbus_free_channels() codepath was apparently forgotten.
> >
> > Fixes: 85d9aa705184 ("Drivers: hv: vmbus: add an API
> vmbus_hvsock_device_unregister()")
> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  drivers/hv/channel_mgmt.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> Shouldn't this go to stable kernels?

Yes; I will resubmit.

Regards,

K. Y

Re: [PATCH 02/15] hyperv: Add a function to detect hv_device

[Greg KH]

On Fri, Dec 02, 2016 at 06:02:29AM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Thursday, December 1, 2016 12:36 PM
> > To: KY Srinivasan <kys@microsoft.com>
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> > <haiyangz@microsoft.com>
> > Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device

Ugh, please fix your email client...

> > 
> > On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com
> > wrote:
> > > From: Haiyang Zhang <haiyangz@microsoft.com>
> > >
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > ---
> > >  drivers/hv/vmbus_drv.c |    6 ++++++
> > >  include/linux/hyperv.h |    2 ++
> > >  2 files changed, 8 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > > index 0276d2e..1730ac0 100644
> > > --- a/drivers/hv/vmbus_drv.c
> > > +++ b/drivers/hv/vmbus_drv.c
> > > @@ -692,6 +692,12 @@ struct onmessage_work_context {
> > >  	struct hv_message msg;
> > >  };
> > >
> > > +bool device_is_hyperv(struct device *dev)
> > > +{
> > > +	return dev->release == vmbus_device_release;
> > > +}
> > > +EXPORT_SYMBOL_GPL(device_is_hyperv);
> > 
> > Wait, eek, no!  That's NOT how you determine a device type, if you
> > really even ever need to do that.
> > 
> > Why are you needing this?  You should always "just know" what type of
> > device a struct device * is, that's what we rely on in the driver model.
> > Otherwise things get messy very very quickly.
> > 
> > Sorry, I can't take this without a ton of justification, and even then,
> > you need to do this correctly (and no, I'm not going to tell you how to
> > do that as I don't like it being done...)
> > 
> Greg,
> 
> To support SR-IOV, netvsc registers for all netdev events. For netdev events related to the
> VF interface, we need to do some special processing. And so, we need to determine
> if the device that is generating the netdev event is Hyper-V device or not
> (passed through the PCI pass through driver). Is this justification sufficient.
> As you have observed, currently there is no user of this API and that is because netvsc
> will be the user. To avoid cross-tree dependency, we wanted to get this functionality in first
> before submitting the netvsc patch.

See, you do have some text for a changelog!

{grumble...}

No, I don't think this is a good justification, where are you going to
put this "check" into the networking stack?  Your driver should only be
binding to devices of this "type" anyway, so by that logic, it already
"knows" that the device is of this type.

In other words, why do you need this and PCI or USB doesn't?  Why is
hyperv "special"?

greg k-h

RE: [PATCH 02/15] hyperv: Add a function to detect hv_device

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 10:48 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> 
> On Fri, Dec 02, 2016 at 06:02:29AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Thursday, December 1, 2016 12:36 PM
> > > To: KY Srinivasan <kys@microsoft.com>
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > > jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> > > <haiyangz@microsoft.com>
> > > Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> 
> Ugh, please fix your email client...
> 
> > >
> > > On Thu, Dec 01, 2016 at 09:28:39AM -0800, kys@exchange.microsoft.com
> > > wrote:
> > > > From: Haiyang Zhang <haiyangz@microsoft.com>
> > > >
> > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > ---
> > > >  drivers/hv/vmbus_drv.c |    6 ++++++
> > > >  include/linux/hyperv.h |    2 ++
> > > >  2 files changed, 8 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > > > index 0276d2e..1730ac0 100644
> > > > --- a/drivers/hv/vmbus_drv.c
> > > > +++ b/drivers/hv/vmbus_drv.c
> > > > @@ -692,6 +692,12 @@ struct onmessage_work_context {
> > > >  	struct hv_message msg;
> > > >  };
> > > >
> > > > +bool device_is_hyperv(struct device *dev)
> > > > +{
> > > > +	return dev->release == vmbus_device_release;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(device_is_hyperv);
> > >
> > > Wait, eek, no!  That's NOT how you determine a device type, if you
> > > really even ever need to do that.
> > >
> > > Why are you needing this?  You should always "just know" what type of
> > > device a struct device * is, that's what we rely on in the driver model.
> > > Otherwise things get messy very very quickly.
> > >
> > > Sorry, I can't take this without a ton of justification, and even then,
> > > you need to do this correctly (and no, I'm not going to tell you how to
> > > do that as I don't like it being done...)
> > >
> > Greg,
> >
> > To support SR-IOV, netvsc registers for all netdev events. For netdev
> events related to the
> > VF interface, we need to do some special processing. And so, we need to
> determine
> > if the device that is generating the netdev event is Hyper-V device or not
> > (passed through the PCI pass through driver). Is this justification sufficient.
> > As you have observed, currently there is no user of this API and that is
> because netvsc
> > will be the user. To avoid cross-tree dependency, we wanted to get this
> functionality in first
> > before submitting the netvsc patch.
> 
> See, you do have some text for a changelog!
> 
> {grumble...}
> 
> No, I don't think this is a good justification, where are you going to
> put this "check" into the networking stack?  Your driver should only be
> binding to devices of this "type" anyway, so by that logic, it already
> "knows" that the device is of this type.
> 
> In other words, why do you need this and PCI or USB doesn't?  Why is
> hyperv "special"?

On Hyper-V, each VF interface (SR-IOV interface)
is paired with an instance of the 
synthetic interface that is managed by netvsc.
When the VF interface comes up, we
need to associate the VF instance with 
the corresponding netvsc instance. To do this
without modifying the VF drivers, netvsc registers 
for netdev events. In the netdev
event handler (in netvsc) currently I am 
doing the association based on the MAC
address - this code is currently committed 
upstream (see drivers/net/hyperv/netvsc_drv.c 
netvsc_register_vf()). Going forward, we want to 
base this association based on a sequence
number that the host publishes both for the 
VF as well as the corresponding
synthetic (netvsc) instance.

You are right, netvsc already knows that the 
devices it is managing belong to vmbus.
Since we are registering for netdev events, 
we will get notified for devices that may not
be vmbus devices and that is where this new 
API will be used. If the device is a vmbus device
we can extract the sequence number to implement the match.

Regards,

K. Y

Re: [PATCH 02/15] hyperv: Add a function to detect hv_device

[Greg KH]

On Fri, Dec 02, 2016 at 07:14:03AM +0000, KY Srinivasan wrote:
> > In other words, why do you need this and PCI or USB doesn't?  Why is
> > hyperv "special"?
> 
> On Hyper-V, each VF interface (SR-IOV interface)
> is paired with an instance of the 
> synthetic interface that is managed by netvsc.
> When the VF interface comes up, we
> need to associate the VF instance with 
> the corresponding netvsc instance. To do this
> without modifying the VF drivers, netvsc registers 
> for netdev events.

Why not modify the VF drivers?  You have the full source to them...

RE: [PATCH 02/15] hyperv: Add a function to detect hv_device

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, December 1, 2016 11:36 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> <haiyangz@microsoft.com>
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> 
> On Fri, Dec 02, 2016 at 07:14:03AM +0000, KY Srinivasan wrote:
> > > In other words, why do you need this and PCI or USB doesn't?  Why is
> > > hyperv "special"?
> >
> > On Hyper-V, each VF interface (SR-IOV interface)
> > is paired with an instance of the
> > synthetic interface that is managed by netvsc.
> > When the VF interface comes up, we
> > need to associate the VF instance with
> > the corresponding netvsc instance. To do this
> > without modifying the VF drivers, netvsc registers
> > for netdev events.
> 
> Why not modify the VF drivers?  You have the full source to them...
Greg,

This is even worse. On Linux, VF drivers are hypervisor agnostic
and I want to keep it that way.

Regards,

K. Y
 

Re: [PATCH 02/15] hyperv: Add a function to detect hv_device

[Greg KH]

On Fri, Dec 02, 2016 at 03:38:51PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Thursday, December 1, 2016 11:36 PM
> > To: KY Srinivasan <kys@microsoft.com>
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> > <haiyangz@microsoft.com>
> > Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> > 
> > On Fri, Dec 02, 2016 at 07:14:03AM +0000, KY Srinivasan wrote:
> > > > In other words, why do you need this and PCI or USB doesn't?  Why is
> > > > hyperv "special"?
> > >
> > > On Hyper-V, each VF interface (SR-IOV interface)
> > > is paired with an instance of the
> > > synthetic interface that is managed by netvsc.
> > > When the VF interface comes up, we
> > > need to associate the VF instance with
> > > the corresponding netvsc instance. To do this
> > > without modifying the VF drivers, netvsc registers
> > > for netdev events.
> > 
> > Why not modify the VF drivers?  You have the full source to them...
> Greg,
> 
> This is even worse. On Linux, VF drivers are hypervisor agnostic
> and I want to keep it that way.

Ok, I really don't know what to suggest, other than this is probably not
the way to do this as no other bus has to.  As I don't see the code that
actually uses this anywhere, it's really impossible to have this
conversation at all :(

greg k-h

RE: [PATCH 02/15] hyperv: Add a function to detect hv_device

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Friday, December 2, 2016 8:03 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: olaf@aepfle.de; jasowang@redhat.com; Haiyang Zhang
> <haiyangz@microsoft.com>; linux-kernel@vger.kernel.org;
> apw@canonical.com; devel@linuxdriverproject.org;
> leann.ogasawara@canonical.com
> Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> 
> On Fri, Dec 02, 2016 at 03:38:51PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Thursday, December 1, 2016 11:36 PM
> > > To: KY Srinivasan <kys@microsoft.com>
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> > > jasowang@redhat.com; leann.ogasawara@canonical.com; Haiyang Zhang
> > > <haiyangz@microsoft.com>
> > > Subject: Re: [PATCH 02/15] hyperv: Add a function to detect hv_device
> > >
> > > On Fri, Dec 02, 2016 at 07:14:03AM +0000, KY Srinivasan wrote:
> > > > > In other words, why do you need this and PCI or USB doesn't?  Why is
> > > > > hyperv "special"?
> > > >
> > > > On Hyper-V, each VF interface (SR-IOV interface)
> > > > is paired with an instance of the
> > > > synthetic interface that is managed by netvsc.
> > > > When the VF interface comes up, we
> > > > need to associate the VF instance with
> > > > the corresponding netvsc instance. To do this
> > > > without modifying the VF drivers, netvsc registers
> > > > for netdev events.
> > >
> > > Why not modify the VF drivers?  You have the full source to them...
> > Greg,
> >
> > This is even worse. On Linux, VF drivers are hypervisor agnostic
> > and I want to keep it that way.
> 
> Ok, I really don't know what to suggest, other than this is probably not
> the way to do this as no other bus has to.  As I don't see the code that
> actually uses this anywhere, it's really impossible to have this
> conversation at all :(

I agree it is difficult to discuss this without
having the code that uses this.
That said, there is currently code in
the tree that disambiguates the netdev
events that netvsc sees - look at the function
get_netvsc_bymac(). This function allows us to 
associate the VF interface that maybe coming up with the
associated netvsc interface using MAC address. What I want to do
is to not use the MAC address but to use a serial number that the
host publishes. I could send the netvsc patches that use this if that
would help here. In any case once we have this functionality, we will be
submitting the patches that use this.

What I am trying to do here is to implement the equivalent of dev_is_pci()
for vmbus.
 
You also had concerns about how we were implementing this functionality.
We could certainly use the same mechanism used in dev_is_pci(). Perhaps
I could also use the same naming convention - dev_is_hv()?

Regards,

K. Y

> 
> greg k-h

Re: [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel

[Dan Carpenter]

On Thu, Dec 01, 2016 at 09:28:41AM -0800, kys@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan <kys@microsoft.com>
> 
> After the channel is rescinded, the host does not read from the rescinded channel.
> Fail writes to a channel that has already been rescinded

What are the user visible effects of this bug?  Presumably none right
since it doesn't need to be back ported.

But it's absolutely impossible to tell from the commit message.

regards,
dan carpenter

RE: [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a rescinded channel

[KY Srinivasan]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Monday, December 5, 2016 2:01 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
> Subject: Re: [PATCH 04/15] Drivers: hv: vmbus: Prevent sending data on a
> rescinded channel
> 
> On Thu, Dec 01, 2016 at 09:28:41AM -0800, kys@exchange.microsoft.com
> wrote:
> > From: K. Y. Srinivasan <kys@microsoft.com>
> >
> > After the channel is rescinded, the host does not read from the rescinded
> channel.
> > Fail writes to a channel that has already been rescinded
> 
> What are the user visible effects of this bug?  Presumably none right
> since it doesn't need to be back ported.
> 
> But it's absolutely impossible to tell from the commit message.

Thanks Dan. I will update the commit log and resend.

K. Y

> 
> regards,
> dan carpenter