RE: [PATCH 1/2] ACPI: Fix the issue that auto SSDT loading is conflict with customized SSDT that is included by the customized DSDT.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Rafael

> From: Rafael J. Wysocki [mailto:rjw@xxxxxxxxxxxxx]
> Sent: Wednesday, April 23, 2014 6:48 PM
> To: Zheng, Lv
> 
> On Wednesday, April 23, 2014 01:21:53 AM Zheng, Lv wrote:
> > Hi,
> >
> > > From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi-owner@xxxxxxxxxxxxxxx] On Behalf Of Rafael J. Wysocki
> > > Sent: Tuesday, April 22, 2014 7:20 PM
> > >
> > > On Tuesday, April 22, 2014 12:50:37 AM Zheng, Lv wrote:
> > > > Hi, Rafael
> > > >
> > > > > From: Rafael J. Wysocki [mailto:rjw@xxxxxxxxxxxxx]
> > > > > Sent: Tuesday, April 22, 2014 5:14 AM
> > > > >
> > > > > On Tuesday, April 08, 2014 10:49:06 AM Lv Zheng wrote:
> > > > > > This patch fixes the following issue:
> > > > > > User can specify a DSDT with SSDT embedded, in which case, no_static_ssdt
> > > > > > must be enforced.  If we don't do that, then:
> > > > > > 1. The namespace object conflicts will result in an AE_ALREADY_EXISTS
> > > > > >    exception;
> > > > > > 2. The namespace objects that are deleted from the original SSDT will be
> > > > > >    restored by the auto loading of the original SSDT.
> > > > > >
> > > > > > Note that the DSDT customization is a compile-time feature, thus the SSDT
> > > > > > inclusion indication of the DSDT customization is also implemented as a
> > > > > > compile-time configurable.
> > > > > >
> > > > > > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
> > > > > > Original-by: Enrico Etxe Arte <goitizena.generoa@xxxxxxxxx>
> > > > > > Signed-off-by: Lv Zheng <lv.zheng@xxxxxxxxx>
> > > > > > ---
> > > > > >  drivers/acpi/Kconfig    |    8 ++++++++
> > > > > >  drivers/acpi/internal.h |    5 +++++
> > > > > >  drivers/acpi/osl.c      |   14 +++++++++++++-
> > > > > >  drivers/acpi/tables.c   |    2 ++
> > > > > >  4 files changed, 28 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > > > > > index c205653..a0b8131 100644
> > > > > > --- a/drivers/acpi/Kconfig
> > > > > > +++ b/drivers/acpi/Kconfig
> > > > > > @@ -212,6 +212,14 @@ config ACPI_CUSTOM_DSDT
> > > > > >  	bool
> > > > > >  	default ACPI_CUSTOM_DSDT_FILE != ""
> > > > > >
> > > > > > +config ACPI_CUSTOM_SSDT_INCLUDED
> > > > > > +	bool "Custom SSDT Tables included"
> > > > > > +	depends on ACPI_CUSTOM_DSDT
> > > > > > +	help
> > > > > > +	  This option turns on acpi.no_static_ssdt by default.
> > > > > > +
> > > > > > +	  If the SSDT code has been merged into the custom DSDT file, say Y.
> > > > >
> > > > > I wonder what the benefit of having a separate .config option for that is?
> > > > >
> > > > > Can't we just use CONFIG_ACPI_CUSTOM_DSDT instead of CONFIG_ACPI_CUSTOM_SSDT_INCLUDED
> > > > > everywhere instead?
> > > > >
> > > >
> > > > It is a different use case.
> > > > Normally, ACPI_CUSTOM_DSDT only allows 1 table to be customized.
> > > > The original bug reporter has to customize the SSDT to make his platform bootable.
> > > > He achieves this by copying the entire SSDT into the DSDT and compile the DSDT
> > > > into a customized binary to be used for ACPI_CUSTOM_DSDT.
> > > > So user can:
> > > >
> > > > 1. customize DSDT without SSDTs copied into it;
> > > > 2. customize DSDT with SSDTs copied into it.
> > > >
> > > > The new config option is used for the new use case - case 2.
> > > > So it seems we can't just use CONFIG_ACPI_CUSTOM_DSDT.
> > > >
> > > > I think the naming of the new option might be confusing.
> > > > It  can be ACPI_CUSTOM_DSDT_WITH_SSDT to indicate the fact that
> > > > it is a sub-option that depends on the ACPI_CUSTOM_DSDT.
> > >
> > > Well, it looks like the solution would be to allow SSDTs and DSDTs to be
> > > customized independently, wouldn't it?
> >
> > First, I'm not talking about initrd table override.
> >
> > For the CONFIG_ACPI_CUSTOM_DSDT feature, I'm afraid the answer is SSDT cannot be independently customized without DSDT
> customization.
> 
> Is there any fundamental problem with that or is it just a limitation in the
> current code?

The SSDT override using DSDT is not a new feature, it is used along with the acpi.no_auto_ssdt boot parameter.
The use case is documented here:
https://01.org/zh/linux-acpi/documentation/overriding-dsdt?langredirect=1
Searching in the internet, there are also many posts from Linux contribution sites training bug reporters using this facility to perform an SSDT override to fix BIOS problems.

This patch is a useful fix for overriding SSDT using DSDT.
It can prevent ACPICA from:
1. generating unwelcomed AE_ALREADY_EXISTS or
2. executing load-time SSDT AML twice.
The latter issue could be a fundamental problem.
AML will be executed upon loading a table.  It will cause the load-time AML in SSDT being executed twice.
As the execution environment has been changed for the second load execution, there are possibilities to trigger wrong result.

Thanks and best regards
-Lv

> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f





[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux