From: Cristian Marussi <cristian.marussi@arm.com>
To: Sudeep Holla <sudeep.holla@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
Cristian Marussi <cristian.marussi@arm.com>,
"Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
Jacky Bai <ping.bai@nxp.com>,
arm-scmi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Peng Fan <peng.fan@nxp.com>
Subject: Re: [PATCH] firmware: arm_scmi: clock: Relax check in scmi_clock_protocol_init
Date: Wed, 25 Mar 2026 12:18:53 +0000 [thread overview]
Message-ID: <acPSrTxMkfdeNOfr@pluto> (raw)
In-Reply-To: <20260325-spiffy-analytic-sloth-eabfea@sudeepholla>
On Wed, Mar 25, 2026 at 12:06:15PM +0000, Sudeep Holla wrote:
> On Tue, Mar 24, 2026 at 04:32:55PM +0100, Geert Uytterhoeven wrote:
> > Hi Cristian,
> >
Hi Sudeep,
> > On Tue, 24 Mar 2026 at 15:35, Cristian Marussi <cristian.marussi@arm.com> wrote:
> > > On Tue, Mar 24, 2026 at 02:15:36PM +0100, Geert Uytterhoeven wrote:
> > > > On Tue, 24 Mar 2026 at 09:41, Cristian Marussi <cristian.marussi@arm.com> wrote:
> > > > > Looking at the spec 3.6.2.5 CLOCK_ATTRIBUTES
> > > > >
> > > > > "This command returns the attributes that are associated with a specific clock. An agent might be allowed access to only
> > > > > a subset of the clocks available in the system. The platform must thus guarantee that clocks that an agent cannot access
> > > > > are not visible to it."
> > > > >
> > > > > ...not sure if this sheds some light or it is ambiguos anyway...I'd say that
> > > > > NOT_FOUND does NOT equate to be invisible...
> > > > >
> > > > > ...BUT at the same time I think that this practice of exposing a non-contiguos
> > > > > set of resources IDs (a set with holes in it) is the a well-known spec-loophole
> > > > > used by many vendors to deploy one single FW image across all of their platforms
> > > > > without having to reconfigure their reosurces IDs ro expose a common set of
> > > > > contiguos IDs like the spec would suggest...
> > > > >
> > > > > Having said that, since we unfortunately left this door open in the
> > > > > implementation, now this loophole has become common practice
> > > > > apparently...
> > > >
> > > > When I first read that paragraph, I was also confused.
> > > > What does "not visible" mean?
> > > > - Not present in the clock ID space exposed to that client of
> > > > the system?
> > > > Yeah, multiple different sequences of contiguous IDs, depending
> > > > on client!
> > >
> > > Yes that is the most spec-compliant interpretation usually; in general
> > > across all protocols the SCMI server, through customized enumeration
> > > results, should provide a per-agent view of the system: this should help
> > > handling shared or virtualized resources, since the agent always see
> > > only the 'illusion' provided by the server...
> > >
> > > ...under this assumption if you dont even need a resource at all (not
> > > RW nor RO) you should NOT even be able to see it...this in turn of
> > > course means that in order to expose a contiguous set of IDs you should
> > > be able to properly configure at build time the FW resources on a per
> > > platform basis...
> > >
> > > > - Return failure on CLOCK_ATTRIBUTES?
> > > > Which is what implementations seem to do.
> > >
> > > Yes this is what is done leveraging the gap in the implementation...I am
> > > not sure that the non-contiguous set of IDs is supported if not by
> > > chance as of now :P (especially in other protocols)
> > >
> > > > The next step in the fun is when the system actually needs to know the
> > > > clock rate of such a clock...
> > >
> > > Well...that seems a bit of wishful thinking ...
> >
> > Unfortunately it is real... [cliffhanger, to be continued... :-]
> >
>
> I am late to the discussion. Based on all the discussion so far, I don't
> want to rush the clk changes from Cristian that adds the hardening yet.
Agreed, the whole series with the iterators and hardening needs fixes from
Geert upfront...
> I won't make it part of my SCMI v7.1 PR. However is it good idea to keep
> it in the next so that we can converge towards some solution in v7.2 ?
>
> So, the question is if I add the fixes from Geert[1] to my queue, will it
> be a good baseline for all these changes and discussions ?
Yes having my iterator series plus Geert fixes on top would be a good
starting point to have a sane kernel...then we'll see how many quirks or
de-Hardening will be needed on top of that to make all firmware
survive...
I will re-test on my side in the coming days but anything that goes on
linux-next from your next of course has a more broad audience...
Thanks,
Cristian
next prev parent reply other threads:[~2026-03-25 12:19 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-24 6:24 [PATCH] firmware: arm_scmi: clock: Relax check in scmi_clock_protocol_init Peng Fan (OSS)
2026-03-24 7:49 ` Sudeep Holla
2026-03-24 8:18 ` Cristian Marussi
2026-03-24 13:15 ` Geert Uytterhoeven
2026-03-24 14:35 ` Cristian Marussi
2026-03-24 15:32 ` Geert Uytterhoeven
2026-03-25 12:06 ` Sudeep Holla
2026-03-25 12:18 ` Cristian Marussi [this message]
2026-03-25 12:14 ` Sudeep Holla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=acPSrTxMkfdeNOfr@pluto \
--to=cristian.marussi@arm.com \
--cc=arm-scmi@vger.kernel.org \
--cc=geert@linux-m68k.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peng.fan@nxp.com \
--cc=peng.fan@oss.nxp.com \
--cc=ping.bai@nxp.com \
--cc=sudeep.holla@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox