LinuxLists.cc - [PATCH V2] Change ACPI IPMI support to "default y"

[permalink] [raw]

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

On Tuesday, February 18, 2014 11:28:29 AM Matthew Garrett wrote:
>
> The ACPI IPMI driver implements IPMI operation region support for the ACPI
> core. Systems that declare ACPI operation regions may reference them at any
> time, including during kernel initialisation. These accesses will fail
> unless the ACPI IPMI driver is present, and undesirable system behaviour
> may result. Set the default to Y in order to encourage distributions and
> users to configure kernels to avoid awkward surprises.

Do you have any examples of problems caused by that or is this just theoretical
at the moment?

Rafael

2014-02-18 23:15:20

[permalink] [raw]

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

On Wed, 2014-02-19 at 00:26 +0100, Rafael J. Wysocki wrote:
> On Tuesday, February 18, 2014 11:28:29 AM Matthew Garrett wrote:
> >
> > The ACPI IPMI driver implements IPMI operation region support for the ACPI
> > core. Systems that declare ACPI operation regions may reference them at any
> > time, including during kernel initialisation. These accesses will fail
> > unless the ACPI IPMI driver is present, and undesirable system behaviour
> > may result. Set the default to Y in order to encourage distributions and
> > users to configure kernels to avoid awkward surprises.
>
> Do you have any examples of problems caused by that or is this just theoretical
> at the moment?

For example, if you load the ACPI power meter driver before you've
installed the ACPI IPMI driver you'll typically get failures (most
vendors implement it via IPMI).

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-18 23:20:55

[permalink] [raw]

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

On Tuesday, February 18, 2014 11:15:08 PM Matthew Garrett wrote:
> On Wed, 2014-02-19 at 00:26 +0100, Rafael J. Wysocki wrote:
> > On Tuesday, February 18, 2014 11:28:29 AM Matthew Garrett wrote:
> > >
> > > The ACPI IPMI driver implements IPMI operation region support for the ACPI
> > > core. Systems that declare ACPI operation regions may reference them at any
> > > time, including during kernel initialisation. These accesses will fail
> > > unless the ACPI IPMI driver is present, and undesirable system behaviour
> > > may result. Set the default to Y in order to encourage distributions and
> > > users to configure kernels to avoid awkward surprises.
> >
> > Do you have any examples of problems caused by that or is this just theoretical
> > at the moment?
>
> For example, if you load the ACPI power meter driver before you've
> installed the ACPI IPMI driver you'll typically get failures (most
> vendors implement it via IPMI).

Well, any specific machine from any specific vendor?

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2014-02-18 23:25:31

[permalink] [raw]

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

On Wed, 2014-02-19 at 00:35 +0100, Rafael J. Wysocki wrote:
> On Tuesday, February 18, 2014 11:15:08 PM Matthew Garrett wrote:
> > For example, if you load the ACPI power meter driver before you've
> > installed the ACPI IPMI driver you'll typically get failures (most
> > vendors implement it via IPMI).
>
> Well, any specific machine from any specific vendor?

The Dell R720 and HP DL385 both seem to have this configuration. I
believe some Ciscos do, too.
--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-19 00:31:11

[permalink] [raw]

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

On Tuesday, February 18, 2014 11:25:02 PM Matthew Garrett wrote:
> On Wed, 2014-02-19 at 00:35 +0100, Rafael J. Wysocki wrote:
> > On Tuesday, February 18, 2014 11:15:08 PM Matthew Garrett wrote:
> > > For example, if you load the ACPI power meter driver before you've
> > > installed the ACPI IPMI driver you'll typically get failures (most
> > > vendors implement it via IPMI).
> >
> > Well, any specific machine from any specific vendor?
>
> The Dell R720 and HP DL385 both seem to have this configuration. I
> believe some Ciscos do, too.

I've queued this up for 3.15 (with the above info added to the changelog).

Thanks!

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2014-02-19 00:53:47

by Corey Minyard

[permalink] [raw]

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

I just queued it up, too, but that's fine. This seems to be a good idea
with the way ACPI is going.

Acked-by: Corey Minyard <[email protected]>

On 02/18/2014 06:45 PM, Rafael J. Wysocki wrote:
> On Tuesday, February 18, 2014 11:25:02 PM Matthew Garrett wrote:
>> On Wed, 2014-02-19 at 00:35 +0100, Rafael J. Wysocki wrote:
>>> On Tuesday, February 18, 2014 11:15:08 PM Matthew Garrett wrote:
>>>> For example, if you load the ACPI power meter driver before you've
>>>> installed the ACPI IPMI driver you'll typically get failures (most
>>>> vendors implement it via IPMI).
>>> Well, any specific machine from any specific vendor?
>> The Dell R720 and HP DL385 both seem to have this configuration. I
>> believe some Ciscos do, too.
> I've queued this up for 3.15 (with the above info added to the changelog).
>
> Thanks!
>

2014-02-20 20:15:05

[permalink] [raw]

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

On Tue, Feb 18, 2014 at 11:28:29AM -0500, Matthew Garrett wrote:
> The ACPI IPMI driver implements IPMI operation region support for the ACPI
> core. Systems that declare ACPI operation regions may reference them at any
> time, including during kernel initialisation. These accesses will fail
> unless the ACPI IPMI driver is present, and undesirable system behaviour
> may result. Set the default to Y in order to encourage distributions and
> users to configure kernels to avoid awkward surprises.

