[PATCH] ACPI: set acpi_enforce_resources to strict

[PATCH] ACPI: set acpi_enforce_resources to strict

[Luca Tettamanti]

The following patch changes the default for "acpi_enforce_resources" option to
"strict", disallowing access by native drivers to IO ports and memory regions
claimed by ACPI firmware.

The patch is mainly aimed to block native hwmon drivers from touching
monitoring chips that ACPI thinks it own; there have reports of such
conflicts in the past, leading to an emergency shutdown of the machine
(due to native IC driver interference ACPI TZ polling ends up reading
garbage); I'm also writing a driver for ASUS ATK0110 ACPI hwmon
interface[1], which will _definitely_ clash with native drivers.

Currently only hwmon SuperIO IC's and SMBus controllers will be affected by
this change. Unfortunately this also means that drivers for those devices will
refuse to load and users may perceive this as regression; consensus - however
- is that we shouldn't touch resources claimed by the firmware because it's too
risky since we do not know what ACPI code may be doing (and it is risky
_right now_, regardless of the new ATK driver); for people who needs to
force the loading of hwmon drivers regardless of what ACPI is doing there's is
always the possibility to set acpi_enforce_resources to "lax".

While I as at it I also documented the parameter ;-)

[1] http://lists.lm-sensors.org/pipermail/lm-sensors/2009-January/025035.html

Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
Blessed-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/kernel-parameters.txt |   16 ++++++++++++++++
 drivers/acpi/osl.c                  |    2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

Index: b/Documentation/kernel-parameters.txt
===================================================================
--- a/Documentation/kernel-parameters.txt	2009-01-06 16:44:29.406069082 +0100
+++ b/Documentation/kernel-parameters.txt	2009-01-06 19:16:57.543996442 +0100
@@ -252,6 +252,22 @@
 			to assume that this machine's pmtimer latches its value
 			and always returns good values.
 
+	acpi_enforce_resources=	[ACPI]
+			{ strict, lax, no }
+			Check for resource conflicts between native drivers
+			and ACPI OperationRegions (SystemIO and System Memory
+			only). IO ports and memory declared in ACPI might be
+			used by the ACPI subsystem in arbitrary AML code and
+			can interfere with legacy drivers.
+			strict (default): access to resources claimed by ACPI
+			is denied; legacy drivers trying to access reserved
+			resources will fail to load.
+			lax: access to resources claimed by ACPI is allowed;
+			legacy drivers trying to access reserved resources
+			will load and a warning message is logged.
+			no: ACPI OperationRegions are not marked as reserved,
+			no further checks are performed.
+
 	agp=		[AGP]
 			{ off | try_unsupported }
 			off: disable AGP support
Index: b/drivers/acpi/osl.c
===================================================================
--- a/drivers/acpi/osl.c	2008-12-14 19:26:02.926046208 +0100
+++ b/drivers/acpi/osl.c	2009-01-06 19:16:31.506869922 +0100
@@ -1077,7 +1077,7 @@
 #define ENFORCE_RESOURCES_LAX    1
 #define ENFORCE_RESOURCES_NO     0
 
-static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
+static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
 
 static int __init acpi_enforce_resources_setup(char *str)
 {

Luca
-- 
Dicono che  il cane sia  il miglior  amico dell'uomo. Secondo me  non e`
vero. Quanti dei vostri amici avete fatto castrare, recentemente?

Re: [PATCH] ACPI: set acpi_enforce_resources to strict

[Hans de Goede]

Luca Tettamanti wrote:
> The following patch changes the default for "acpi_enforce_resources" option to
> "strict", disallowing access by native drivers to IO ports and memory regions
> claimed by ACPI firmware.
> 

Hi Luca (et all)

I know it was me who proposed to send a mail about this to the ACPI list, but 
when I did I had an RFC mail in mind, not a patch.

I've been discussing this with Jean Delvare (he is back from his holliday), and 
he agrees that in the end this is the right thing to do. But this will probably 
stop the smbus controller drivers (Jean is the i2c subsystem maintainer) from 
loading on quite a few motherboards. Given that ACPI claims it is using these, 
this would actually be a good thing, but still it will probably make for a lot 
of unhappy users. So his call (and I support him there) is that it is too early 
to do that.

<Intermezzo to get those not involved in earlier discussion up to speed>

For those not familiar with the hwmon subsystem. Some (one or 2?) release ago a 
couple of new acpi functions were added:
acpi_check_resource_conflict()
acpi_check_region()
acpi_check_mem_region()

Which can be called by drivers to see if they are not trying to do IO to any 
ranges claimed by ACPI.

I did a full grep for these in 2.6.28, and they are only used by the smbus / 
i2c controller drivers there. In 2.6.29 the hwmon drivers for the hwmon part of 
superio IC's starts checking these too.

The behaviour of these 3 functions gets controlled by the global 
acpi_enforce_resources kernel (cmdline) option (and changing this from lax to 
strict will only influence these 3 methods). The default currently is lax, 
which will cause these methods to only issue a warning about conflicts, but not 
report any actual conflicts to the caller. Changing this to strict would 
actually report the conflict, stopping the smbus drivers (and hwmon superio) 
drivers from loading. As said before: Given that ACPI claims it is using these, 
this would actually be a good thing. But it is a change from current behaviour 
so quite some people will probably start yelling.

</intermezzo>

So back to the discussion about changing the default of acpi_enforce_resources 
to strict, Jean and I have been discussing this on IRC and we feel it will most 
likely cause too much pain. So we would like to suggest to make the default 
depend up on the motherboard. Our plan is to have the default be "auto" and 
that will mean "lax", unless the motherboard is atk0110 (Asus ACPI interface 
for reading hwmon data) capable and in that case "auto" will mean "strict"

The plan is then to merge this acpi subsystem change and the atk0110 driver at 
the same time, so that people will get different hwmon capabilities, but won't 
loose hwmon capabilities all together. Important note: this is meant as an 
temporary state of affairs, the end goal is to make the checking strict.

Luca do you think you could do a patch implementing the described "auto" value 
for acpi_enforce_resources ?

ACPI people, would this be acceptable as a temporary solution ?

Thanks & Regards,

Hans

Re: [lm-sensors] [PATCH] ACPI: set acpi_enforce_resources to strict

[Hans de Goede]

Luca Tettamanti wrote:
> The following patch changes the default for "acpi_enforce_resources" option to
> "strict", disallowing access by native drivers to IO ports and memory regions
> claimed by ACPI firmware.
> 

Hi Luca (et all)

I know it was me who proposed to send a mail about this to the ACPI list, but 
when I did I had an RFC mail in mind, not a patch.

I've been discussing this with Jean Delvare (he is back from his holliday), and 
he agrees that in the end this is the right thing to do. But this will probably 
stop the smbus controller drivers (Jean is the i2c subsystem maintainer) from 
loading on quite a few motherboards. Given that ACPI claims it is using these, 
this would actually be a good thing, but still it will probably make for a lot 
of unhappy users. So his call (and I support him there) is that it is too early 
to do that.

<Intermezzo to get those not involved in earlier discussion up to speed>

For those not familiar with the hwmon subsystem. Some (one or 2?) release ago a 
couple of new acpi functions were added:
acpi_check_resource_conflict()
acpi_check_region()
acpi_check_mem_region()

Which can be called by drivers to see if they are not trying to do IO to any 
ranges claimed by ACPI.

I did a full grep for these in 2.6.28, and they are only used by the smbus / 
i2c controller drivers there. In 2.6.29 the hwmon drivers for the hwmon part of 
superio IC's starts checking these too.

The behaviour of these 3 functions gets controlled by the global 
acpi_enforce_resources kernel (cmdline) option (and changing this from lax to 
strict will only influence these 3 methods). The default currently is lax, 
which will cause these methods to only issue a warning about conflicts, but not 
report any actual conflicts to the caller. Changing this to strict would 
actually report the conflict, stopping the smbus drivers (and hwmon superio) 
drivers from loading. As said before: Given that ACPI claims it is using these, 
this would actually be a good thing. But it is a change from current behaviour 
so quite some people will probably start yelling.

</intermezzo>

So back to the discussion about changing the default of acpi_enforce_resources 
to strict, Jean and I have been discussing this on IRC and we feel it will most 
likely cause too much pain. So we would like to suggest to make the default 
depend up on the motherboard. Our plan is to have the default be "auto" and 
that will mean "lax", unless the motherboard is atk0110 (Asus ACPI interface 
for reading hwmon data) capable and in that case "auto" will mean "strict"

The plan is then to merge this acpi subsystem change and the atk0110 driver at 
the same time, so that people will get different hwmon capabilities, but won't 
loose hwmon capabilities all together. Important note: this is meant as an 
temporary state of affairs, the end goal is to make the checking strict.

Luca do you think you could do a patch implementing the described "auto" value 
for acpi_enforce_resources ?

ACPI people, would this be acceptable as a temporary solution ?

Thanks & Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] ACPI: set acpi_enforce_resources to strict

[Luca Tettamanti]

On Thu, Jan 8, 2009 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
[...]
> So back to the discussion about changing the default of
> acpi_enforce_resources to strict, Jean and I have been discussing this on
> IRC and we feel it will most likely cause too much pain. So we would like to
> suggest to make the default depend up on the motherboard. Our plan is to
> have the default be "auto" and that will mean "lax", unless the motherboard
> is atk0110 (Asus ACPI interface for reading hwmon data) capable and in that
> case "auto" will mean "strict"

Hum, that would mean putting ATK specific code into ACPI subsystem.
It's not pretty :S

> The plan is then to merge this acpi subsystem change and the atk0110 driver
> at the same time, so that people will get different hwmon capabilities, but
> won't loose hwmon capabilities all together. Important note: this is meant
> as an temporary state of affairs, the end goal is to make the checking
> strict.
>
> Luca do you think you could do a patch implementing the described "auto"
> value for acpi_enforce_resources ?

So the logic would be:
- if ATK node is present and the driver is enabled -> strict
- otherwise -> auto

I don't if I can use the ACPI interpreter early enough to check the
DSDT though; AFAICS the alternatives are:
- DMI which would require an entry for each mobo (notebooks don't have
the ATK interface)
- an initcall in the ATK driver that overrides the value of
acpi_enforce_resources to strict

Luca

Re: [lm-sensors] [PATCH] ACPI: set acpi_enforce_resources to strict

[Luca Tettamanti]

