diff mbox

[V2] Change ACPI IPMI support to "default y"

Message ID 1392740909-2079-1-git-send-email-matthew.garrett@nebula.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Matthew Garrett Feb. 18, 2014, 4:28 p.m. UTC
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.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
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(-)

Comments

Matthew Garrett Feb. 18, 2014, 11:15 p.m. UTC | #1
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 <matthew.garrett@nebula.com>
Matthew Garrett Feb. 18, 2014, 11:25 p.m. UTC | #2
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 <matthew.garrett@nebula.com>
Rafael J. Wysocki Feb. 18, 2014, 11:26 p.m. UTC | #3
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

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki Feb. 18, 2014, 11:35 p.m. UTC | #4
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?
Rafael J. Wysocki Feb. 19, 2014, 12:45 a.m. UTC | #5
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!
Corey Minyard Feb. 19, 2014, 12:53 a.m. UTC | #6
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 <cminyard@mvista.com>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russ Anderson Feb. 20, 2014, 8:14 p.m. UTC | #7
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 <rja@sgi.com>


> Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
> ---
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
Matthew Garrett Feb. 20, 2014, 8:16 p.m. UTC | #8
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 <matthew.garrett@nebula.com>
Russ Anderson Feb. 20, 2014, 8:40 p.m. UTC | #9
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.
Matthew Garrett Feb. 20, 2014, 8:46 p.m. UTC | #10
T24gVGh1LCAyMDE0LTAyLTIwIGF0IDE0OjQwIC0wNjAwLCBSdXNzIEFuZGVyc29uIHdyb3RlOg0K
DQo+IFdoeSBidWlsZCBhIGRyaXZlciBpbnRvIHRoZSBrZXJuZWw/DQoNCkJlY2F1c2UgaXQgcHJv
dmlkZXMgZnVuY3Rpb25hbGl0eSB0aGF0IG90aGVyIGRyaXZlcnMgbWF5IG5lZWQgd2l0aG91dA0K
dGhlcmUgYmVpbmcgYW55IG1lY2hhbmlzbSB0byBwcm92aWRlIGFuIGV4cGxpY2l0IGRlcGVuZGVu
Y3kuIFRoZSBzYW1lDQpyZWFzb24gd2UgYnVpbGQgdGhlIEFDUEkgZW1iZWRkZWQgY29udHJvbGxl
ciBkcml2ZXIgaW50byB0aGUga2VybmVsLg0KDQo+IFRoZSByZWFzb24gaXBtaV9zaSBpcyANCj4g
YSBkcml2ZXIgaXMgc28gc3lzdGVtcyB0aGF0IHdhbnQgaXQgY2FuIGxvYWQgaXQgYW5kIHN5c3Rl
bXMNCj4gdGhhdCBkbyBub3Qgd2FudCBpdCBkbyBub3QgaGF2ZSB0byBsb2FkIGl0LiAgUGx1cyB5
b3UgY2FuDQo+IHN0b3Avc3RhcnQgbW9kdWxlcyB3aXRob3V0IHJlYm9vdGluZy4gIFlvdSBjYW4g
Y2hhbmdlIG1vZHVsZQ0KPiBwYXJhbWV0ZXJzIHdpdGhvdXQgcmVib290aW5nLg0KDQpZb3UgY2Fu
IGNoYW5nZSBtb2R1bGUgcGFyYW1ldGVycyB3aXRob3V0IHJlYm9vdGluZyBhbnl3YXkgLSB0aGVy
ZSdzIGFuDQppbnRlcmZhY2UgZm9yIGl0IGluIHN5c2ZzLg0KDQo+IFRoZXJlIGFyZSBhbnkgbnVt
YmVyIG9mIHJlYXNvbnMgd2h5IGEgQk1DIG1heSBub3QgcmVzcG9uZC4NCj4gQk1DcyBhcmUgbm90
b3Jpb3VzIGZvciBiZWluZyBmbGFrZXksIHdpdGggZGlmZmVyZW50IHR5cGVzDQo+IG9mIEJNQ3Mg
dGhhdCBtYXkgb3IgbWF5IG5vdCBiZSByZWxpYWJsZS4gIFlvdSBkbyBub3Qgd2FudA0KPiB0byBt
YWtlIHRoZSBrZXJuZWwgYm9vdCBkZXBlbmRlbnQgb24gYW4gdW5yZWxpYWJsZSBjb21wb25lbnQu
DQoNCllvdSBhcHBlYXIgdG8gYmUgc2F5aW5nICJTR0kgc2hpcCBoYXJkd2FyZSB0aGF0IGRvZXNu
J3Qgd29yay4gV2UgZG9uJ3QNCmtub3cgd2h5IGl0IGRvZXNuJ3Qgd29yayBhbmQgd2UncmUgbm90
IGludGVyZXN0ZWQgaW4gZml4aW5nIGl0LCBzbyB3ZSdkDQpwcmVmZXIgdGhlIGRlZmF1bHQga2Vy
bmVsIGNvbmZpZ3VyYXRpb24gdG8gYmUgYnJva2VuIi4gVGhhdCBkb2Vzbid0IHNlZW0NCmxpa2Ug
YW4gZXNwZWNpYWxseSBjb21wZWxsaW5nIGFyZ3VtZW50Lg0KDQo+IFRoaXMgaXMgYWxzbyBhIHBy
b2JsZW0gZm9yIHN5c3RlbXMgd2l0aCBmdW5jdGlvbmFsIEJNQ3MuICBPdXINCj4gbGFyZ2UgY2x1
c3RlciBzeXN0ZW1zIGRvIGFsbCBJUE1JIHRyYWZmaWMgKG1vbml0b3JpbmcpIHRocm91Z2gNCj4g
YSBzeXN0ZW0gY29udHJvbGxlciBiYWNrIGRvb3IuICBXZSBkbyBub3Qgd2FudCB0aGUga2VybmVs
DQo+IGRvaW5nIElQTUkgY29tbWFuZHMgb24gdGhvc2Ugc3lzdGVtcy4NCg0KV2h5IG5vdD8NCg0K
LS0gDQpNYXR0aGV3IEdhcnJldHQgPG1hdHRoZXcuZ2FycmV0dEBuZWJ1bGEuY29tPg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russ Anderson Feb. 20, 2014, 8:59 p.m. UTC | #11
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.
Matthew Garrett Feb. 20, 2014, 9 p.m. UTC | #12
T24gVGh1LCAyMDE0LTAyLTIwIGF0IDE0OjU5IC0wNjAwLCBSdXNzIEFuZGVyc29uIHdyb3RlOg0K
PiBPbiBUaHUsIEZlYiAyMCwgMjAxNCBhdCAwODo0NjowNFBNICswMDAwLCBNYXR0aGV3IEdhcnJl
dHQgd3JvdGU6DQo+ID4gT24gVGh1LCAyMDE0LTAyLTIwIGF0IDE0OjQwIC0wNjAwLCBSdXNzIEFu
ZGVyc29uIHdyb3RlOg0KPiA+IA0KPiA+ID4gVGhpcyBpcyBhbHNvIGEgcHJvYmxlbSBmb3Igc3lz
dGVtcyB3aXRoIGZ1bmN0aW9uYWwgQk1Dcy4gIE91cg0KPiA+ID4gbGFyZ2UgY2x1c3RlciBzeXN0
ZW1zIGRvIGFsbCBJUE1JIHRyYWZmaWMgKG1vbml0b3JpbmcpIHRocm91Z2gNCj4gPiA+IGEgc3lz
dGVtIGNvbnRyb2xsZXIgYmFjayBkb29yLiAgV2UgZG8gbm90IHdhbnQgdGhlIGtlcm5lbA0KPiA+
ID4gZG9pbmcgSVBNSSBjb21tYW5kcyBvbiB0aG9zZSBzeXN0ZW1zLg0KPiA+IA0KPiA+IFdoeSBu
b3Q/DQo+IA0KPiBCZWNhdXNlIHNvbWUgY3VzdG9tZXJzIHdhbnQgdG8gdXNlIGNwdSBjeWNsZXMg
Zm9yIHRoZWlyDQo+IGFwcGxpY2F0aW9uIGFuZCBsZXQgdGhlIGlwbWkgbW9uaXRvcmluZyBnbyBv
biB0aHJvdWdoDQo+IHRoZSBzeXN0ZW0gY29udHJvbGxlciBuZXR3b3JrLg0KDQpXaHkgaXMgaXQg
Z2VuZXJhdGluZyBhbnkgc2lnbmlmaWNhbnQgYW1vdW50IG9mIENQVSBsb2FkPyBXZSdyZSBub3QN
CnRhbGtpbmcgYWJvdXQgYSBoaWdoLWJhbmR3aWR0aCBpbnRlcmZhY2UgaGVyZS4NCg0KLS0gDQpN
YXR0aGV3IEdhcnJldHQgPG1hdHRoZXcuZ2FycmV0dEBuZWJ1bGEuY29tPg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russ Anderson Feb. 20, 2014, 9:28 p.m. UTC | #13
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.
Matthew Garrett Feb. 20, 2014, 9:39 p.m. UTC | #14
T24gVGh1LCAyMDE0LTAyLTIwIGF0IDE1OjI4IC0wNjAwLCBSdXNzIEFuZGVyc29uIHdyb3RlOg0K
DQo+IEZvciBzb21lIGN1c3RvbWVycyBfYW55XyBhbW91bnQgaXMgc2lnbmlmaWNhbnQsIGVzcGVj
aWFsbHkNCj4gb24gbGFyZ2UgY2x1c3RlcmVkIHN5c3RlbXMgd2hlcmUgdGhlIGFtb3VudCBpcyBt
dWx0aXBsaWVkDQo+IGJ5IHRlbnMgb3IgaHVuZHJlZHMgb2YgdGhvdXNhbmRzIG9mIG5vZGVzLg0K
PiANCj4gWW91IG1hbnkgbm90IHRoaW5rIHdhc3RpbmcgdGhlaXIgY3B1IGN5Y2xlcyBpcyBpbXBv
cnRhbnQsIGJ1dCB0aGV5IGRvLg0KDQpUaGVuIHRoZXkgc2hvdWxkIGJlIHJ1bm5pbmcgbG9jYWxs
eSBidWlsdCBrZXJuZWxzIGluIG9yZGVyIHRvIGVuc3VyZQ0KdGhhdCB0aGVyZSdzIG5vIHByb2Js
ZW1hdGljIGNvZGUgcnVubmluZyBhdCBhbGwuIERpc3RyaWJ1dGlvbiBrZXJuZWxzDQp3aWxsIGFs
d2F5cyBjb250YWluIGNvZGUgdGhhdCBzb21lIGN1c3RvbWVycyB3b24ndCBiZSBpbnRlcmVzdGVk
IGluLCBhbmQNCnNvbWUgb2YgdGhhdCBjb2RlIHdpbGwgZW5kIHVwIGV4ZWN1dGluZy4NCg0KSWYg
eW91IGhhdmUgc3BlY2lmaWMgYnVnIHJlcG9ydHMsIHRoYXQgd291bGQgYmUgaGVscGZ1bC4gQnV0
IHlvdSdyZSBub3QNCmRlc2NyaWJpbmcgYWN0dWFsIGZhaWx1cmUgY29uZGl0aW9ucyBvciBzaG93
aW5nIGFueSB3aWxsaW5nbmVzcyB0bw0KZmlndXJlIG91dCB3aGF0IHRoZSB1bmRlcmx5aW5nIHBy
b2JsZW0gaXMuIFRoZSBCTUMgaXMgZ2VuZXJhdGluZw0KbWVzc2FnZXMgYW5kIGhhbmRpbmcgdGhl
bSBvZmYgdG8gdGhlIGhvc3QuIElmIHRoYXQncyB0YWtpbmcgbW9yZSB0aGFuIGENCnRpbnkgbnVt
YmVyIG9mIGN5Y2xlcywgd2h5PyBPbmNlIHdlIGtub3cgdGhlIGFuc3dlciB0byB0aGF0LCB3ZSBj
YW4NCmFjdHVhbGx5IHB1dCBzb21lIGVmZm9ydCBpbnRvIGZpeGluZyBpdC4NCg0KLS0gDQpNYXR0
aGV3IEdhcnJldHQgPG1hdHRoZXcuZ2FycmV0dEBuZWJ1bGEuY29tPg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russ Anderson Feb. 20, 2014, 9:49 p.m. UTC | #15
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.
Matthew Garrett Feb. 20, 2014, 9:51 p.m. UTC | #16
T24gVGh1LCAyMDE0LTAyLTIwIGF0IDE1OjQ5IC0wNjAwLCBSdXNzIEFuZGVyc29uIHdyb3RlOg0K
PiBPbiBUaHUsIEZlYiAyMCwgMjAxNCBhdCAwODo0NjowNFBNICswMDAwLCBNYXR0aGV3IEdhcnJl
dHQgd3JvdGU6DQo+ID4gWW91IGFwcGVhciB0byBiZSBzYXlpbmcgIlNHSSBzaGlwIGhhcmR3YXJl
IHRoYXQgZG9lc24ndCB3b3JrLiBXZSBkb24ndA0KPiA+IGtub3cgd2h5IGl0IGRvZXNuJ3Qgd29y
ayBhbmQgd2UncmUgbm90IGludGVyZXN0ZWQgaW4gZml4aW5nIGl0LCBzbyB3ZSdkDQo+ID4gcHJl
ZmVyIHRoZSBkZWZhdWx0IGtlcm5lbCBjb25maWd1cmF0aW9uIHRvIGJlIGJyb2tlbiIuIFRoYXQg
ZG9lc24ndCBzZWVtDQo+ID4gbGlrZSBhbiBlc3BlY2lhbGx5IGNvbXBlbGxpbmcgYXJndW1lbnQu
DQo+IA0KPiBZb3UgYXBwZWFyIHRvIG5vdCB1bmRlcnN0YW5kIHRoZSBtZXNzIHRoaXMgY2F1c2Vz
LiAgV2UgaGF2ZSBoaXQNCj4gdGhpcyBvbiAzcmQgcGFydHkgYm9hcmRzLCBub3QganVzdCBTR0kg
YnVpbHQgaGFyZHdhcmUgKG11Y2gNCj4gb2Ygd2hpY2ggaXMgaW5kdXN0cnkgc3RhbmRhcmQgY29t
cG9uZW50cykuICBJdCBjYXVzZXMgcHJvYmxlbXMNCj4gYm90aCBmb3IgZmxha2V5IGhhcmR3YXJl
IF9hbmRfIGZ1bmN0aW9uaW5nIGhhcmR3YXJlLg0KDQpCdWcgbnVtYmVycz8NCg0KLS0gDQpNYXR0
aGV3IEdhcnJldHQgPG1hdHRoZXcuZ2FycmV0dEBuZWJ1bGEuY29tPg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russ Anderson Feb. 20, 2014, 10:06 p.m. UTC | #17
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?
Matthew Garrett Feb. 20, 2014, 10:26 p.m. UTC | #18
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 <matthew.garrett@nebula.com>
Russ Anderson Feb. 20, 2014, 10:45 p.m. UTC | #19
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 Feb. 20, 2014, 11:01 p.m. UTC | #20
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. 

