Re: [PATCH v5 18/18] Documentation: ACPI for ARM64

[Hanjun Guo]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/