Skip to content

Commit 113386c

Browse files
Naman Jainliuw
authored andcommitted
Drivers: hv: vmbus: Wait for boot-time offers during boot and resume
Channel offers are requested during VMBus initialization and resume from hibernation. Add support to wait for all boot-time channel offers to be delivered and processed before returning from vmbus_request_offers. This is in analogy to a PCI bus not returning from probe until it has scanned all devices on the bus. Without this, user mode can race with VMBus initialization and miss channel offers. User mode has no way to work around this other than sleeping for a while, since there is no way to know when VMBus has finished processing boot-time offers. With this added functionality, remove earlier logic which keeps track of count of offered channels post resume from hibernation. Once all offers delivered message is received, no further boot-time offers are going to be received. Consequently, logic to prevent suspend from happening after previous resume had missing offers, is also removed. Co-developed-by: John Starks <[email protected]> Signed-off-by: John Starks <[email protected]> Signed-off-by: Naman Jain <[email protected]> Reviewed-by: Easwar Hariharan <[email protected]> Reviewed-by: Saurabh Sengar <[email protected]> Reviewed-by: Michael Kelley <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Wei Liu <[email protected]> Message-ID: <[email protected]>
1 parent 5fa1da9 commit 113386c

File tree

4 files changed

+51
-44
lines changed

4 files changed

+51
-44
lines changed

drivers/hv/channel_mgmt.c

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -944,16 +944,6 @@ void vmbus_initiate_unload(bool crash)
944944
vmbus_wait_for_unload();
945945
}
946946

947-
static void check_ready_for_resume_event(void)
948-
{
949-
/*
950-
* If all the old primary channels have been fixed up, then it's safe
951-
* to resume.
952-
*/
953-
if (atomic_dec_and_test(&vmbus_connection.nr_chan_fixup_on_resume))
954-
complete(&vmbus_connection.ready_for_resume_event);
955-
}
956-
957947
static void vmbus_setup_channel_state(struct vmbus_channel *channel,
958948
struct vmbus_channel_offer_channel *offer)
959949
{
@@ -1109,8 +1099,6 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
11091099

11101100
/* Add the channel back to the array of channels. */
11111101
vmbus_channel_map_relid(oldchannel);
1112-
check_ready_for_resume_event();
1113-
11141102
mutex_unlock(&vmbus_connection.channel_mutex);
11151103
return;
11161104
}
@@ -1296,13 +1284,28 @@ EXPORT_SYMBOL_GPL(vmbus_hvsock_device_unregister);
12961284

12971285
/*
12981286
* vmbus_onoffers_delivered -
1299-
* This is invoked when all offers have been delivered.
1287+
* The CHANNELMSG_ALLOFFERS_DELIVERED message arrives after all
1288+
* boot-time offers are delivered. A boot-time offer is for the primary
1289+
* channel for any virtual hardware configured in the VM at the time it boots.
1290+
* Boot-time offers include offers for physical devices assigned to the VM
1291+
* via Hyper-V's Discrete Device Assignment (DDA) functionality that are
1292+
* handled as virtual PCI devices in Linux (e.g., NVMe devices and GPUs).
1293+
* Boot-time offers do not include offers for VMBus sub-channels. Because
1294+
* devices can be hot-added to the VM after it is booted, additional channel
1295+
* offers that aren't boot-time offers can be received at any time after the
1296+
* all-offers-delivered message.
13001297
*
1301-
* Nothing to do here.
1298+
* SR-IOV NIC Virtual Functions (VFs) assigned to a VM are not considered
1299+
* to be assigned to the VM at boot-time, and offers for VFs may occur after
1300+
* the all-offers-delivered message. VFs are optional accelerators to the
1301+
* synthetic VMBus NIC and are effectively hot-added only after the VMBus
1302+
* NIC channel is opened (once it knows the guest can support it, via the
1303+
* sriov bit in the netvsc protocol).
13021304
*/
13031305
static void vmbus_onoffers_delivered(
13041306
struct vmbus_channel_message_header *hdr)
13051307
{
1308+
complete(&vmbus_connection.all_offers_delivered_event);
13061309
}
13071310

13081311
/*
@@ -1578,7 +1581,8 @@ void vmbus_onmessage(struct vmbus_channel_message_header *hdr)
15781581
}
15791582

15801583
/*
1581-
* vmbus_request_offers - Send a request to get all our pending offers.
1584+
* vmbus_request_offers - Send a request to get all our pending offers
1585+
* and wait for all boot-time offers to arrive.
15821586
*/
15831587
int vmbus_request_offers(void)
15841588
{
@@ -1596,6 +1600,10 @@ int vmbus_request_offers(void)
15961600

15971601
msg->msgtype = CHANNELMSG_REQUESTOFFERS;
15981602

1603+
/*
1604+
* This REQUESTOFFERS message will result in the host sending an all
1605+
* offers delivered message after all the boot-time offers are sent.
1606+
*/
15991607
ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_message_header),
16001608
true);
16011609

