[PATCH 2/2] nvme-pci: disable hmb on idle suspend

[Kai-Heng Feng]

On Wed, Jul 28, 2021 at 12:40 AM Keith Busch <kbusch at kernel.org> wrote:
>
> An idle suspend may or may not disable host memory access from devices
> placed in low power mode. Either way, it should always be safe to
> disable the host memory buffer prior to entering the low power mode, and
> this should also always be faster than a full device shutdown.
>
> Cc: Kai-Heng Feng <kai.heng.feng at canonical.com>
> Signed-off-by: Keith Busch <kbusch at kernel.org>

Positive test result from user:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1912057/comments/74

Kai-Heng


> ---
>  drivers/nvme/host/pci.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index e8f3b32131d2..42d4cbe31a28 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -3130,8 +3130,13 @@ static int nvme_resume(struct device *dev)
>
>         if (ndev->last_ps == U32_MAX ||
>             nvme_set_power_state(ctrl, ndev->last_ps) != 0)
> -               return nvme_try_sched_reset(&ndev->ctrl);
> +               goto reset;
> +       if (ctrl->hmpre && nvme_setup_host_mem(ndev))
> +               goto reset;
> +
>         return 0;
> +reset:
> +       return nvme_try_sched_reset(ctrl);
>  }
>
>  static int nvme_suspend(struct device *dev)
> @@ -3155,15 +3160,9 @@ static int nvme_suspend(struct device *dev)
>          * the PCI bus layer to put it into D3 in order to take the PCIe link
>          * down, so as to allow the platform to achieve its minimum low-power
>          * state (which may not be possible if the link is up).
> -        *
> -        * If a host memory buffer is enabled, shut down the device as the NVMe
> -        * specification allows the device to access the host memory buffer in
> -        * host DRAM from all power states, but hosts will fail access to DRAM
> -        * during S3.
>          */
>         if (pm_suspend_via_firmware() || !ctrl->npss ||
>             !pcie_aspm_enabled(pdev) ||
> -           ndev->nr_host_mem_descs ||
>             (ndev->ctrl.quirks & NVME_QUIRK_SIMPLE_SUSPEND))
>                 return nvme_disable_prepare_reset(ndev, true);
>
> @@ -3174,6 +3173,17 @@ static int nvme_suspend(struct device *dev)
>         if (ctrl->state != NVME_CTRL_LIVE)
>                 goto unfreeze;
>
> +       /*
> +        * Host memory access may not be successful in a system suspend state,
> +        * but the specification allows the controller to access memory in a
> +        * non-operational power state.
> +        */
> +       if (ndev->hmb) {
> +               ret = nvme_set_host_mem(ndev, 0);
> +               if (ret < 0)
> +                       goto unfreeze;
> +       }
> +
>         ret = nvme_get_power_state(ctrl, &ndev->last_ps);
>         if (ret < 0)
>                 goto unfreeze;
> --
> 2.25.4
>