A guest can wait indefinitely in wait_for_response() for the host to
send either a rescind message or a packet completion. If the
host does not send either, the guest can remain blocked with no
diagnostic indicating a reason.
This was observed during a guest kernel upgrade in which the
host-side application handling the PCI channel faulted, causing the
guest to never receive the completion request.
Add a warning in wait_for_response() when the wait exceeds
a timeout so that such a hang is visible in the guest's kernel log
and can be correlated with host-side state.

Suggested-by: Hamza Mahfooz <[email protected]>
Suggested-by: Naman Jain <[email protected]>
Suggested-by: Michael Kelley <[email protected]>
Signed-off-by: Sahil Chandna <[email protected]>
---
Changes since v1:
- Removed periodic warning to one time warning in 2 minutes
- Include vmbus relid and stuck PCI msg.
Link to v1: 
https://lore.kernel.org/all/[email protected]/
---
 drivers/pci/controller/pci-hyperv.c | 48 +++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c 
b/drivers/pci/controller/pci-hyperv.c
index 89816a2bd7cd..ca9d8efd748c 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1040,19 +1040,40 @@ static void put_pcichild(struct hv_pci_dev *hpdev)

 /*
  * There is no good way to get notified from vmbus_onoffer_rescind(),
- * so let's use polling here, since this is not a hot path.
+ * so let's use polling here, since this is not a hot path. If
+ * wait_for_response() has been polling for PCI_RESPONSE_HANG_TIMEOUT_SEC
+ * without either a rescind or completion, add a warning.
  */
+#define PCI_RESPONSE_HANG_TIMEOUT_SEC 120
+
 static int wait_for_response(struct hv_device *hdev,
-                            struct completion *comp)
+                            struct completion *comp,
+                            const char *msg_type)
 {
+       unsigned long delay = secs_to_jiffies(PCI_RESPONSE_HANG_TIMEOUT_SEC);
+       u64 timeout = get_jiffies_64() + delay;
+       bool warned = false;
+
        while (true) {
                if (hdev->channel->rescind) {
                        dev_warn_once(&hdev->device, "The device is gone.\n");
                        return -ENODEV;
                }

-               if (wait_for_completion_timeout(comp, HZ / 10))
+               if (wait_for_completion_timeout(comp, HZ / 10)) {
+                       if (warned)
+                               dev_warn(&hdev->device,
+                                        "Late %s completion arrived.\n", 
msg_type);
                        break;
+               }
+
+               if (!warned && time_after64(get_jiffies_64(), timeout)) {
+                       dev_err(&hdev->device,
+                               "%s stuck waiting for response, relid = %u\n",
+                               msg_type, hdev->channel->offermsg.child_relid);
+
+                       warned = true;
+               }
        }

        return 0;
@@ -1518,7 +1539,8 @@ static int hv_read_config_block(struct pci_dev *pdev, 
void *buf,
        if (ret)
                return ret;

-       ret = wait_for_response(hbus->hdev, &comp_pkt.comp_pkt.host_event);
+       ret = wait_for_response(hbus->hdev, &comp_pkt.comp_pkt.host_event,
+                               "PCI_READ_BLOCK");
        if (ret)
                return ret;

@@ -1607,7 +1629,8 @@ static int hv_write_config_block(struct pci_dev *pdev, 
void *buf,
        if (ret)
                return ret;

-       ret = wait_for_response(hbus->hdev, &comp_pkt.host_event);
+       ret = wait_for_response(hbus->hdev, &comp_pkt.host_event,
+                               "PCI_WRITE_BLOCK");
        if (ret)
                return ret;

@@ -2624,7 +2647,8 @@ static struct hv_pci_dev *new_pcichild_device(struct 
hv_pcibus_device *hbus,
        if (ret)
                goto error;

-       if (wait_for_response(hbus->hdev, &comp_pkt.host_event))
+       if (wait_for_response(hbus->hdev, &comp_pkt.host_event,
+                             "PCI_QUERY_RESOURCE_REQUIREMENTS"))
                goto error;

        hpdev->desc = *desc;
@@ -3256,7 +3280,8 @@ static int hv_pci_protocol_negotiation(struct hv_device 
*hdev,
                                (unsigned long)pkt, VM_PKT_DATA_INBAND,
                                VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
                if (!ret)
-                       ret = wait_for_response(hdev, &comp_pkt.host_event);
+                       ret = wait_for_response(hdev, &comp_pkt.host_event,
+                                               "PCI_QUERY_PROTOCOL_VERSION");

                if (ret) {
                        dev_err(&hdev->device,
@@ -3476,7 +3501,8 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
                               (unsigned long)pkt, VM_PKT_DATA_INBAND,
                               VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
        if (!ret)
-               ret = wait_for_response(hdev, &comp_pkt.host_event);
+               ret = wait_for_response(hdev, &comp_pkt.host_event,
+                                       "PCI_BUS_D0ENTRY");

        if (ret)
                goto exit;
@@ -3553,7 +3579,8 @@ static int hv_pci_query_relations(struct hv_device *hdev)
        ret = vmbus_sendpacket(hdev->channel, &message, sizeof(message),
                               0, VM_PKT_DATA_INBAND, 0);
        if (!ret)
-               ret = wait_for_response(hdev, &comp);
+               ret = wait_for_response(hdev, &comp,
+                                       "PCI_QUERY_BUS_RELATIONS");

        /*
         * In the case of fast device addition/removal, it's possible that
@@ -3644,7 +3671,8 @@ static int hv_send_resources_allocated(struct hv_device 
*hdev)
                                VM_PKT_DATA_INBAND,
                                VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
                if (!ret)
-                       ret = wait_for_response(hdev, &comp_pkt.host_event);
+                       ret = wait_for_response(hdev, &comp_pkt.host_event,
+                                               "PCI_RESOURCE_ASSIGNED");
                if (ret)
                        break;

--
2.53.0

Reply via email to