Could you be more specific?  Is the problem kernel code
trying to use IPMI functionality before the ipmi_si driver
is loaded?  Or something else?

What is the "undesirable system behaviour"?


> Set the default to Y in order to encourage distributions and
> users to configure kernels to avoid awkward surprises.

What are the "awkward surprises"?

Thanks,
Matthew Garrett Feb. 20, 2014, 11:09 p.m. UTC | #21
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 <matthew.garrett@nebula.com>
Russ Anderson Feb. 20, 2014, 11:59 p.m. UTC | #22
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.  :-)
Rafael J. Wysocki Feb. 21, 2014, 12:10 a.m. UTC | #23
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 <rja@sgi.com>
> 
> 
> > Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
> > ---
> > 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
> 
>
Matthew Garrett Feb. 21, 2014, 12:13 a.m. UTC | #24
T24gVGh1LCAyMDE0LTAyLTIwIGF0IDE3OjU5IC0wNjAwLCBSdXNzIEFuZGVyc29uIHdyb3RlOg0K
PiBPbiBUaHUsIEZlYiAyMCwgMjAxNCBhdCAxMTowOTo0MlBNICswMDAwLCBNYXR0aGV3IEdhcnJl
dHQgd3JvdGU6DQo+ID4gT24gVGh1LCAyMDE0LTAyLTIwIGF0IDE2OjQ1IC0wNjAwLCBSdXNzIEFu
ZGVyc29uIHdyb3RlOg0KPiA+ID4gDQo+ID4gPiBUaGUgQUNQSSBzcGVjIHJlcXVpcmVzIElQTUkg
ZnVuY3Rpb25hbGl0eSBiZWZvcmUgYSBtb2R1bGUgbG9hZHMgYXQNCj4gPiA+IGJvb3QgdGltZT8g
IEFuZCB0aGUga2VybmVsIGlzICpicm9rZW4qIGlmIGl0IGRvZXMgbm90IHN1cHBvcnQgQUNJUCBJ
UE1JDQo+ID4gPiBmdW5jdGlvbmFsaXR5IGJlZm9yZSBtb2R1bGUgbG9hZCB0aW1lPyAgUmVhbGx5
Pw0KPiA+IA0KPiA+IFRoZXJlJ3Mgbm8gbWVjaGFuaXNtIHRvIGVuc3VyZSB0aGF0IElQTUkgc3Vw
cG9ydCB3aWxsIGJlIGxvYWRlZCBiZWZvcmUNCj4gPiBBQ1BJIGNhbGxzIGF0dGVtcHQgdG8gYWNj
ZXNzIElQTUkgb3BlcmF0aW9uIHJlZ2lvbnMuIFJlYWxseS4NCj4gDQo+IEFuZCBubyBtZWNoYW5p
c20gY2FuIGJlIGFkZGVkIHRvIGVuc3VyZSB0aGF0IEFDUEkgY2FsbCBhcmUNCj4gbm90IGF0dGVt
cHRlZCBiZWZvcmUgSVBNSSBpcyBpbml0aWFsaXplZD8gIEEgZmxhZyBvciBsb2NrDQo+IG9yIGV4
cG9ydGVkIHN5bWJvbCBpbmRpY2F0aW5nIElQTUkgc3VwcG9ydCBpcyByZWFkeS4NCg0KQUNQSSBm
dW5jdGlvbnMgYXJlIGEgYmxhY2sgYm94IHRvIGRyaXZlcnMuIFlvdSBtYWtlIGFuIEFDUEkgY2Fs
bCwgdGhlDQpBTUwgY29kZSBkb2VzIHNvbWV0aGluZy4gV2UgY291bGQgYmxvY2sgdGhlcmUsIGJ1
dCB3aGF0J3MgdGhlIGRyaXZlcg0Kc3VwcG9zZWQgdG8gZG8gYXQgdGhhdCBwb2ludD8gVGhlIGNv
cmUgY291bGQgY2FsbCBvdXQgdG8gYSBtb2R1bGUNCmxvYWRlciwgYnV0IGlmIHRoZSBkcml2ZXIg
aXMgYnVpbHQgaW4gYW5kIElQTUkgaXNuJ3QgdGhlbiB5b3UnbGwgZW5kIHVwDQp3aXRoIGEgNjAg
c2Vjb25kIHBhdXNlIGluIGJvb3QgYW5kIGEgZHJpdmVyIHRoYXQgZG9lc24ndCB3b3JrLiANCg0K
PiA+ID4gPiBBQ1BJIDQuMCBpbmNsdWRlcyBzdXBwb3J0IGZvciBJUE1JIG9wZXJhdGlvbiByZWdp
b25zLiBNb2R1bGFyIElQTUkgbWVhbnMNCj4gPiA+ID4gdGhhdCB0aGUga2VybmVsIHdpbGwgc3Bl
bmQgYSBzaWduaWZpY2FudCBhbW91bnQgb2YgdGltZSAocG90ZW50aWFsbHkNCj4gPiA+ID4gdW50
aWwgYSB1c2VyIG1hbnVhbGx5IGxvYWRzIGEgZHJpdmVyKSBmYWlsaW5nIHRvIGltcGxlbWVudCBw
YXJ0IG9mIHRoZQ0KPiA+ID4gPiBJUE1JIHNwZWNpZmljYXRpb24uIFRoYXQncyBhIHByb2JsZW0s
IGFuZCB0aGUgY29ycmVjdCBmaXggaXMgdG8gZW5zdXJlDQo+ID4gPiA+IHRoYXQgdGhlIGtlcm5l
bCBhbHdheXMgaW1wbGVtZW50cyBJUE1JIHN1cHBvcnQuDQo+ID4gPiANCj4gPiA+IFRoZSBBQ1BJ
IHNwZWMgc2F5cyBpcG1pX3NpIGNhbm5vdCBiZSBhIGRyaXZlcj8gIFJlYWxseT8NCj4gPiA+IFdo
YXQgaXMgdGhlIHJlYWwgcHJvYmxlbSB5b3UgYXJlIHRyeWluZyB0byBzb2x2ZT8NCj4gPiANCj4g
PiBUaGUgbW9zdCBzdHJhaWdodGZvcndhcmQgY2FzZSBpcyB0aGF0IG9mIGFuIEFDUEkgcG93ZXIg
bWV0ZXIuDQo+IA0KPiBTbyBpdCBpcyBqdXN0IGEgbWF0dGVyIG9mIG1ha2luZyBzdXJlIGlwbWlf
c2kgbW9kdWxlcyBsb2FkcyBiZWZvcmUNCj4gdGhlIEFDUEkgcG93ZXIgbWV0ZXIgbW9kdWxlIGxv
YWRzLCByaWdodD8gIG1vZHVsZSBkZXBlbmRlbmN5IGlzc3VlLg0KDQpObywgYmVjYXVzZSB0aGUg
cG93ZXIgbWV0ZXIgZHJpdmVyIGhhcyBubyB3YXkgb2Yga25vd2luZyB0aGF0IGEgdmVuZG9yDQpo
YXMgaW1wbGVtZW50ZWQgdGhpcyBpbnRlcmZhY2UgdmlhIElQTUkuICpBbnkqIEFDUEkgZW50cnkg
cG9pbnQgY291bGQNCnRoZW9yZXRpY2FsbHkgcmVmZXJlbmNlIElQTUkgY29kZSwgZXZlbiB0aGUg
X0lOSSBtZXRob2QgdGhhdCdzIGNhbGxlZA0KZHVyaW5nIEFDUEkgY29yZSBpbml0LiBJZiBpdCBk
b2VzLCBhbmQgaWYgeW91IGRvbid0IGhhdmUgYnVpbHQtaW4gQUNQSQ0Kc3VwcG9ydCwgeW91J2Qg
ZmFpbCBBQ1BJIGluaXRpYWxpc2F0aW9uIGFuZCB0aGluZ3Mgd291bGQgZ28gZG93bmhpbGwNCmZy
b20gdGhlcmUuDQoNCihJIGRvbid0IHRoaW5rIGZhaWx1cmUgb2YgdGhpcyBtYWduaXR1ZGUgaXMg
YWN0dWFsbHkgKmxpa2VseSosIGJ1dCBpdA0Kd291bGQgYmUgc3BlYy1jb21wbGlhbnQpDQoNCj4g
PiBJJ3ZlIHJlcGVhdGVkbHkgYXNrZWQgZm9yIHlvdSB0byBwcm92aWRlIGRldGFpbGVkIGRlc2Ny
aXB0aW9ucyBvZiB0aGUNCj4gPiBwcm9ibGVtcyB5b3UndmUgc2VlbiBiZWNhdXNlIEkgaGF2ZSBh
IGdlbnVpbmUgaW50ZXJlc3QgaW4gZml4aW5nIHRoZW0uDQo+ID4gSWYgeW91J3JlIGp1c3QgZ29p
bmcgdG8gY2hpbGRpc2hseSByZWZ1c2UgdGhlbiB0aGlzIGRpc2N1c3Npb24gaXMNCj4gPiBwb2lu
dGxlc3MuIA0KPiANCj4gVGhlIGRpc3RybyBjYXNlcyBJIHdvdWxkIHBvaW50IHlvdSBhdCBhcmUg
bWFya2VkIHByaXZhdGUuIA0KPiBBbmQgeW91IGRvIG5vdCBoYXZlIGFjY2VzcyB0byBvdXIgaW50
ZXJuYWwgc3VwcG9ydCBzeXN0ZW0uDQo+IEEgc2ltcGxlIGdvb2dsZSBzZWFyY2ggZm9yICJraXBt
aTAiIHNob3dzIGEgbG90IG9mIHJlcG9ydHMgb2YNCj4gaGlnaCBjcHUgdXRpbGl6YXRpb24uICAN
Cg0KQW5kIG5vYm9keSBzZWVtcyB0byBoYXZlIHB1dCBhbnkgZWZmb3J0IGludG8gZmlndXJpbmcg
b3V0IHdoYXQgdGhlDQp1bmRlcmx5aW5nIGNhdXNlIGlzLiBJcyBpdCBzcGlubmluZyBiZWNhdXNl
IHRoZXJlIGFyZSBtZXNzYWdlcz8gSWYgc28sDQppcyBpdCBiZWNhdXNlIHRoZSBCTUMgd291bGQg
cmVhbGx5IGxpa2Ugc29tZSBraW5kIG9mIHJlc3BvbnNlIHRvIHRob3NlDQptZXNzYWdlcz8gSXMg
aXQgc3Bpbm5pbmcgYmVjYXVzZSB0aGUgQk1DIGlzIHdlZGdlZD8gSWYgc28sIGNhbiB3ZSBkZXRl
Y3QNCnRoYXQgY2FzZSwgZmxhZyBpdCBhcyBicm9rZW4gYW5kIGNsZWFubHkgZGlzZW5nYWdlPw0K
DQpXZSdyZSBydW5uaW5nIHN5c3RlbXMgZnJvbSBhIHdpZGUgcmFuZ2Ugb2YgdmVuZG9ycyAoaW5j
bHVkaW5nIGJhc2ljYWxseQ0KYWxsIHRoZSBUaWVyIDEgc2VydmVyIG1hbnVmYWN0dXJlcnMsIHBs
dXMgc29tZSB3aGl0ZWJveCksIHVzZSBJUE1JDQpmdW5jdGlvbmFsaXR5IGhlYXZpbHkgYW5kIGdl
bnVpbmVseSBkbyBub3Qgc2VlIHRoZSBkZXNjcmliZWQgcHJvYmxlbXMuIEkNCmRvbid0IHRoaW5r
IHRoZXJlJ3MgZXZpZGVuY2Ugb2Ygd2lkZXNwcmVhZCBicmVha2FnZSwgYW5kIHdoZXJlIGl0IGRv
ZXMNCmV4aXN0IHdlIHNob3VsZCB0cmVhdCBpdCBhcyB3ZSB3b3VsZCBhbnkgb3RoZXIgYnVnIC0g
ZGlhZ25vc2UgdGhlDQp1bmRlcmx5aW5nIHByb2JsZW0gYW5kIGZpeCBpdC4NCg0KLS0gDQpNYXR0
aGV3IEdhcnJldHQgPG1hdHRoZXcuZ2FycmV0dEBuZWJ1bGEuY29tPg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lv Zheng Feb. 21, 2014, 2:17 a.m. UTC | #25
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: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] 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          rja@sgi.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Corey Minyard Feb. 21, 2014, 1:37 p.m. UTC | #26
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
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Garrett Feb. 21, 2014, 4:12 p.m. UTC | #27
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 <matthew.garrett@nebula.com>
Russ Anderson Feb. 21, 2014, 4:33 p.m. UTC | #28
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: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] 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          rja@sgi.com
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russ Anderson Feb. 21, 2014, 4:53 p.m. UTC | #29
On Fri, Feb 21, 2014 at 12:13:13AM +0000, Matthew Garrett wrote:
> 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. 