No, please do not build the ipmi_si driver into the kernel.
Not all systems want, or need, the ipmi_si driver.

The distro that added this change created all sorts of support
problems. Problems include kipmi0 spinning at 100% of cpu
(creating a performance hit) and long boot delays (as the
kernel tries to talk to a BMC that will never respond).
It has been a big mess.

Nacked-by: Russ Anderson <[email protected]>

> Signed-off-by: Matthew Garrett <[email protected]>
> ---
> Actually, I guess we also want this on CONFIG_IPMI_HANDLER for least
> surprise
>
> drivers/acpi/Kconfig | 2 +-
> drivers/char/ipmi/Kconfig | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 4770de5..0e6aab9 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -162,7 +162,7 @@ config ACPI_PROCESSOR
> config ACPI_IPMI
> tristate "IPMI"
> depends on IPMI_SI
> - default n
> + default y
> help
> This driver enables the ACPI to access the BMC controller. And it
> uses the IPMI request/response message to communicate with BMC
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index 0baa8fa..eea8464 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -5,6 +5,7 @@
> menuconfig IPMI_HANDLER
> tristate 'IPMI top-level message handler'
> depends on HAS_IOMEM
> + default y if ACPI
> help
> This enables the central IPMI message handler, required for IPMI
> to work.
> @@ -45,6 +46,7 @@ config IPMI_DEVICE_INTERFACE
>
> config IPMI_SI
> tristate 'IPMI System Interface handler'
> + default y if ACPI
> help
> Provides a driver for System Interfaces (KCS, SMIC, BT).
> Currently, only KCS and SMIC are supported. If
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Russ Anderson, Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc [email protected]

2014-02-20 20:16:27

[permalink] [raw]

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

On Thu, 2014-02-20 at 14:14 -0600, Russ Anderson wrote:

> The distro that added this change created all sorts of support
> problems. Problems include kipmi0 spinning at 100% of cpu
> (creating a performance hit) and long boot delays (as the
> kernel tries to talk to a BMC that will never respond).
> It has been a big mess.

Why is the BMC not responding? Why is kipmi0 at 100%? Why are we not
fixing those bugs?

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-20 20:40:32

[permalink] [raw]

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

On Thu, Feb 20, 2014 at 08:16:22PM +0000, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:14 -0600, Russ Anderson wrote:
>
> > The distro that added this change created all sorts of support
> > problems. Problems include kipmi0 spinning at 100% of cpu
> > (creating a performance hit) and long boot delays (as the
> > kernel tries to talk to a BMC that will never respond).
> > It has been a big mess.
>
> Why is the BMC not responding? Why is kipmi0 at 100%? Why are we not
> fixing those bugs?

Why build a driver into the kernel? The reason ipmi_si is
a driver is so systems that want it can load it and systems
that do not want it do not have to load it. Plus you can
stop/start modules without rebooting. You can change module
parameters without rebooting.

There are any number of reasons why a BMC may not respond.
BMCs are notorious for being flakey, with different types
of BMCs that may or may not be reliable. You do not want
to make the kernel boot dependent on an unreliable component.

This is also a problem for systems with functional BMCs. Our
large cluster systems do all IPMI traffic (monitoring) through
a system controller back door. We do not want the kernel
doing IPMI commands on those systems. On those systems we
simply do not load the ipmi_si module. Building ipmi_si
into the kernel means adding kernel boot line options to
turn ipmi_si back off again.

--
Russ Anderson, Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc [email protected]

2014-02-20 20:46:21

[permalink] [raw]

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

On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:

> Why build a driver into the kernel?

Because it provides functionality that other drivers may need without
there being any mechanism to provide an explicit dependency. The same
reason we build the ACPI embedded controller driver into the kernel.

> The reason ipmi_si is
> a driver is so systems that want it can load it and systems
> that do not want it do not have to load it. Plus you can
> stop/start modules without rebooting. You can change module
> parameters without rebooting.

You can change module parameters without rebooting anyway - there's an
interface for it in sysfs.

> There are any number of reasons why a BMC may not respond.
> BMCs are notorious for being flakey, with different types
> of BMCs that may or may not be reliable. You do not want
> to make the kernel boot dependent on an unreliable component.

You appear to be saying "SGI ship hardware that doesn't work. We don't
know why it doesn't work and we're not interested in fixing it, so we'd
prefer the default kernel configuration to be broken". That doesn't seem
like an especially compelling argument.

> This is also a problem for systems with functional BMCs. Our
> large cluster systems do all IPMI traffic (monitoring) through
> a system controller back door. We do not want the kernel
> doing IPMI commands on those systems.

Why not?

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-20 20:59:06

[permalink] [raw]

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

On Thu, Feb 20, 2014 at 08:46:04PM +0000, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
>
> > This is also a problem for systems with functional BMCs. Our
> > large cluster systems do all IPMI traffic (monitoring) through
> > a system controller back door. We do not want the kernel
> > doing IPMI commands on those systems.
>
> Why not?

Because some customers want to use cpu cycles for their
application and let the ipmi monitoring go on through
the system controller network.

--
Russ Anderson, Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc [email protected]

2014-02-20 21:00:54

[permalink] [raw]

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

