RE: [PATCH 2/4] PCI: hv: Add the support of hibernation
> From: Lorenzo Pieralisi > Sent: Thursday, September 26, 2019 9:31 AM > > On Wed, Sep 11, 2019 at 11:38:20PM +, Dexuan Cui wrote: > > Implement the suspend/resume callbacks for hibernation. > > > > hv_pci_suspend() needs to prevent any new work from being queued: a later > > patch will address this issue. > > I don't see why you have two separate patches, the second one fixing the > previous (this one). Squash them together and merge them as such, or > there is something I am missing here. > > Lorenzo I was trying to make it easier to review the changes by spliting the changes into 2 smaller patches. :-) I'll merge patch 2/4 and patch 3/4 into one patch, and post a v2. Thanks, -- Dexuan
Re: [PATCH 2/4] PCI: hv: Add the support of hibernation
On Wed, Sep 11, 2019 at 11:38:20PM +, Dexuan Cui wrote:
> Implement the suspend/resume callbacks for hibernation.
>
> hv_pci_suspend() needs to prevent any new work from being queued: a later
> patch will address this issue.
I don't see why you have two separate patches, the second one fixing the
previous (this one). Squash them together and merge them as such, or
there is something I am missing here.
Lorenzo
> Signed-off-by: Dexuan Cui
> ---
> drivers/pci/controller/pci-hyperv.c | 76
> +
> 1 file changed, 76 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c
> b/drivers/pci/controller/pci-hyperv.c
> index 03fa039..3b77a3a 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1398,6 +1398,23 @@ static void prepopulate_bars(struct hv_pcibus_device
> *hbus)
>
> spin_lock_irqsave(>device_list_lock, flags);
>
> + /*
> + * Clear the memory enable bit, in case it's already set. This occurs
> + * in the suspend path of hibernation, where the device is suspended,
> + * resumed and suspended again: see hibernation_snapshot() and
> + * hibernation_platform_enter().
> + *
> + * If the memory enable bit is already set, Hyper-V sliently ignores
> + * the below BAR updates, and the related PCI device driver can not
> + * work, because reading from the device register(s) always returns
> + * 0x.
> + */
> + list_for_each_entry(hpdev, >children, list_entry) {
> + _hv_pcifront_read_config(hpdev, PCI_COMMAND, 2, );
> + command &= ~PCI_COMMAND_MEMORY;
> + _hv_pcifront_write_config(hpdev, PCI_COMMAND, 2, command);
> + }
> +
> /* Pick addresses for the BARs. */
> do {
> list_for_each_entry(hpdev, >children, list_entry) {
> @@ -2737,6 +2754,63 @@ static int hv_pci_remove(struct hv_device *hdev)
> return ret;
> }
>
> +static int hv_pci_suspend(struct hv_device *hdev)
> +{
> + struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
> + int ret;
> +
> + /* XXX: Need to prevent any new work from being queued. */
> + flush_workqueue(hbus->wq);
> +
> + ret = hv_pci_bus_exit(hdev, true);
> + if (ret)
> + return ret;
> +
> + vmbus_close(hdev->channel);
> +
> + return 0;
> +}
> +
> +static int hv_pci_resume(struct hv_device *hdev)
> +{
> + struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
> + enum pci_protocol_version_t version[1];
> + int ret;
> +
> + hbus->state = hv_pcibus_init;
> +
> + ret = vmbus_open(hdev->channel, pci_ring_size, pci_ring_size, NULL, 0,
> + hv_pci_onchannelcallback, hbus);
> + if (ret)
> + return ret;
> +
> + /* Only use the version that was in use before hibernation. */
> + version[0] = pci_protocol_version;
> + ret = hv_pci_protocol_negotiation(hdev, version, 1);
> + if (ret)
> + goto out;
> +
> + ret = hv_pci_query_relations(hdev);
> + if (ret)
> + goto out;
> +
> + ret = hv_pci_enter_d0(hdev);
> + if (ret)
> + goto out;
> +
> + ret = hv_send_resources_allocated(hdev);
> + if (ret)
> + goto out;
> +
> + prepopulate_bars(hbus);
> +
> + hbus->state = hv_pcibus_installed;
> + return 0;
> +out:
> + vmbus_close(hdev->channel);
> + return ret;
> +}
> +
> static const struct hv_vmbus_device_id hv_pci_id_table[] = {
> /* PCI Pass-through Class ID */
> /* 44C4F61D--4400-9D52-802E27EDE19F */
> @@ -2751,6 +2825,8 @@ static int hv_pci_remove(struct hv_device *hdev)
> .id_table = hv_pci_id_table,
> .probe = hv_pci_probe,
> .remove = hv_pci_remove,
> + .suspend= hv_pci_suspend,
> + .resume = hv_pci_resume,
> };
>
> static void __exit exit_hv_pci_drv(void)
> --
> 1.8.3.1
>
[PATCH 2/4] PCI: hv: Add the support of hibernation
Implement the suspend/resume callbacks for hibernation.
hv_pci_suspend() needs to prevent any new work from being queued: a later
patch will address this issue.
Signed-off-by: Dexuan Cui
---
drivers/pci/controller/pci-hyperv.c | 76 +
1 file changed, 76 insertions(+)
diff --git a/drivers/pci/controller/pci-hyperv.c
b/drivers/pci/controller/pci-hyperv.c
index 03fa039..3b77a3a 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1398,6 +1398,23 @@ static void prepopulate_bars(struct hv_pcibus_device
*hbus)
spin_lock_irqsave(>device_list_lock, flags);
+ /*
+* Clear the memory enable bit, in case it's already set. This occurs
+* in the suspend path of hibernation, where the device is suspended,
+* resumed and suspended again: see hibernation_snapshot() and
+* hibernation_platform_enter().
+*
+* If the memory enable bit is already set, Hyper-V sliently ignores
+* the below BAR updates, and the related PCI device driver can not
+* work, because reading from the device register(s) always returns
+* 0x.
+*/
+ list_for_each_entry(hpdev, >children, list_entry) {
+ _hv_pcifront_read_config(hpdev, PCI_COMMAND, 2, );
+ command &= ~PCI_COMMAND_MEMORY;
+ _hv_pcifront_write_config(hpdev, PCI_COMMAND, 2, command);
+ }
+
/* Pick addresses for the BARs. */
do {
list_for_each_entry(hpdev, >children, list_entry) {
@@ -2737,6 +2754,63 @@ static int hv_pci_remove(struct hv_device *hdev)
return ret;
}
+static int hv_pci_suspend(struct hv_device *hdev)
+{
+ struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
+ int ret;
+
+ /* XXX: Need to prevent any new work from being queued. */
+ flush_workqueue(hbus->wq);
+
+ ret = hv_pci_bus_exit(hdev, true);
+ if (ret)
+ return ret;
+
+ vmbus_close(hdev->channel);
+
+ return 0;
+}
+
+static int hv_pci_resume(struct hv_device *hdev)
+{
+ struct hv_pcibus_device *hbus = hv_get_drvdata(hdev);
+ enum pci_protocol_version_t version[1];
+ int ret;
+
+ hbus->state = hv_pcibus_init;
+
+ ret = vmbus_open(hdev->channel, pci_ring_size, pci_ring_size, NULL, 0,
+hv_pci_onchannelcallback, hbus);
+ if (ret)
+ return ret;
+
+ /* Only use the version that was in use before hibernation. */
+ version[0] = pci_protocol_version;
+ ret = hv_pci_protocol_negotiation(hdev, version, 1);
+ if (ret)
+ goto out;
+
+ ret = hv_pci_query_relations(hdev);
+ if (ret)
+ goto out;
+
+ ret = hv_pci_enter_d0(hdev);
+ if (ret)
+ goto out;
+
+ ret = hv_send_resources_allocated(hdev);
+ if (ret)
+ goto out;
+
+ prepopulate_bars(hbus);
+
+ hbus->state = hv_pcibus_installed;
+ return 0;
+out:
+ vmbus_close(hdev->channel);
+ return ret;
+}
+
static const struct hv_vmbus_device_id hv_pci_id_table[] = {
/* PCI Pass-through Class ID */
/* 44C4F61D--4400-9D52-802E27EDE19F */
@@ -2751,6 +2825,8 @@ static int hv_pci_remove(struct hv_device *hdev)
.id_table = hv_pci_id_table,
.probe = hv_pci_probe,
.remove = hv_pci_remove,
+ .suspend= hv_pci_suspend,
+ .resume = hv_pci_resume,
};
static void __exit exit_hv_pci_drv(void)
--
1.8.3.1
