Re: [PATCH 0/3] ACPI: Serialize battery methods and INT3403 _TMP

[Rong Zhang <i@rong.moe>]

Hi Hans,

On Thu, 2026-10-15 at 15:13 +0200, Hans de Goede wrote:
> Hi,
> 
> On 15-Oct-26 13:45, Rong Zhang wrote:
> > Hi Ali,
> > 
> > Thanks for working on this!
> > 
> > The series is generally in a good shape. I just have a few minor
> > comments.
> > 
> > On Wed, 2026-10-14 at 16:26 +0300, Ali Ahmet Memiş wrote:
> > > As discussed in [1], add exclusive variants of acpi_evaluate_object()
> > > and acpi_evaluate_integer() and use them for the battery methods and
> > > INT3403 _TMP, which interleave badly on ThinkPad P1 Gen2 and P53.
> > > 
> > > UCSI (bko221065) is not converted.  
> > > 
> > 
> > Please leave a link to the bko bug, so that readers don't need to
> > assemble the link themselves.
> > 
> > > There the contention is on a
> > 
> > After carefully auditing the ASL code again, I've just found the root
> > cause of the mutex acquisition timeout.
> > 
> > It's hard to notice, but \_SB.PCI0.LPC0.EC0.RDER being declared as
> > "Serialized" causes an ABBA deadlock:
> > 
> > Thread 1 (battery)                         Thread 2 (ideapad-laptop)
> > -----------------------------------------  ---------------------------------------
> > serialize \_SB.BAT0._BIX                   serialize \_SB.PCI0.LPC0.EC0.VPC0.GBMD
> > evaluate \_SB.BAT0._BIX                    evaluate \_SB.PCI0.LPC0.EC0.VPC0.GBMD
> >   acquire ECMT [A]
> >   serialize \_SB.PCI0.LPC0.EC0.RDIF          serialize \_SB.PCI0.LPC0.EC0.RDER [B]
> >   evaluate \_SB.PCI0.LPC0.EC0.RDIF           evaluate \_SB.PCI0.LPC0.EC0.RDER
> >     serialize \_SB.PCI0.LPC0.EC0.RDER [B]      acquire ECMT [A]
> > 
> > That's why merging battery notifications significantly reduced the
> > frequency of mutex acquisition timeout.
> > 
> > > firmware AML mutex that _Qxx EC queries also take, which a wrapper
> > > around acpi_evaluate_object() cannot cover, and Rong's notification
> > > merging in 7.3 already reduces it on the battery side.
> > 
> > Lock contention is not the root cause, and EC queries and UCSI _DSM are
> > just innocent victims of the ABBA deadlock.
> 
> Hmm, ok so since \_SB.BAT0._BIX takes ECMT itself instead of relying
> on \_SB.PCI0.LPC0.EC0.RDER taking it (1), 
> 

I guess the intent of holding ECMT in _BIX was to prevent race conditions
in multi-threaded interpreters. IOW, the firmware authors probably took
multi-threaded interpreter into their consideration.

        If ((Acquire (^^PCI0.LPC0.EC0.ECMT, 0x2000) == 0x00))
        {
            If (((^^PCI0.LPC0.EC0.BTDV && ^^PCI0.LPC0.EC0.BTFC) && ^^PCI0.LPC0.EC0.BTDC))
            {
                BPK1 [0x01] = 0x00
                Local0 = ^^PCI0.LPC0.EC0.BTDC /* \_SB_.PCI0.LPC0.EC0_.BTDC */
                Local0 *= 0x0A
                BPK1 [0x02] = Local0
                Local0 = ^^PCI0.LPC0.EC0.BTFC /* \_SB_.PCI0.LPC0.EC0_.BTFC */
                Local0 *= 0x0A
                BPK1 [0x03] = Local0
                BPK1 [0x05] = ^^PCI0.LPC0.EC0.BTDV /* \_SB_.PCI0.LPC0.EC0_.BTDV */
                Local0 = ^^PCI0.LPC0.EC0.BTFC /* \_SB_.PCI0.LPC0.EC0_.BTFC */
                Local0 *= 0x0A
                Local0 *= 0x0A
                Divide (Local0, 0x64, Local1, Local2)
                BPK1 [0x06] = Local2
                Local0 = ^^PCI0.LPC0.EC0.BTFC /* \_SB_.PCI0.LPC0.EC0_.BTFC */
                Local0 *= 0x0A
                Local0 *= 0x02
                Divide (Local0, 0x64, Local1, Local2)
                BPK1 [0x07] = Local2
                BPK1 [0x08] = ^^PCI0.LPC0.EC0.BTCC /* \_SB_.PCI0.LPC0.EC0_.BTCC */
                BPK1 [0x09] = 0x251C
            }

            BPK1 [0x10] = ToString (^^PCI0.LPC0.EC0.RDIF (0x01), Ones)
            ToDecimalString (^^PCI0.LPC0.EC0.BTSN, Local0)
            BPK1 [0x11] = ToBuffer (Local0)
            BPK1 [0x12] = ToString (^^PCI0.LPC0.EC0.RDIF (0x02), Ones)
            BPK1 [0x13] = ToString (^^PCI0.LPC0.EC0.RDIF (0x03), Ones)
            Release (^^PCI0.LPC0.EC0.ECMT)
        }

> this means that not just
> the ideapad-laptop GBMD method call but anything calling
> \_SB.PCI0.LPC0.EC0.RDER may cause this abba dead-lock, right?
> Are we sure nothing else is calling  \_SB.PCI0.LPC0.EC0.RDER ?

I checked all code paths with ECMT involved, and concluded a call graph
with all faulty methods:

                      ideapad-laptop
                            |
                            v
   battery                 GBMD
     |                      |
     v                      v
   _BIX -> ECMT -> RDIF -> RDER -> ECMT
                    ^       ^
                    |       |
    +---------------+-------+
    |
   GSBI -> ECMT -> SMTE/SMTF -> ECMT

GSBI: \_SB.PCI0.LPC0.EC0.VPC0.GSBI
SMTE: \_SB.PCI0.LPC0.EC0.VPC0.SMTE
SMTF: \_SB.PCI0.LPC0.EC0.VPC0.SMTF

All methods in the graph are never called by any method or driver that is
not in the graph.

So battery and ideapad-laptop are the only drivers that could potentially
trigger the ABBA deadlock.

> 
> Regards,
> 
> Hans
> 
> 
> 1) I guess ACPI mutexes may be taken nested from the same thread

Right. ACPI mutexes are re-entrant. IMO it's a broken design by
encouraging firmware authors to write broken ASL code.

Thanks,
Rong

> 
>