diff mbox

[v5,18/18] Documentation: ACPI for ARM64

Message ID 54ABC2CB.6@linaro.org
State New
Headers show

Commit Message

Hanjun Guo Jan. 6, 2015, 11:11 a.m. UTC
On 2015年01月05日 19:05, Catalin Marinas wrote:
> On Sun, Jan 04, 2015 at 09:39:24AM +0000, Hanjun Guo wrote:
>> On 2014年12月25日 01:18, Catalin Marinas wrote:
>> [...]
>>>
>>> In addition to the above and _DSD requirements/banning, I would also add
>>> some clear statements around:
>>>
>>> _OSC: only global/published capabilities are allowed. For
>>> device-specific _OSC we need a process or maybe we can ban them entirely
>>> and rely on _DSD once we clarify the process.
>>>
>>> _OSI: firmware must not check for certain _OSI strings. Here I'm not
>>> sure what we would have to do for ARM Linux. Reporting "Windows" does
>>> not make any sense but not reporting anything can, as Matthew Garrett
>>> pointed out, can be interpreted by firmware as "Linux". In addition to
>>> any statements in this document, I suggest you patch
>>> drivers/acpi/acpica/utosi.c accordingly, maybe report "Linux" for ARM
>>> and print a kernel warning so that we notice earlier.
>>>
>>> ACPI_OS_NAME: this is globally defined as "Microsoft Windows NT". It
>>> doesn't make much sense in the ARM context. Could we change it to
>>> "Linux" when CONFIG_ARM64?

I think we can introduce a Kconfig such as CONFIG_ACPI_OS_NAME_LINUX,
selected by ARM64 and change ACPI_OS_NAME to "Linux" when
CONFIG_ACPI_OS_NAME_LINUX defined. (we can not add CONFIG_ARM64 in
ACPICA code directly since it will be used by windows too)

some code like below:


>>
>> We will work on this both on ASWG and linux ACPI driver side, as Dong
>> and Charles pointed out, _OSI things can be solved in ACPI spec, when
>> that is done, we can modify the kernel driver to fix the problems above.
>
> Which driver?

the ACPICA core driver as you suggested, sorry for the confusion.

>
> What about ACPI_OS_NAME? Would you suggest it is fine to report
> "Microsoft Windows NT" on an ARM system? That _OS_ not _OSI.

No, not at all. I prefer "Linux"
In include/acpi/acconfig.h, when ACPI_OS_NAME defined, it says:
"OS name, used for the _OS object.  The _OS object is essentially
obsolete,..."
for some legacy reasons, we needed  "Microsoft Windows NT", but ACPI
for ARM64 on linux is totally new, I think we can change it to
"Linux" when CONFIG_ARM64 as you suggested.

Thanks
Hanjun
--
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/

Comments

