[PATCH V2 0/5] Drivers: hv: vmbus: Cleanup the vmbus unload path

[PATCH V2 0/5] Drivers: hv: vmbus: Cleanup the vmbus unload path

[K. Y. Srinivasan]

This patch-set have several fixes to enable the clean unload of the vmbus.
Typically, vmbus will not be unloadable when Linux is hosted on
Hyper-V since the driver managing the root device needs the vmbus driver.

In this version of the patch-set, the patch Drivers: hv: vmbus: Implement
the protocol for tearing down vmbus was modified based on feedback from
Vitaly Kuznetsov.

Dexuan Cui (1):
  hv: vmbus_free_channels(): remove the redundant free_channel()

K. Y. Srinivasan (2):
  drivers: hv: vmbus: Get rid of some unused definitions
  Drivers: hv: vmbus: Implement the protocol for tearing down vmbus
    state

Vitaly Kuznetsov (2):
  Drivers: hv: vmbus: introduce vmbus_acpi_remove
  Drivers: hv: vmbus: unregister panic notifier on module unload

 drivers/hv/channel_mgmt.c |   36 +++++++++++++++++++++++++++++++++---
 drivers/hv/connection.c   |    5 +++++
 drivers/hv/hyperv_vmbus.h |    2 ++
 drivers/hv/vmbus_drv.c    |   16 +++++++++++++++-
 include/linux/hyperv.h    |   20 +-------------------
 5 files changed, 56 insertions(+), 23 deletions(-)

-- 
1.7.4.1

[PATCH V2 1/5] Drivers: hv: vmbus: introduce vmbus_acpi_remove

[K. Y. Srinivasan]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

In case we do request_resource() in vmbus_acpi_add() we need to tear it down
to be able to load the driver again. Otherwise the following crash in observed
when hv_vmbus unload/load sequence is performed on a Generation2 instance:

[   38.165701] BUG: unable to handle kernel paging request at ffffffffa00075a0
[   38.166315] IP: [<ffffffff8107dc5f>] __request_resource+0x2f/0x50
[   38.166315] PGD 1f34067 PUD 1f35063 PMD 3f723067 PTE 0
[   38.166315] Oops: 0000 [#1] SMP
[   38.166315] Modules linked in: hv_vmbus(+) [last unloaded: hv_vmbus]
[   38.166315] CPU: 0 PID: 267 Comm: modprobe Not tainted 3.19.0-rc5_bug923184+ #486
[   38.166315] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v1.0 11/26/2012
[   38.166315] task: ffff88003f401cb0 ti: ffff88003f60c000 task.ti: ffff88003f60c000
[   38.166315] RIP: 0010:[<ffffffff8107dc5f>]  [<ffffffff8107dc5f>] __request_resource+0x2f/0x50
[   38.166315] RSP: 0018:ffff88003f60fb58  EFLAGS: 00010286

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

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index c85235e..0d8d1d7 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1035,6 +1035,15 @@ acpi_walk_err:
 	return ret_val;
 }
 