On Thu, 2014-02-20 at 14:59 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 08:46:04PM +0000, Matthew Garrett wrote:
> > On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
> >
> > > This is also a problem for systems with functional BMCs. Our
> > > large cluster systems do all IPMI traffic (monitoring) through
> > > a system controller back door. We do not want the kernel
> > > doing IPMI commands on those systems.
> >
> > Why not?
>
> Because some customers want to use cpu cycles for their
> application and let the ipmi monitoring go on through
> the system controller network.

Why is it generating any significant amount of CPU load? We're not
talking about a high-bandwidth interface here.

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-20 21:28:57

[permalink] [raw]

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

On Thu, Feb 20, 2014 at 09:00:48PM +0000, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:59 -0600, Russ Anderson wrote:
> > On Thu, Feb 20, 2014 at 08:46:04PM +0000, Matthew Garrett wrote:
> > > On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
> > >
> > > > This is also a problem for systems with functional BMCs. Our
> > > > large cluster systems do all IPMI traffic (monitoring) through
> > > > a system controller back door. We do not want the kernel
> > > > doing IPMI commands on those systems.
> > >
> > > Why not?
> >
> > Because some customers want to use cpu cycles for their
> > application and let the ipmi monitoring go on through
> > the system controller network.
>
> Why is it generating any significant amount of CPU load? We're not
> talking about a high-bandwidth interface here.

For some customers _any_ amount is significant, especially
on large clustered systems where the amount is multiplied
by tens or hundreds of thousands of nodes.

You many not think wasting their cpu cycles is important, but they do.

--
Russ Anderson, Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc [email protected]

2014-02-20 21:39:45

[permalink] [raw]

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

On Thu, 2014-02-20 at 15:28 -0600, Russ Anderson wrote:

> For some customers _any_ amount is significant, especially
> on large clustered systems where the amount is multiplied
> by tens or hundreds of thousands of nodes.
>
> You many not think wasting their cpu cycles is important, but they do.

Then they should be running locally built kernels in order to ensure
that there's no problematic code running at all. Distribution kernels
will always contain code that some customers won't be interested in, and
some of that code will end up executing.

If you have specific bug reports, that would be helpful. But you're not
describing actual failure conditions or showing any willingness to
figure out what the underlying problem is. The BMC is generating
messages and handing them off to the host. If that's taking more than a
tiny number of cycles, why? Once we know the answer to that, we can
actually put some effort into fixing it.

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-20 21:49:16

[permalink] [raw]

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

On Thu, Feb 20, 2014 at 08:46:04PM +0000, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
>
> > There are any number of reasons why a BMC may not respond.
> > BMCs are notorious for being flakey, with different types
> > of BMCs that may or may not be reliable. You do not want
> > to make the kernel boot dependent on an unreliable component.
>
> You appear to be saying "SGI ship hardware that doesn't work. We don't
> know why it doesn't work and we're not interested in fixing it, so we'd
> prefer the default kernel configuration to be broken". That doesn't seem
> like an especially compelling argument.

You appear to not understand the mess this causes. We have hit
this on 3rd party boards, not just SGI built hardware (much
of which is industry standard components). It causes problems
both for flakey hardware _and_ functioning hardware.

--
Russ Anderson, Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc [email protected]

2014-02-20 21:51:31

[permalink] [raw]

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

On Thu, 2014-02-20 at 15:49 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 08:46:04PM +0000, Matthew Garrett wrote:
> > You appear to be saying "SGI ship hardware that doesn't work. We don't
> > know why it doesn't work and we're not interested in fixing it, so we'd
> > prefer the default kernel configuration to be broken". That doesn't seem
> > like an especially compelling argument.
>
> You appear to not understand the mess this causes. We have hit
> this on 3rd party boards, not just SGI built hardware (much
> of which is industry standard components). It causes problems
> both for flakey hardware _and_ functioning hardware.

Bug numbers?

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-20 22:07:01

[permalink] [raw]

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

On Thu, Feb 20, 2014 at 09:39:23PM +0000, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 15:28 -0600, Russ Anderson wrote:
>
> > For some customers _any_ amount is significant, especially
> > on large clustered systems where the amount is multiplied
> > by tens or hundreds of thousands of nodes.
> >
> > You many not think wasting their cpu cycles is important, but they do.
>
> Then they should be running locally built kernels in order to ensure

Why don't YOU run a locally built kernel?

> that there's no problematic code running at all. Distribution kernels
> will always contain code that some customers won't be interested in, and
> some of that code will end up executing.

We support multiple distros so if one does something the customer
does not like/need we can steer them to other distros.

> If you have specific bug reports, that would be helpful. But you're not
> describing actual failure conditions or showing any willingness to
> figure out what the underlying problem is.

You can't fix your problem without creating problems for
others to fix?

--
Russ Anderson, Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc [email protected]

2014-02-20 22:26:50

[permalink] [raw]

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

On Thu, 2014-02-20 at 16:06 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 09:39:23PM +0000, Matthew Garrett wrote:
> > On Thu, 2014-02-20 at 15:28 -0600, Russ Anderson wrote:
> >
> > > For some customers _any_ amount is significant, especially
> > > on large clustered systems where the amount is multiplied
> > > by tens or hundreds of thousands of nodes.
> > >
> > > You many not think wasting their cpu cycles is important, but they do.
> >
> > Then they should be running locally built kernels in order to ensure
>
> Why don't YOU run a locally built kernel?