On Thu, Jan 8, 2009 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
[...]
> So back to the discussion about changing the default of
> acpi_enforce_resources to strict, Jean and I have been discussing this on
> IRC and we feel it will most likely cause too much pain. So we would like to
> suggest to make the default depend up on the motherboard. Our plan is to
> have the default be "auto" and that will mean "lax", unless the motherboard
> is atk0110 (Asus ACPI interface for reading hwmon data) capable and in that
> case "auto" will mean "strict"

Hum, that would mean putting ATK specific code into ACPI subsystem.
It's not pretty :S

> The plan is then to merge this acpi subsystem change and the atk0110 driver
> at the same time, so that people will get different hwmon capabilities, but
> won't loose hwmon capabilities all together. Important note: this is meant
> as an temporary state of affairs, the end goal is to make the checking
> strict.
>
> Luca do you think you could do a patch implementing the described "auto"
> value for acpi_enforce_resources ?

So the logic would be:
- if ATK node is present and the driver is enabled -> strict
- otherwise -> auto

I don't if I can use the ACPI interpreter early enough to check the
DSDT though; AFAICS the alternatives are:
- DMI which would require an entry for each mobo (notebooks don't have
the ATK interface)
- an initcall in the ATK driver that overrides the value of
acpi_enforce_resources to strict

Luca

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] ACPI: set acpi_enforce_resources to strict

[Hans de Goede]

Luca Tettamanti wrote:
> On Thu, Jan 8, 2009 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> [...]
>> So back to the discussion about changing the default of
>> acpi_enforce_resources to strict, Jean and I have been discussing this on
>> IRC and we feel it will most likely cause too much pain. So we would like to
>> suggest to make the default depend up on the motherboard. Our plan is to
>> have the default be "auto" and that will mean "lax", unless the motherboard
>> is atk0110 (Asus ACPI interface for reading hwmon data) capable and in that
>> case "auto" will mean "strict"
> 
> Hum, that would mean putting ATK specific code into ACPI subsystem.
> It's not pretty :S
> 

Agreed, an alternative Jean and I discussed was to make the default strict for 
all Asus boards, that would be easier (simple strcmp on baseboard 
manufacturer). I don't know how large the atk detection code is.

>> The plan is then to merge this acpi subsystem change and the atk0110 driver
>> at the same time, so that people will get different hwmon capabilities, but
>> won't loose hwmon capabilities all together. Important note: this is meant
>> as an temporary state of affairs, the end goal is to make the checking
>> strict.
>>
>> Luca do you think you could do a patch implementing the described "auto"
>> value for acpi_enforce_resources ?
> 
> So the logic would be:
> - if ATK node is present and the driver is enabled -> strict
> - otherwise -> auto
> 

No the logic would be:
if (acpi_enforce_resources == auto) {
   if (atk0110)
     acpi_enforce_resources = strict;
   else
     acpi_enforce_resources = lax;
}


> - DMI which would require an entry for each mobo (notebooks don't have
> the ATK interface)

Yes, if the atk0110 case is too trouble some, changing this over to make auto 
be strict on all Asus boards is ok too.

> - an initcall in the ATK driver that overrides the value of
> acpi_enforce_resources to strict

That won't work, the atk0110 driver might be a module and the conflicting 
drivers could be loaded first.

Regards,

Hans

Re: [lm-sensors] [PATCH] ACPI: set acpi_enforce_resources to strict

[Hans de Goede]

Luca Tettamanti wrote:
> On Thu, Jan 8, 2009 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> [...]
>> So back to the discussion about changing the default of
>> acpi_enforce_resources to strict, Jean and I have been discussing this on
>> IRC and we feel it will most likely cause too much pain. So we would like to
>> suggest to make the default depend up on the motherboard. Our plan is to
>> have the default be "auto" and that will mean "lax", unless the motherboard
>> is atk0110 (Asus ACPI interface for reading hwmon data) capable and in that
>> case "auto" will mean "strict"
> 
> Hum, that would mean putting ATK specific code into ACPI subsystem.
> It's not pretty :S
> 

Agreed, an alternative Jean and I discussed was to make the default strict for 
all Asus boards, that would be easier (simple strcmp on baseboard 
manufacturer). I don't know how large the atk detection code is.

>> The plan is then to merge this acpi subsystem change and the atk0110 driver
>> at the same time, so that people will get different hwmon capabilities, but
>> won't loose hwmon capabilities all together. Important note: this is meant
>> as an temporary state of affairs, the end goal is to make the checking
>> strict.
>>
>> Luca do you think you could do a patch implementing the described "auto"
>> value for acpi_enforce_resources ?
> 
> So the logic would be:
> - if ATK node is present and the driver is enabled -> strict
> - otherwise -> auto
> 

No the logic would be:
if (acpi_enforce_resources = auto) {
   if (atk0110)
     acpi_enforce_resources = strict;
   else
     acpi_enforce_resources = lax;
}


> - DMI which would require an entry for each mobo (notebooks don't have
> the ATK interface)

Yes, if the atk0110 case is too trouble some, changing this over to make auto 
be strict on all Asus boards is ok too.

> - an initcall in the ATK driver that overrides the value of
> acpi_enforce_resources to strict

That won't work, the atk0110 driver might be a module and the conflicting 
drivers could be loaded first.

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] ACPI: set acpi_enforce_resources to strict

[Luca Tettamanti]

