Message-ID: <20150106112929.GB8829@e104818-lin.cambridge.arm.com>
Date:	Tue, 6 Jan 2015 11:29:29 +0000
From:	Catalin Marinas <catalin.marinas@....com>
To:	Hanjun Guo <hanjun.guo@...aro.org>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Mark Rutland <Mark.Rutland@....com>,
	Olof Johansson <olof@...om.net>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	Will Deacon <Will.Deacon@....com>,
	"graeme.gregory@...aro.org" <graeme.gregory@...aro.org>,
	Arnd Bergmann <arnd@...db.de>,
	Sudeep Holla <Sudeep.Holla@....com>,
	"jcm@...hat.com" <jcm@...hat.com>,
	Jason Cooper <jason@...edaemon.net>,
	Marc Zyngier <Marc.Zyngier@....com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
	Robert Richter <rric@...nel.org>,
	Lv Zheng <lv.zheng@...el.com>,
	Robert Moore <robert.moore@...el.com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
	Liviu Dudau <Liviu.Dudau@....com>,
	Randy Dunlap <rdunlap@...radead.org>,
	Charles Garcia-Tobin <Charles.Garcia-Tobin@....com>,
	"Kangkang.Shen@...wei.com" <Kangkang.Shen@...wei.com>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linaro-acpi@...ts.linaro.org" <linaro-acpi@...ts.linaro.org>,
	Al Stone <al.stone@...aro.org>
Subject: Re: [PATCH v5 18/18] Documentation: ACPI for ARM64

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?

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists