| Message ID | 1568245086-70601-3-git-send-email-decui@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Enhance pci-hyperv to support hibernation | expand |
On Wed, Sep 11, 2019 at 11:38:20PM +0000, 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 <decui@microsoft.com> > --- > 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(&hbus->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 > + * 0xFFFFFFFF. > + */ > + list_for_each_entry(hpdev, &hbus->children, list_entry) { > + _hv_pcifront_read_config(hpdev, PCI_COMMAND, 2, &command); > + command &= ~PCI_COMMAND_MEMORY; > + _hv_pcifront_write_config(hpdev, PCI_COMMAND, 2, command); > + } > + > /* Pick addresses for the BARs. */ > do { > list_for_each_entry(hpdev, &hbus->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-4444-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 >
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Sent: Thursday, September 26, 2019 9:31 AM > > On Wed, Sep 11, 2019 at 11:38:20PM +0000, 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
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(&hbus->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 + * 0xFFFFFFFF. + */ + list_for_each_entry(hpdev, &hbus->children, list_entry) { + _hv_pcifront_read_config(hpdev, PCI_COMMAND, 2, &command); + command &= ~PCI_COMMAND_MEMORY; + _hv_pcifront_write_config(hpdev, PCI_COMMAND, 2, command); + } + /* Pick addresses for the BARs. */ do { list_for_each_entry(hpdev, &hbus->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-4444-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)
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 <decui@microsoft.com> --- drivers/pci/controller/pci-hyperv.c | 76 +++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)