Il Sun, Jan 11, 2009 at 08:04:56PM +0100, Hans de Goede ha scritto: 
> Luca Tettamanti wrote:
>> On Thu, Jan 8, 2009 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> [...]
>>> So back to the discussion about changing the default of
>>> acpi_enforce_resources to strict, Jean and I have been discussing this on
>>> IRC and we feel it will most likely cause too much pain. So we would like to
>>> suggest to make the default depend up on the motherboard. Our plan is to
>>> have the default be "auto" and that will mean "lax", unless the motherboard
>>> is atk0110 (Asus ACPI interface for reading hwmon data) capable and in that
>>> case "auto" will mean "strict"
>>
>> Hum, that would mean putting ATK specific code into ACPI subsystem.
>> It's not pretty :S
>>
>
> Agreed, an alternative Jean and I discussed was to make the default 
> strict for all Asus boards, that would be easier (simple strcmp on 
> baseboard manufacturer). I don't know how large the atk detection code 
> is.

Pretty small, 25 LoC.

>>> The plan is then to merge this acpi subsystem change and the atk0110 driver
>>> at the same time, so that people will get different hwmon capabilities, but
>>> won't loose hwmon capabilities all together. Important note: this is meant
>>> as an temporary state of affairs, the end goal is to make the checking
>>> strict.
>>>
>>> Luca do you think you could do a patch implementing the described "auto"
>>> value for acpi_enforce_resources ?
>>
>> So the logic would be:
>> - if ATK node is present and the driver is enabled -> strict
>> - otherwise -> auto
>>
>
> No the logic would be:
> if (acpi_enforce_resources == auto) {
>   if (atk0110)
>     acpi_enforce_resources = strict;
>   else
>     acpi_enforce_resources = lax;
> }

Detection is pretty easy, the only problem is that I had to hijack the
fs_initcal level, because the code shall run _after_ the interpreter has
been initialized (subsys) but before native drivers have a chance to
poke the HW. So this what I've come up with (I just wrote it - I plan to
reboot tomorrow to test it...):

---
 Documentation/kernel-parameters.txt |   19 +++++++++++++
 drivers/acpi/osl.c                  |   49 ++++++++++++++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 2 deletions(-)

Index: linux-2.6.git/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.git.orig/Documentation/kernel-parameters.txt	2009-01-12 21:21:34.573074955 +0100
+++ linux-2.6.git/Documentation/kernel-parameters.txt	2009-01-12 21:22:01.164076567 +0100
@@ -251,6 +251,25 @@
 			to assume that this machine's pmtimer latches its value
 			and always returns good values.
 
+	acpi_enforce_resources=	[ACPI]
+			{ strict, auto, lax, no }
+			Check for resource conflicts between native drivers
+			and ACPI OperationRegions (SystemIO and System Memory
+			only). IO ports and memory declared in ACPI might be
+			used by the ACPI subsystem in arbitrary AML code and
+			can interfere with legacy drivers.
+			strict: access to resources claimed by ACPI is denied;
+			legacy drivers trying to access reserved resources
+			will fail to load.
+			auto (default): try and detect ACPI devices with known
+			ACPI drivers; escalates the setting to "strict" if such
+			a device is found, otherwise behaves like "lax".
+			lax: access to resources claimed by ACPI is allowed;
+			legacy drivers trying to access reserved resources
+			will load and a warning message is logged.
+			no: ACPI OperationRegions are not marked as reserved,
+			no further checks are performed.
+
 	agp=		[AGP]
 			{ off | try_unsupported }
 			off: disable AGP support
Index: linux-2.6.git/drivers/acpi/osl.c
===================================================================
--- linux-2.6.git.orig/drivers/acpi/osl.c	2009-01-12 21:21:34.549076302 +0100
+++ linux-2.6.git/drivers/acpi/osl.c	2009-01-12 21:22:01.164076567 +0100
@@ -1063,6 +1063,9 @@
  * in arbitrary AML code and can interfere with legacy drivers.
  * acpi_enforce_resources= can be set to:
  *
+ *   - auto             (3)
+ *     -> detect possible conflicts with ACPI drivers and switch to
+ *     strict if needed, otherwise act like lax
  *   - strict           (2)
  *     -> further driver trying to access the resources will not load
  *   - lax (default)    (1)
@@ -1073,11 +1076,12 @@
  *     -> ACPI Operation Region resources will not be registered
  *
  */
-#define ENFORCE_RESOURCES_STRICT 2
+#define ENFORCE_RESOURCES_STRICT 3
+#define ENFORCE_RESOURCES_AUTO   2
 #define ENFORCE_RESOURCES_LAX    1
 #define ENFORCE_RESOURCES_NO     0
 