When you say "if the driver is built in" which driver are you
talking about?  I thought the issue was making sure ipmi_si 
driver was loaded before power meter driver.



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

Again, it sounds like as long as ipmi_si driver is loaded before
power meter driver, power meter driver is fine.
Lv Zheng Feb. 24, 2014, 12:48 a.m. UTC | #30
SGksDQoNCj4gRnJvbTogbGludXgtYWNwaS1vd25lckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzps
aW51eC1hY3BpLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24gQmVoYWxmIE9mIE1hdHRoZXcgR2Fy
cmV0dA0KPiBTZW50OiBTYXR1cmRheSwgRmVicnVhcnkgMjIsIDIwMTQgMTI6MTMgQU0NCj4gDQo+
IE9uIEZyaSwgMjAxNC0wMi0yMSBhdCAwMjoxNyArMDAwMCwgWmhlbmcsIEx2IHdyb3RlOg0KPiAN
Cj4gPiBJbiBmYWN0IHRoZXJlIGlzIGEgd29ya2Fyb3VuZCBzb2x1dGlvbiBJJ3ZlIHBvc3RlZCBo
ZXJlOg0KPiA+IGh0dHBzOi8vcGF0Y2h3b3JrLmtlcm5lbC5vcmcvcGF0Y2gvMjgzMTg1MS8NCj4g
PiBUaGUgdXBkYXRlZCB2ZXJzaW9uIG9mIHRoaXMgcGF0Y2ggY2FuIGJlIGZvdW5kIGF0Og0KPiA+
IGh0dHBzOi8vYnVnemlsbGEua2VybmVsLm9yZy9hdHRhY2htZW50LmNnaT9pZD0xMTI2MTENCj4g
PiBJdCBpcyB0aGUgYWNwaS1pcG1pMTMucGF0Y2ggZmlsZS4NCj4gDQo+IEkgZG9uJ3QgdGhpbmsg
dGhhdCBzb2x2ZXMgdGhlIHVuZGVybHlpbmcgcHJvYmxlbT8gV2UncmUgc3RpbGwgbGVmdCB3aXRo
DQo+IG5vIGd1YXJhbnRlZSBvZiBvcmRlcmluZyBiZXR3ZWVuIEFDUEkgSVBNSSBvcGVyYXRpb24g
cmVnaW9uIHN1cHBvcnQNCj4gYmVpbmcgYXZhaWxhYmxlIGFuZCB0aGUgbG9hZGluZyBvZiBhIGRy
aXZlciB0aGF0IG1heSByZWx5IG9uIGl0Lg0KDQpZZXMuIEFzIGl0IGlzIGEgYml0IGZhciBiZXlv
bmQgdGhlIGJ1ZyByZXBvcnQsIEkgZGlkbid0IHdvcmsgb24gaXQuDQpCdXQgYWN0dWFsbHksIEkg
aGF2ZSBzb2x1dGlvbiB0byBhY2hpZXZlIHRoaXMuDQoNCj4gPiBJIHRoaW5rIHRoZXJlIHNob3Vs
ZCBiZSByZWxhdGlvbnNoaXAgYmV0d2VlbiBBQ1BJIHJlZ2lvbiBhbmQgTGludXgga2VybmVsIG1v
ZHVsZXMuDQo+ID4gSW4gZmFjdCBvbiB0aGUgd2VsbC1rbm93biBvcGVyYXRpbmcgc3lzdGVtLCBf
UkVHIGlzIGFsd2F5cyBpbnZva2VkIGF0IHRoZSBlbmQgb2YgdGhlIElSUF9QTlBfU1RBUlRfREVW
SUNFIGNvbXBsZXRpb25zLg0KPiA+IEJ1dCB3ZSBtYXkgc3RpbGwgYmUgYWJsZSB0byByZXR1cm4g
LUVQUk9CRV9ERUZFUiBpbiB0aGUgcG93ZXIgbWV0ZXIgZHJpdmVyIHdoZW4gaXQgZGV0ZWN0cyBm
YWlsdXJlcyBjYXVzZWQgYnkgdGhlIHJlYWRpbmVzcyBzdGF0ZQ0KPiBvZiB0aGUgcmVnaW9uIGhh
bmRsZXJzLg0KPiANCj4gV2UgZG9uJ3QgaGF2ZSBhIGd1YXJhbnRlZSB0aGF0IGl0J2xsIGhhcHBl
biBhdCBwcm9iZSB0aW1lLiBUaGUNCj4gY2FwYWJpbGl0aWVzIGxpc3QgbWF5IGJlIHN0YXRpYywg
YW5kIHNvIHdlJ2xsIGdldCBvdXIgZmlyc3QgZmFpbHVyZSBvbg0KPiBhbiBhdHRlbXB0ZWQgdXNl
cnNwYWNlIGFjY2VzcyBpbnN0ZWFkLg0KDQpUaGlzIGlzIHRoZSBpc3N1ZSByZWxhdGVkIHRvIHRo
ZSBJUE1JIG9wZXJhdGlvbiByZWdpb24sIGFuZCBpdCBjYW4gYmUgc29sdmVkIGJ5IGEgInJlcXVl
c3RfbW9kdWxlKCkiIGNhbGwuDQpUaGUgLUVQUk9CRV9ERUZFUiBJIG1lbnRpb25lZCBpcyBhYm91
dCB0aGUgcXVhbGl0eSBvZiAgYWNwaSBwb3dlciBtZXRlciBkcml2ZXIgaXRzZWxmLg0KDQo+IEl0
IG1heSBiZSB0aGF0IENvcmV5J3Mgc3VnZ2VzdGlvbiBzYXRpc2ZpZXMgdGhlIGNvbmNlcm5zIFJ1
c3MgcmFpc2VkLg0KPiBCdWlsZGluZyBJUE1JIGluIGJ1dCBvbmx5IHByb2JpbmcgYnkgZGVmYXVs
dCBvbiBzeXN0ZW1zIHRoYXQgZGVjbGFyZSBhDQo+IEJNQyBpbiBBQ1BJIG9yIERNSSBzaG91bGQg
YXZvaWQgYm9vdC10aW1lIGRlbGF5cyB3aGlsZSBzdGlsbCBlbnN1cmluZw0KPiB0aGF0IHRoZSBm
dW5jdGlvbmFsaXR5IGlzIGF2YWlsYWJsZSBvbiBzeXN0ZW1zIHdoZXJlIHRoZXJlJ3MgYSByZWFs
DQo+IGNoYW5jZSB0aGF0IHRoZSBmaXJtd2FyZSBleHBlY3RzIHRoZSBPUyB0byBwcm92aWRlIHN1
cHBvcnQuDQoNCklmIHlvdSB0YWtlIGEgbG9vayBhdCB0aGlzIHBhdGNoIChodHRwczovL3BhdGNo
d29yay5rZXJuZWwub3JnL3BhdGNoLzI4MzE4NTEvKSwgeW91J2xsIHNlZSBpdCBpcyBwb3NzaWJs
ZSB0byBhZGQgcmVxdWVzdF9tb2R1bGUoKSBpbiBhY3BpX3JlZ2lvbl9kZWZhdWx0X2hhbmRsZXIo
KS4NClRoZSBjYWxsYmFjayBpcyBpbnZva2VkIHdoZW4gdGhlIGZpcnN0IElQTUkgcmVnaW9uIGFj
Y2VzcyBoYXBwZW5zIGFuZCBpdCBpcyBpbiB0aGUgcHJvY2VzcyBjb250ZXh0IHdpdGhvdXQgYW55
IGxvY2tpbmcuDQpJJ2QgcmF0aGVyIHRvIGVtYmVkIGFjcGlfaXBtaSBpbnRvIGlwbWlfc2kgbW9k
dWxlIHdpdGggdGhpcyBmaXg6DQpodHRwczovL2J1Z3ppbGxhLmtlcm5lbC5vcmcvYXR0YWNobWVu
dC5jZ2k/aWQ9MTEyNjExIChhY3BpLWlwbWkxNC5wYXRjaCkuDQpBZnRlciB0aGF0LCBhIHJlcXVl
c3RfbW9kdWxlKCkgaW52b2NhdGlvbiBvZiBpcG1pX3NpIGluIGFjcGlfcmVnaW9uX2RlZmF1bHRf
aGFuZGxlcigpIGNhbiBzb2x2ZSB3aGF0IHlvdSBhcmUgd29ycnlpbmcgYWJvdXQuDQoNCldoYXQn
cyB5b3VyIG9waW5pb24/DQoNClRoYW5rcyBhbmQgYmVzdCByZWdhcmRzDQotTHYNCg0KPiAtLQ0K
PiBNYXR0aGV3IEdhcnJldHQgPG1hdHRoZXcuZ2FycmV0dEBuZWJ1bGEuY29tPg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek March 12, 2014, 11 p.m. UTC | #31
On Tue 2014-02-18 23:15:08, 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).