+static int vmbus_acpi_remove(struct acpi_device *device)
+{
+	int ret = 0;
+
+	if (hyperv_mmio.start && hyperv_mmio.end)
+		ret = release_resource(&hyperv_mmio);
+	return ret;
+}
+
 static const struct acpi_device_id vmbus_acpi_device_ids[] = {
 	{"VMBUS", 0},
 	{"VMBus", 0},
@@ -1047,6 +1056,7 @@ static struct acpi_driver vmbus_acpi_driver = {
 	.ids = vmbus_acpi_device_ids,
 	.ops = {
 		.add = vmbus_acpi_add,
+		.remove = vmbus_acpi_remove,
 	},
 };
 
-- 
1.7.4.1

[PATCH V2 2/5] Drivers: hv: vmbus: unregister panic notifier on module unload

[K. Y. Srinivasan]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Commit 96c1d0581d00f7abe033350edb021a9d947d8d81 ("Drivers: hv: vmbus: Add
support for VMBus panic notifier handler") introduced
atomic_notifier_chain_register() call on module load. We also need to call
atomic_notifier_chain_unregister() on module unload as otherwise the following
crash is observed when we bring hv_vmbus back:

[   39.788877] BUG: unable to handle kernel paging request at ffffffffa00078a8
[   39.788877] IP: [<ffffffff8109d63f>] notifier_call_chain+0x3f/0x80
...
[   39.788877] Call Trace:
[   39.788877]  [<ffffffff8109de7d>] __atomic_notifier_call_chain+0x5d/0x90
...
[   39.788877]  [<ffffffff8109d788>] ? atomic_notifier_chain_register+0x38/0x70
[   39.788877]  [<ffffffff8109d767>] ? atomic_notifier_chain_register+0x17/0x70
[   39.788877]  [<ffffffffa002814f>] hv_acpi_init+0x14f/0x1000 [hv_vmbus]
[   39.788877]  [<ffffffff81002144>] do_one_initcall+0xd4/0x210

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

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 0d8d1d7..7870a90 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1108,6 +1108,10 @@ static void __exit vmbus_exit(void)
 	hv_synic_clockevents_cleanup();
 	hv_remove_vmbus_irq();
 	vmbus_free_channels();
+	if (ms_hyperv.features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+		atomic_notifier_chain_unregister(&panic_notifier_list,
+						 &hyperv_panic_block);
+	}
 	bus_unregister(&hv_bus);
 	hv_cleanup();
 	for_each_online_cpu(cpu)
-- 
1.7.4.1

[PATCH V2 3/5] hv: vmbus_free_channels(): remove the redundant free_channel()

[K. Y. Srinivasan]

From: Dexuan Cui <decui@microsoft.com>

free_channel() has been invoked in
vmbus_remove() -> hv_process_channel_removal(), or vmbus_remove() ->
... -> vmbus_close_internal() -> hv_process_channel_removal().

We also change to use list_for_each_entry_safe(), because the entry
is removed in hv_process_channel_removal().

This patch fixes a bug in the vmbus unload path.

Thank Dan Carpenter for finding the issue!

Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 0eeb1b3..865a3af 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -212,11 +212,16 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
 
 void vmbus_free_channels(void)
 {
-	struct vmbus_channel *channel;
+	struct vmbus_channel *channel, *tmp;
+
+	list_for_each_entry_safe(channel, tmp, &vmbus_connection.chn_list,
+		listentry) {
+		/* if we don't set rescind to true, vmbus_close_internal()
+		 * won't invoke hv_process_channel_removal().
+		 */
+		channel->rescind = true;
 
-	list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
 		vmbus_device_unregister(channel->device_obj);
-		free_channel(channel);
 	}
 }
 
-- 
1.7.4.1

[PATCH V2 4/5] drivers: hv: vmbus: Get rid of some unused definitions

[K. Y. Srinivasan]

Get rid of some unused definitions.

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

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 1744148..e29ccdd 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -389,10 +389,6 @@ enum vmbus_channel_message_type {
 	CHANNELMSG_INITIATE_CONTACT		= 14,
 	CHANNELMSG_VERSION_RESPONSE		= 15,
 	CHANNELMSG_UNLOAD			= 16,
-#ifdef VMBUS_FEATURE_PARENT_OR_PEER_MEMORY_MAPPED_INTO_A_CHILD
-	CHANNELMSG_VIEWRANGE_ADD		= 17,
-	CHANNELMSG_VIEWRANGE_REMOVE		= 18,
-#endif
 	CHANNELMSG_COUNT
 };
 
@@ -549,21 +545,6 @@ struct vmbus_channel_gpadl_torndown {
 	u32 gpadl;
 } __packed;
 
-#ifdef VMBUS_FEATURE_PARENT_OR_PEER_MEMORY_MAPPED_INTO_A_CHILD
-struct vmbus_channel_view_range_add {
-	struct vmbus_channel_message_header header;
-	PHYSICAL_ADDRESS viewrange_base;
-	u64 viewrange_length;
-	u32 child_relid;
-} __packed;
-
-struct vmbus_channel_view_range_remove {
-	struct vmbus_channel_message_header header;
-	PHYSICAL_ADDRESS viewrange_base;
-	u32 child_relid;
-} __packed;
-#endif
-
 struct vmbus_channel_relid_released {
 	struct vmbus_channel_message_header header;
 	u32 child_relid;
-- 
1.7.4.1

[PATCH V2 5/5] Drivers: hv: vmbus: Implement the protocol for tearing down vmbus state

[K. Y. Srinivasan]

Implement the protocol for tearing down the monitor state established with
the host.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Tested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes in V2: Call vmbus_disconnect earlier - Vitaly Kuznetsov <vkuznets@redhat.com>

 drivers/hv/channel_mgmt.c |   25 +++++++++++++++++++++++++
 drivers/hv/connection.c   |    5 +++++
 drivers/hv/hyperv_vmbus.h |    2 ++
 drivers/hv/vmbus_drv.c    |    2 +-
 include/linux/hyperv.h    |    1 +
 5 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 865a3af..4b9d89a 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -422,6 +422,30 @@ static void init_vp_index(struct vmbus_channel *channel, const uuid_le *type_gui
 }
 
 /*
+ * vmbus_unload_response - Handler for the unload response.
+ */
+static void vmbus_unload_response(struct vmbus_channel_message_header *hdr)
+{
+	/*
+	 * This is a global event; just wakeup the waiting thread.
+	 * Once we successfully unload, we can cleanup the monitor state.
+	 */
+	complete(&vmbus_connection.unload_event);
+}
+
+void vmbus_initiate_unload(void)
+{
+	struct vmbus_channel_message_header hdr;
+
+	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));
+
+	wait_for_completion(&vmbus_connection.unload_event);
+}
+
+/*
  * vmbus_onoffer - Handler for channel offers from vmbus in parent partition.
  *
  */
@@ -717,6 +741,7 @@ struct vmbus_channel_message_table_entry
 	{CHANNELMSG_INITIATE_CONTACT,		0, NULL},
 	{CHANNELMSG_VERSION_RESPONSE,		1, vmbus_onversion_response},
 	{CHANNELMSG_UNLOAD,			0, NULL},
+	{CHANNELMSG_UNLOAD_RESPONSE,		1, vmbus_unload_response},
 };
 
 /*
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index b27220a..acd50e9 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -227,6 +227,11 @@ cleanup:
 
 void vmbus_disconnect(void)
 {
+	/*
+	 * First send the unload request to the host.
+	 */
+	vmbus_initiate_unload();
+
 	if (vmbus_connection.work_queue) {
 		drain_workqueue(vmbus_connection.work_queue);
 		destroy_workqueue(vmbus_connection.work_queue);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 138d663..cddc0c9 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -647,6 +647,7 @@ struct vmbus_connection {
 
 	atomic_t next_gpadl_handle;
 
+	struct completion  unload_event;
 	/*
 	 * Represents channel interrupts. Each bit position represents a
 	 * channel.  When a channel sends an interrupt via VMBUS, it finds its
@@ -741,6 +742,7 @@ void hv_vss_onchannelcallback(void *);
 int hv_fcopy_init(struct hv_util_service *);
 void hv_fcopy_deinit(void);
 void hv_fcopy_onchannelcallback(void *);
+void vmbus_initiate_unload(void);
 
 static inline void hv_poll_channel(struct vmbus_channel *channel,
 				   void (*cb)(void *))
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 7870a90..2b56260 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1106,6 +1106,7 @@ static void __exit vmbus_exit(void)
 
 	vmbus_connection.conn_state = DISCONNECTED;
 	hv_synic_clockevents_cleanup();
+	vmbus_disconnect();
 	hv_remove_vmbus_irq();
 	vmbus_free_channels();
 	if (ms_hyperv.features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
@@ -1118,7 +1119,6 @@ static void __exit vmbus_exit(void)
 		smp_call_function_single(cpu, hv_synic_cleanup, NULL, 1);
 	acpi_bus_unregister_driver(&vmbus_acpi_driver);
 	hv_cpu_hotplug_quirk(false);
-	vmbus_disconnect();
 }
 
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index e29ccdd..ea93486 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -389,6 +389,7 @@ enum vmbus_channel_message_type {
 	CHANNELMSG_INITIATE_CONTACT		= 14,
 	CHANNELMSG_VERSION_RESPONSE		= 15,
 	CHANNELMSG_UNLOAD			= 16,
+	CHANNELMSG_UNLOAD_RESPONSE		= 17,
 	CHANNELMSG_COUNT
 };
 
-- 
1.7.4.1