[focal:linux-azure, bionic:linux-azure-4.15][PATCH 0/5] Fix kdump Over Network

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[focal:linux-azure, bionic:linux-azure-4.15][PATCH 0/5] Fix kdump Over Network

Kelsey Skunberg
BugLink: https://bugs.launchpad.net/bugs/1883261

[Impact]

Microsoft would like to request two kdump related fixes in all releases
supported on Azure. The two commits are:

c81992e7f4aa1 ("PCI: hv: Retry PCI bus D0 entry on invalid device
state")
83cc3508ffaa6 ("PCI: hv: Fix the PCI HyperV probe failure path
to release resource properly")

These are in the virtual PCI driver for Hyper-V. The customer visible
symptom is that the network is not functional in the kdump kernel, so
the dump file must be stored on the local disk and cannot be written
over the network.

The problem only occurs when Accelerated Networking is enabled. It’s a
relatively obscure scenario, which is why the problem has not surfaced
before now. But we have an important customer who wants the
“dump-file-over-the-network” functionality to work.

For bionic/linux-azure-4.15, the following additional patch needs to be
backported first to allow the requested patches to apply cleanly:

a8e37506e79a ("PCI: hv: Reorganize the code in preparation of
hibernation")

[Test Case]

- Apply requested patches and boot into updated kernel
- Verify Accelerated Networking is enabled
- Set up kdump
- configure kdump to use SSH
- Test the crash dump mechanism and verify the kernel crash dump appears
  on the selected remote server

Further details for setting up kdump through testing can be found here:
https://ubuntu.com/server/docs/kernel-crash-dump

[Regression Potential]

Patches are only targeted to azure kernels.

Patches are desgiend to release allocated resources remaining after
error cases in hv_pci_probe() or PCI devices not being shut down
properly. if those resources are still not correctly released, then
entering D0 state in kdump kernel could continue to fail.

Potential for finding regression with freeing resources or still failing to
enter D0 state in the kdump kernel even after all resources have been
released.  

Build & boot tested. Verified kdump works as intended over SSH after
patches are applied.

Both 5.4 and 4.15 test kernels were sent to Microsoft. Both kernels
signed off on and verified to resolve problem.


Changes for Bionic/linux-azure-4.15:


Dexuan Cui (1):
  PCI: hv: Reorganize the code in preparation of hibernation

Wei Hu (2):
  PCI: hv: Fix the PCI HyperV probe failure path to release resource
    properly
  PCI: hv: Retry PCI bus D0 entry on invalid device state

 drivers/pci/host/pci-hyperv.c | 101 +++++++++++++++++++++++++++-------
 1 file changed, 81 insertions(+), 20 deletions(-)


Changes for Focal/linux-azure:

Wei Hu (2):
  PCI: hv: Fix the PCI HyperV probe failure path to release resource
    properly
  PCI: hv: Retry PCI bus D0 entry on invalid device state

 drivers/pci/controller/pci-hyperv.c | 60 ++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 6 deletions(-)

--
2.25.1

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[bionic/linux-azure-4.15][PATCH 1/3] PCI: hv: Reorganize the code in preparation of hibernation

Kelsey Skunberg
From: Dexuan Cui <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1883261

There is no functional change. This is just preparatory for a later
patch which adds the hibernation support for the pci-hyperv driver.

Signed-off-by: Dexuan Cui <[hidden email]>
Signed-off-by: Lorenzo Pieralisi <[hidden email]>
Reviewed-by: Michael Kelley <[hidden email]>
(backported from commit a8e37506e79a7a08d0ee217f389fa1710e7a2ea4)
[KelseyS: Changes made in drivers/pci/host/ instead of
drivers/pci/controller. Context changes in hv_pcu_bus_exit()]
Signed-off-by: Kelsey Skunberg <[hidden email]>
---
 drivers/pci/host/pci-hyperv.c | 46 +++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index f5cb6b5f5515..1c822eb10c7a 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -2270,7 +2270,9 @@ static void hv_pci_onchannelcallback(void *context)
  * failing if the host doesn't support the necessary protocol
  * level.
  */
-static int hv_pci_protocol_negotiation(struct hv_device *hdev)
+static int hv_pci_protocol_negotiation(struct hv_device *hdev,
+      enum pci_protocol_version_t version[],
+      int num_version)
 {
  struct pci_version_request *version_req;
  struct hv_pci_compl comp_pkt;
@@ -2294,8 +2296,8 @@ static int hv_pci_protocol_negotiation(struct hv_device *hdev)
  version_req = (struct pci_version_request *)&pkt->message;
  version_req->message_type.type = PCI_QUERY_PROTOCOL_VERSION;
 
- for (i = 0; i < ARRAY_SIZE(pci_protocol_versions); i++) {
- version_req->protocol_version = pci_protocol_versions[i];
+ for (i = 0; i < num_version; i++) {
+ version_req->protocol_version = version[i];
  ret = vmbus_sendpacket(hdev->channel, version_req,
  sizeof(struct pci_version_request),
  (unsigned long)pkt, VM_PKT_DATA_INBAND,
@@ -2311,7 +2313,7 @@ static int hv_pci_protocol_negotiation(struct hv_device *hdev)
  }
 
  if (comp_pkt.completion_status >= 0) {
- pci_protocol_version = pci_protocol_versions[i];
+ pci_protocol_version = version[i];
  dev_info(&hdev->device,
  "PCI VMBus probing: Using version %#x\n",
  pci_protocol_version);
@@ -2820,7 +2822,8 @@ static int hv_pci_probe(struct hv_device *hdev,
 
  hv_set_drvdata(hdev, hbus);
 
- ret = hv_pci_protocol_negotiation(hdev);
+ ret = hv_pci_protocol_negotiation(hdev, pci_protocol_versions,
+  ARRAY_SIZE(pci_protocol_versions));
  if (ret)
  goto close;
 
@@ -2894,7 +2897,7 @@ static int hv_pci_probe(struct hv_device *hdev,
  return ret;
 }
 
-static void hv_pci_bus_exit(struct hv_device *hdev)
+static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
 {
  struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
  struct {
@@ -2910,17 +2913,21 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
  * access the per-channel ringbuffer any longer.
  */
  if (hdev->channel->rescind)
- return;
+ return 0;
 
- /* Delete any children which might still exist. */
- dr = kzalloc(sizeof(*dr), GFP_KERNEL);
- if (dr && hv_pci_start_relations_work(hbus, dr))
- kfree(dr);
+ if (!hibernating) {
+ /* Delete any children which might still exist. */
+ dr = kzalloc(sizeof(*dr), GFP_KERNEL);
+ if (dr && hv_pci_start_relations_work(hbus, dr))
+ kfree(dr);
+ }
 
  ret = hv_send_resources_released(hdev);
- if (ret)
+ if (ret) {
  dev_err(&hdev->device,
  "Couldn't send resources released packet(s)\n");
+ return ret;
+ }
 
  memset(&pkt.teardown_packet, 0, sizeof(pkt.teardown_packet));
  init_completion(&comp_pkt.host_event);
@@ -2933,8 +2940,14 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
        (unsigned long)&pkt.teardown_packet,
        VM_PKT_DATA_INBAND,
        VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
- if (!ret)
- wait_for_completion_timeout(&comp_pkt.host_event, 10 * HZ);
+ if (ret)
+ return ret;
+
+ if (wait_for_completion_timeout(&comp_pkt.host_event, 10 * HZ) == 0)
+ return -ETIMEDOUT;
+
+ return 0;
+
 }
 
 /**
@@ -2946,6 +2959,7 @@ static void hv_pci_bus_exit(struct hv_device *hdev)
 static int hv_pci_remove(struct hv_device *hdev)
 {
  struct hv_pcibus_device *hbus;
+ int ret;
 
  hbus = hv_get_drvdata(hdev);
  if (hbus->state == hv_pcibus_installed) {
@@ -2958,7 +2972,7 @@ static int hv_pci_remove(struct hv_device *hdev)
  hbus->state = hv_pcibus_removed;
  }
 
- hv_pci_bus_exit(hdev);
+ ret = hv_pci_bus_exit(hdev, false);
 
  vmbus_close(hdev->channel);
 
@@ -2975,7 +2989,7 @@ static int hv_pci_remove(struct hv_device *hdev)
  hv_put_dom_num(hbus->sysdata.domain);
 
  free_page((unsigned long)hbus);
- return 0;
+ return ret;
 }
 
 static const struct hv_vmbus_device_id hv_pci_id_table[] = {
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[bionic/linux-azure-4.15][PATCH 2/3] PCI: hv: Fix the PCI HyperV probe failure path to release resource properly

Kelsey Skunberg
In reply to this post by Kelsey Skunberg
From: Wei Hu <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1883261

In some error cases in hv_pci_probe(), allocated resources are not freed.

Fix this by adding a field to keep track of the high water mark for slots
that have resources allocated to them.  In case of an error, this high
water mark is used to know which slots have resources that must be released.
Since slots are numbered starting with zero, a value of -1 indicates no
slots have been allocated resources.  There may be unused slots in the range
between slot 0 and the high water mark slot, but these slots are already
ignored by the existing code in the allocate and release loops with the call
to get_pcichild_wslot().

Link: https://lore.kernel.org/r/20200507050211.10923-1-weh@...
Signed-off-by: Wei Hu <[hidden email]>
Signed-off-by: Lorenzo Pieralisi <[hidden email]>
Reviewed-by: Michael Kelley <[hidden email]>
(backported from commit 83cc3508ffaa6e2cd364d29418d35fab6f069b51)
[KelseyS: Changes made in drivers/pci/host/ instead of
drivers/pci/controller]
Signed-off-by: Kelsey Skunberg <[hidden email]>
---
 drivers/pci/host/pci-hyperv.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 1c822eb10c7a..f15642e16886 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -499,6 +499,9 @@ struct hv_pcibus_device {
  struct msi_controller msi_chip;
  struct irq_domain *irq_domain;
 
+ /* Highest slot of child device with resources allocated */
+ int wslot_res_allocated;
+
  /* hypercall arg, must not cross page boundary */
  struct retarget_msi_interrupt retarget_msi_interrupt_params;
 
@@ -2597,7 +2600,7 @@ static int hv_send_resources_allocated(struct hv_device *hdev)
  struct hv_pci_dev *hpdev;
  struct pci_packet *pkt;
  size_t size_res;
- u32 wslot;
+ int wslot;
  int ret;
 
  size_res = (pci_protocol_version < PCI_PROTOCOL_VERSION_1_2)
@@ -2650,6 +2653,8 @@ static int hv_send_resources_allocated(struct hv_device *hdev)
  comp_pkt.completion_status);
  break;
  }
+
+ hbus->wslot_res_allocated = wslot;
  }
 
  kfree(pkt);
@@ -2668,10 +2673,10 @@ static int hv_send_resources_released(struct hv_device *hdev)
  struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
  struct pci_child_message pkt;
  struct hv_pci_dev *hpdev;
- u32 wslot;
+ int wslot;
  int ret;
 
- for (wslot = 0; wslot < 256; wslot++) {
+ for (wslot = hbus->wslot_res_allocated; wslot >= 0; wslot--) {
  hpdev = get_pcichild_wslot(hbus, wslot);
  if (!hpdev)
  continue;
@@ -2686,8 +2691,12 @@ static int hv_send_resources_released(struct hv_device *hdev)
        VM_PKT_DATA_INBAND, 0);
  if (ret)
  return ret;
+
+ hbus->wslot_res_allocated = wslot - 1;
  }
 
+ hbus->wslot_res_allocated = -1;
+
  return 0;
 }
 
@@ -2768,6 +2777,7 @@ static int hv_pci_probe(struct hv_device *hdev,
  if (!hbus)
  return -ENOMEM;
  hbus->state = hv_pcibus_init;
+ hbus->wslot_res_allocated = -1;
 
  /*
  * The PCI bus "domain" is what is called "segment" in ACPI and other
@@ -2860,7 +2870,7 @@ static int hv_pci_probe(struct hv_device *hdev,
 
  ret = hv_pci_allocate_bridge_windows(hbus);
  if (ret)
- goto free_irq_domain;
+ goto exit_d0;
 
  ret = hv_send_resources_allocated(hdev);
  if (ret)
@@ -2878,6 +2888,8 @@ static int hv_pci_probe(struct hv_device *hdev,
 
 free_windows:
  hv_pci_free_bridge_windows(hbus);
+exit_d0:
+ (void) hv_pci_bus_exit(hdev, true);
 free_irq_domain:
  irq_domain_remove(hbus->irq_domain);
 free_fwnode:
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[bionic/linux-azure-4.15][PATCH 3/3] PCI: hv: Retry PCI bus D0 entry on invalid device state

Kelsey Skunberg
In reply to this post by Kelsey Skunberg
From: Wei Hu <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1883261

When kdump is triggered, some PCI devices may have not been shut down
cleanly before the kdump kernel starts.

This causes the initial attempt to enter D0 state in the kdump kernel to
fail with invalid device state returned from Hyper-V host.

When this happens, explicitly call hv_pci_bus_exit() and retry to enter
the D0 state.

Link: https://lore.kernel.org/r/20200507050300.10974-1-weh@...
Signed-off-by: Wei Hu <[hidden email]>
[[hidden email]: commit log]
Signed-off-by: Lorenzo Pieralisi <[hidden email]>
Reviewed-by: Michael Kelley <[hidden email]>
(backported from commit c81992e7f4aa19a055dbff5bd6c6d5ff9408f2fb)
[KelseyS: Changes made in drivers/pci/host/ instead of
drivers/pci/controller]
Signed-off-by: Kelsey Skunberg <[hidden email]>
---
 drivers/pci/host/pci-hyperv.c | 39 +++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index f15642e16886..33d430e5d94b 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -2489,6 +2489,8 @@ static void hv_free_config_window(struct hv_pcibus_device *hbus)
  vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH);
 }
 
+static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs);
+
 /**
  * hv_pci_enter_d0() - Bring the "bus" into the D0 power state
  * @hdev: VMBus's tracking struct for this root PCI bus
@@ -2501,8 +2503,10 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
  struct pci_bus_d0_entry *d0_entry;
  struct hv_pci_compl comp_pkt;
  struct pci_packet *pkt;
+ bool retry = true;
  int ret;
 
+enter_d0_retry:
  /*
  * Tell the host that the bus is ready to use, and moved into the
  * powered-on state.  This includes telling the host which region
@@ -2528,6 +2532,37 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
 
  if (ret)
  goto exit;
+ /*
+ * In certain case (Kdump) the pci device of interest was
+ * not cleanly shut down and resource is still held on host
+ * side, the host could return invalid device status.
+ * We need to explicitly request host to release the resource
+ * and try to enter D0 again.
+ */
+ if (comp_pkt.completion_status < 0 && retry) {
+ retry = false;
+
+ dev_err(&hdev->device, "Retrying D0 Entry\n");
+
+ /*
+ * Hv_pci_bus_exit() calls hv_send_resource_released()
+ * to free up resources of its child devices.
+ * In the kdump kernel we need to set the
+ * wslot_res_allocated to 255 so it scans all child
+ * devices to release resources allocated in the
+ * normal kernel before panic happened.
+ */
+ hbus->wslot_res_allocated = 255;
+
+ ret = hv_pci_bus_exit(hdev, true);
+
+ if (ret == 0) {
+ kfree(pkt);
+ goto enter_d0_retry;
+ }
+ dev_err(&hdev->device,
+ "Retrying D0 failed with ret %d\n", ret);
+ }
 
  if (comp_pkt.completion_status < 0) {
  dev_err(&hdev->device,
@@ -2909,7 +2944,7 @@ static int hv_pci_probe(struct hv_device *hdev,
  return ret;
 }
 
-static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
+static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
 {
  struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
  struct {
@@ -2927,7 +2962,7 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
  if (hdev->channel->rescind)
  return 0;
 
- if (!hibernating) {
+ if (!keep_devs) {
  /* Delete any children which might still exist. */
  dr = kzalloc(sizeof(*dr), GFP_KERNEL);
  if (dr && hv_pci_start_relations_work(hbus, dr))
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[focal/linux-azure][PATCH 1/2] PCI: hv: Fix the PCI HyperV probe failure path to release resource properly

Kelsey Skunberg
In reply to this post by Kelsey Skunberg
From: Wei Hu <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1883261

In some error cases in hv_pci_probe(), allocated resources are not freed.

Fix this by adding a field to keep track of the high water mark for slots
that have resources allocated to them.  In case of an error, this high
water mark is used to know which slots have resources that must be released.
Since slots are numbered starting with zero, a value of -1 indicates no
slots have been allocated resources.  There may be unused slots in the range
between slot 0 and the high water mark slot, but these slots are already
ignored by the existing code in the allocate and release loops with the call
to get_pcichild_wslot().

Link: https://lore.kernel.org/r/20200507050211.10923-1-weh@...
Signed-off-by: Wei Hu <[hidden email]>
Signed-off-by: Lorenzo Pieralisi <[hidden email]>
Reviewed-by: Michael Kelley <[hidden email]>
(cherry picked from commit 83cc3508ffaa6e2cd364d29418d35fab6f069b51)
Signed-off-by: Kelsey Skunberg <[hidden email]>
---
 drivers/pci/controller/pci-hyperv.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 5d074d78da97..27e65f0f3250 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -519,6 +519,9 @@ struct hv_pcibus_device {
 
  struct workqueue_struct *wq;
 
+ /* Highest slot of child device with resources allocated */
+ int wslot_res_allocated;
+
  /* hypercall arg, must not cross page boundary */
  struct retarget_msi_interrupt retarget_msi_interrupt_params;
 
@@ -2899,7 +2902,7 @@ static int hv_send_resources_allocated(struct hv_device *hdev)
  struct hv_pci_dev *hpdev;
  struct pci_packet *pkt;
  size_t size_res;
- u32 wslot;
+ int wslot;
  int ret;
 
  size_res = (pci_protocol_version < PCI_PROTOCOL_VERSION_1_2)
@@ -2952,6 +2955,8 @@ static int hv_send_resources_allocated(struct hv_device *hdev)
  comp_pkt.completion_status);
  break;
  }
+
+ hbus->wslot_res_allocated = wslot;
  }
 
  kfree(pkt);
@@ -2970,10 +2975,10 @@ static int hv_send_resources_released(struct hv_device *hdev)
  struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
  struct pci_child_message pkt;
  struct hv_pci_dev *hpdev;
- u32 wslot;
+ int wslot;
  int ret;
 
- for (wslot = 0; wslot < 256; wslot++) {
+ for (wslot = hbus->wslot_res_allocated; wslot >= 0; wslot--) {
  hpdev = get_pcichild_wslot(hbus, wslot);
  if (!hpdev)
  continue;
@@ -2988,8 +2993,12 @@ static int hv_send_resources_released(struct hv_device *hdev)
        VM_PKT_DATA_INBAND, 0);
  if (ret)
  return ret;
+
+ hbus->wslot_res_allocated = wslot - 1;
  }
 
+ hbus->wslot_res_allocated = -1;
+
  return 0;
 }
 
@@ -3071,6 +3080,7 @@ static int hv_pci_probe(struct hv_device *hdev,
  if (!hbus)
  return -ENOMEM;
  hbus->state = hv_pcibus_init;
+ hbus->wslot_res_allocated = -1;
 
  /*
  * The PCI bus "domain" is what is called "segment" in ACPI and other
@@ -3170,7 +3180,7 @@ static int hv_pci_probe(struct hv_device *hdev,
 
  ret = hv_pci_allocate_bridge_windows(hbus);
  if (ret)
- goto free_irq_domain;
+ goto exit_d0;
 
  ret = hv_send_resources_allocated(hdev);
  if (ret)
@@ -3188,6 +3198,8 @@ static int hv_pci_probe(struct hv_device *hdev,
 
 free_windows:
  hv_pci_free_bridge_windows(hbus);
+exit_d0:
+ (void) hv_pci_bus_exit(hdev, true);
 free_irq_domain:
  irq_domain_remove(hbus->irq_domain);
 free_fwnode:
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[focal/linux-azure][PATCH 2/2] PCI: hv: Retry PCI bus D0 entry on invalid device state

Kelsey Skunberg
In reply to this post by Kelsey Skunberg
From: Wei Hu <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1883261

When kdump is triggered, some PCI devices may have not been shut down
cleanly before the kdump kernel starts.

This causes the initial attempt to enter D0 state in the kdump kernel to
fail with invalid device state returned from Hyper-V host.

When this happens, explicitly call hv_pci_bus_exit() and retry to enter
the D0 state.

Link: https://lore.kernel.org/r/20200507050300.10974-1-weh@...
Signed-off-by: Wei Hu <[hidden email]>
[[hidden email]: commit log]
Signed-off-by: Lorenzo Pieralisi <[hidden email]>
Reviewed-by: Michael Kelley <[hidden email]>
(cherry picked from commit c81992e7f4aa19a055dbff5bd6c6d5ff9408f2fb)
Signed-off-by: Kelsey Skunberg <[hidden email]>
---
 drivers/pci/controller/pci-hyperv.c | 40 +++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 27e65f0f3250..2c92d020729f 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -2791,6 +2791,8 @@ static void hv_free_config_window(struct hv_pcibus_device *hbus)
  vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH);
 }
 
+static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs);
+
 /**
  * hv_pci_enter_d0() - Bring the "bus" into the D0 power state
  * @hdev: VMBus's tracking struct for this root PCI bus
@@ -2803,8 +2805,10 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
  struct pci_bus_d0_entry *d0_entry;
  struct hv_pci_compl comp_pkt;
  struct pci_packet *pkt;
+ bool retry = true;
  int ret;
 
+enter_d0_retry:
  /*
  * Tell the host that the bus is ready to use, and moved into the
  * powered-on state.  This includes telling the host which region
@@ -2831,6 +2835,38 @@ static int hv_pci_enter_d0(struct hv_device *hdev)
  if (ret)
  goto exit;
 
+ /*
+ * In certain case (Kdump) the pci device of interest was
+ * not cleanly shut down and resource is still held on host
+ * side, the host could return invalid device status.
+ * We need to explicitly request host to release the resource
+ * and try to enter D0 again.
+ */
+ if (comp_pkt.completion_status < 0 && retry) {
+ retry = false;
+
+ dev_err(&hdev->device, "Retrying D0 Entry\n");
+
+ /*
+ * Hv_pci_bus_exit() calls hv_send_resource_released()
+ * to free up resources of its child devices.
+ * In the kdump kernel we need to set the
+ * wslot_res_allocated to 255 so it scans all child
+ * devices to release resources allocated in the
+ * normal kernel before panic happened.
+ */
+ hbus->wslot_res_allocated = 255;
+
+ ret = hv_pci_bus_exit(hdev, true);
+
+ if (ret == 0) {
+ kfree(pkt);
+ goto enter_d0_retry;
+ }
+ dev_err(&hdev->device,
+ "Retrying D0 failed with ret %d\n", ret);
+ }
+
  if (comp_pkt.completion_status < 0) {
  dev_err(&hdev->device,
  "PCI Pass-through VSP failed D0 Entry with status %x\n",
@@ -3219,7 +3255,7 @@ static int hv_pci_probe(struct hv_device *hdev,
  return ret;
 }
 
-static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
+static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
 {
  struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
  struct {
@@ -3237,7 +3273,7 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool hibernating)
  if (hdev->channel->rescind)
  return 0;
 
- if (!hibernating) {
+ if (!keep_devs) {
  /* Delete any children which might still exist. */
  dr = kzalloc(sizeof(*dr), GFP_KERNEL);
  if (dr && hv_pci_start_relations_work(hbus, dr))
--
2.25.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [focal:linux-azure, bionic:linux-azure-4.15][PATCH 0/5] Fix kdump Over Network

Stefan Bader-2
In reply to this post by Kelsey Skunberg
On 07.10.20 23:16, Kelsey Skunberg wrote:

> BugLink: https://bugs.launchpad.net/bugs/1883261
>
> [Impact]
>
> Microsoft would like to request two kdump related fixes in all releases
> supported on Azure. The two commits are:
>
> c81992e7f4aa1 ("PCI: hv: Retry PCI bus D0 entry on invalid device
> state")
> 83cc3508ffaa6 ("PCI: hv: Fix the PCI HyperV probe failure path
> to release resource properly")
>
> These are in the virtual PCI driver for Hyper-V. The customer visible
> symptom is that the network is not functional in the kdump kernel, so
> the dump file must be stored on the local disk and cannot be written
> over the network.
>
> The problem only occurs when Accelerated Networking is enabled. It’s a
> relatively obscure scenario, which is why the problem has not surfaced
> before now. But we have an important customer who wants the
> “dump-file-over-the-network” functionality to work.
>
> For bionic/linux-azure-4.15, the following additional patch needs to be
> backported first to allow the requested patches to apply cleanly:
>
> a8e37506e79a ("PCI: hv: Reorganize the code in preparation of
> hibernation")
>
> [Test Case]
>
> - Apply requested patches and boot into updated kernel
> - Verify Accelerated Networking is enabled
> - Set up kdump
> - configure kdump to use SSH
> - Test the crash dump mechanism and verify the kernel crash dump appears
>   on the selected remote server
>
> Further details for setting up kdump through testing can be found here:
> https://ubuntu.com/server/docs/kernel-crash-dump
>
> [Regression Potential]
>
> Patches are only targeted to azure kernels.
>
> Patches are desgiend to release allocated resources remaining after
> error cases in hv_pci_probe() or PCI devices not being shut down
> properly. if those resources are still not correctly released, then
> entering D0 state in kdump kernel could continue to fail.
>
> Potential for finding regression with freeing resources or still failing to
> enter D0 state in the kdump kernel even after all resources have been
> released.  
>
> Build & boot tested. Verified kdump works as intended over SSH after
> patches are applied.
>
> Both 5.4 and 4.15 test kernels were sent to Microsoft. Both kernels
> signed off on and verified to resolve problem.
>
>
> Changes for Bionic/linux-azure-4.15:
>
>
> Dexuan Cui (1):
>   PCI: hv: Reorganize the code in preparation of hibernation
>
> Wei Hu (2):
>   PCI: hv: Fix the PCI HyperV probe failure path to release resource
>     properly
>   PCI: hv: Retry PCI bus D0 entry on invalid device state
>
>  drivers/pci/host/pci-hyperv.c | 101 +++++++++++++++++++++++++++-------
>  1 file changed, 81 insertions(+), 20 deletions(-)
>
>
> Changes for Focal/linux-azure:
>
> Wei Hu (2):
>   PCI: hv: Fix the PCI HyperV probe failure path to release resource
>     properly
>   PCI: hv: Retry PCI bus D0 entry on invalid device state
>
>  drivers/pci/controller/pci-hyperv.c | 60 ++++++++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 6 deletions(-)
>
> --
> 2.25.1
>
Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [focal:linux-azure, bionic:linux-azure-4.15][PATCH 0/5] Fix kdump Over Network

Colin Ian King-2
In reply to this post by Kelsey Skunberg
On 07/10/2020 22:16, Kelsey Skunberg wrote:

> BugLink: https://bugs.launchpad.net/bugs/1883261
>
> [Impact]
>
> Microsoft would like to request two kdump related fixes in all releases
> supported on Azure. The two commits are:
>
> c81992e7f4aa1 ("PCI: hv: Retry PCI bus D0 entry on invalid device
> state")
> 83cc3508ffaa6 ("PCI: hv: Fix the PCI HyperV probe failure path
> to release resource properly")
>
> These are in the virtual PCI driver for Hyper-V. The customer visible
> symptom is that the network is not functional in the kdump kernel, so
> the dump file must be stored on the local disk and cannot be written
> over the network.
>
> The problem only occurs when Accelerated Networking is enabled. It’s a
> relatively obscure scenario, which is why the problem has not surfaced
> before now. But we have an important customer who wants the
> “dump-file-over-the-network” functionality to work.
>
> For bionic/linux-azure-4.15, the following additional patch needs to be
> backported first to allow the requested patches to apply cleanly:
>
> a8e37506e79a ("PCI: hv: Reorganize the code in preparation of
> hibernation")
>
> [Test Case]
>
> - Apply requested patches and boot into updated kernel
> - Verify Accelerated Networking is enabled
> - Set up kdump
> - configure kdump to use SSH
> - Test the crash dump mechanism and verify the kernel crash dump appears
>   on the selected remote server
>
> Further details for setting up kdump through testing can be found here:
> https://ubuntu.com/server/docs/kernel-crash-dump
>
> [Regression Potential]
>
> Patches are only targeted to azure kernels.
>
> Patches are desgiend to release allocated resources remaining after
> error cases in hv_pci_probe() or PCI devices not being shut down
> properly. if those resources are still not correctly released, then
> entering D0 state in kdump kernel could continue to fail.
>
> Potential for finding regression with freeing resources or still failing to
> enter D0 state in the kdump kernel even after all resources have been
> released.  
>
> Build & boot tested. Verified kdump works as intended over SSH after
> patches are applied.
>
> Both 5.4 and 4.15 test kernels were sent to Microsoft. Both kernels
> signed off on and verified to resolve problem.
>
>
> Changes for Bionic/linux-azure-4.15:
>
>
> Dexuan Cui (1):
>   PCI: hv: Reorganize the code in preparation of hibernation
>
> Wei Hu (2):
>   PCI: hv: Fix the PCI HyperV probe failure path to release resource
>     properly
>   PCI: hv: Retry PCI bus D0 entry on invalid device state
>
>  drivers/pci/host/pci-hyperv.c | 101 +++++++++++++++++++++++++++-------
>  1 file changed, 81 insertions(+), 20 deletions(-)
>
>
> Changes for Focal/linux-azure:
>
> Wei Hu (2):
>   PCI: hv: Fix the PCI HyperV probe failure path to release resource
>     properly
>   PCI: hv: Retry PCI bus D0 entry on invalid device state
>
>  drivers/pci/controller/pci-hyperv.c | 60 ++++++++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 6 deletions(-)
>
> --
> 2.25.1
>

Thanks Kelsey; backports look good to me, good test case and results, I
think the regression potential vs benefit looks sane, so..

Acked-by: Colin Ian King <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[B:azure-4.15]: [focal:linux-azure, bionic:linux-azure-4.15][PATCH 0/5] Fix kdump Over Network

Ian May
In reply to this post by Kelsey Skunberg
Applied to Bionic/azure-4.15-next

Thanks!
Ian

On 2020-10-07 15:16:35 , Kelsey Skunberg wrote:

> BugLink: https://bugs.launchpad.net/bugs/1883261
>
> [Impact]
>
> Microsoft would like to request two kdump related fixes in all releases
> supported on Azure. The two commits are:
>
> c81992e7f4aa1 ("PCI: hv: Retry PCI bus D0 entry on invalid device
> state")
> 83cc3508ffaa6 ("PCI: hv: Fix the PCI HyperV probe failure path
> to release resource properly")
>
> These are in the virtual PCI driver for Hyper-V. The customer visible
> symptom is that the network is not functional in the kdump kernel, so
> the dump file must be stored on the local disk and cannot be written
> over the network.
>
> The problem only occurs when Accelerated Networking is enabled. It’s a
> relatively obscure scenario, which is why the problem has not surfaced
> before now. But we have an important customer who wants the
> “dump-file-over-the-network” functionality to work.
>
> For bionic/linux-azure-4.15, the following additional patch needs to be
> backported first to allow the requested patches to apply cleanly:
>
> a8e37506e79a ("PCI: hv: Reorganize the code in preparation of
> hibernation")
>
> [Test Case]
>
> - Apply requested patches and boot into updated kernel
> - Verify Accelerated Networking is enabled
> - Set up kdump
> - configure kdump to use SSH
> - Test the crash dump mechanism and verify the kernel crash dump appears
>   on the selected remote server
>
> Further details for setting up kdump through testing can be found here:
> https://ubuntu.com/server/docs/kernel-crash-dump
>
> [Regression Potential]
>
> Patches are only targeted to azure kernels.
>
> Patches are desgiend to release allocated resources remaining after
> error cases in hv_pci_probe() or PCI devices not being shut down
> properly. if those resources are still not correctly released, then
> entering D0 state in kdump kernel could continue to fail.
>
> Potential for finding regression with freeing resources or still failing to
> enter D0 state in the kdump kernel even after all resources have been
> released.  
>
> Build & boot tested. Verified kdump works as intended over SSH after
> patches are applied.
>
> Both 5.4 and 4.15 test kernels were sent to Microsoft. Both kernels
> signed off on and verified to resolve problem.
>
>
> Changes for Bionic/linux-azure-4.15:
>
>
> Dexuan Cui (1):
>   PCI: hv: Reorganize the code in preparation of hibernation
>
> Wei Hu (2):
>   PCI: hv: Fix the PCI HyperV probe failure path to release resource
>     properly
>   PCI: hv: Retry PCI bus D0 entry on invalid device state
>
>  drivers/pci/host/pci-hyperv.c | 101 +++++++++++++++++++++++++++-------
>  1 file changed, 81 insertions(+), 20 deletions(-)
>
>
> Changes for Focal/linux-azure:
>
> Wei Hu (2):
>   PCI: hv: Fix the PCI HyperV probe failure path to release resource
>     properly
>   PCI: hv: Retry PCI bus D0 entry on invalid device state
>
>  drivers/pci/controller/pci-hyperv.c | 60 ++++++++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 6 deletions(-)
>
> --
> 2.25.1
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[F:azure]: [focal:linux-azure, bionic:linux-azure-4.15][PATCH 0/5] Fix kdump Over Network

Ian May
In reply to this post by Kelsey Skunberg
Applied to Focal/azure

Thanks,
Ian

On 2020-10-07 15:16:35 , Kelsey Skunberg wrote:

> BugLink: https://bugs.launchpad.net/bugs/1883261
>
> [Impact]
>
> Microsoft would like to request two kdump related fixes in all releases
> supported on Azure. The two commits are:
>
> c81992e7f4aa1 ("PCI: hv: Retry PCI bus D0 entry on invalid device
> state")
> 83cc3508ffaa6 ("PCI: hv: Fix the PCI HyperV probe failure path
> to release resource properly")
>
> These are in the virtual PCI driver for Hyper-V. The customer visible
> symptom is that the network is not functional in the kdump kernel, so
> the dump file must be stored on the local disk and cannot be written
> over the network.
>
> The problem only occurs when Accelerated Networking is enabled. It’s a
> relatively obscure scenario, which is why the problem has not surfaced
> before now. But we have an important customer who wants the
> “dump-file-over-the-network” functionality to work.
>
> For bionic/linux-azure-4.15, the following additional patch needs to be
> backported first to allow the requested patches to apply cleanly:
>
> a8e37506e79a ("PCI: hv: Reorganize the code in preparation of
> hibernation")
>
> [Test Case]
>
> - Apply requested patches and boot into updated kernel
> - Verify Accelerated Networking is enabled
> - Set up kdump
> - configure kdump to use SSH
> - Test the crash dump mechanism and verify the kernel crash dump appears
>   on the selected remote server
>
> Further details for setting up kdump through testing can be found here:
> https://ubuntu.com/server/docs/kernel-crash-dump
>
> [Regression Potential]
>
> Patches are only targeted to azure kernels.
>
> Patches are desgiend to release allocated resources remaining after
> error cases in hv_pci_probe() or PCI devices not being shut down
> properly. if those resources are still not correctly released, then
> entering D0 state in kdump kernel could continue to fail.
>
> Potential for finding regression with freeing resources or still failing to
> enter D0 state in the kdump kernel even after all resources have been
> released.  
>
> Build & boot tested. Verified kdump works as intended over SSH after
> patches are applied.
>
> Both 5.4 and 4.15 test kernels were sent to Microsoft. Both kernels
> signed off on and verified to resolve problem.
>
>
> Changes for Bionic/linux-azure-4.15:
>
>
> Dexuan Cui (1):
>   PCI: hv: Reorganize the code in preparation of hibernation
>
> Wei Hu (2):
>   PCI: hv: Fix the PCI HyperV probe failure path to release resource
>     properly
>   PCI: hv: Retry PCI bus D0 entry on invalid device state
>
>  drivers/pci/host/pci-hyperv.c | 101 +++++++++++++++++++++++++++-------
>  1 file changed, 81 insertions(+), 20 deletions(-)
>
>
> Changes for Focal/linux-azure:
>
> Wei Hu (2):
>   PCI: hv: Fix the PCI HyperV probe failure path to release resource
>     properly
>   PCI: hv: Retry PCI bus D0 entry on invalid device state
>
>  drivers/pci/controller/pci-hyperv.c | 60 ++++++++++++++++++++++++++---
>  1 file changed, 54 insertions(+), 6 deletions(-)
>
> --
> 2.25.1
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team