-static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
+static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_AUTO;
 
 static int __init acpi_enforce_resources_setup(char *str)
 {
@@ -1086,6 +1090,8 @@
 
 	if (!strcmp("strict", str))
 		acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
+	else if (!strcmp("auto", str))
+		acpi_enforce_resources = ENFORCE_RESOURCES_AUTO;
 	else if (!strcmp("lax", str))
 		acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
 	else if (!strcmp("no", str))
@@ -1096,6 +1102,40 @@
 
 __setup("acpi_enforce_resources=", acpi_enforce_resources_setup);
 
+static int __init acpi_detect_asus_atk(void)
+{
+	struct acpi_buffer output;
+	union acpi_object *hid;
+	acpi_status ret;
+
+	if (acpi_enforce_resources != ENFORCE_RESOURCES_AUTO)
+		return 0;
+
+	output.pointer = NULL;
+	output.length = ACPI_ALLOCATE_BUFFER;
+	ret = acpi_evaluate_object_typed(NULL, "\\_SB.PCI0.SBRG.ASOC._HID",
+			NULL, &output, ACPI_TYPE_STRING);
+	if (ret == AE_OK) {
+		hid = output.pointer;
+
+		if (!strcmp("ATK0110", hid->string.pointer)) {
+			printk(KERN_DEBUG "Asus ATK0110 interface detected: "
+					"enforcing resource checking.\n");
+
+			acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
+		} else {
+			acpi_enforce_resources =  ENFORCE_RESOURCES_LAX;
+		}
+
+		ACPI_FREE(output.pointer);
+	} else {
+		acpi_enforce_resources =  ENFORCE_RESOURCES_LAX;
+	}
+
+	return 0;
+}
+fs_initcall(acpi_detect_asus_atk);
+
 /* Check for resource conflicts between ACPI OperationRegions and native
  * drivers */
 int acpi_check_resource_conflict(struct resource *res)


Luca
-- 
Ligabue canta: "Tutti vogliono viaggiare in primaaaa..."
Io ci ho provato e dopo un chilometro ho fuso il motore e bruciato
la frizione...

Re: [lm-sensors] [PATCH] ACPI: set acpi_enforce_resources to strict

[Luca Tettamanti]

Il Sun, Jan 11, 2009 at 08:04:56PM +0100, Hans de Goede ha scritto: 
> Luca Tettamanti wrote:
>> On Thu, Jan 8, 2009 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> [...]
>>> So back to the discussion about changing the default of
>>> acpi_enforce_resources to strict, Jean and I have been discussing this on
>>> IRC and we feel it will most likely cause too much pain. So we would like to
>>> suggest to make the default depend up on the motherboard. Our plan is to
>>> have the default be "auto" and that will mean "lax", unless the motherboard
>>> is atk0110 (Asus ACPI interface for reading hwmon data) capable and in that
>>> case "auto" will mean "strict"
>>
>> Hum, that would mean putting ATK specific code into ACPI subsystem.
>> It's not pretty :S
>>
>
> Agreed, an alternative Jean and I discussed was to make the default 
> strict for all Asus boards, that would be easier (simple strcmp on 
> baseboard manufacturer). I don't know how large the atk detection code 
> is.

Pretty small, 25 LoC.

>>> The plan is then to merge this acpi subsystem change and the atk0110 driver
>>> at the same time, so that people will get different hwmon capabilities, but
>>> won't loose hwmon capabilities all together. Important note: this is meant
>>> as an temporary state of affairs, the end goal is to make the checking
>>> strict.
>>>
>>> Luca do you think you could do a patch implementing the described "auto"
>>> value for acpi_enforce_resources ?
>>
>> So the logic would be:
>> - if ATK node is present and the driver is enabled -> strict
>> - otherwise -> auto
>>
>
> No the logic would be:
> if (acpi_enforce_resources = auto) {
>   if (atk0110)
>     acpi_enforce_resources = strict;
>   else
>     acpi_enforce_resources = lax;
> }

Detection is pretty easy, the only problem is that I had to hijack the
fs_initcal level, because the code shall run _after_ the interpreter has
been initialized (subsys) but before native drivers have a chance to
poke the HW. So this what I've come up with (I just wrote it - I plan to
reboot tomorrow to test it...):

---
 Documentation/kernel-parameters.txt |   19 +++++++++++++
 drivers/acpi/osl.c                  |   49 ++++++++++++++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 2 deletions(-)

Index: linux-2.6.git/Documentation/kernel-parameters.txt
=================================--- linux-2.6.git.orig/Documentation/kernel-parameters.txt	2009-01-12 21:21:34.573074955 +0100
+++ linux-2.6.git/Documentation/kernel-parameters.txt	2009-01-12 21:22:01.164076567 +0100
@@ -251,6 +251,25 @@
 			to assume that this machine's pmtimer latches its value
 			and always returns good values.
 
+	acpi_enforce_resources=	[ACPI]
+			{ strict, auto, lax, no }
+			Check for resource conflicts between native drivers
+			and ACPI OperationRegions (SystemIO and System Memory
+			only). IO ports and memory declared in ACPI might be
+			used by the ACPI subsystem in arbitrary AML code and
+			can interfere with legacy drivers.
+			strict: access to resources claimed by ACPI is denied;
+			legacy drivers trying to access reserved resources
+			will fail to load.
+			auto (default): try and detect ACPI devices with known
+			ACPI drivers; escalates the setting to "strict" if such
+			a device is found, otherwise behaves like "lax".
+			lax: access to resources claimed by ACPI is allowed;
+			legacy drivers trying to access reserved resources
+			will load and a warning message is logged.
+			no: ACPI OperationRegions are not marked as reserved,
+			no further checks are performed.
+
 	agp=		[AGP]
 			{ off | try_unsupported }
 			off: disable AGP support