Hanjun Guo Jan. 6, 2015, 1:50 p.m. UTC | #1
On 2015年01月06日 19:29, Catalin Marinas wrote:
> On Tue, Jan 06, 2015 at 11:11:07AM +0000, Hanjun Guo wrote:
>> On 2015年01月05日 19:05, Catalin Marinas wrote:
>>> On Sun, Jan 04, 2015 at 09:39:24AM +0000, Hanjun Guo wrote:
>>>> On 2014年12月25日 01:18, Catalin Marinas wrote:
>>>> [...]
>>>>>
>>>>> In addition to the above and _DSD requirements/banning, I would also add
>>>>> some clear statements around:
>>>>>
>>>>> _OSC: only global/published capabilities are allowed. For
>>>>> device-specific _OSC we need a process or maybe we can ban them entirely
>>>>> and rely on _DSD once we clarify the process.
>>>>>
>>>>> _OSI: firmware must not check for certain _OSI strings. Here I'm not
>>>>> sure what we would have to do for ARM Linux. Reporting "Windows" does
>>>>> not make any sense but not reporting anything can, as Matthew Garrett
>>>>> pointed out, can be interpreted by firmware as "Linux". In addition to
>>>>> any statements in this document, I suggest you patch
>>>>> drivers/acpi/acpica/utosi.c accordingly, maybe report "Linux" for ARM
>>>>> and print a kernel warning so that we notice earlier.
>>>>>
>>>>> ACPI_OS_NAME: this is globally defined as "Microsoft Windows NT". It
>>>>> doesn't make much sense in the ARM context. Could we change it to
>>>>> "Linux" when CONFIG_ARM64?
>>
>> I think we can introduce a Kconfig such as CONFIG_ACPI_OS_NAME_LINUX,
>> selected by ARM64 and change ACPI_OS_NAME to "Linux" when
>> CONFIG_ACPI_OS_NAME_LINUX defined. (we can not add CONFIG_ARM64 in
>> ACPICA code directly since it will be used by windows too)
>>
>> some code like below:
>
> This looks fine for me (with some minor comments below) but I'm not an
> ACPI expert to say there wouldn't be any issues.
>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index b1f9a20..de567a3 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -1,5 +1,6 @@
>>    config ARM64
>>           def_bool y
>> +       select ACPI_OS_NAME_LINUX if ACPI
>>           select ARCH_BINFMT_ELF_RANDOMIZE_PIE
>>           select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>>           select ARCH_HAS_GCOV_PROFILE_ALL
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 8951cef..11a10ac 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -369,6 +369,10 @@ config ACPI_REDUCED_HARDWARE_ONLY
>>
>>             If you are unsure what to do, do not enable this option.
>>
>> +config ACPI_OS_NAME_LINUX
>> +       bool "Using Linux for _OS method" if EXPERT
>> +       def_bool n
>
> No need for a default n, it is off by default. Alternatively you could
> say:
>
> 	default y if ARM64

ok.

>
>> +
>>    source "drivers/acpi/apei/Kconfig"
>>
>>    config ACPI_EXTLOG
>> diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h
>> index 5a0a3e5..db5e13e 100644
>> --- a/include/acpi/acconfig.h
>> +++ b/include/acpi/acconfig.h
>> @@ -69,7 +69,11 @@
>>     * code that will not execute the _OSI method unless _OS matches the
>> string
>>     * below.  Therefore, change this string at your own risk.
>>     */
>> +#ifndef ACPI_OS_NAME_USING_LINUX
>>    #define ACPI_OS_NAME                    "Microsoft Windows NT"
>> +#else
>> +#define ACPI_OS_NAME                    "Linux"
>> +#endif
>
> Can you not use CONFIG_ACPI_OS_NAME_LINUX directly here without
> introducing another macro?

acconfig.h is part of ACPICA core and will be shared by windows and
other OS, so use CONFIG from Linux in this file is not allowed I think.

>
>>>> We will work on this both on ASWG and linux ACPI driver side, as Dong
>>>> and Charles pointed out, _OSI things can be solved in ACPI spec, when
>>>> that is done, we can modify the kernel driver to fix the problems above.
>>>
>>> Which driver?
>>
>> the ACPICA core driver as you suggested, sorry for the confusion.
>>
>>> What about ACPI_OS_NAME? Would you suggest it is fine to report
>>> "Microsoft Windows NT" on an ARM system? That _OS_ not _OSI.
>>
>> No, not at all. I prefer "Linux"
>> In include/acpi/acconfig.h, when ACPI_OS_NAME defined, it says:
>> "OS name, used for the _OS object.  The _OS object is essentially
>> obsolete,..."
>> for some legacy reasons, we needed  "Microsoft Windows NT", but ACPI
>> for ARM64 on linux is totally new, I think we can change it to
>> "Linux" when CONFIG_ARM64 as you suggested.
>
> We could ignore this change for now if we don't expect the _OS object to
> be used at all. But do we have any other way to check the AML code for
> this? Would FWTS catch such obsolete cases?