Because I'm trying to ensure that the default behaviour of the kernel is
to *work*. Defaulting to having IPMI be modular means that the default
behaviour of the kernel, as far as the ACPI spec goes, is to be broken.

>> If you have specific bug reports, that would be helpful. But you're not
> > describing actual failure conditions or showing any willingness to
> > figure out what the underlying problem is.
>
> You can't fix your problem without creating problems for
> others to fix?

ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
that the kernel will spend a significant amount of time (potentially
until a user manually loads a driver) failing to implement part of the
IPMI specification. That's a problem, and the correct fix is to ensure
that the kernel always implements IPMI support.

Now, you've described some other problems. I don't disagree that those
are problems. The correct thing for us to do with those problems is to
fix them, not to simply change the kernel defaults such that it's
possible for users to choose between two differently broken states. I'm
absolutely willing to help, as long as you're willing to put some
reasonable amount of effort into describing them.

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-20 22:45:33

[permalink] [raw]

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

On Thu, Feb 20, 2014 at 10:26:45PM +0000, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 16:06 -0600, Russ Anderson wrote:
> > On Thu, Feb 20, 2014 at 09:39:23PM +0000, Matthew Garrett wrote:
> > > On Thu, 2014-02-20 at 15:28 -0600, Russ Anderson wrote:
> > >
> > > > For some customers _any_ amount is significant, especially
> > > > on large clustered systems where the amount is multiplied
> > > > by tens or hundreds of thousands of nodes.
> > > >
> > > > You many not think wasting their cpu cycles is important, but they do.
> > >
> > > Then they should be running locally built kernels in order to ensure
> >
> > Why don't YOU run a locally built kernel?
>
> Because I'm trying to ensure that the default behaviour of the kernel is
> to *work*. Defaulting to having IPMI be modular means that the default
> behaviour of the kernel, as far as the ACPI spec goes, is to be broken.

The ACPI spec requires IPMI functionality before a module loads at
boot time? And the kernel is *broken* if it does not support ACIP IPMI
functionality before module load time? Really?

> >> If you have specific bug reports, that would be helpful. But you're not
> > > describing actual failure conditions or showing any willingness to
> > > figure out what the underlying problem is.
> >
> > You can't fix your problem without creating problems for
> > others to fix?
>
> ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> that the kernel will spend a significant amount of time (potentially
> until a user manually loads a driver) failing to implement part of the
> IPMI specification. That's a problem, and the correct fix is to ensure
> that the kernel always implements IPMI support.

The ACPI spec says ipmi_si cannot be a driver? Really?
What is the real problem you are trying to solve?

> Now, you've described some other problems. I don't disagree that those
> are problems. The correct thing for us to do with those problems is to
> fix them, not to simply change the kernel defaults such that it's
> possible for users to choose between two differently broken states. I'm
> absolutely willing to help, as long as you're willing to put some
> reasonable amount of effort into describing them.

How about ACPI IPMI functionality starts when the ipmi_si
module loads at boot time.

--
Russ Anderson, Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc [email protected]

2014-02-20 23:01:32

[permalink] [raw]

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

2014-02-20 23:09:49

[permalink] [raw]

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

On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 10:26:45PM +0000, Matthew Garrett wrote:

> > Because I'm trying to ensure that the default behaviour of the kernel is
> > to *work*. Defaulting to having IPMI be modular means that the default
> > behaviour of the kernel, as far as the ACPI spec goes, is to be broken.
>
> The ACPI spec requires IPMI functionality before a module loads at
> boot time? And the kernel is *broken* if it does not support ACIP IPMI
> functionality before module load time? Really?

There's no mechanism to ensure that IPMI support will be loaded before
ACPI calls attempt to access IPMI operation regions. Really.

> > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> > that the kernel will spend a significant amount of time (potentially
> > until a user manually loads a driver) failing to implement part of the
> > IPMI specification. That's a problem, and the correct fix is to ensure
> > that the kernel always implements IPMI support.
>
> The ACPI spec says ipmi_si cannot be a driver? Really?
> What is the real problem you are trying to solve?

The most straightforward case is that of an ACPI power meter. Several
vendors implement this with an IPMI operation region. Calling any of the
power meter functions will trigger access to that IPMI operation region,
which will fail. This may result in driver initialisation failing. There
is no express dependency between the power meter driver and ipmi_si,
because the spec envisages IPMI support as basic kernel functionality.
It's meant to be there before you start loading any other drivers.

> > Now, you've described some other problems. I don't disagree that those
> > are problems. The correct thing for us to do with those problems is to
> > fix them, not to simply change the kernel defaults such that it's
> > possible for users to choose between two differently broken states. I'm
> > absolutely willing to help, as long as you're willing to put some
> > reasonable amount of effort into describing them.
>
> How about ACPI IPMI functionality starts when the ipmi_si
> module loads at boot time.

I've repeatedly asked for you to provide detailed descriptions of the
problems you've seen because I have a genuine interest in fixing them.
If you're just going to childishly refuse then this discussion is
pointless.

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-20 23:55:31

[permalink] [raw]

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

