Re: [PATCH 2/4] PCI: hv: Add the support of hibernation

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

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
>