Would the right solution be to implement dependency between power
meter and IMPI?
									Pavel
Matthew Garrett March 12, 2014, 11:22 p.m. UTC | #32
On Thu, 2014-03-13 at 00:00 +0100, Pavel Machek wrote:
> On Tue 2014-02-18 23:15:08, 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).

> 

> Would the right solution be to implement dependency between power

> meter and IMPI?


No. The power meter driver knows nothing about IPMI. It makes no IPMI
calls. There's no requirement that a vendor implement it via IPMI.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
Pavel Machek March 13, 2014, 7:22 a.m. UTC | #33
On Wed 2014-03-12 23:22:49, Matthew Garrett wrote:
> On Thu, 2014-03-13 at 00:00 +0100, Pavel Machek wrote:
> > On Tue 2014-02-18 23:15:08, 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).
> > 
> > Would the right solution be to implement dependency between power
> > meter and IMPI?
> 
> No. The power meter driver knows nothing about IPMI. It makes no IPMI
> calls. There's no requirement that a vendor implement it via IPMI.

Yet you claim that IMPI is needed for that, and that's why you made
IMPI default.

So ... do we need dmi-based blacklist?
									Pavel
Matthew Garrett March 13, 2014, 7:24 a.m. UTC | #34
On Thu, 2014-03-13 at 08:22 +0100, Pavel Machek wrote:
> On Wed 2014-03-12 23:22:49, Matthew Garrett wrote:

> > No. The power meter driver knows nothing about IPMI. It makes no IPMI

> > calls. There's no requirement that a vendor implement it via IPMI.

> 

> Yet you claim that IMPI is needed for that, and that's why you made

> IMPI default.


I claim that the ACPI spec defines the behaviour of IPMI operation
regions, and so we should default IPMI to Y in order to (by default)
implement the ACPI spec.

> So ... do we need dmi-based blacklist?


I don't see why.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
Pavel Machek March 13, 2014, 8:38 a.m. UTC | #35
On Thu 2014-03-13 07:24:36, Matthew Garrett wrote:
> On Thu, 2014-03-13 at 08:22 +0100, Pavel Machek wrote:
> > On Wed 2014-03-12 23:22:49, Matthew Garrett wrote:
> > > No. The power meter driver knows nothing about IPMI. It makes no IPMI
> > > calls. There's no requirement that a vendor implement it via IPMI.
> > 
> > Yet you claim that IMPI is needed for that, and that's why you made
> > IMPI default.
> 
> I claim that the ACPI spec defines the behaviour of IPMI operation
> regions, and so we should default IPMI to Y in order to (by default)
> implement the ACPI spec.
> 
> > So ... do we need dmi-based blacklist?
> 
> I don't see why.

Your reasoning for default y was that "power meter depends on
this". Then, claim that "power meter does not officially depend on it"
so it would be wrong to have a dependency.

