linux-kernel - Re: [RFC PATCH 0/3] Introduce SCMI Quirks framework

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: <Z_5lJA74OugEbB-S@pluto>
Date: Tue, 15 Apr 2025 14:55:33 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: Johan Hovold <johan@...nel.org>
Cc: Cristian Marussi <cristian.marussi@....com>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	arm-scmi@...r.kernel.org, sudeep.holla@....com,
	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: [RFC PATCH 0/3] Introduce SCMI Quirks framework

On Fri, Apr 04, 2025 at 03:22:57PM +0200, Johan Hovold wrote:
> Hi Cristian,
> 
> On Tue, Apr 01, 2025 at 01:25:42PM +0100, Cristian Marussi wrote:
> 
> > with the increasing adoption of SCMI across arm64 ecosystems, we have to
> > start considering how to deal and take care of out-of-spec SCMI firmware
> > platforms that are actively deployed in the wild, in a consistent manner.
> > 
> > This small series introduces a simple framework, based on static_keys,
> > that allows a user to:
> > 
> > - define a quirk and its matching conditions; quirks can match based on:
> > 
> >   	compatible / Vendor_ID / Sub_Vendor_ID / ImplVersion
> > 
> >   from the longest matching sequence down to the shortest.
> > 
> >   When the SCMI core stack boots it will enable the matching quirks
> >   depending on the information gathered from the platform via Base
> >   protocol: any NULL/0 match condition is ignored during matching and is
> >   interpreted as ANY, so you can decide to match on a very specific
> >   combination of compatibles and FW versions OR simply on a compatible.
> > 
> > - define a quirk code-block: simply a block of code, meant to play the
> >   magic quirk trick, defined in the proximity of where it will be used
> >   and gated by an implicit quirk static-key associated with the defined
> >   quirk
> 
> > Any feedback and testing is very much welcome.
> 
> I'm hitting the below lockdep splat when running with this series and
> the FC quirk enabled.
> 
> Johan
> 
> 
> [    8.399581] ======================================================
> [    8.399582] WARNING: possible circular locking dependency detected
> [    8.399583] 6.14.0 #103 Not tainted
> [    8.399584] ------------------------------------------------------
> [    8.399584] kworker/u49:5/165 is trying to acquire lock:
> [    8.399586] ffff65d004d36320 (&info->protocols_mtx){+.+.}-{4:4}, at: scmi_get_protocol_instance+0x4c/0x5c0
> [    8.399596] 
>                but task is already holding lock:
> [    8.399596] ffff65d00a895348 (&ni->pending_mtx){+.+.}-{4:4}, at: __scmi_event_handler_get_ops+0x70/0x47c
> [    8.399600] 
>                which lock already depends on the new lock.
> 
> [    8.399601] 
>                the existing dependency chain (in reverse order) is:
> [    8.399601] 
>                -> #4 (&ni->pending_mtx){+.+.}-{4:4}:

Hi Johan,

I have reproduced this when running with LOCKDEP and quirks enabled:
it was due to the fact that static_branch_enable() takes a cpu_lock
internally and I was triggering all of this from Base protocol init
which takes a number of other mutexes...

I moved SCMI quirk initialiation after Base protocol initialization
in scmi_probe and it is gone...and indeed was not needed anyway so
early.

I'll post a new V1 for quirks in a short while including this change.

Thanks,
Cristian