Index: linux-2.6.git/drivers/acpi/osl.c
=================================--- linux-2.6.git.orig/drivers/acpi/osl.c	2009-01-12 21:21:34.549076302 +0100
+++ linux-2.6.git/drivers/acpi/osl.c	2009-01-12 21:22:01.164076567 +0100
@@ -1063,6 +1063,9 @@
  * in arbitrary AML code and can interfere with legacy drivers.
  * acpi_enforce_resources= can be set to:
  *
+ *   - auto             (3)
+ *     -> detect possible conflicts with ACPI drivers and switch to
+ *     strict if needed, otherwise act like lax
  *   - strict           (2)
  *     -> further driver trying to access the resources will not load
  *   - lax (default)    (1)
@@ -1073,11 +1076,12 @@
  *     -> ACPI Operation Region resources will not be registered
  *
  */
-#define ENFORCE_RESOURCES_STRICT 2
+#define ENFORCE_RESOURCES_STRICT 3
+#define ENFORCE_RESOURCES_AUTO   2
 #define ENFORCE_RESOURCES_LAX    1
 #define ENFORCE_RESOURCES_NO     0
 
-static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
+static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_AUTO;
 
 static int __init acpi_enforce_resources_setup(char *str)
 {
@@ -1086,6 +1090,8 @@
 
 	if (!strcmp("strict", str))
 		acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
+	else if (!strcmp("auto", str))
+		acpi_enforce_resources = ENFORCE_RESOURCES_AUTO;
 	else if (!strcmp("lax", str))
 		acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
 	else if (!strcmp("no", str))
@@ -1096,6 +1102,40 @@
 
 __setup("acpi_enforce_resources=", acpi_enforce_resources_setup);
 
+static int __init acpi_detect_asus_atk(void)
+{
+	struct acpi_buffer output;
+	union acpi_object *hid;
+	acpi_status ret;
+
+	if (acpi_enforce_resources != ENFORCE_RESOURCES_AUTO)
+		return 0;
+
+	output.pointer = NULL;
+	output.length = ACPI_ALLOCATE_BUFFER;
+	ret = acpi_evaluate_object_typed(NULL, "\\_SB.PCI0.SBRG.ASOC._HID",
+			NULL, &output, ACPI_TYPE_STRING);
+	if (ret = AE_OK) {
+		hid = output.pointer;
+
+		if (!strcmp("ATK0110", hid->string.pointer)) {
+			printk(KERN_DEBUG "Asus ATK0110 interface detected: "
+					"enforcing resource checking.\n");
+
+			acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
+		} else {
+			acpi_enforce_resources =  ENFORCE_RESOURCES_LAX;
+		}
+
+		ACPI_FREE(output.pointer);
+	} else {
+		acpi_enforce_resources =  ENFORCE_RESOURCES_LAX;
+	}
+
+	return 0;
+}
+fs_initcall(acpi_detect_asus_atk);
+
 /* Check for resource conflicts between ACPI OperationRegions and native
  * drivers */
 int acpi_check_resource_conflict(struct resource *res)


Luca
-- 
Ligabue canta: "Tutti vogliono viaggiare in primaaaa..."
Io ci ho provato e dopo un chilometro ho fuso il motore e bruciato
la frizione...

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] ACPI: set acpi_enforce_resources to strict

[Hans de Goede]

Luca Tettamanti wrote:
> Il Sun, Jan 11, 2009 at 08:04:56PM +0100, Hans de Goede ha scritto: 
>> Luca Tettamanti wrote:
>>> On Thu, Jan 8, 2009 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> [...]
>>>> So back to the discussion about changing the default of
>>>> acpi_enforce_resources to strict, Jean and I have been discussing this on
>>>> IRC and we feel it will most likely cause too much pain. So we would like to
>>>> suggest to make the default depend up on the motherboard. Our plan is to
>>>> have the default be "auto" and that will mean "lax", unless the motherboard
>>>> is atk0110 (Asus ACPI interface for reading hwmon data) capable and in that
>>>> case "auto" will mean "strict"
>>> Hum, that would mean putting ATK specific code into ACPI subsystem.
>>> It's not pretty :S
>>>
>> Agreed, an alternative Jean and I discussed was to make the default 
>> strict for all Asus boards, that would be easier (simple strcmp on 
>> baseboard manufacturer). I don't know how large the atk detection code 
>> is.
> 
> Pretty small, 25 LoC.
> 

<snip>

> Detection is pretty easy, the only problem is that I had to hijack the
> fs_initcal level, because the code shall run _after_ the interpreter has
> been initialized (subsys) but before native drivers have a chance to
> poke the HW. So this what I've come up with (I just wrote it - I plan to
> reboot tomorrow to test it...):
> 

Looks good to me, Len would something like this be acceptable?

Regards,

Hans