Defaults are not right solution; system should still work if I select
non-default settings. Which you claim is not a case, but you don't see
why you should fix it.
								Pavel
Corey Minyard March 13, 2014, 1:29 p.m. UTC | #36
On 03/13/2014 03:38 AM, Pavel Machek wrote:
> On Thu 2014-03-13 07:24:36, Matthew Garrett wrote:
>> On Thu, 2014-03-13 at 08:22 +0100, Pavel Machek wrote:
>>> On Wed 2014-03-12 23:22:49, Matthew Garrett wrote:
>>>> No. The power meter driver knows nothing about IPMI. It makes no IPMI
>>>> calls. There's no requirement that a vendor implement it via IPMI.
>>> Yet you claim that IMPI is needed for that, and that's why you made
>>> IMPI default.
>> I claim that the ACPI spec defines the behaviour of IPMI operation
>> regions, and so we should default IPMI to Y in order to (by default)
>> implement the ACPI spec.
>>
>>> So ... do we need dmi-based blacklist?
>> I don't see why.
> Your reasoning for default y was that "power meter depends on
> this". Then, claim that "power meter does not officially depend on it"
> so it would be wrong to have a dependency.

I believe the correct statement is "On some systems power meter depends
on it".

>
> Defaults are not right solution; system should still work if I select
> non-default settings. Which you claim is not a case, but you don't see
> why you should fix it.

If something is implemented using IPMI, then IPMI has to be there to
use it.  Matthew's statement was:

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

Other things besides the power meter can be implemented using IPMI, it's
up to the vendor.  How will those things work if IPMI is not available?

-corey

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek March 16, 2014, 8:53 a.m. UTC | #37
Hi!

> > Defaults are not right solution; system should still work if I select
> > non-default settings. Which you claim is not a case, but you don't see
> > why you should fix it.
> 
> If something is implemented using IPMI, then IPMI has to be there to
> use it.  Matthew's statement was:
> 
> 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).

Yes, and this failure is not fixed with different default.

									Pavel
diff mbox

Patch

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