I'm not sure, I will check it and get back when I have the answer.

Thanks
Hanjun
--
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
Graeme Gregory Jan. 6, 2015, 1:54 p.m. UTC | #2
On 6 January 2015 at 13:50, Hanjun Guo <hanjun.guo@linaro.org> wrote:
> On 2015年01月06日 19:29, Catalin Marinas wrote:
>>
>> On Tue, Jan 06, 2015 at 11:11:07AM +0000, Hanjun Guo wrote:
>>>
>>> On 2015年01月05日 19:05, Catalin Marinas wrote:
>>>>
>>>> On Sun, Jan 04, 2015 at 09:39:24AM +0000, Hanjun Guo wrote:
>>>>>
>>>>> On 2014年12月25日 01:18, Catalin Marinas wrote:
>>>>> [...]
>>>>>>
>>>>>>
>>>>>> In addition to the above and _DSD requirements/banning, I would also
>>>>>> add
>>>>>> some clear statements around:
>>>>>>
>>>>>> _OSC: only global/published capabilities are allowed. For
>>>>>> device-specific _OSC we need a process or maybe we can ban them
>>>>>> entirely
>>>>>> and rely on _DSD once we clarify the process.
>>>>>>
>>>>>> _OSI: firmware must not check for certain _OSI strings. Here I'm not
>>>>>> sure what we would have to do for ARM Linux. Reporting "Windows" does
>>>>>> not make any sense but not reporting anything can, as Matthew Garrett
>>>>>> pointed out, can be interpreted by firmware as "Linux". In addition to
>>>>>> any statements in this document, I suggest you patch
>>>>>> drivers/acpi/acpica/utosi.c accordingly, maybe report "Linux" for ARM
>>>>>> and print a kernel warning so that we notice earlier.
>>>>>>
>>>>>> ACPI_OS_NAME: this is globally defined as "Microsoft Windows NT". It
>>>>>> doesn't make much sense in the ARM context. Could we change it to
>>>>>> "Linux" when CONFIG_ARM64?
>>>
>>>
>>> I think we can introduce a Kconfig such as CONFIG_ACPI_OS_NAME_LINUX,
>>> selected by ARM64 and change ACPI_OS_NAME to "Linux" when
>>> CONFIG_ACPI_OS_NAME_LINUX defined. (we can not add CONFIG_ARM64 in
>>> ACPICA code directly since it will be used by windows too)
>>>
>>> some code like below:
>>
>>
>> This looks fine for me (with some minor comments below) but I'm not an
>> ACPI expert to say there wouldn't be any issues.
>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index b1f9a20..de567a3 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -1,5 +1,6 @@
>>>    config ARM64
>>>           def_bool y
>>> +       select ACPI_OS_NAME_LINUX if ACPI
>>>           select ARCH_BINFMT_ELF_RANDOMIZE_PIE
>>>           select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>>>           select ARCH_HAS_GCOV_PROFILE_ALL
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index 8951cef..11a10ac 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -369,6 +369,10 @@ config ACPI_REDUCED_HARDWARE_ONLY
>>>
>>>             If you are unsure what to do, do not enable this option.
>>>
>>> +config ACPI_OS_NAME_LINUX
>>> +       bool "Using Linux for _OS method" if EXPERT
>>> +       def_bool n
>>
>>
>> No need for a default n, it is off by default. Alternatively you could
>> say:
>>
>>         default y if ARM64
>
>
> ok.
>
>>
>>> +
>>>    source "drivers/acpi/apei/Kconfig"
>>>
>>>    config ACPI_EXTLOG
>>> diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h
>>> index 5a0a3e5..db5e13e 100644
>>> --- a/include/acpi/acconfig.h
>>> +++ b/include/acpi/acconfig.h
>>> @@ -69,7 +69,11 @@
>>>     * code that will not execute the _OSI method unless _OS matches the
>>> string
>>>     * below.  Therefore, change this string at your own risk.
>>>     */
>>> +#ifndef ACPI_OS_NAME_USING_LINUX
>>>    #define ACPI_OS_NAME                    "Microsoft Windows NT"
>>> +#else
>>> +#define ACPI_OS_NAME                    "Linux"
>>> +#endif
>>
>>
>> Can you not use CONFIG_ACPI_OS_NAME_LINUX directly here without
>> introducing another macro?
>
>
> acconfig.h is part of ACPICA core and will be shared by windows and
> other OS, so use CONFIG from Linux in this file is not allowed I think.
>