On Thursday, February 20, 2014 02:14:58 PM Russ Anderson wrote:
> On Tue, Feb 18, 2014 at 11:28:29AM -0500, Matthew Garrett wrote:
> > The ACPI IPMI driver implements IPMI operation region support for the ACPI
> > core. Systems that declare ACPI operation regions may reference them at any
> > time, including during kernel initialisation. These accesses will fail
> > unless the ACPI IPMI driver is present, and undesirable system behaviour
> > may result. Set the default to Y in order to encourage distributions and
> > users to configure kernels to avoid awkward surprises.
>
> No, please do not build the ipmi_si driver into the kernel.
> Not all systems want, or need, the ipmi_si driver.
>
> The distro that added this change created all sorts of support
> problems.

Which distro was that?

Do you have any pointers to specific bug reports related to that?

Rafael

> Problems include kipmi0 spinning at 100% of cpu
> (creating a performance hit) and long boot delays (as the
> kernel tries to talk to a BMC that will never respond).
> It has been a big mess.
>
> Nacked-by: Russ Anderson <[email protected]>
>
>
> > Signed-off-by: Matthew Garrett <[email protected]>
> > ---
> > Actually, I guess we also want this on CONFIG_IPMI_HANDLER for least
> > surprise
> >
> > drivers/acpi/Kconfig | 2 +-
> > drivers/char/ipmi/Kconfig | 2 ++
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 4770de5..0e6aab9 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -162,7 +162,7 @@ config ACPI_PROCESSOR
> > config ACPI_IPMI
> > tristate "IPMI"
> > depends on IPMI_SI
> > - default n
> > + default y
> > help
> > This driver enables the ACPI to access the BMC controller. And it
> > uses the IPMI request/response message to communicate with BMC
> > diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> > index 0baa8fa..eea8464 100644
> > --- a/drivers/char/ipmi/Kconfig
> > +++ b/drivers/char/ipmi/Kconfig
> > @@ -5,6 +5,7 @@
> > menuconfig IPMI_HANDLER
> > tristate 'IPMI top-level message handler'
> > depends on HAS_IOMEM
> > + default y if ACPI
> > help
> > This enables the central IPMI message handler, required for IPMI
> > to work.
> > @@ -45,6 +46,7 @@ config IPMI_DEVICE_INTERFACE
> >
> > config IPMI_SI
> > tristate 'IPMI System Interface handler'
> > + default y if ACPI
> > help
> > Provides a driver for System Interfaces (KCS, SMIC, BT).
> > Currently, only KCS and SMIC are supported. If
>
>

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2014-02-20 23:59:10

[permalink] [raw]

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

On Thu, Feb 20, 2014 at 11:09:42PM +0000, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> > On Thu, Feb 20, 2014 at 10:26:45PM +0000, Matthew Garrett wrote:
>
> > > Because I'm trying to ensure that the default behaviour of the kernel is
> > > to *work*. Defaulting to having IPMI be modular means that the default
> > > behaviour of the kernel, as far as the ACPI spec goes, is to be broken.
> >
> > The ACPI spec requires IPMI functionality before a module loads at
> > boot time? And the kernel is *broken* if it does not support ACIP IPMI
> > functionality before module load time? Really?
>
> There's no mechanism to ensure that IPMI support will be loaded before
> ACPI calls attempt to access IPMI operation regions. Really.

And no mechanism can be added to ensure that ACPI call are
not attempted before IPMI is initialized? A flag or lock
or exported symbol indicating IPMI support is ready.

> > > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> > > that the kernel will spend a significant amount of time (potentially
> > > until a user manually loads a driver) failing to implement part of the
> > > IPMI specification. That's a problem, and the correct fix is to ensure
> > > that the kernel always implements IPMI support.
> >
> > The ACPI spec says ipmi_si cannot be a driver? Really?
> > What is the real problem you are trying to solve?
>
> The most straightforward case is that of an ACPI power meter.

So it is just a matter of making sure ipmi_si modules loads before
the ACPI power meter module loads, right? module dependency issue.

> Several
> vendors implement this with an IPMI operation region. Calling any of the
> power meter functions will trigger access to that IPMI operation region,
> which will fail. This may result in driver initialisation failing. There
> is no express dependency between the power meter driver and ipmi_si,
> because the spec envisages IPMI support as basic kernel functionality.
> It's meant to be there before you start loading any other drivers.

The spec "envisages"? I get there is a dependency, that IPMI driver
needs to be loaded before ACIP power meter. This isn't the first
case of a driver being dependent on another driver. That doesn't
mean IPMI driver must be built into the kernel.

> > > Now, you've described some other problems. I don't disagree that those
> > > are problems. The correct thing for us to do with those problems is to
> > > fix them, not to simply change the kernel defaults such that it's
> > > possible for users to choose between two differently broken states. I'm
> > > absolutely willing to help, as long as you're willing to put some
> > > reasonable amount of effort into describing them.
> >
> > How about ACPI IPMI functionality starts when the ipmi_si
> > module loads at boot time.
>
> I've repeatedly asked for you to provide detailed descriptions of the
> problems you've seen because I have a genuine interest in fixing them.
> If you're just going to childishly refuse then this discussion is
> pointless.

The distro cases I would point you at are marked private.
And you do not have access to our internal support system.
A simple google search for "kipmi0" shows a lot of reports of
high cpu utilization.

And I'm old enough to appreciate being called childishly. :-)

--
Russ Anderson, Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc [email protected]

2014-02-21 00:13:31

[permalink] [raw]

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

On Thu, 2014-02-20 at 17:59 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 11:09:42PM +0000, Matthew Garrett wrote:
> > On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> > >
> > > The ACPI spec requires IPMI functionality before a module loads at
> > > boot time? And the kernel is *broken* if it does not support ACIP IPMI
> > > functionality before module load time? Really?
> >
> > There's no mechanism to ensure that IPMI support will be loaded before
> > ACPI calls attempt to access IPMI operation regions. Really.
>
> And no mechanism can be added to ensure that ACPI call are
> not attempted before IPMI is initialized? A flag or lock
> or exported symbol indicating IPMI support is ready.

ACPI functions are a black box to drivers. You make an ACPI call, the
AML code does something. We could block there, but what's the driver
supposed to do at that point? The core could call out to a module
loader, but if the driver is built in and IPMI isn't then you'll end up
with a 60 second pause in boot and a driver that doesn't work.

> > > > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> > > > that the kernel will spend a significant amount of time (potentially
> > > > until a user manually loads a driver) failing to implement part of the
> > > > IPMI specification. That's a problem, and the correct fix is to ensure
> > > > that the kernel always implements IPMI support.
> > >
> > > The ACPI spec says ipmi_si cannot be a driver? Really?
> > > What is the real problem you are trying to solve?
> >
> > The most straightforward case is that of an ACPI power meter.
>
> So it is just a matter of making sure ipmi_si modules loads before
> the ACPI power meter module loads, right? module dependency issue.

No, because the power meter driver has no way of knowing that a vendor
has implemented this interface via IPMI. *Any* ACPI entry point could
theoretically reference IPMI code, even the _INI method that's called
during ACPI core init. If it does, and if you don't have built-in ACPI
support, you'd fail ACPI initialisation and things would go downhill
from there.

(I don't think failure of this magnitude is actually *likely*, but it
would be spec-compliant)

> > I've repeatedly asked for you to provide detailed descriptions of the
> > problems you've seen because I have a genuine interest in fixing them.
> > If you're just going to childishly refuse then this discussion is
> > pointless.
>
> The distro cases I would point you at are marked private.
> And you do not have access to our internal support system.
> A simple google search for "kipmi0" shows a lot of reports of
> high cpu utilization.

And nobody seems to have put any effort into figuring out what the
underlying cause is. Is it spinning because there are messages? If so,
is it because the BMC would really like some kind of response to those
messages? Is it spinning because the BMC is wedged? If so, can we detect
that case, flag it as broken and cleanly disengage?

We're running systems from a wide range of vendors (including basically
all the Tier 1 server manufacturers, plus some whitebox), use IPMI
functionality heavily and genuinely do not see the described problems. I
don't think there's evidence of widespread breakage, and where it does
exist we should treat it as we would any other bug - diagnose the
underlying problem and fix it.

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-21 02:17:46

by Zheng, Lv

[permalink] [raw]

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

Hi,

Sorry for interrupting you.
I have some information that may be helpful for your discussion.
Please find them in the inlined replies.
Well, I don't want to join the fight, just for your informations. :-)

> From: [email protected] [mailto:[email protected]] On Behalf Of Russ Anderson
> Sent: Friday, February 21, 2014 7:59 AM
>
> On Thu, Feb 20, 2014 at 11:09:42PM +0000, Matthew Garrett wrote:
> > On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> > > On Thu, Feb 20, 2014 at 10:26:45PM +0000, Matthew Garrett wrote:
> >
> > > > Because I'm trying to ensure that the default behaviour of the kernel is
> > > > to *work*. Defaulting to having IPMI be modular means that the default
> > > > behaviour of the kernel, as far as the ACPI spec goes, is to be broken.
> > >
> > > The ACPI spec requires IPMI functionality before a module loads at
> > > boot time? And the kernel is *broken* if it does not support ACIP IPMI
> > > functionality before module load time? Really?
> >
> > There's no mechanism to ensure that IPMI support will be loaded before
> > ACPI calls attempt to access IPMI operation regions. Really.
>
> And no mechanism can be added to ensure that ACPI call are
> not attempted before IPMI is initialized? A flag or lock
> or exported symbol indicating IPMI support is ready.

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.

This solution may change the meaning of ACPI spec defined _REG.
So we may need a better solution.

But after merging this patch, it is safe to unload acpi_ipmi at users' wishes.
Without solutions to solve region handler uninstallation races, it is not safe to unload acpi_ipmi module.
You can see crashes in the description of this patch.

The ipmi_si module is using a different way to unload itself which has not been tested by me.
You can find it in Documentation/IPMI.txt by searching "hot and remove of interfaces" in this file.

>
> > > > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> > > > that the kernel will spend a significant amount of time (potentially
> > > > until a user manually loads a driver) failing to implement part of the
> > > > IPMI specification. That's a problem, and the correct fix is to ensure
> > > > that the kernel always implements IPMI support.
> > >
> > > The ACPI spec says ipmi_si cannot be a driver? Really?
> > > What is the real problem you are trying to solve?
> >
> > The most straightforward case is that of an ACPI power meter.
>
> So it is just a matter of making sure ipmi_si modules loads before
> the ACPI power meter module loads, right? module dependency issue.

I agree.
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.

I didn't work further on this issue when solving the reported bug:
https://bugzilla.kernel.org/show_bug.cgi?id=46741