@@ -1607,6 +1615,29 @@ int vmbus_request_offers(void)
16071615
goto cleanup;
16081616
}
16091617

1618+
/*
1619+
* Wait for the host to send all boot-time offers.
1620+
* Keeping it as a best-effort mechanism, where a warning is
1621+
* printed if a timeout occurs, and execution is resumed.
1622+
*/
1623+
if (!wait_for_completion_timeout(&vmbus_connection.all_offers_delivered_event,
1624+
secs_to_jiffies(60))) {
1625+
pr_warn("timed out waiting for all boot-time offers to be delivered.\n");
1626+
}
1627+
1628+
/*
1629+
* Flush handling of offer messages (which may initiate work on
1630+
* other work queues).
1631+
*/
1632+
flush_workqueue(vmbus_connection.work_queue);
1633+
1634+
/*
1635+
* Flush workqueue for processing the incoming offers. Subchannel
1636+
* offers and their processing can happen later, so there is no need to
1637+
* flush that workqueue here.
1638+
*/
1639+
flush_workqueue(vmbus_connection.handle_primary_chan_wq);
1640+
16101641
cleanup:
16111642
kfree(msginfo);
16121643

drivers/hv/connection.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ struct vmbus_connection vmbus_connection = {
3434

3535
.ready_for_suspend_event = COMPLETION_INITIALIZER(
3636
vmbus_connection.ready_for_suspend_event),
37-
.ready_for_resume_event = COMPLETION_INITIALIZER(
38-
vmbus_connection.ready_for_resume_event),
37+
.all_offers_delivered_event = COMPLETION_INITIALIZER(
38+
vmbus_connection.all_offers_delivered_event),
3939
};
4040
EXPORT_SYMBOL_GPL(vmbus_connection);
4141

drivers/hv/hyperv_vmbus.h

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -287,18 +287,10 @@ struct vmbus_connection {
287287
struct completion ready_for_suspend_event;
288288

289289
/*
290-
* The number of primary channels that should be "fixed up"
291-
* upon resume: these channels are re-offered upon resume, and some
292-
* fields of the channel offers (i.e. child_relid and connection_id)
293-
* can change, so the old offermsg must be fixed up, before the resume
294-
* callbacks of the VSC drivers start to further touch the channels.
290+
* Completed once the host has offered all boot-time channels.
291+
* Note that some channels may still be under process on a workqueue.
295292
*/
296-
atomic_t nr_chan_fixup_on_resume;
297-
/*
298-
* vmbus_bus_resume() waits for "nr_chan_fixup_on_resume" to
299-
* drop to zero.
300-
*/
301-
struct completion ready_for_resume_event;
293+
struct completion all_offers_delivered_event;
302294
};
303295

304296

drivers/hv/vmbus_drv.c

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2427,11 +2427,6 @@ static int vmbus_bus_suspend(struct device *dev)
24272427
if (atomic_read(&vmbus_connection.nr_chan_close_on_suspend) > 0)
24282428
wait_for_completion(&vmbus_connection.ready_for_suspend_event);
24292429

2430-
if (atomic_read(&vmbus_connection.nr_chan_fixup_on_resume) != 0) {
2431-
pr_err("Can not suspend due to a previous failed resuming\n");
2432-
return -EBUSY;
2433-
}
2434-
24352430
mutex_lock(&vmbus_connection.channel_mutex);
24362431

24372432
list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
@@ -2456,17 +2451,12 @@ static int vmbus_bus_suspend(struct device *dev)
24562451
pr_err("Sub-channel not deleted!\n");
24572452
WARN_ON_ONCE(1);
24582453
}
2459-
2460-
atomic_inc(&vmbus_connection.nr_chan_fixup_on_resume);
24612454
}
24622455

24632456
mutex_unlock(&vmbus_connection.channel_mutex);
24642457

24652458
vmbus_initiate_unload(false);
24662459

2467-
/* Reset the event for the next resume. */
2468-
reinit_completion(&vmbus_connection.ready_for_resume_event);
2469-
24702460
return 0;
24712461
}
24722462

@@ -2502,14 +2492,8 @@ static int vmbus_bus_resume(struct device *dev)
25022492
if (ret != 0)
25032493
return ret;
25042494

2505-
WARN_ON(atomic_read(&vmbus_connection.nr_chan_fixup_on_resume) == 0);
2506-
25072495
vmbus_request_offers();
25082496

2509-
if (wait_for_completion_timeout(
2510-
&vmbus_connection.ready_for_resume_event, secs_to_jiffies(10)) == 0)
2511-
pr_err("Some vmbus device is missing after suspending?\n");
2512-
25132497
/* Reset the event for the next suspend. */
25142498
reinit_completion(&vmbus_connection.ready_for_suspend_event);
25152499

0 commit comments

Comments
 (0)