linux-kernel - Re: [PATCH 2/4] firmware: arm

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: <aAdya5rephGNP_Tw@hovoldconsulting.com>
Date: Tue, 22 Apr 2025 12:41:47 +0200
From: Johan Hovold <johan@...nel.org>
To: Sudeep Holla <sudeep.holla@....com>,
	Cristian Marussi <cristian.marussi@....com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	arm-scmi@...r.kernel.org, james.quinlan@...adcom.com,
	f.fainelli@...il.com, vincent.guittot@...aro.org,
	peng.fan@....nxp.com, michal.simek@....com, quic_sibis@...cinc.com,
	dan.carpenter@...aro.org, maz@...nel.org
Subject: Re: [PATCH 2/4] firmware: arm_scmi: Add Quirks framework

On Thu, Apr 17, 2025 at 03:41:56PM +0100, Sudeep Holla wrote:
> On Thu, Apr 17, 2025 at 12:10:25PM +0100, Cristian Marussi wrote:
> > On Thu, Apr 17, 2025 at 10:44:24AM +0200, Johan Hovold wrote:
> > > On Wed, Apr 16, 2025 at 05:37:13PM +0100, Cristian Marussi wrote:
> > > > On Wed, Apr 16, 2025 at 06:00:37PM +0200, Johan Hovold wrote:
> > > > > On Tue, Apr 15, 2025 at 03:29:31PM +0100, Cristian Marussi wrote:
> > > 
> > > > > > +static void scmi_enable_matching_quirks(struct scmi_info *info)
> > > > > > +{
> > > > > > +	struct scmi_revision_info *rev = &info->version;
> > > > > > +	const char *compatible = NULL;
> > > > > > +	struct device_node *root;
> > > > > > +
> > > > > > +	root = of_find_node_by_path("/");
> > > > > > +	if (root) {
> > > > > > +		of_property_read_string(root, "compatible", &compatible);
> > > > > 
> > > > > Looks like you still only allow matching on the most specific compatible
> > > > > string.
> > > > > 
> > > > > As we discussed in the RFC thread, this will result in one quirk entry
> > > > > for each machine in a SoC family in case the issue is not machine
> > > > > specific.
> 
> Agreed, but we can predict that. You can infer just from the current state
> of affairs. Today all machines based on soc X may need the quirk but the
> firmware may vary across machines with same SoC.

Sure, I was just highlighting this limitation in the current
implementation...

> > > I was referring to the need to match on other compatible strings than
> > > the most specific one. For the ThinkPad T14s the strings are:
> > > 
> > > 	"lenovo,thinkpad-t14s-lcd", "lenovo,thinkpad-t14s",
> > > 	"qcom,x1e78100", "qcom,x1e80100"
> > > 
> > > Here you most certainly would not want to match on
> > > "lenovo,thinkpad-t14s-lcd" but rather on "lenovo,thinkpad-t14s" or one
> > > of the SoC compatibles.

...and the fact that even if you want to avoid matching on SoC
compatible, the current implementation seems to be too limited to allow
matching on machine compatibles generally (i.e. given that there may be
variants of a particular machine).

We may not even need this for the FC quirk, this was just a general
observation.

> > > of_machine_is_compatible() can be used to match on any compatible
> > > string, but not sure if that fits with your current implementation.
> > > 
> 
> I was thinking about the same when I looked at the code. Using it is
> more elegant IMO.

It would be more flexible, even if you never intend to accept any quirks
that matches for an entire SoC.

> > ...moreover this kind of carpet-quirking that hides the issue on any possible
> > fw version gives ZERO incentives to the aforementioned vendor to fix its
> > firmware...(or it fw-release process)...

If a vendor truly only cares about some other OS then perhaps this
argument isn't that strong and we'll just increase our own maintenance
burden.

> > Indeed, cross posting from your other mail thread, as of now we cannot
> > even be sure if the Vendor has somehow already updated the SCMI-related
> > firmware NEITHER we can be sure about this in the future since it has not
> > even confirmed how they are (or they will) be handling the Impl_Version field...
> > 
> > Having said that, I would add that in this specific case (FC quirk) since the
> > only way to make use of all of this SCMI stuff from the MicroZoft/ACPI world
> > is having working FCs and, since it's clear that our lovely vendor wont
> > certainly break this other lovely OS way of working with SCMI, MAYBE it could
> > be safe to just apply the quirk to this Vendor forever no matter what the
> > platform or FW version will be in the future...(so not using compats at all)

My understanding is that the version has been bumped now albeit for
other reasons than fixing this particular bug. And since enabling FC for
these messages should be safe we will probably be able to match on
vendor/impl_version here.

Sibi is looking into this and should be able to provide an answer
shortly.

Johan