>
> > Several
> > vendors implement this with an IPMI operation region. Calling any of the
> > power meter functions will trigger access to that IPMI operation region,
> > which will fail. This may result in driver initialisation failing. There
> > is no express dependency between the power meter driver and ipmi_si,
> > because the spec envisages IPMI support as basic kernel functionality.
> > It's meant to be there before you start loading any other drivers.
>
> The spec "envisages"? I get there is a dependency, that IPMI driver
> needs to be loaded before ACIP power meter. This isn't the first
> case of a driver being dependent on another driver. That doesn't
> mean IPMI driver must be built into the kernel.
>
> > > > Now, you've described some other problems. I don't disagree that those
> > > > are problems. The correct thing for us to do with those problems is to
> > > > fix them, not to simply change the kernel defaults such that it's
> > > > possible for users to choose between two differently broken states. I'm
> > > > absolutely willing to help, as long as you're willing to put some
> > > > reasonable amount of effort into describing them.
> > >
> > > How about ACPI IPMI functionality starts when the ipmi_si
> > > module loads at boot time.

Actually, I have a solution implemented this.
You can find it at:
https://bugzilla.kernel.org/attachment.cgi?id=112611
It is the acpi-ipmi14.patch file.

The patch will hand the maintenance-ship of acpi_ipmi to IPMI community.
I'm not sure it is proper to merge it by Linux upstreams.

Thanks and best regards
-Lv

> >
> > I've repeatedly asked for you to provide detailed descriptions of the
> > problems you've seen because I have a genuine interest in fixing them.
> > If you're just going to childishly refuse then this discussion is
> > pointless.
>
> The distro cases I would point you at are marked private.
> And you do not have access to our internal support system.
> A simple google search for "kipmi0" shows a lot of reports of
> high cpu utilization.
>
> And I'm old enough to appreciate being called childishly. :-)
>
> --
> Russ Anderson, Kernel and Performance Software Team Manager
> SGI - Silicon Graphics Inc [email protected]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-02-21 13:37:24

by Corey Minyard

[permalink] [raw]

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

On 02/20/2014 03:00 PM, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:59 -0600, Russ Anderson wrote:
>> On Thu, Feb 20, 2014 at 08:46:04PM +0000, Matthew Garrett wrote:
>>> On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
>>>
>>>> This is also a problem for systems with functional BMCs. Our
>>>> large cluster systems do all IPMI traffic (monitoring) through
>>>> a system controller back door. We do not want the kernel
>>>> doing IPMI commands on those systems.
>>> Why not?
>> Because some customers want to use cpu cycles for their
>> application and let the ipmi monitoring go on through
>> the system controller network.
> Why is it generating any significant amount of CPU load? We're not
> talking about a high-bandwidth interface here.
>

I believe that problem is fixed now, at least the one with kipmid using
lots of CPU.

However, the basic problem is that hardware vendors produce hardware
that sucks and then expect software to fix all the problems. Most IPMI
interfaces don't have interrupts, so they have to be polled. Then they
add important interfaces on top of it like firmware upgrade and ACPI and
expect it to perform well. If vendors would just have an interrupt for
IPMI, 99% of these problems would go away.

If there are still issues with lots of CPU being used, then the problem
is most likely non-compliant or just broken hardware. I've seen enough
of that.

One thing we can do is remove the default interface probing for IPMI.
Even though the spec has it, all modern hardware should have it
specified in ACPI or device tree. That should fix all the slow boot
problems, at least. If a user wants to add a default interface, they
can use the interface to dynamically add it after boot time.

-corey

2014-02-21 15:51:59

[permalink] [raw]

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

On Fri, 2014-02-21 at 07:37 -0600, Corey Minyard wrote:

> However, the basic problem is that hardware vendors produce hardware
> that sucks and then expect software to fix all the problems. Most IPMI
> interfaces don't have interrupts, so they have to be polled. Then they
> add important interfaces on top of it like firmware upgrade and ACPI and
> expect it to perform well. If vendors would just have an interrupt for
> IPMI, 99% of these problems would go away.

Not going to disagree. The impact on power consumption is also pretty
awful. I should re-read the spec to figure out whether we can
legitimately get away with not doing that.

> One thing we can do is remove the default interface probing for IPMI.
> Even though the spec has it, all modern hardware should have it
> specified in ACPI or device tree. That should fix all the slow boot
> problems, at least. If a user wants to add a default interface, they
> can use the interface to dynamically add it after boot time.

Something like this (untested)?

diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index eea8464..5126230 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -52,6 +52,16 @@ config IPMI_SI
Currently, only KCS and SMIC are supported. If
you are using IPMI, you should probably say "y" here.

+config IPMI_PROBE_DEFAULTS
+ bool 'Probe for all possible IPMI interfaces by default'
+ help
+ Modern systems will usually expose IPMI interfaces via a discoverable
+ firmware mechanism such as ACPI or DMI. Older systems do not, and so
+ the driver is forced to probe hardware manually. This may cause boot
+ delays. Say "n" here to disable this manual probing. IPMI will then
+ only be available on older systems if the "ipmi_si_intf.trydefaults=1"
+ boot argument is passed.
+
config IPMI_WATCHDOG
tristate 'IPMI Watchdog Timer'
help
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 03f4189..82c7d56 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1230,7 +1230,11 @@ static bool si_tryplatform = 1;
#ifdef CONFIG_PCI
static bool si_trypci = 1;
#endif
+#ifdef CONFIG_IPMI_PROBE_DEFAULTS
static bool si_trydefaults = 1;
+#else
+static bool si_trydefaults;
+#endif
static char *si_type[SI_MAX_PARMS];
#define MAX_SI_TYPE_STR 30
static char si_type_str[MAX_SI_TYPE_STR];

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-21 16:13:00