We could just propse
#ifndef ACPI_OS_NAME
#define ACPI_OS_NAME "Microsoft Windows NT"
#endif

to acpica maintainers. This will not alter Windows or other software usage
but we can then override it in Linux when/if we want to.

Graeme
--
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
Hanjun Guo Jan. 6, 2015, 1:59 p.m. UTC | #3
On 2015年01月06日 21:54, G Gregory wrote:
> On 6 January 2015 at 13:50, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>> On 2015年01月06日 19:29, Catalin Marinas wrote:
>>>
>>> On Tue, Jan 06, 2015 at 11:11:07AM +0000, Hanjun Guo wrote:
>>>>
>>>> On 2015年01月05日 19:05, Catalin Marinas wrote:
>>>>>
>>>>> On Sun, Jan 04, 2015 at 09:39:24AM +0000, Hanjun Guo wrote:
>>>>>>
>>>>>> On 2014年12月25日 01:18, Catalin Marinas wrote:
>>>>>> [...]
>>>>>>>
>>>>>>>
>>>>>>> In addition to the above and _DSD requirements/banning, I would also
>>>>>>> add
>>>>>>> some clear statements around:
>>>>>>>
>>>>>>> _OSC: only global/published capabilities are allowed. For
>>>>>>> device-specific _OSC we need a process or maybe we can ban them
>>>>>>> entirely
>>>>>>> and rely on _DSD once we clarify the process.
>>>>>>>
>>>>>>> _OSI: firmware must not check for certain _OSI strings. Here I'm not
>>>>>>> sure what we would have to do for ARM Linux. Reporting "Windows" does
>>>>>>> not make any sense but not reporting anything can, as Matthew Garrett
>>>>>>> pointed out, can be interpreted by firmware as "Linux". In addition to
>>>>>>> any statements in this document, I suggest you patch
>>>>>>> drivers/acpi/acpica/utosi.c accordingly, maybe report "Linux" for ARM
>>>>>>> and print a kernel warning so that we notice earlier.
>>>>>>>
>>>>>>> ACPI_OS_NAME: this is globally defined as "Microsoft Windows NT". It
>>>>>>> doesn't make much sense in the ARM context. Could we change it to
>>>>>>> "Linux" when CONFIG_ARM64?
>>>>
>>>>
>>>> I think we can introduce a Kconfig such as CONFIG_ACPI_OS_NAME_LINUX,
>>>> selected by ARM64 and change ACPI_OS_NAME to "Linux" when
>>>> CONFIG_ACPI_OS_NAME_LINUX defined. (we can not add CONFIG_ARM64 in
>>>> ACPICA code directly since it will be used by windows too)
>>>>
>>>> some code like below:
>>>
>>>
>>> This looks fine for me (with some minor comments below) but I'm not an
>>> ACPI expert to say there wouldn't be any issues.
>>>
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index b1f9a20..de567a3 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -1,5 +1,6 @@
>>>>     config ARM64
>>>>            def_bool y
>>>> +       select ACPI_OS_NAME_LINUX if ACPI
>>>>            select ARCH_BINFMT_ELF_RANDOMIZE_PIE
>>>>            select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>>>>            select ARCH_HAS_GCOV_PROFILE_ALL
>>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>>> index 8951cef..11a10ac 100644
>>>> --- a/drivers/acpi/Kconfig
>>>> +++ b/drivers/acpi/Kconfig
>>>> @@ -369,6 +369,10 @@ config ACPI_REDUCED_HARDWARE_ONLY
>>>>
>>>>              If you are unsure what to do, do not enable this option.
>>>>
>>>> +config ACPI_OS_NAME_LINUX
>>>> +       bool "Using Linux for _OS method" if EXPERT
>>>> +       def_bool n
>>>
>>>
>>> No need for a default n, it is off by default. Alternatively you could
>>> say:
>>>
>>>          default y if ARM64
>>
>>
>> ok.
>>
>>>
>>>> +
>>>>     source "drivers/acpi/apei/Kconfig"
>>>>
>>>>     config ACPI_EXTLOG
>>>> diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h
>>>> index 5a0a3e5..db5e13e 100644
>>>> --- a/include/acpi/acconfig.h
>>>> +++ b/include/acpi/acconfig.h
>>>> @@ -69,7 +69,11 @@
>>>>      * code that will not execute the _OSI method unless _OS matches the
>>>> string
>>>>      * below.  Therefore, change this string at your own risk.
>>>>      */
>>>> +#ifndef ACPI_OS_NAME_USING_LINUX
>>>>     #define ACPI_OS_NAME                    "Microsoft Windows NT"
>>>> +#else
>>>> +#define ACPI_OS_NAME                    "Linux"
>>>> +#endif
>>>
>>>
>>> Can you not use CONFIG_ACPI_OS_NAME_LINUX directly here without
>>> introducing another macro?
>>
>>
>> acconfig.h is part of ACPICA core and will be shared by windows and
>> other OS, so use CONFIG from Linux in this file is not allowed I think.
>>
>
> We could just propse
> #ifndef ACPI_OS_NAME
> #define ACPI_OS_NAME "Microsoft Windows NT"
> #endif
>
> to acpica maintainers. This will not alter Windows or other software usage
> but we can then override it in Linux when/if we want to.

