linux-kernel - Re: [Patch v15 2/5] iommu/arm-smmu: Invoke pm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAFQd5BrJppNSo8ETT6ywjuLEULk4nAvWDQEuNiMCvdddeUq1A@mail.gmail.com>
Date:   Thu, 30 Aug 2018 18:33:53 +0900
From:   Tomasz Figa <tfiga@...omium.org>
To:     Vivek Gautam <vivek.gautam@...eaurora.org>
Cc:     "list@....net:IOMMU DRIVERS <iommu@...ts.linux-foundation.org>, Joerg
        Roedel <joro@...tes.org>," <joro@...tes.org>,
        Rob Herring <robh+dt@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Will Deacon <will.deacon@....com>,
        "list@....net:IOMMU DRIVERS <iommu@...ts.linux-foundation.org>, Joerg
        Roedel <joro@...tes.org>," <iommu@...ts.linux-foundation.org>,
        devicetree@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        freedreno <freedreno@...ts.freedesktop.org>,
        Mark Rutland <mark.rutland@....com>,
        Linux PM <linux-pm@...r.kernel.org>, sboyd@...nel.org,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Alex Williamson <alex.williamson@...hat.com>,
        Andy Gross <andy.gross@...aro.org>
Subject: Re: [Patch v15 2/5] iommu/arm-smmu: Invoke pm_runtime during probe,
 add/remove device

On Thu, Aug 30, 2018 at 6:22 PM Vivek Gautam
<vivek.gautam@...eaurora.org> wrote:
>
> On Mon, Aug 27, 2018 at 4:27 PM Vivek Gautam
> <vivek.gautam@...eaurora.org> wrote:
> >
> > From: Sricharan R <sricharan@...eaurora.org>
> >
> > The smmu device probe/remove and add/remove master device callbacks
> > gets called when the smmu is not linked to its master, that is without
> > the context of the master device. So calling runtime apis in those places
> > separately.
> >
> > Signed-off-by: Sricharan R <sricharan@...eaurora.org>
> > [vivek: Cleanup pm runtime calls]
> > Signed-off-by: Vivek Gautam <vivek.gautam@...eaurora.org>
> > Reviewed-by: Tomasz Figa <tfiga@...omium.org>
> > Tested-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> > ---
> >
> > Changes since v14:
> >  - none.
> >
> >  drivers/iommu/arm-smmu.c | 101 +++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 93 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > index a81224bc6637..23b4a60149b6 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
>
> [snip]
>
> > @@ -2131,6 +2188,26 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> >         if (err)
> >                 return err;
> >
> > +       /*
> > +        * We want to avoid touching dev->power.lock in fastpaths unless
> > +        * it's really going to do something useful - pm_runtime_enabled()
> > +        * can serve as an ideal proxy for that decision. So, conditionally
> > +        * enable pm_runtime.
> > +        */
> > +       if (dev->pm_domain)
> > +               pm_runtime_enable(dev);
> > +
> > +       err = arm_smmu_rpm_get(smmu);
>
> We shouldn't be doing a runtime_get() yet, as this eventually calls
> arm_smmu_device_reset().
> arm_smmu_device_reset() should be called only after arm_smmu_device_cfg_probe().
> So, I plan to replace the pm_runtime_get/put() calls in probe() with
> simple clk_bulk_enable()
> to let the driver initialize smmu, and at the end of the probe we can
> disable the clocks and
> enable runtime pm over the device to let it take care of the device further-on.
>

We can avoid the explicit clock disable by just calling
pm_runtime_set_active() before pm_runtime_enable(), assuming that what
probe does is symmetrical with the suspend callback, which would be
called after the latter.

Best regards,
Tomasz