Re: [PATCH] ACPI: battery: Re-read battery information in the complete callback

[Rafael J. Wysocki (Intel)]

Hi Hans,

On Tue, Oct 13, 2026 at 3:36 PM <johannes.goede@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Rafael,
>
> On 13-Oct-26 13:58, Rafael J. Wysocki (Intel) wrote:
> > On Tue, Oct 13, 2026 at 11:33 AM <johannes.goede@xxxxxxxxxxxxxxxx> wrote:
> >>
> >> Hi,
> >>
> >> On 13-Oct-26 00:46, Ali Ahmet Memiş wrote:
> >>> Since commit d33e89d12295 ("thermal: core: Suspend thermal zones later
> >>> and resume them earlier"), thermal zones are resumed from dpm_complete(),
> >>> before the PM_POST_SUSPEND notifier that re-reads _BIX and _BST. On
> >>> ThinkPad P1 Gen2 and P53, INT3403 _TMP calls \GTST which switches the EC
> >>> page selector (HIID) without taking the BATM mutex used by the battery
> >>> methods. ACPICA releases the interpreter lock around EC accesses
> >>
> >> I'm wondering if this "ACPICA releases the interpreter lock around EC
> >> accesses" is not the real problem here.
> >
> > Yes, it most likely is.
> >
> > What actually happens is that ACPICA drops the interpreter lock and
> > the namespace lock in several cases listed in the
> > acpi_ex_exit_interpreter() documentation comment, the majority of
> > which involve having to wait.
> >
> > The bottom line though is that the interpreter and namespace locks
> > cannot be expected to be held throughout AML evaluation because it is
> > not known in advance if any of the cases in question will appear.
> >
> >> Recently I've learned / heard
> >> from a trusted source that Windows ACPI subsystem is very much single
> >> threaded. So it is nice that ACPI has these mutexes and other stuff
> >> to support multi-threading but as long as Windows ACPI interpreter is
> >> not multi-threaded I think we may need to consider to copy Windows
> >> behavior.
> >
> > This is also my conclusion, at least to some extent.
> >
> > The problem with it though is that in some cases parallel evaluation
> > of AML is actually useful.  For example, say two PCI devices are
> > powered up at the same time and AML waits for each of them.  The async
> > suspend/resume aligns those waits to take place simultaneously instead
> > of serializing them which allows some time to be saved.
> >
> >> So maybe the EC accesses should keep the interpreter lock locked during
> >> EC accesses.
> >
> > The problem here is that it is hard to say in advance whether or not
> > _BIX or _BST will access the EC and whether or not that will clash
> > with _TMP evaluation for a seemingly unrelated thermal zone.
> >
> >> And even go further and maybe we need to introduce a big-acpi-lock,
> >> which I guess the interpreter lock more or less is? And just serialize
> >> all ACPI accesses since most ACPI tables clearly do not handle
> >> multi-threaded access to ACPI methods well ?
> >
> > I would be inclined to do so except for device power management
> > methods, that is _ON, _OFF, and _PSx, that are not the problematic
> > ones, at least as far as I know.
>
> That sounds like a reasonable compromise which should fix both
> this case as well as help with bko221065 .
>
> >> Another example of multi-threaded ACPI calls causing problems is:
> >>
> >> https://bugzilla.kernel.org/show_bug.cgi?id=221065
> >
> > Right.
> >
> > So I guess the idea would be to add locking to acpi_evaluate_object()?
>
> Yes that seems like a nice single point of entry to do this where
> all the various helpers eventually end up.

That, of course, would require updating ACPICA code and I'm not sure
if doing it upstream is useful.  For example, if some hosts already
use locking around acpi_evaluate_object(), this modification won't
help them and it will certainly be a change in behavior.

This means that we'll likely end up with a Linux-specific divergence
from the upstream which can be lived with, but it won't be nice.

> > An alternative might be to introduce an "exclusive" variant of
> > acpi_evaluate_object() with built-in locking and use it in the cases
> > that are known-problematic.
>
> So here we have _TMP and _BIX / _BST and in bko221065 we have
> a bunch of again batteyr methods + UCSI racing. I'm afraid if
> we add something like acpi_evaluate_object_exclusive() we'll end
> up eventually using that almost everywhere.

While I think that your concern is valid, this can be done without
touching the ACPICA code at all.

Also, we don't actually know how many "bad" cases there are and so far
we have identified a few.  If they are not too widespread, changing
behavior everywhere for everyone may not be adequate.

> Maybe do things the other way around and serializing all the calls
> by default and adding an acpi_evaluate_object_unlocked() to be used
> in cases where we want the speedup of multi-threading like
> the _ON, _OFF and _PSx you mentioned above ?

Yes, depending on which way is more practical, that can be done.