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] [day] [month] [year] [list]

Message-ID: <2c36d66dc3eaf8f0f9778dd6a1d45806e7d9bcdd.camel@linaro.org>
Date: Mon, 06 Oct 2025 08:06:26 +0100
From: André Draszik <andre.draszik@...aro.org>
To: Sam Protsenko <semen.protsenko@...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 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.

Cheers,
Andre'