Re: [PATCH] ACPI: set acpi_enforce_resources to strict

[Hans de Goede <hdegoede@redhat.com>]

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