On Tue, Nov 19, 2019 at 11:16:56PM -0800, Dexuan Cui wrote:
> Implement the suspend/resume callbacks.
>
> We must make sure there is no pending work items before we call
> vmbus_close().
Where ? Why ? Imagine a developer reading this log to try to understand
why you made this change, do you really think this commit log is
informative in its current form ?
I am not asking a book but this is a significant feature please make
an effort to explain it (I can update the log for you but please
write one and I shall do it).
Lorenzo
> Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
> ---
>
> Changes in v2: this patch is a simple merge of 2 previous smaller patches,
> accordign to the suggestion of Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>.
>
> drivers/pci/controller/pci-hyperv.c | 107 +++++++++++++++++++++++++++-
> 1 file changed, 105 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 65f18f840ce9..e71eb6e0bfdd 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -455,6 +455,7 @@ enum hv_pcibus_state {
> hv_pcibus_init = 0,
> hv_pcibus_probed,
> hv_pcibus_installed,
> + hv_pcibus_removing,
> hv_pcibus_removed,
> hv_pcibus_maximum
> };
> @@ -1681,6 +1682,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) {
> @@ -2107,6 +2125,12 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
> unsigned long flags;
> bool pending_dr;
>
> + if (hbus->state == hv_pcibus_removing) {
> + dev_info(&hbus->hdev->device,
> + "PCI VMBus BUS_RELATIONS: ignored\n");
> + return;
> + }
> +
> dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT);
> if (!dr_wrk)
> return;
> @@ -2223,11 +2247,19 @@ static void hv_eject_device_work(struct work_struct *work)
> */
> static void hv_pci_eject_device(struct hv_pci_dev *hpdev)
> {
> + struct hv_pcibus_device *hbus = hpdev->hbus;
> + struct hv_device *hdev = hbus->hdev;
> +
> + if (hbus->state == hv_pcibus_removing) {
> + dev_info(&hdev->device, "PCI VMBus EJECT: ignored\n");
> + return;
> + }
> +
> hpdev->state = hv_pcichild_ejecting;
> get_pcichild(hpdev);
> INIT_WORK(&hpdev->wrk, hv_eject_device_work);
> - get_hvpcibus(hpdev->hbus);
> - queue_work(hpdev->hbus->wq, &hpdev->wrk);
> + get_hvpcibus(hbus);
> + queue_work(hbus->wq, &hpdev->wrk);
> }
>
> /**
> @@ -3107,6 +3139,75 @@ 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);
> + enum hv_pcibus_state old_state;
> + int ret;
> +
> + tasklet_disable(&hdev->channel->callback_event);
> +
> + /* Change the hbus state to prevent new work items. */
> + old_state = hbus->state;
> + if (hbus->state == hv_pcibus_installed)
> + hbus->state = hv_pcibus_removing;
> +
> + tasklet_enable(&hdev->channel->callback_event);
> +
> + if (old_state != hv_pcibus_installed)
> + return -EINVAL;
> +
> + 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 */
> @@ -3121,6 +3222,8 @@ static struct hv_driver hv_pci_drv = {
> .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)
> --
> 2.19.1
>