[permalink] [raw]

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

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.

> 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.

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.

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-02-21 16:33:07

[permalink] [raw]

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

On Fri, Feb 21, 2014 at 02:17:12AM +0000, Zheng, Lv wrote:
> Hi,
>
> Sorry for interrupting you.
> I have some information that may be helpful for your discussion.
> Please find them in the inlined replies.
> Well, I don't want to join the fight, just for your informations. :-)

I don't want to join the fight, either.

I have not looked at your code changes but the description
looks like the right direction.

> > From: [email protected] [mailto:[email protected]] On Behalf Of Russ Anderson
> > Sent: Friday, February 21, 2014 7:59 AM
> >
> > On Thu, Feb 20, 2014 at 11:09:42PM +0000, Matthew Garrett wrote:
> > > On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> > > > On Thu, Feb 20, 2014 at 10:26:45PM +0000, Matthew Garrett wrote:
> > >
> > > > > Because I'm trying to ensure that the default behaviour of the kernel is
> > > > > to *work*. Defaulting to having IPMI be modular means that the default
> > > > > behaviour of the kernel, as far as the ACPI spec goes, is to be broken.
> > > >
> > > > The ACPI spec requires IPMI functionality before a module loads at
> > > > boot time? And the kernel is *broken* if it does not support ACIP IPMI
> > > > functionality before module load time? Really?
> > >
> > > There's no mechanism to ensure that IPMI support will be loaded before
> > > ACPI calls attempt to access IPMI operation regions. Really.
> >
> > And no mechanism can be added to ensure that ACPI call are
> > not attempted before IPMI is initialized? A flag or lock
> > or exported symbol indicating IPMI support is ready.
>
> 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.
>
> This solution may change the meaning of ACPI spec defined _REG.
> So we may need a better solution.
>
> But after merging this patch, it is safe to unload acpi_ipmi at users' wishes.
> Without solutions to solve region handler uninstallation races, it is not safe to unload acpi_ipmi module.
> You can see crashes in the description of this patch.
>
> The ipmi_si module is using a different way to unload itself which has not been tested by me.
> You can find it in Documentation/IPMI.txt by searching "hot and remove of interfaces" in this file.
>
> >
> > > > > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> > > > > that the kernel will spend a significant amount of time (potentially
> > > > > until a user manually loads a driver) failing to implement part of the
> > > > > IPMI specification. That's a problem, and the correct fix is to ensure
> > > > > that the kernel always implements IPMI support.
> > > >
> > > > The ACPI spec says ipmi_si cannot be a driver? Really?
> > > > What is the real problem you are trying to solve?
> > >
> > > The most straightforward case is that of an ACPI power meter.
> >
> > So it is just a matter of making sure ipmi_si modules loads before
> > the ACPI power meter module loads, right? module dependency issue.
>
> I agree.
> 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.
>
> I didn't work further on this issue when solving the reported bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=46741
>
> >
> > > Several
> > > vendors implement this with an IPMI operation region. Calling any of the
> > > power meter functions will trigger access to that IPMI operation region,
> > > which will fail. This may result in driver initialisation failing. There
> > > is no express dependency between the power meter driver and ipmi_si,
> > > because the spec envisages IPMI support as basic kernel functionality.
> > > It's meant to be there before you start loading any other drivers.
> >
> > The spec "envisages"? I get there is a dependency, that IPMI driver
> > needs to be loaded before ACIP power meter. This isn't the first
> > case of a driver being dependent on another driver. That doesn't
> > mean IPMI driver must be built into the kernel.
> >
> > > > > Now, you've described some other problems. I don't disagree that those
> > > > > are problems. The correct thing for us to do with those problems is to
> > > > > fix them, not to simply change the kernel defaults such that it's
> > > > > possible for users to choose between two differently broken states. I'm
> > > > > absolutely willing to help, as long as you're willing to put some
> > > > > reasonable amount of effort into describing them.
> > > >
> > > > How about ACPI IPMI functionality starts when the ipmi_si
> > > > module loads at boot time.
>
> Actually, I have a solution implemented this.
> You can find it at:
> https://bugzilla.kernel.org/attachment.cgi?id=112611
> It is the acpi-ipmi14.patch file.
>
> The patch will hand the maintenance-ship of acpi_ipmi to IPMI community.
> I'm not sure it is proper to merge it by Linux upstreams.
>
> Thanks and best regards
> -Lv
>
> > >
> > > I've repeatedly asked for you to provide detailed descriptions of the
> > > problems you've seen because I have a genuine interest in fixing them.
> > > If you're just going to childishly refuse then this discussion is
> > > pointless.
> >
> > The distro cases I would point you at are marked private.
> > And you do not have access to our internal support system.
> > A simple google search for "kipmi0" shows a lot of reports of
> > high cpu utilization.
> >
> > And I'm old enough to appreciate being called childishly. :-)
> >
> > --
> > Russ Anderson, Kernel and Performance Software Team Manager
> > SGI - Silicon Graphics Inc [email protected]
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Russ Anderson, Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc [email protected]

2014-02-21 16:54:05