Re: [RFC PATCH 3/5] iommu/arm-smmu-v3: Implement pm_runtime & system sleep ops

[Pranjal Shrivastava <praan@google.com>]

On Thu, Mar 20, 2025 at 10:33:41PM +0000, Mostafa Saleh wrote:
> On Wed, Mar 19, 2025 at 12:43 AM Pranjal Shrivastava <praan@google.com> wrote:
> >
> > Implement pm_runtime and system sleep ops for arm-smmu-v3. The smmu is
> > disabled as part of the suspend callbacks after ensuring the completion
> > of all pending commands and is configured to abort any transactions that
> > happen after disabling the smmu. The smmu shall be reinitialized in the
> > resume callback by invoking the `arm_smmu_device_reset` helper.
> >
> > The MSIs are freed as part of the suspend and are re-allocated during
> > the resume operation.
> >
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > ---
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 106 ++++++++++++++++++++
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |   2 +
> >  2 files changed, 108 insertions(+)
> >
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index f65d9bca0392..caf750470772 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/pci.h>
> >  #include <linux/pci-ats.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> >  #include <linux/string_choices.h>
> >  #include <kunit/visibility.h>
> >  #include <uapi/linux/iommufd.h>
> > @@ -108,6 +109,32 @@ static const char * const event_class_str[] = {
> >
> >  static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master);
> >
> > +static int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
> > +{
> > +       int ret;
> > +
> > +       if (pm_runtime_enabled(smmu->dev)) {
> > +               ret = pm_runtime_resume_and_get(smmu->dev);
> > +               if (ret < 0) {
> > +                       dev_err(smmu->dev, "Failed to resume device: %d\n", ret);
> > +                       return ret;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
> > +{
> > +       int ret;
> > +
> > +       if (pm_runtime_enabled(smmu->dev)) {
> > +               ret = pm_runtime_put_autosuspend(smmu->dev);
> > +               if (ret < 0)
> > +                       dev_err(smmu->dev, "Failed to suspend device: %d\n", ret);
> > +       }
> > +}
> > +
> >  static void parse_driver_options(struct arm_smmu_device *smmu)
> >  {
> >         int i = 0;
> > @@ -4850,6 +4877,84 @@ static void arm_smmu_device_shutdown(struct platform_device *pdev)
> >         arm_smmu_device_disable(smmu);
> >  }
> >
> > +
> > +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
> > +{
> > +       struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> > +
> > +       /* We might get the vcmdq */
> > +       struct arm_smmu_cmdq_ent cmd = {
> > +               .opcode = smmu->features & ARM_SMMU_FEAT_E2H ?
> > +                       CMDQ_OP_TLBI_EL2_VA : CMDQ_OP_TLBI_NH_VA,
> > +       };
> > +
> > +       struct arm_smmu_cmdq *cmdq = arm_smmu_get_cmdq(smmu, &cmd);
> > +       struct arm_smmu_ll_queue *llq = &cmdq->q.llq;
> > +
> > +       /*
> > +        * Since suspend is invoked when all clients have been
> > +        * we don't expect more commands to be added to the cmdq.
> > +        * Thus, wait for all existing commands to complete.
> > +        */
> > +       arm_smmu_cmdq_shared_lock(cmdq);
> > +       arm_smmu_cmdq_poll_until_empty(smmu, cmdq, llq);
> > +       arm_smmu_cmdq_shared_unlock(cmdq);
> > +
> > +       /* Disable all queues */
> > +       arm_smmu_device_disable(smmu);
> > +
> > +       /* Abort all transactions to avoid spurious bypass */
> > +       arm_smmu_update_gbpa(smmu, GBPA_ABORT, 0);
> > +
> > +       /* Free all MSIs (re-allocated on resume) */
> > +       arm_smmu_free_msis(dev);
> > +
> > +       dev_dbg(dev, "Suspending smmu\n");
> > +       return 0;
> > +}
> > +
> > +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
> > +{
> > +       int ret;
> > +       struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> > +
> > +       dev_dbg(dev, "Resuming device\n");
> > +
> > +       /*
> > +        * The reset will re-initialize all the queues with the base addr,
> > +        * prod and cons maintained within struct arm_smmu_device as well as
> > +        * re-wire the IRQs and/or enable MSIs and install relevant handlers.
> > +        */
> > +       ret = arm_smmu_device_reset(smmu);
> > +
> > +       if (ret)
> > +               dev_err(dev, "Failed to reset during resume operation: %d\n", ret);
> > +
> > +       return ret;
> > +}
> > +
> > +static int __maybe_unused arm_smmu_pm_suspend(struct device *dev)
> > +{
> > +       if (pm_runtime_suspended(dev))
> > +               return 0;
> > +
> > +       return arm_smmu_runtime_suspend(dev);
> > +}
> > +
> > +static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
> > +{
> > +       if (pm_runtime_suspended(dev))
> > +               return 0;
> > +
> > +       return arm_smmu_runtime_resume(dev);
> > +}
> > +
> > +static const struct dev_pm_ops arm_smmu_pm_ops = {
> > +       SET_SYSTEM_SLEEP_PM_OPS(arm_smmu_pm_suspend, arm_smmu_pm_resume)
> 
> I guess, as there is no specific logic in the system suspend/resume handlers,
> SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
> would be enough?

Ack. Thanks for this! :)

> 
> Thanks,
> Mostafa
> 

-Praan

> > +       SET_RUNTIME_PM_OPS(arm_smmu_runtime_suspend,
> > +                          arm_smmu_runtime_resume, NULL)
> > +};
> > +
> >  static const struct of_device_id arm_smmu_of_match[] = {
> >         { .compatible = "arm,smmu-v3", },
> >         { },
> > @@ -4866,6 +4971,7 @@ static struct platform_driver arm_smmu_driver = {
> >         .driver = {
> >                 .name                   = "arm-smmu-v3",
> >                 .of_match_table         = arm_smmu_of_match,
> > +               .pm                     = &arm_smmu_pm_ops,
> >                 .suppress_bind_attrs    = true,
> >         },
> >         .probe  = arm_smmu_device_probe,
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > index bd9d7c85576a..6bac83a511e2 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> > @@ -505,6 +505,8 @@ static inline unsigned int arm_smmu_cdtab_l2_idx(unsigned int ssid)
> >  #define MSI_IOVA_BASE                  0x8000000
> >  #define MSI_IOVA_LENGTH                        0x100000
> >
> > +#define RPM_AUTOSUSPEND_DELAY_MS       15
> > +
> >  enum pri_resp {
> >         PRI_RESP_DENY = 0,
> >         PRI_RESP_FAIL = 1,
> > --
> > 2.49.0.rc1.451.g8f38331e32-goog
> >