> ---
>  Documentation/kernel-parameters.txt |   19 +++++++++++++
>  drivers/acpi/osl.c                  |   49 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 66 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6.git/Documentation/kernel-parameters.txt
> ===================================================================
> --- linux-2.6.git.orig/Documentation/kernel-parameters.txt	2009-01-12 21:21:34.573074955 +0100
> +++ linux-2.6.git/Documentation/kernel-parameters.txt	2009-01-12 21:22:01.164076567 +0100
> @@ -251,6 +251,25 @@
>  			to assume that this machine's pmtimer latches its value
>  			and always returns good values.
>  
> +	acpi_enforce_resources=	[ACPI]
> +			{ strict, auto, lax, no }
> +			Check for resource conflicts between native drivers
> +			and ACPI OperationRegions (SystemIO and System Memory
> +			only). IO ports and memory declared in ACPI might be
> +			used by the ACPI subsystem in arbitrary AML code and
> +			can interfere with legacy drivers.
> +			strict: access to resources claimed by ACPI is denied;
> +			legacy drivers trying to access reserved resources
> +			will fail to load.
> +			auto (default): try and detect ACPI devices with known
> +			ACPI drivers; escalates the setting to "strict" if such
> +			a device is found, otherwise behaves like "lax".
> +			lax: access to resources claimed by ACPI is allowed;
> +			legacy drivers trying to access reserved resources
> +			will load and a warning message is logged.
> +			no: ACPI OperationRegions are not marked as reserved,
> +			no further checks are performed.
> +
>  	agp=		[AGP]
>  			{ off | try_unsupported }
>  			off: disable AGP support
> Index: linux-2.6.git/drivers/acpi/osl.c
> ===================================================================
> --- linux-2.6.git.orig/drivers/acpi/osl.c	2009-01-12 21:21:34.549076302 +0100
> +++ linux-2.6.git/drivers/acpi/osl.c	2009-01-12 21:22:01.164076567 +0100
> @@ -1063,6 +1063,9 @@
>   * in arbitrary AML code and can interfere with legacy drivers.
>   * acpi_enforce_resources= can be set to:
>   *
> + *   - auto             (3)
> + *     -> detect possible conflicts with ACPI drivers and switch to
> + *     strict if needed, otherwise act like lax
>   *   - strict           (2)
>   *     -> further driver trying to access the resources will not load
>   *   - lax (default)    (1)
> @@ -1073,11 +1076,12 @@
>   *     -> ACPI Operation Region resources will not be registered
>   *
>   */
> -#define ENFORCE_RESOURCES_STRICT 2
> +#define ENFORCE_RESOURCES_STRICT 3
> +#define ENFORCE_RESOURCES_AUTO   2
>  #define ENFORCE_RESOURCES_LAX    1
>  #define ENFORCE_RESOURCES_NO     0
>  
> -static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
> +static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_AUTO;
>  
>  static int __init acpi_enforce_resources_setup(char *str)
>  {
> @@ -1086,6 +1090,8 @@
>  
>  	if (!strcmp("strict", str))
>  		acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
> +	else if (!strcmp("auto", str))
> +		acpi_enforce_resources = ENFORCE_RESOURCES_AUTO;
>  	else if (!strcmp("lax", str))
>  		acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
>  	else if (!strcmp("no", str))
> @@ -1096,6 +1102,40 @@
>  
>  __setup("acpi_enforce_resources=", acpi_enforce_resources_setup);
>  
> +static int __init acpi_detect_asus_atk(void)
> +{
> +	struct acpi_buffer output;
> +	union acpi_object *hid;
> +	acpi_status ret;
> +
> +	if (acpi_enforce_resources != ENFORCE_RESOURCES_AUTO)
> +		return 0;
> +
> +	output.pointer = NULL;
> +	output.length = ACPI_ALLOCATE_BUFFER;
> +	ret = acpi_evaluate_object_typed(NULL, "\\_SB.PCI0.SBRG.ASOC._HID",
> +			NULL, &output, ACPI_TYPE_STRING);
> +	if (ret == AE_OK) {
> +		hid = output.pointer;
> +
> +		if (!strcmp("ATK0110", hid->string.pointer)) {
> +			printk(KERN_DEBUG "Asus ATK0110 interface detected: "
> +					"enforcing resource checking.\n");
> +
> +			acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
> +		} else {
> +			acpi_enforce_resources =  ENFORCE_RESOURCES_LAX;
> +		}
> +
> +		ACPI_FREE(output.pointer);
> +	} else {
> +		acpi_enforce_resources =  ENFORCE_RESOURCES_LAX;
> +	}
> +
> +	return 0;
> +}
> +fs_initcall(acpi_detect_asus_atk);
> +
>  /* Check for resource conflicts between ACPI OperationRegions and native
>   * drivers */
>  int acpi_check_resource_conflict(struct resource *res)
> 
> 
> Luca

Re: [lm-sensors] [PATCH] ACPI: set acpi_enforce_resources to strict

[Hans de Goede]

Luca Tettamanti wrote:
> Il Sun, Jan 11, 2009 at 08:04:56PM +0100, Hans de Goede ha scritto: 
>> Luca Tettamanti wrote:
>>> On Thu, Jan 8, 2009 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> [...]
>>>> So back to the discussion about changing the default of
>>>> acpi_enforce_resources to strict, Jean and I have been discussing this on
>>>> IRC and we feel it will most likely cause too much pain. So we would like to
>>>> suggest to make the default depend up on the motherboard. Our plan is to
>>>> have the default be "auto" and that will mean "lax", unless the motherboard
>>>> is atk0110 (Asus ACPI interface for reading hwmon data) capable and in that
>>>> case "auto" will mean "strict"
>>> Hum, that would mean putting ATK specific code into ACPI subsystem.
>>> It's not pretty :S
>>>
>> Agreed, an alternative Jean and I discussed was to make the default 
>> strict for all Asus boards, that would be easier (simple strcmp on 
>> baseboard manufacturer). I don't know how large the atk detection code 
>> is.
> 
> Pretty small, 25 LoC.
> 

<snip>

> Detection is pretty easy, the only problem is that I had to hijack the
> fs_initcal level, because the code shall run _after_ the interpreter has
> been initialized (subsys) but before native drivers have a chance to
> poke the HW. So this what I've come up with (I just wrote it - I plan to
> reboot tomorrow to test it...):
> 

