linux-kernel - Re: [PATCH 3/3] soc: samsung: gs101-pmu: implement access tables for read and write

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: <CAPLW+4mmbtFvdyijMQ+xG1S_FQeZyHt+cYJgycWTmpsnNNmamg@mail.gmail.com>
Date: Tue, 7 Oct 2025 14:06:54 -0500
From: Sam Protsenko <semen.protsenko@...aro.org>
To: André Draszik <andre.draszik@...aro.org>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>, 
	Peter Griffin <peter.griffin@...aro.org>, Tudor Ambarus <tudor.ambarus@...aro.org>, 
	Will McVicker <willmcvicker@...gle.com>, kernel-team@...roid.com, 
	linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] soc: samsung: gs101-pmu: implement access tables for
 read and write

On Mon, Oct 6, 2025 at 2:06 AM André Draszik <andre.draszik@...aro.org> wrote:
>
> On Fri, 2025-10-03 at 13:24 -0500, Sam Protsenko wrote:
> > On Thu, Oct 2, 2025 at 5:33 AM André Draszik <andre.draszik@...aro.org> wrote:
> > >
> > > Accessing non-existent PMU registers causes an SError, halting the
> > > system.
> > >
> > > Implement read and write access tables for the gs101-PMU to specify
> > > which registers are read- and/or writable to avoid that SError.
> > >
> > > Signed-off-by: André Draszik <andre.draszik@...aro.org>
> >
> > I think having "Fixes:" tag would be justified here?
>
> I decided against, because IMHO it's not a bug fix as such, it's a new feature.
> >
>
> > > ---
> > > Note there are checkpatch warnings 'Macros with complex values should
> > > be enclosed in parentheses' and 'Macro argument reuse' for macros like
> > > CLUSTER_CPU_RANGE(). Since they are used in an initialiser, the only
> > > way to get rid of the warnings is to avoid the macros and duplicate all
> > > the related register ranges I believe, which I'd rather not due to the
> > > sheer amount of similar blocks.
> > > ---
> > >  drivers/soc/samsung/gs101-pmu.c             | 306 ++++++++++++++++++++++++-
> > >  include/linux/soc/samsung/exynos-regs-pmu.h | 343 +++++++++++++++++++++++++++-
> > >  2 files changed, 640 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/soc/samsung/gs101-pmu.c b/drivers/soc/samsung/gs101-pmu.c
> > > index b5a535822ec830b751e36a33121e2a03ef2ebcb2..5be1cbfa58c95e466bbdf954923f324f74460783 100644
> > > --- a/drivers/soc/samsung/gs101-pmu.c
> > > +++ b/drivers/soc/samsung/gs101-pmu.c
> > > @@ -8,6 +8,7 @@
> > >  #include <linux/array_size.h>
> > >  #include <linux/soc/samsung/exynos-pmu.h>
> > >  #include <linux/soc/samsung/exynos-regs-pmu.h>
> > > +#include <linux/regmap.h>
> >
> > If you decide to add this line to exynos-pmu.h (as I commented in the
> > preceding patch), it can then be omitted here.
> >
> > >
> > >  #include "exynos-pmu.h"
> > >
> > > @@ -19,9 +20,312 @@
> > >  #define TENSOR_PMUREG_WRITE            1
> > >  #define TENSOR_PMUREG_RMW              2
>
> [...]
>
> > > +#define CLUSTER_NONCPU_RANGE(cl)                                       \
> > > +       regmap_reg_range(GS101_CLUSTER_NONCPU_IN(cl),                   \
> > > +                        GS101_CLUSTER_NONCPU_IN(cl)),                  \
> > > +       regmap_reg_range(GS101_CLUSTER_NONCPU_INT_IN(cl),               \
> > > +                        GS101_CLUSTER_NONCPU_INT_IN(cl)),              \
> > > +       regmap_reg_range(GS101_CLUSTER_NONCPU_DUALRAIL_CTRL_IN(cl),     \
> > > +                        GS101_CLUSTER_NONCPU_DUALRAIL_CTRL_IN(cl))
> > > +
> > > +       CLUSTER_NONCPU_RANGE(0),
> > > +       CLUSTER_NONCPU_RANGE(1),
> > > +       CLUSTER_NONCPU_RANGE(2),
> > > +       regmap_reg_range(GS101_CLUSTER_NONCPU_INT_EN(2),
> > > +                        GS101_CLUSTER_NONCPU_INT_DIR(2)),
> > > +#undef CLUSTER_NONCPU_RANGE
> > > +
> > > +#define SUBBLK_RANGE(blk)                                              \
> >
> > Reusing the same names for different macros seems a bit confusing. But
> > that might be just a matter of my taste, so no strong opinion.
>
> And I OTOH explicitly picked the same name because it's the same block, just
> for r/o instead of r/w :-)
>
> [...]
>
> >
> > That's quite an extensive list of registers! Does this PMU driver
> > really have to cover all of those?
>
> That's what all Samsung PMU drivers do, it's the PMU region after all. Also,
> in the gs101 case, only the PMU driver knows how to do the secure access: Various
> other drivers have references to this PMU regmap, e.g. phy drivers for isolation
> (USB & UFS) and upcoming PD driver will do too. We don't want to reimplement
> secure access in all of those.
>

Ok, thanks for explaining this.

Reviewed-by: Sam Protsenko <semen.protsenko@...aro.org>

> Cheers,
> Andre'