[PATCH 4/4] firmware: arm_scmi: Validate Powercap domains before state access
Sudeep Holla
sudeep.holla at kernel.org
Wed May 20 01:10:11 PDT 2026
On Tue, May 19, 2026 at 11:04:41AM +0100, Cristian Marussi wrote:
> On Sun, May 17, 2026 at 08:02:43PM +0100, Sudeep Holla wrote:
> > Powercap protocol v2 keeps local enable and last-cap state per
> > domain. Some public operations indexed that state before checking that
> > the supplied domain id was valid, and cap_enable_get() updated it even
> > when cap_get() failed.
> >
> > Validate the domain before touching the per-domain state and only
> > refresh cached enable state after a successful cap_get().
> >
>
[...]
> > /*
> > * Report always real platform state; platform could have ignored
> > * a previous disable request. Default true on any error.
> > */
> > ret = scmi_powercap_cap_get(ph, domain_id, &power_cap);
> > - if (!ret)
> > + if (!ret) {
> > *enable = !!power_cap;
> >
> > - /* Update internal state with current real platform state */
> > - pi->states[domain_id].enabled = *enable;
> > + /* Update internal state with current real platform state */
> > + pi->states[domain_id].enabled = *enable;
> > + }
>
> Mmm, this changes the logic as stated in the above comments...now the
> problem is recalling WHY I adopted this logic :<
>
The comment captures only ignored disabled request in which case
enabled=true is correct. But I fail to understand what would happen
if the previous call is scmi_powercap_cap_enable_set(..,false); and
assume the scmi_powercap_cap_get() from scmi_powercap_cap_enable_set()
also returned false, but failure to get the state in
scmi_powercap_cap_enable_get() will update it to true.
I wonder if it is due to the SCMI versions to maintain compatibility or the
kernel default sysfs value. scmi_powercap_cap_set() treats cached disabled
state specially. If cap_enable_get() defaulted the cache to false after
a failed read, later cap updates could be suppressed even though the platform
might actually still be enabled. Defaulting to true is the conservative
choice. I will drop this change/hunk. I will assume your reviewed by with
that dropped, please shout if not.
The generic powercap sysfs layer itself defaults enabled to true, but if a
driver get_enable() returns an error it reports false. SCMI avoids that by
returning success with true I assume.
--
Regards,
Sudeep
More information about the linux-arm-kernel
mailing list