Looks good to me, Len would something like this be acceptable?

Regards,

Hans


> ---
>  Documentation/kernel-parameters.txt |   19 +++++++++++++
>  drivers/acpi/osl.c                  |   49 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 66 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6.git/Documentation/kernel-parameters.txt
> =================================> --- linux-2.6.git.orig/Documentation/kernel-parameters.txt	2009-01-12 21:21:34.573074955 +0100
> +++ linux-2.6.git/Documentation/kernel-parameters.txt	2009-01-12 21:22:01.164076567 +0100
> @@ -251,6 +251,25 @@
>  			to assume that this machine's pmtimer latches its value
>  			and always returns good values.
>  
> +	acpi_enforce_resources=	[ACPI]
> +			{ strict, auto, lax, no }
> +			Check for resource conflicts between native drivers
> +			and ACPI OperationRegions (SystemIO and System Memory
> +			only). IO ports and memory declared in ACPI might be
> +			used by the ACPI subsystem in arbitrary AML code and
> +			can interfere with legacy drivers.
> +			strict: access to resources claimed by ACPI is denied;
> +			legacy drivers trying to access reserved resources
> +			will fail to load.
> +			auto (default): try and detect ACPI devices with known
> +			ACPI drivers; escalates the setting to "strict" if such
> +			a device is found, otherwise behaves like "lax".
> +			lax: access to resources claimed by ACPI is allowed;
> +			legacy drivers trying to access reserved resources
> +			will load and a warning message is logged.
> +			no: ACPI OperationRegions are not marked as reserved,
> +			no further checks are performed.
> +
>  	agp=		[AGP]
>  			{ off | try_unsupported }
>  			off: disable AGP support
> Index: linux-2.6.git/drivers/acpi/osl.c
> =================================> --- linux-2.6.git.orig/drivers/acpi/osl.c	2009-01-12 21:21:34.549076302 +0100
> +++ linux-2.6.git/drivers/acpi/osl.c	2009-01-12 21:22:01.164076567 +0100
> @@ -1063,6 +1063,9 @@
>   * in arbitrary AML code and can interfere with legacy drivers.
>   * acpi_enforce_resources= can be set to:
>   *
> + *   - auto             (3)
> + *     -> detect possible conflicts with ACPI drivers and switch to
> + *     strict if needed, otherwise act like lax
>   *   - strict           (2)
>   *     -> further driver trying to access the resources will not load
>   *   - lax (default)    (1)
> @@ -1073,11 +1076,12 @@
>   *     -> ACPI Operation Region resources will not be registered
>   *
>   */
> -#define ENFORCE_RESOURCES_STRICT 2
> +#define ENFORCE_RESOURCES_STRICT 3
> +#define ENFORCE_RESOURCES_AUTO   2
>  #define ENFORCE_RESOURCES_LAX    1
>  #define ENFORCE_RESOURCES_NO     0
>  
> -static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
> +static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_AUTO;
>  
>  static int __init acpi_enforce_resources_setup(char *str)
>  {
> @@ -1086,6 +1090,8 @@
>  
>  	if (!strcmp("strict", str))
>  		acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
> +	else if (!strcmp("auto", str))
> +		acpi_enforce_resources = ENFORCE_RESOURCES_AUTO;
>  	else if (!strcmp("lax", str))
>  		acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
>  	else if (!strcmp("no", str))
> @@ -1096,6 +1102,40 @@
>  
>  __setup("acpi_enforce_resources=", acpi_enforce_resources_setup);
>  
> +static int __init acpi_detect_asus_atk(void)
> +{
> +	struct acpi_buffer output;
> +	union acpi_object *hid;
> +	acpi_status ret;
> +
> +	if (acpi_enforce_resources != ENFORCE_RESOURCES_AUTO)
> +		return 0;
> +
> +	output.pointer = NULL;
> +	output.length = ACPI_ALLOCATE_BUFFER;
> +	ret = acpi_evaluate_object_typed(NULL, "\\_SB.PCI0.SBRG.ASOC._HID",
> +			NULL, &output, ACPI_TYPE_STRING);
> +	if (ret = AE_OK) {
> +		hid = output.pointer;
> +
> +		if (!strcmp("ATK0110", hid->string.pointer)) {
> +			printk(KERN_DEBUG "Asus ATK0110 interface detected: "
> +					"enforcing resource checking.\n");
> +
> +			acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
> +		} else {
> +			acpi_enforce_resources =  ENFORCE_RESOURCES_LAX;
> +		}
> +
> +		ACPI_FREE(output.pointer);
> +	} else {
> +		acpi_enforce_resources =  ENFORCE_RESOURCES_LAX;
> +	}
> +
> +	return 0;
> +}
> +fs_initcall(acpi_detect_asus_atk);
> +
>  /* Check for resource conflicts between ACPI OperationRegions and native
>   * drivers */
>  int acpi_check_resource_conflict(struct resource *res)
> 
> 
> Luca

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors