diff mbox

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

Message ID 54ABC2CB.6@linaro.org (mailing list archive)
State New, archived
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

Comments

Catalin Marinas Jan. 6, 2015, 11:29 a.m. UTC | #1
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

> +
>   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?

> >> 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?
Hanjun Guo Jan. 6, 2015, 1:50 p.m. UTC | #2
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
Graeme Gregory Jan. 6, 2015, 1:54 p.m. UTC | #3
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
Hanjun Guo Jan. 6, 2015, 1:59 p.m. UTC | #4
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
Arnd Bergmann Jan. 6, 2015, 2:05 p.m. UTC | #5
On Tuesday 06 January 2015 11:29:29 Catalin Marinas wrote:
> > >> 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?

How about just leaving it out? It's clearly not used for anything
good, so I don't see the point in passing either Linux or "Microsoft
Windows NT" here.

	Arnd
Catalin Marinas Jan. 6, 2015, 2:16 p.m. UTC | #6
On Tue, Jan 06, 2015 at 02:05:12PM +0000, Arnd Bergmann wrote:
> On Tuesday 06 January 2015 11:29:29 Catalin Marinas wrote:
> > > >> 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?
> 
> How about just leaving it out? It's clearly not used for anything
> good, so I don't see the point in passing either Linux or "Microsoft
> Windows NT" here.

Do you mean defining it to NULL (so it ends up as NULL in
acpi_gbl_pre_defined_names) or removing "_OS_" entirely from that array?
I really can't tell what the implications are.
Charles Garcia-Tobin Jan. 6, 2015, 2:37 p.m. UTC | #7
> -----Original Message-----
> From: Catalin Marinas [mailto:catalin.marinas@arm.com]
> Sent: 06 January 2015 14:17
> To: Arnd Bergmann
> Cc: linux-arm-kernel@lists.infradead.org; hanjun.guo@linaro.org; Mark
> Rutland; linaro-acpi@lists.linaro.org; Will Deacon; Lv Zheng; Rob
> Herring; Lorenzo Pieralisi; Al Stone; Daniel Lezcano; Robert Moore;
> linux-acpi@vger.kernel.org; jcm@redhat.com; grant.likely@linaro.org;
> Charles Garcia-Tobin; Robert Richter; Jason Cooper; Marc Zyngier; Liviu
> Dudau; Mark Brown; Bjorn Helgaas; graeme.gregory@linaro.org;
> Kangkang.Shen@huawei.com; Randy Dunlap; Rafael J. Wysocki; linux-
> kernel@vger.kernel.org; Sudeep Holla; Olof Johansson
> Subject: Re: [PATCH v5 18/18] Documentation: ACPI for ARM64
> 
> On Tue, Jan 06, 2015 at 02:05:12PM +0000, Arnd Bergmann wrote:
> > On Tuesday 06 January 2015 11:29:29 Catalin Marinas wrote:
> > > > >> 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?
> >
> > How about just leaving it out? It's clearly not used for anything
> > good, so I don't see the point in passing either Linux or "Microsoft
> > Windows NT" here.
> 
> Do you mean defining it to NULL (so it ends up as NULL in
> acpi_gbl_pre_defined_names) or removing "_OS_" entirely from that
> array?
> I really can't tell what the implications are.

To me, given that we don't want to use it in ARM64, it would make sense to
have some method to configurably:
0. Leave as is
1. Warn for usage
2. Panic
With a configurability method that allows FWTS to make use of it, and
therefore catch usages of the method.

Cheers

Charles


> 
> --
> Catalin
Jon Masters Jan. 6, 2015, 4:37 p.m. UTC | #8
On 01/06/2015 09:16 AM, Catalin Marinas wrote:
> On Tue, Jan 06, 2015 at 02:05:12PM +0000, Arnd Bergmann wrote:
>> On Tuesday 06 January 2015 11:29:29 Catalin Marinas wrote:
>>>>>> 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?
>>
>> How about just leaving it out? It's clearly not used for anything
>> good, so I don't see the point in passing either Linux or "Microsoft
>> Windows NT" here.
> 
> Do you mean defining it to NULL (so it ends up as NULL in
> acpi_gbl_pre_defined_names) or removing "_OS_" entirely from that array?
> I really can't tell what the implications are.

Setting it to NULL is almost certainly (99%+ likely IMO) going to result
in some firmware crashing nastily somewhere.

I think I've mentioned before that we're using the ACPI patches in an
early access program for partners that we announced last year. I've
pinged the team internally and asked that we swiftly change to use
whatever name is agreed in this thread so that we can get a lot of other
folks to begin testing with a correctly reported OS name.

Jon.
Arnd Bergmann Jan. 9, 2015, 11:12 p.m. UTC | #9
On Tuesday 06 January 2015 11:37:08 Jon Masters wrote:
> On 01/06/2015 09:16 AM, Catalin Marinas wrote:
> > On Tue, Jan 06, 2015 at 02:05:12PM +0000, Arnd Bergmann wrote:
> >> On Tuesday 06 January 2015 11:29:29 Catalin Marinas wrote:
> >>>>>> 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?
> >>
> >> How about just leaving it out? It's clearly not used for anything
> >> good, so I don't see the point in passing either Linux or "Microsoft
> >> Windows NT" here.
> > 
> > Do you mean defining it to NULL (so it ends up as NULL in
> > acpi_gbl_pre_defined_names) or removing "_OS_" entirely from that array?
> > I really can't tell what the implications are.
> 
> Setting it to NULL is almost certainly (99%+ likely IMO) going to result
> in some firmware crashing nastily somewhere.
> 
> I think I've mentioned before that we're using the ACPI patches in an
> early access program for partners that we announced last year. I've
> pinged the team internally and asked that we swiftly change to use
> whatever name is agreed in this thread so that we can get a lot of other
> folks to begin testing with a correctly reported OS name.

If we have to put something in there, I'd vote for keeping the current
string.  It's not clear whether Microsoft would follow the change to
use "Linux" as the _OS_ string, and we probably don't want them to
use incompatible IDs here.

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