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

[Rafael J. Wysocki (Intel)]

Hi Hans,

On Wed, Oct 14, 2026 at 11:58 AM <johannes.goede@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Rafael,
>
> On 13-Oct-26 19:20, Rafael J. Wysocki (Intel) wrote:
> > 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.
>
> Ah, I see.
>
> >>> 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.
>
> Not needing to touch ACPICA is a valid concern. So I agree lets go
> with a wrapper for paths which we know need to be serialized for now.
>
> And then add that around the battery methods + _TMP + UCSI methods
> for starters I guess ?

Yes.

> ###
>
> Should we also stop releasing the interpreter lock in the EC code?

Which EC code do you mean in particular?

> All these things (battery, _TMP, UCSI) seem to have in common that
> they might talk to the EC in some cases?

Yes, they are EC-based in many cases.

> Where as e.g. your powering-up of PCI slots example is unlikely to
> talk to the EC ?

Yup.