Re: [PATCH v6 2/3] PCI/NPEM: Add Native PCIe Enclosure Management support

[Lukas Wunner <lukas@wunner.de>]

On Thu, Aug 15, 2024 at 12:42:48PM -0500, Bjorn Helgaas wrote:
> The dependency of _DSM on IPMI sounds like a purely ACPI problem.  Is
> there no mechanism in ACPI to express that dependency?

Unfortunately there doesn't seem to be one. :(


> If _DSM claims the function is supported before the IPMI driver is
> ready, that sounds like a BIOS defect to me.
> 
> If we're stuck with this, maybe the comment can be reworded.  "Lazy
> loading" in a paragraph that also mentions initcalls and the
> "ACPI_IPMI" module makes it sound like we're talking about loading the
> *module* lazily, not just (IIUC) reading the LED status lazily.
> 
> Maybe it could also explicitly say that the GET_STATE_DSM function
> depends on IPMI.
> 
> I'm unhappy that we're getting our arm twisted here.  If functionality
> depends on IPMI, there really needs to be a way for OSPM to manage
> that dependency.  If we're working around a firmware defect, we need
> to be clear about that.

AFAICS lazy initialization of active indications was architected
such that it is retried on every LED access until it succeeds:

npem->active_inds_initialized is only set to true once
npem->ops->get_active_indications() returns successfully.
I'm assuming that the DSM method fails as it should on
inaccessibility of the IPMI OpRegion.

So users may see errors in dmesg when they access LEDs if IPMI drivers
have not been loaded yet, but once they're loaded, those errors will
go away and LED access should start working flawlessly.

This way of lazily initializing the cached active_indications bit mask
doesn't cost us much as far as code complexity is concerned, but should
make things work 99.999% of the time on quirky platforms.

Thanks,

Lukas