RE: [PATCH V2] Change ACPI IPMI support to "default y"

[Zheng, Lv]

Hi,

> From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi-owner@xxxxxxxxxxxxxxx] On Behalf Of Matthew Garrett
> Sent: Saturday, February 22, 2014 12:13 AM
> 
> On Fri, 2014-02-21 at 02:17 +0000, Zheng, Lv wrote:
> 
> > In fact there is a workaround solution I've posted here:
> > https://patchwork.kernel.org/patch/2831851/
> > The updated version of this patch can be found at:
> > https://bugzilla.kernel.org/attachment.cgi?id=112611
> > It is the acpi-ipmi13.patch file.
> 
> I don't think that solves the underlying problem? We're still left with
> no guarantee of ordering between ACPI IPMI operation region support
> being available and the loading of a driver that may rely on it.

Yes. As it is a bit far beyond the bug report, I didn't work on it.
But actually, I have solution to achieve this.

> > I think there should be relationship between ACPI region and Linux kernel modules.
> > In fact on the well-known operating system, _REG is always invoked at the end of the IRP_PNP_START_DEVICE completions.
> > But we may still be able to return -EPROBE_DEFER in the power meter driver when it detects failures caused by the readiness state
> of the region handlers.
> 
> We don't have a guarantee that it'll happen at probe time. The
> capabilities list may be static, and so we'll get our first failure on
> an attempted userspace access instead.

This is the issue related to the IPMI operation region, and it can be solved by a "request_module()" call.
The -EPROBE_DEFER I mentioned is about the quality of  acpi power meter driver itself.

> It may be that Corey's suggestion satisfies the concerns Russ raised.
> Building IPMI in but only probing by default on systems that declare a
> BMC in ACPI or DMI should avoid boot-time delays while still ensuring
> that the functionality is available on systems where there's a real
> chance that the firmware expects the OS to provide support.

If you take a look at this patch (https://patchwork.kernel.org/patch/2831851/), you'll see it is possible to add request_module() in acpi_region_default_handler().
The callback is invoked when the first IPMI region access happens and it is in the process context without any locking.
I'd rather to embed acpi_ipmi into ipmi_si module with this fix:
https://bugzilla.kernel.org/attachment.cgi?id=112611 (acpi-ipmi14.patch).
After that, a request_module() invocation of ipmi_si in acpi_region_default_handler() can solve what you are worrying about.

What's your opinion?

Thanks and best regards
-Lv

> --
> Matthew Garrett <matthew.garrett@xxxxxxxxxx>
N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nlj·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~†­†Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ0¶ìå