linux-kernel - Re: [PATCH] soc: apple: apple-pmgr-pwrstate: Switch to IRQ-safe mode

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: <CAOgh=FwEpMGx-F6GZumOBkBYShtU_SVUWTzZ+WWY0Euc-75YGQ@mail.gmail.com>
Date:   Mon, 23 Jan 2023 11:22:53 +0000
From:   Eric Curtin <ecurtin@...hat.com>
To:     Hector Martin <marcan@...can.st>
Cc:     Sven Peter <sven@...npeter.dev>,
        Alyssa Rosenzweig <alyssa@...enzweig.io>,
        Janne Grunau <j@...nau.net>, asahi@...ts.linux.dev,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soc: apple: apple-pmgr-pwrstate: Switch to IRQ-safe mode

On Mon, 23 Jan 2023 at 07:01, Hector Martin <marcan@...can.st> wrote:
>
> This requires changing the reset path locking primitives to the spinlock
> path in genpd, instead of the mutex path.
>
> Signed-off-by: Hector Martin <marcan@...can.st>
> ---

It seems we need this to avoid a race from reading #asahi-dev IRC,
commit message could be more detailed here.

Reviewed-by: Eric Curtin <ecurtin@...hat.com>

Is mise le meas/Regards,

Eric Curtin

>  drivers/soc/apple/apple-pmgr-pwrstate.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/apple/apple-pmgr-pwrstate.c b/drivers/soc/apple/apple-pmgr-pwrstate.c
> index e1122288409a..a3e2bc1d2686 100644
> --- a/drivers/soc/apple/apple-pmgr-pwrstate.c
> +++ b/drivers/soc/apple/apple-pmgr-pwrstate.c
> @@ -116,8 +116,9 @@ static int apple_pmgr_ps_power_off(struct generic_pm_domain *genpd)
>  static int apple_pmgr_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
>  {
>         struct apple_pmgr_ps *ps = rcdev_to_apple_pmgr_ps(rcdev);
> +       unsigned long flags;
>
> -       mutex_lock(&ps->genpd.mlock);
> +       spin_lock_irqsave(&ps->genpd.slock, flags);
>
>         if (ps->genpd.status == GENPD_STATE_OFF)
>                 dev_err(ps->dev, "PS 0x%x: asserting RESET while powered down\n", ps->offset);
> @@ -129,7 +130,7 @@ static int apple_pmgr_reset_assert(struct reset_controller_dev *rcdev, unsigned
>         regmap_update_bits(ps->regmap, ps->offset, APPLE_PMGR_FLAGS | APPLE_PMGR_RESET,
>                            APPLE_PMGR_RESET);
>
> -       mutex_unlock(&ps->genpd.mlock);
> +       spin_unlock_irqrestore(&ps->genpd.slock, flags);
>
>         return 0;
>  }
> @@ -137,8 +138,9 @@ static int apple_pmgr_reset_assert(struct reset_controller_dev *rcdev, unsigned
>  static int apple_pmgr_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
>  {
>         struct apple_pmgr_ps *ps = rcdev_to_apple_pmgr_ps(rcdev);
> +       unsigned long flags;
>
> -       mutex_lock(&ps->genpd.mlock);
> +       spin_lock_irqsave(&ps->genpd.slock, flags);
>
>         dev_dbg(ps->dev, "PS 0x%x: deassert reset\n", ps->offset);
>         regmap_update_bits(ps->regmap, ps->offset, APPLE_PMGR_FLAGS | APPLE_PMGR_RESET, 0);
> @@ -147,7 +149,7 @@ static int apple_pmgr_reset_deassert(struct reset_controller_dev *rcdev, unsigne
>         if (ps->genpd.status == GENPD_STATE_OFF)
>                 dev_err(ps->dev, "PS 0x%x: RESET was deasserted while powered down\n", ps->offset);
>
> -       mutex_unlock(&ps->genpd.mlock);
> +       spin_unlock_irqrestore(&ps->genpd.slock, flags);
>
>         return 0;
>  }
> @@ -222,6 +224,7 @@ static int apple_pmgr_ps_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> +       ps->genpd.flags |= GENPD_FLAG_IRQ_SAFE;
>         ps->genpd.name = name;
>         ps->genpd.power_on = apple_pmgr_ps_power_on;
>         ps->genpd.power_off = apple_pmgr_ps_power_off;
> --
> 2.35.1
>
>