this is better and looks great to me :)

Thanks
Hanjun

--
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
diff mbox

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index b1f9a20..de567a3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1,5 +1,6 @@ 
  config ARM64
         def_bool y
+       select ACPI_OS_NAME_LINUX if ACPI
         select ARCH_BINFMT_ELF_RANDOMIZE_PIE
         select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
         select ARCH_HAS_GCOV_PROFILE_ALL
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 8951cef..11a10ac 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -369,6 +369,10 @@  config ACPI_REDUCED_HARDWARE_ONLY

           If you are unsure what to do, do not enable this option.

+config ACPI_OS_NAME_LINUX
+       bool "Using Linux for _OS method" if EXPERT
+       def_bool n
+
  source "drivers/acpi/apei/Kconfig"

  config ACPI_EXTLOG
diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h
index 5a0a3e5..db5e13e 100644
--- a/include/acpi/acconfig.h
+++ b/include/acpi/acconfig.h
@@ -69,7 +69,11 @@ 
   * code that will not execute the _OSI method unless _OS matches the 
string
   * below.  Therefore, change this string at your own risk.
   */
+#ifndef ACPI_OS_NAME_USING_LINUX
  #define ACPI_OS_NAME                    "Microsoft Windows NT"
+#else
+#define ACPI_OS_NAME                    "Linux"
+#endif

  /* Maximum objects in the various object caches */

diff --git a/include/acpi/platform/aclinux.h 
b/include/acpi/platform/aclinux.h
index 1ba7c19..45d51d2 100644
--- a/include/acpi/platform/aclinux.h
+++ b/include/acpi/platform/aclinux.h
@@ -69,6 +69,10 @@ 
  #define ACPI_REDUCED_HARDWARE 1
  #endif

+#ifdef CONFIG_ACPI_OS_NAME_LINUX
+#define ACPI_OS_NAME_USING_LINUX 1
+#endif
+
  #include <linux/string.h>
  #include <linux/kernel.h>
  #include <linux/ctype.h>