[PATCH 0/2] ACPI: Fix issues related to the DSDT customization.

[PATCH 0/2] ACPI: Fix issues related to the DSDT customization.

[Lv Zheng]

It is reported that there are several issues related to the DSDT
custmization.  This patchset fixes such trivial issues.

References: https://bugzilla.kernel.org/show_bug.cgi?id=69711

Lv Zheng (2):
  ACPI: Fix the issue that auto SSDT loading is conflict with
    customized SSDT that is included by the customized DSDT.
  ACPI: Fix the issue that the DSDT local copy is conflict with the
    customized DSDT.

 drivers/acpi/Kconfig    |    8 ++++++++
 drivers/acpi/bus.c      |    9 ++++++++-
 drivers/acpi/internal.h |    5 +++++
 drivers/acpi/osl.c      |   14 +++++++++++++-
 drivers/acpi/tables.c   |    2 ++
 5 files changed, 36 insertions(+), 2 deletions(-)

-- 
1.7.10

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

[Lv Zheng]

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@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 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.
+
 config ACPI_INITRD_TABLE_OVERRIDE
 	bool "ACPI tables override via initrd"
 	depends on BLK_DEV_INITRD && X86
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 9573913..0d127ab 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -57,6 +57,11 @@ void acpi_cmos_rtc_init(void);
 #else
 static inline void acpi_cmos_rtc_init(void) {}
 #endif
+#ifdef CONFIG_ACPI_CUSTOM_SSDT_INCLUDED
+void __init acpi_ssdt_customized(void);
+#else
+static inline void acpi_ssdt_customized(void) {}
+#endif
 
 extern bool acpi_force_hot_remove;
 
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 9aeae41..273b69a 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -1770,15 +1770,27 @@ acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object)
 }
 #endif
 
-static int __init acpi_no_static_ssdt_setup(char *s)
+static void __init acpi_set_no_static_ssdt(void)
 {
 	acpi_gbl_disable_ssdt_table_install = TRUE;
 	pr_info("ACPI: static SSDT installation disabled\n");
+}
+
+#ifdef CONFIG_ACPI_CUSTOM_SSDT_INCLUDED
+void __init acpi_ssdt_customized(void)
+{
+	acpi_set_no_static_ssdt();
+}
+#else
+static int __init acpi_no_static_ssdt_setup(char *s)
+{
+	acpi_set_no_static_ssdt();
 
 	return 0;
 }
 
 early_param("acpi_no_static_ssdt", acpi_no_static_ssdt_setup);
+#endif
 
 static int __init acpi_disable_return_repair(char *s)
 {
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 2178229..aaf2177 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -184,6 +184,7 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
 	}
 }
 
+#include "internal.h"
 
 int __init
 acpi_table_parse_entries(char *id,
@@ -333,6 +334,7 @@ int __init acpi_table_init(void)
 {
 	acpi_status status;
 
+	acpi_ssdt_customized();
 	status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
 	if (ACPI_FAILURE(status))
 		return -EINVAL;
-- 
1.7.10

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

[Rafael J. Wysocki]

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@gmail.com>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> ---
>  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?


> +
>  config ACPI_INITRD_TABLE_OVERRIDE
>  	bool "ACPI tables override via initrd"
>  	depends on BLK_DEV_INITRD && X86
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 9573913..0d127ab 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -57,6 +57,11 @@ void acpi_cmos_rtc_init(void);
>  #else
>  static inline void acpi_cmos_rtc_init(void) {}
>  #endif
> +#ifdef CONFIG_ACPI_CUSTOM_SSDT_INCLUDED
> +void __init acpi_ssdt_customized(void);
> +#else
> +static inline void acpi_ssdt_customized(void) {}
> +#endif
>  
>  extern bool acpi_force_hot_remove;
>  
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 9aeae41..273b69a 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -1770,15 +1770,27 @@ acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object)
>  }
>  #endif
>  
> -static int __init acpi_no_static_ssdt_setup(char *s)
> +static void __init acpi_set_no_static_ssdt(void)
>  {
>  	acpi_gbl_disable_ssdt_table_install = TRUE;
>  	pr_info("ACPI: static SSDT installation disabled\n");
> +}
> +
> +#ifdef CONFIG_ACPI_CUSTOM_SSDT_INCLUDED
> +void __init acpi_ssdt_customized(void)
> +{
> +	acpi_set_no_static_ssdt();
> +}
> +#else
> +static int __init acpi_no_static_ssdt_setup(char *s)
> +{
> +	acpi_set_no_static_ssdt();
>  
>  	return 0;
>  }
>  
>  early_param("acpi_no_static_ssdt", acpi_no_static_ssdt_setup);
> +#endif
>  
>  static int __init acpi_disable_return_repair(char *s)
>  {
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index 2178229..aaf2177 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -184,6 +184,7 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
>  	}
>  }
>  
> +#include "internal.h"
>  
>  int __init
>  acpi_table_parse_entries(char *id,
> @@ -333,6 +334,7 @@ int __init acpi_table_init(void)
>  {
>  	acpi_status status;
>  
> +	acpi_ssdt_customized();
>  	status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
>  	if (ACPI_FAILURE(status))
>  		return -EINVAL;
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

[Zheng, Lv]

Hi, Rafael

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> 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@gmail.com>
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > ---
> >  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.

Thanks and best regards
-Lv

> 
> > +
> >  config ACPI_INITRD_TABLE_OVERRIDE
> >  	bool "ACPI tables override via initrd"
> >  	depends on BLK_DEV_INITRD && X86
> > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> > index 9573913..0d127ab 100644
> > --- a/drivers/acpi/internal.h
> > +++ b/drivers/acpi/internal.h
> > @@ -57,6 +57,11 @@ void acpi_cmos_rtc_init(void);
> >  #else
> >  static inline void acpi_cmos_rtc_init(void) {}
> >  #endif
> > +#ifdef CONFIG_ACPI_CUSTOM_SSDT_INCLUDED
> > +void __init acpi_ssdt_customized(void);
> > +#else
> > +static inline void acpi_ssdt_customized(void) {}
> > +#endif
> >
> >  extern bool acpi_force_hot_remove;
> >
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index 9aeae41..273b69a 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -1770,15 +1770,27 @@ acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object)
> >  }
> >  #endif
> >
> > -static int __init acpi_no_static_ssdt_setup(char *s)
> > +static void __init acpi_set_no_static_ssdt(void)
> >  {
> >  	acpi_gbl_disable_ssdt_table_install = TRUE;
> >  	pr_info("ACPI: static SSDT installation disabled\n");
> > +}
> > +
> > +#ifdef CONFIG_ACPI_CUSTOM_SSDT_INCLUDED
> > +void __init acpi_ssdt_customized(void)
> > +{
> > +	acpi_set_no_static_ssdt();
> > +}
> > +#else
> > +static int __init acpi_no_static_ssdt_setup(char *s)
> > +{
> > +	acpi_set_no_static_ssdt();
> >
> >  	return 0;
> >  }
> >
> >  early_param("acpi_no_static_ssdt", acpi_no_static_ssdt_setup);
> > +#endif
> >
> >  static int __init acpi_disable_return_repair(char *s)
> >  {
> > diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> > index 2178229..aaf2177 100644
> > --- a/drivers/acpi/tables.c
> > +++ b/drivers/acpi/tables.c
> > @@ -184,6 +184,7 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
> >  	}
> >  }
> >
> > +#include "internal.h"
> >
> >  int __init
> >  acpi_table_parse_entries(char *id,
> > @@ -333,6 +334,7 @@ int __init acpi_table_init(void)
> >  {
> >  	acpi_status status;
> >
> > +	acpi_ssdt_customized();
> >  	status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
> >  	if (ACPI_FAILURE(status))
> >  		return -EINVAL;
> >
> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

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

[Rafael J. Wysocki]

On Tuesday, April 22, 2014 12:50:37 AM Zheng, Lv wrote:
> Hi, Rafael
> 
> > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > 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@gmail.com>
> > > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > > ---
> > >  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?

So that the user in question actually can leave his DSDT as is and customize
the SSDT only, no?

Rafael

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

[Zheng, Lv]

Hi,

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] 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@rjwysocki.net]
> > > 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@gmail.com>
> > > > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > > > ---
> > > >  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.
The SSDT that the user wants to override should be embedded into the CONFIG_ACPI_CUSTOM_DSDT_FILE.

Let me show you the related code below.

drivers/acpi/osl.c:

#ifdef CONFIG_ACPI_CUSTOM_DSDT
#include CONFIG_ACPI_CUSTOM_DSDT_FILE
#endif
Note this file will contain an output of iasl -tc dsdt.asl, which defines a byte array named as AmlCode.

#ifdef CONFIG_ACPI_CUSTOM_DSDT
	if (strncmp(existing_table->signature, "DSDT", 4) == 0)
		*new_table = (struct acpi_table_header *)AmlCode;
#endif
Only DSDT signature can trigger override CONFIG_ACPI_CUSTOM_DSDT.

So this new use case currently depends on CONFIG_ACPI_CUSTOM_DSDT.

> So that the user in question actually can leave his DSDT as is and customize
> the SSDT only, no?

So the answer is no here.

Thanks and best regards
-Lv

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

[Rafael J. Wysocki]

On Wednesday, April 23, 2014 01:21:53 AM Zheng, Lv wrote:
> Hi,
> 
> > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] 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@rjwysocki.net]
> > > > 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@gmail.com>
> > > > > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > > > > ---
> > > > >  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?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

[Zheng, Lv]

Hi, Rafael

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> 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@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] 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@rjwysocki.net]
> > > > > 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@gmail.com>
> > > > > > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > > > > > ---
> > > > > >  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.

[PATCH 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.

[Lv Zheng]

This patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
---
 drivers/acpi/bus.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index e7e5844..c113e82 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -52,13 +52,20 @@ struct proc_dir_entry *acpi_root_dir;
 EXPORT_SYMBOL(acpi_root_dir);
 
 #ifdef CONFIG_X86
+#ifndef CONFIG_ACPI_CUSTOM_DSDT
 static int set_copy_dsdt(const struct dmi_system_id *id)
 {
-	printk(KERN_NOTICE "%s detected - "
+	pr_info("ACPI: %s detected - "
 		"force copy of DSDT to local memory\n", id->ident);
 	acpi_gbl_copy_dsdt_locally = 1;
 	return 0;
 }
+#else
+static inline int set_copy_dsdt(const struct dmi_system_id *id)
+{
+	return 0;
+}
+#endif
 
 static struct dmi_system_id dsdt_dmi_table[] __initdata = {
 	/*
-- 
1.7.10

Re: [PATCH 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.

[Rafael J. Wysocki]

On Tuesday, April 08, 2014 10:49:12 AM Lv Zheng wrote:
> This patch fixes the following issue:
> If DSDT is customized, no local DSDT copy is needed.
> 
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Signed-off-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
> ---
>  drivers/acpi/bus.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index e7e5844..c113e82 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -52,13 +52,20 @@ struct proc_dir_entry *acpi_root_dir;
>  EXPORT_SYMBOL(acpi_root_dir);
>  
>  #ifdef CONFIG_X86
> +#ifndef CONFIG_ACPI_CUSTOM_DSDT

Can you please use #ifdef instead of #ifndef?

So do

#ifdef CONFIG_ACPI_CUSTOM_DSDT
static inline int set_copy_dsdt(const struct dmi_system_id *id) { return 0; }
#else

etc.

>  static int set_copy_dsdt(const struct dmi_system_id *id)
>  {
> -	printk(KERN_NOTICE "%s detected - "
> +	pr_info("ACPI: %s detected - "

And how is this change related to the rest of the patch?

>  		"force copy of DSDT to local memory\n", id->ident);
>  	acpi_gbl_copy_dsdt_locally = 1;
>  	return 0;
>  }
> +#else
> +static inline int set_copy_dsdt(const struct dmi_system_id *id)
> +{
> +	return 0;
> +}
> +#endif
>  
>  static struct dmi_system_id dsdt_dmi_table[] __initdata = {
>  	/*
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

RE: [PATCH 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.

[Zheng, Lv]

Hi, Rafael

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Tuesday, April 22, 2014 5:16 AM
> 
> On Tuesday, April 08, 2014 10:49:12 AM Lv Zheng wrote:
> > This patch fixes the following issue:
> > If DSDT is customized, no local DSDT copy is needed.
> >
> > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > Signed-off-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
> > ---
> >  drivers/acpi/bus.c |    9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index e7e5844..c113e82 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -52,13 +52,20 @@ struct proc_dir_entry *acpi_root_dir;
> >  EXPORT_SYMBOL(acpi_root_dir);
> >
> >  #ifdef CONFIG_X86
> > +#ifndef CONFIG_ACPI_CUSTOM_DSDT
> 
> Can you please use #ifdef instead of #ifndef?
> 
> So do
> 
> #ifdef CONFIG_ACPI_CUSTOM_DSDT
> static inline int set_copy_dsdt(const struct dmi_system_id *id) { return 0; }
> #else
> 
> etc.

OK.

> 
> >  static int set_copy_dsdt(const struct dmi_system_id *id)
> >  {
> > -	printk(KERN_NOTICE "%s detected - "
> > +	pr_info("ACPI: %s detected - "
> 
> And how is this change related to the rest of the patch?

It doesn't seem to be a part of this fix.
OK, I'll remove it.

Thanks and best regards
-Lv

> 
> >  		"force copy of DSDT to local memory\n", id->ident);
> >  	acpi_gbl_copy_dsdt_locally = 1;
> >  	return 0;
> >  }
> > +#else
> > +static inline int set_copy_dsdt(const struct dmi_system_id *id)
> > +{
> > +	return 0;
> > +}
> > +#endif
> >
> >  static struct dmi_system_id dsdt_dmi_table[] __initdata = {
> >  	/*
> >
> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

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

[Lv Zheng]

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@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 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 c0160bb..6fef394 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -233,6 +233,14 @@ config ACPI_CUSTOM_DSDT
 	bool
 	default ACPI_CUSTOM_DSDT_FILE != ""
 
+config ACPI_CUSTOM_DSDT_WITH_SSDT
+	bool "Customize DSDT with SSDT tables included"
+	depends on ACPI_CUSTOM_DSDT
+	help
+	  This option turns on acpi.no_static_ssdt by default.
+
+	  If the SSDT tables has been merged into the custom DSDT file, say Y.
+
 config ACPI_INITRD_TABLE_OVERRIDE
 	bool "ACPI tables override via initrd"
 	depends on BLK_DEV_INITRD && X86
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 9573913..f84ef59 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -57,6 +57,11 @@ void acpi_cmos_rtc_init(void);
 #else
 static inline void acpi_cmos_rtc_init(void) {}
 #endif
+#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
+void __init acpi_ssdt_customized(void);
+#else
+static inline void acpi_ssdt_customized(void) {}
+#endif
 
 extern bool acpi_force_hot_remove;
 
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 9aeae41..089e55d 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -1770,15 +1770,27 @@ acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object)
 }
 #endif
 
-static int __init acpi_no_static_ssdt_setup(char *s)
+static void __init acpi_set_no_static_ssdt(void)
 {
 	acpi_gbl_disable_ssdt_table_install = TRUE;
 	pr_info("ACPI: static SSDT installation disabled\n");
+}
+
+#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
+void __init acpi_ssdt_customized(void)
+{
+	acpi_set_no_static_ssdt();
+}
+#else
+static int __init acpi_no_static_ssdt_setup(char *s)
+{
+	acpi_set_no_static_ssdt();
 
 	return 0;
 }
 
 early_param("acpi_no_static_ssdt", acpi_no_static_ssdt_setup);
+#endif
 
 static int __init acpi_disable_return_repair(char *s)
 {
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 2178229..aaf2177 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -184,6 +184,7 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
 	}
 }
 
+#include "internal.h"
 
 int __init
 acpi_table_parse_entries(char *id,
@@ -333,6 +334,7 @@ int __init acpi_table_init(void)
 {
 	acpi_status status;
 
+	acpi_ssdt_customized();
 	status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
 	if (ACPI_FAILURE(status))
 		return -EINVAL;
-- 
1.7.10

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

[Rafael J. Wysocki]

On 5/9/2014 10:44 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@gmail.com>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> ---
>   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 c0160bb..6fef394 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -233,6 +233,14 @@ config ACPI_CUSTOM_DSDT
>   	bool
>   	default ACPI_CUSTOM_DSDT_FILE != ""
>   
> +config ACPI_CUSTOM_DSDT_WITH_SSDT
> +	bool "Customize DSDT with SSDT tables included"
> +	depends on ACPI_CUSTOM_DSDT

If you think that we really have to have a separate CONFIG_ option here, 
I would prefer ACPI_CUSTOM_DSDT to become a menu with a choice to build 
it either with or without embedded SSDTs.

> +	help
> +	  This option turns on acpi.no_static_ssdt by default.

This description requires the user building the kernel to know what the 
above command line option means which isn't very useful.

> +
> +	  If the SSDT tables has been merged into the custom DSDT file, say Y.
> +
>   config ACPI_INITRD_TABLE_OVERRIDE
>   	bool "ACPI tables override via initrd"
>   	depends on BLK_DEV_INITRD && X86
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 9573913..f84ef59 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -57,6 +57,11 @@ void acpi_cmos_rtc_init(void);
>   #else
>   static inline void acpi_cmos_rtc_init(void) {}
>   #endif
> +#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
> +void __init acpi_ssdt_customized(void);
> +#else
> +static inline void acpi_ssdt_customized(void) {}
> +#endif
>   
>   extern bool acpi_force_hot_remove;
>   
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 9aeae41..089e55d 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -1770,15 +1770,27 @@ acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object)
>   }
>   #endif
>   
> -static int __init acpi_no_static_ssdt_setup(char *s)
> +static void __init acpi_set_no_static_ssdt(void)
>   {
>   	acpi_gbl_disable_ssdt_table_install = TRUE;
>   	pr_info("ACPI: static SSDT installation disabled\n");
> +}
> +
> +#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
> +void __init acpi_ssdt_customized(void)
> +{
> +	acpi_set_no_static_ssdt();
> +}
> +#else
> +static int __init acpi_no_static_ssdt_setup(char *s)
> +{
> +	acpi_set_no_static_ssdt();
>   
>   	return 0;
>   }
>   
>   early_param("acpi_no_static_ssdt", acpi_no_static_ssdt_setup);
> +#endif
>   
>   static int __init acpi_disable_return_repair(char *s)
>   {
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index 2178229..aaf2177 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -184,6 +184,7 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
>   	}
>   }
>   
> +#include "internal.h"
>   
>   int __init
>   acpi_table_parse_entries(char *id,
> @@ -333,6 +334,7 @@ int __init acpi_table_init(void)
>   {
>   	acpi_status status;
>   
> +	acpi_ssdt_customized();
>   	status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
>   	if (ACPI_FAILURE(status))
>   		return -EINVAL;

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

[Lv Zheng]

Hi, Rafael

Thanks for the review.

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Sent: Friday, May 09, 2014 6:40 PM


> On 5/9/2014 10:44 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@gmail.com>
>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> ---
>>   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 c0160bb..6fef394 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -233,6 +233,14 @@ config ACPI_CUSTOM_DSDT
>>   bool
>>   default ACPI_CUSTOM_DSDT_FILE != ""
>>   +config ACPI_CUSTOM_DSDT_WITH_SSDT
>> + bool "Customize DSDT with SSDT tables included"
>> + depends on ACPI_CUSTOM_DSDT
>
> If you think that we really have to have a separate CONFIG_ option here, I
> would prefer ACPI_CUSTOM_DSDT to become a menu with a choice to build it
> either with or without embedded SSDTs.
>

Ah! I see what you meant now.
The new config should be put below the ACPI_CUSTOM_DSDT_FILE, while
depending on ACPI_CUSTOM_DSDT cannot achieve this.
OK. I'll try to find a way.
Besides, with the cleanup of ACPICA table manager, now it is possible to
merge acpi_os_physical_table_override() with acpi_os_table_override().
So we could have a menu and put the both of the override mechanisms together
under it.

>> + help
>> +   This option turns on acpi.no_static_ssdt by default.
>
> This description requires the user building the kernel to know what the
> above command line option means which isn't very useful.

This is from the original patch.  Let me purify it.
And this comment remindes me that there is one more cleanup I need to do.

The acpi.no_static_ssdt was created for only this use case:
http://bugzilla.kernel.org/show_bug.cgi?id=3774
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=67effe8f

And over so long time, there is no situation other than this to use this
boot parameter.
The use case is a compile-time behavior.
So IMO the runtime kernel parameter should be deleted after introducing this
new kconfig item.

Thanks and best regards
-Lv

>
>> +
>> +   If the SSDT tables has been merged into the custom DSDT file, say Y.
>> +
>>   config ACPI_INITRD_TABLE_OVERRIDE
>>   bool "ACPI tables override via initrd"
>>   depends on BLK_DEV_INITRD && X86
>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>> index 9573913..f84ef59 100644
>> --- a/drivers/acpi/internal.h
>> +++ b/drivers/acpi/internal.h
>> @@ -57,6 +57,11 @@ void acpi_cmos_rtc_init(void);
>>   #else
>>   static inline void acpi_cmos_rtc_init(void) {}
>>   #endif
>> +#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
>> +void __init acpi_ssdt_customized(void);
>> +#else
>> +static inline void acpi_ssdt_customized(void) {}
>> +#endif
>>   extern bool acpi_force_hot_remove;
>>   diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> index 9aeae41..089e55d 100644
>> --- a/drivers/acpi/osl.c
>> +++ b/drivers/acpi/osl.c
>> @@ -1770,15 +1770,27 @@ acpi_status acpi_os_release_object(acpi_cache_t *
>> cache, void *object)
>>   }
>>   #endif
>>   -static int __init acpi_no_static_ssdt_setup(char *s)
>> +static void __init acpi_set_no_static_ssdt(void)
>>   {
>>   acpi_gbl_disable_ssdt_table_install = TRUE;
>>   pr_info("ACPI: static SSDT installation disabled\n");
>> +}
>> +
>> +#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
>> +void __init acpi_ssdt_customized(void)
>> +{
>> + acpi_set_no_static_ssdt();
>> +}
>> +#else
>> +static int __init acpi_no_static_ssdt_setup(char *s)
>> +{
>> + acpi_set_no_static_ssdt();
>>   return 0;
>>   }
>>   early_param("acpi_no_static_ssdt", acpi_no_static_ssdt_setup);
>> +#endif
>>   static int __init acpi_disable_return_repair(char *s)
>>   {
>> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
>> index 2178229..aaf2177 100644
>> --- a/drivers/acpi/tables.c
>> +++ b/drivers/acpi/tables.c
>> @@ -184,6 +184,7 @@ void acpi_table_print_madt_entry(struct
>> acpi_subtable_header *header)
>>   }
>>   }
>>   +#include "internal.h"
>>   int __init
>>   acpi_table_parse_entries(char *id,
>> @@ -333,6 +334,7 @@ int __init acpi_table_init(void)
>>   {
>>   acpi_status status;
>>   + acpi_ssdt_customized();
>>   status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
>>   if (ACPI_FAILURE(status))
>>   return -EINVAL;
>

[PATCH v2 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.

[Lv Zheng]

This patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
---
 drivers/acpi/bus.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 610261a..cfd2318 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -52,6 +52,12 @@ struct proc_dir_entry *acpi_root_dir;
 EXPORT_SYMBOL(acpi_root_dir);
 
 #ifdef CONFIG_X86
+#ifdef CONFIG_ACPI_CUSTOM_DSDT
+static inline int set_copy_dsdt(const struct dmi_system_id *id)
+{
+	return 0;
+}
+#else
 static int set_copy_dsdt(const struct dmi_system_id *id)
 {
 	printk(KERN_NOTICE "%s detected - "
@@ -59,6 +65,7 @@ static int set_copy_dsdt(const struct dmi_system_id *id)
 	acpi_gbl_copy_dsdt_locally = 1;
 	return 0;
 }
+#endif
 
 static struct dmi_system_id dsdt_dmi_table[] __initdata = {
 	/*
-- 
1.7.10

Re: [PATCH v2 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.

[Rafael J. Wysocki]

On Friday, May 09, 2014 04:44:19 PM Lv Zheng wrote:
> This patch fixes the following issue:
> If DSDT is customized, no local DSDT copy is needed.
> 
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Signed-off-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>

This can go in without [1/2], right?

If so, do we need it in -stable?

> ---
>  drivers/acpi/bus.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 610261a..cfd2318 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -52,6 +52,12 @@ struct proc_dir_entry *acpi_root_dir;
>  EXPORT_SYMBOL(acpi_root_dir);
>  
>  #ifdef CONFIG_X86
> +#ifdef CONFIG_ACPI_CUSTOM_DSDT
> +static inline int set_copy_dsdt(const struct dmi_system_id *id)
> +{
> +	return 0;
> +}
> +#else
>  static int set_copy_dsdt(const struct dmi_system_id *id)
>  {
>  	printk(KERN_NOTICE "%s detected - "
> @@ -59,6 +65,7 @@ static int set_copy_dsdt(const struct dmi_system_id *id)
>  	acpi_gbl_copy_dsdt_locally = 1;
>  	return 0;
>  }
> +#endif
>  
>  static struct dmi_system_id dsdt_dmi_table[] __initdata = {
>  	/*
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

RE: [PATCH v2 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.

[Zheng, Lv]

Hi, Rafael

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Monday, May 12, 2014 8:29 AM
> 
> On Friday, May 09, 2014 04:44:19 PM Lv Zheng wrote:
> > This patch fixes the following issue:
> > If DSDT is customized, no local DSDT copy is needed.
> >
> > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > Signed-off-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
> 
> This can go in without [1/2], right?

Yes.

> If so, do we need it in -stable?

I'm not sure.
According to the report, the bug fix is for a special use case.
Users still can have a running Linux by using initrd SSDT customization.
If you think it is a stable material, the stable tag should be:
CC: <stable@vger.kernel.org> # 2.6.35.x: aa2110cb: ACPI: add boot option acpi=copy_dsdt to fix corrupt DSDT

Thanks and best regards
-Lv

> 
> > ---
> >  drivers/acpi/bus.c |    7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 610261a..cfd2318 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -52,6 +52,12 @@ struct proc_dir_entry *acpi_root_dir;
> >  EXPORT_SYMBOL(acpi_root_dir);
> >
> >  #ifdef CONFIG_X86
> > +#ifdef CONFIG_ACPI_CUSTOM_DSDT
> > +static inline int set_copy_dsdt(const struct dmi_system_id *id)
> > +{
> > +	return 0;
> > +}
> > +#else
> >  static int set_copy_dsdt(const struct dmi_system_id *id)
> >  {
> >  	printk(KERN_NOTICE "%s detected - "
> > @@ -59,6 +65,7 @@ static int set_copy_dsdt(const struct dmi_system_id *id)
> >  	acpi_gbl_copy_dsdt_locally = 1;
> >  	return 0;
> >  }
> > +#endif
> >
> >  static struct dmi_system_id dsdt_dmi_table[] __initdata = {
> >  	/*
> >
> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH v3 1/2] ACPI: Cleanup to convert acpi.no_static_ssdt into a compile-time configurable.

[Lv Zheng]

User can specify a DSDT with SSDT embedded, in which case, no_static_ssdt
must be enforced during compile-time.  If we don't do that and forget to
specify acpi.no_static_ssdt as a boot parameter, then:
1. The AML executable will be executed twice, the second execution is
   running under a changed environment.
2. The namespace object conflicts will result in an AE_ALREADY_EXISTS
   exception;
3. The namespace objects that are deleted from the original SSDT will be
   restored by the auto loading of the original SSDT.

Note that:
1. The DSDT customization is a compile-time feature, thus the SSDT
   inclusion indication of the DSDT customization should also be
   implemented as a compile-time configurable.
2. According to the commit log:
    Commit: 67effe8fff32f60bdf51cba484766ba6003005bb
    Subject: ACPI: add "acpi_no_auto_ssdt" bootparam
   The acpi.no_static_ssdt (originally acpi.no_auto_ssdt) was introduced to
   be used for this use case.  And over a long time, there is no other
   situation than this requires this boot parameter.
This patch thus converts the runtime kernel parameter into the compile-time
configuration item.

References: https://bugzilla.kernel.org/show_bug.cgi?id=3774
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
Original-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 Documentation/kernel-parameters.txt |   10 ----------
 drivers/acpi/Kconfig                |   32 ++++++++++++++++++++++++++------
 drivers/acpi/internal.h             |    5 +++++
 drivers/acpi/osl.c                  |    8 +++-----
 drivers/acpi/tables.c               |    2 ++
 5 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 831fb1e..ceba0d6 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -237,16 +237,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			This feature is enabled by default.
 			This option allows to turn off the feature.
 
-	acpi_no_static_ssdt	[HW,ACPI]
-			Disable installation of static SSDTs at early boot time
-			By default, SSDTs contained in the RSDT/XSDT will be
-			installed automatically and they will appear under
-			/sys/firmware/acpi/tables.
-			This option turns off this feature.
-			Note that specifying this option does not affect
-			dynamic table installation which will install SSDT
-			tables to /sys/firmware/acpi/tables/dynamic.
-
 	acpica_no_return_repair [HW, ACPI]
 			Disable AML predefined validation mechanism
 			This mechanism can repair the evaluation result to make
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index c0160bb..51dc3b8 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -216,22 +216,40 @@ config ACPI_NUMA
 	depends on (X86 || IA64)
 	default y if IA64_GENERIC || IA64_SGI_SN2
 
-config ACPI_CUSTOM_DSDT_FILE
-	string "Custom DSDT Table file to include"
-	default ""
+menu "Table override"
+
+menuconfig ACPI_CUSTOM_DSDT
+	bool "Customize DSDT"
 	depends on !STANDALONE
 	help
 	  This option supports a custom DSDT by linking it into the kernel.
 	  See Documentation/acpi/dsdt-override.txt
 
+	  If unsure, say N.
+
+if ACPI_CUSTOM_DSDT
+
+config ACPI_CUSTOM_DSDT_FILE
+	string "Custom DSDT Table file to include"
+	default ""
+	help
 	  Enter the full path name to the file which includes the AmlCode
 	  declaration.
 
 	  If unsure, don't enter a file name.
 
-config ACPI_CUSTOM_DSDT
-	bool
-	default ACPI_CUSTOM_DSDT_FILE != ""
+config ACPI_CUSTOM_DSDT_WITH_SSDT
+	bool "Customize DSDT with static SSDTs included"
+	depends on ACPI_CUSTOM_DSDT
+	help
+	  This option allows a DSDT override to optionally be a DSDT+SSDT
+	  override.  In which case, Linux should stop loading static SSDTs
+          from RSDT/XSDT.  Otherwise, the load time AML execution of the
+	  SSDTs will be errornously executed twice.
+
+	  If unsure, say N.
+
+endif
 
 config ACPI_INITRD_TABLE_OVERRIDE
 	bool "ACPI tables override via initrd"
@@ -243,6 +261,8 @@ config ACPI_INITRD_TABLE_OVERRIDE
 	  initrd, therefore it's safe to say Y.
 	  See Documentation/acpi/initrd_table_override.txt for details
 
+endmenu
+
 config ACPI_DEBUG
 	bool "Debug Statements"
 	default n
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 9573913..f84ef59 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -57,6 +57,11 @@ void acpi_cmos_rtc_init(void);
 #else
 static inline void acpi_cmos_rtc_init(void) {}
 #endif
+#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
+void __init acpi_ssdt_customized(void);
+#else
+static inline void acpi_ssdt_customized(void) {}
+#endif
 
 extern bool acpi_force_hot_remove;
 
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 9aeae41..c6024c2 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -1770,15 +1770,13 @@ acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object)
 }
 #endif
 
-static int __init acpi_no_static_ssdt_setup(char *s)
+#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
+void __init acpi_ssdt_customized(void)
 {
 	acpi_gbl_disable_ssdt_table_install = TRUE;
 	pr_info("ACPI: static SSDT installation disabled\n");
-
-	return 0;
 }
-
-early_param("acpi_no_static_ssdt", acpi_no_static_ssdt_setup);
+#endif
 
 static int __init acpi_disable_return_repair(char *s)
 {
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 2178229..aaf2177 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -184,6 +184,7 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
 	}
 }
 
+#include "internal.h"
 
 int __init
 acpi_table_parse_entries(char *id,
@@ -333,6 +334,7 @@ int __init acpi_table_init(void)
 {
 	acpi_status status;
 
+	acpi_ssdt_customized();
 	status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
 	if (ACPI_FAILURE(status))
 		return -EINVAL;
-- 
1.7.10

Re: [PATCH v3 1/2] ACPI: Cleanup to convert acpi.no_static_ssdt into a compile-time configurable.

[Rafael J. Wysocki]

On Monday, May 12, 2014 03:49:59 PM Lv Zheng wrote:
> User can specify a DSDT with SSDT embedded, in which case, no_static_ssdt
> must be enforced during compile-time.  If we don't do that and forget to
> specify acpi.no_static_ssdt as a boot parameter, then:
> 1. The AML executable will be executed twice, the second execution is
>    running under a changed environment.
> 2. The namespace object conflicts will result in an AE_ALREADY_EXISTS
>    exception;
> 3. The namespace objects that are deleted from the original SSDT will be
>    restored by the auto loading of the original SSDT.
> 
> Note that:
> 1. The DSDT customization is a compile-time feature, thus the SSDT
>    inclusion indication of the DSDT customization should also be
>    implemented as a compile-time configurable.
> 2. According to the commit log:
>     Commit: 67effe8fff32f60bdf51cba484766ba6003005bb
>     Subject: ACPI: add "acpi_no_auto_ssdt" bootparam
>    The acpi.no_static_ssdt (originally acpi.no_auto_ssdt) was introduced to
>    be used for this use case.  And over a long time, there is no other
>    situation than this requires this boot parameter.
> This patch thus converts the runtime kernel parameter into the compile-time
> configuration item.
> 
> References: https://bugzilla.kernel.org/show_bug.cgi?id=3774
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
> Original-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> ---
>  Documentation/kernel-parameters.txt |   10 ----------
>  drivers/acpi/Kconfig                |   32 ++++++++++++++++++++++++++------
>  drivers/acpi/internal.h             |    5 +++++
>  drivers/acpi/osl.c                  |    8 +++-----
>  drivers/acpi/tables.c               |    2 ++
>  5 files changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 831fb1e..ceba0d6 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -237,16 +237,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			This feature is enabled by default.
>  			This option allows to turn off the feature.
>  
> -	acpi_no_static_ssdt	[HW,ACPI]
> -			Disable installation of static SSDTs at early boot time
> -			By default, SSDTs contained in the RSDT/XSDT will be
> -			installed automatically and they will appear under
> -			/sys/firmware/acpi/tables.
> -			This option turns off this feature.
> -			Note that specifying this option does not affect
> -			dynamic table installation which will install SSDT
> -			tables to /sys/firmware/acpi/tables/dynamic.
> -
>  	acpica_no_return_repair [HW, ACPI]
>  			Disable AML predefined validation mechanism
>  			This mechanism can repair the evaluation result to make
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index c0160bb..51dc3b8 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -216,22 +216,40 @@ config ACPI_NUMA
>  	depends on (X86 || IA64)
>  	default y if IA64_GENERIC || IA64_SGI_SN2
>  
> -config ACPI_CUSTOM_DSDT_FILE
> -	string "Custom DSDT Table file to include"
> -	default ""
> +menu "Table override"
> +
> +menuconfig ACPI_CUSTOM_DSDT

I would make this a choice, like the "Kernel compression mode" (in init/Kconfig).

Then ACPI_CUSTOM_DSDT_ONLY and ACPI_CUSTOM_DSDT_WITH_SSDT may be the options
and they both would select ACPI_CUSTOM_DSDT (otherwise invisible to the user), and ->

> +	bool "Customize DSDT"
>  	depends on !STANDALONE
>  	help
>  	  This option supports a custom DSDT by linking it into the kernel.
>  	  See Documentation/acpi/dsdt-override.txt
>  
> +	  If unsure, say N.
> +
> +if ACPI_CUSTOM_DSDT
> +
> +config ACPI_CUSTOM_DSDT_FILE

-> this would work for both.

Would that make sense?

> +	string "Custom DSDT Table file to include"
> +	default ""
> +	help
>  	  Enter the full path name to the file which includes the AmlCode
>  	  declaration.
>  
>  	  If unsure, don't enter a file name.
>  
> -config ACPI_CUSTOM_DSDT
> -	bool
> -	default ACPI_CUSTOM_DSDT_FILE != ""
> +config ACPI_CUSTOM_DSDT_WITH_SSDT
> +	bool "Customize DSDT with static SSDTs included"
> +	depends on ACPI_CUSTOM_DSDT
> +	help
> +	  This option allows a DSDT override to optionally be a DSDT+SSDT
> +	  override.  In which case, Linux should stop loading static SSDTs
> +          from RSDT/XSDT.  Otherwise, the load time AML execution of the
> +	  SSDTs will be errornously executed twice.
> +
> +	  If unsure, say N.
> +
> +endif
>  
>  config ACPI_INITRD_TABLE_OVERRIDE
>  	bool "ACPI tables override via initrd"
> @@ -243,6 +261,8 @@ config ACPI_INITRD_TABLE_OVERRIDE
>  	  initrd, therefore it's safe to say Y.
>  	  See Documentation/acpi/initrd_table_override.txt for details
>  
> +endmenu
> +
>  config ACPI_DEBUG
>  	bool "Debug Statements"
>  	default n
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 9573913..f84ef59 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -57,6 +57,11 @@ void acpi_cmos_rtc_init(void);
>  #else
>  static inline void acpi_cmos_rtc_init(void) {}
>  #endif
> +#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
> +void __init acpi_ssdt_customized(void);
> +#else
> +static inline void acpi_ssdt_customized(void) {}
> +#endif
>  
>  extern bool acpi_force_hot_remove;
>  
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 9aeae41..c6024c2 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -1770,15 +1770,13 @@ acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object)
>  }
>  #endif
>  
> -static int __init acpi_no_static_ssdt_setup(char *s)
> +#ifdef CONFIG_ACPI_CUSTOM_DSDT_WITH_SSDT
> +void __init acpi_ssdt_customized(void)
>  {
>  	acpi_gbl_disable_ssdt_table_install = TRUE;
>  	pr_info("ACPI: static SSDT installation disabled\n");
> -
> -	return 0;
>  }
> -
> -early_param("acpi_no_static_ssdt", acpi_no_static_ssdt_setup);
> +#endif
>  
>  static int __init acpi_disable_return_repair(char *s)
>  {
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index 2178229..aaf2177 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -184,6 +184,7 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
>  	}
>  }
>  
> +#include "internal.h"
>  
>  int __init
>  acpi_table_parse_entries(char *id,
> @@ -333,6 +334,7 @@ int __init acpi_table_init(void)
>  {
>  	acpi_status status;
>  
> +	acpi_ssdt_customized();
>  	status = acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
>  	if (ACPI_FAILURE(status))
>  		return -EINVAL;
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v3 1/2] ACPI: Cleanup to convert acpi.no_static_ssdt into a compile-time configurable.

[Rafael J. Wysocki]

On Tuesday, May 20, 2014 12:49:04 AM Rafael J. Wysocki wrote:
> On Monday, May 12, 2014 03:49:59 PM Lv Zheng wrote:
> > User can specify a DSDT with SSDT embedded, in which case, no_static_ssdt
> > must be enforced during compile-time.  If we don't do that and forget to
> > specify acpi.no_static_ssdt as a boot parameter, then:
> > 1. The AML executable will be executed twice, the second execution is
> >    running under a changed environment.
> > 2. The namespace object conflicts will result in an AE_ALREADY_EXISTS
> >    exception;
> > 3. The namespace objects that are deleted from the original SSDT will be
> >    restored by the auto loading of the original SSDT.
> > 
> > Note that:
> > 1. The DSDT customization is a compile-time feature, thus the SSDT
> >    inclusion indication of the DSDT customization should also be
> >    implemented as a compile-time configurable.
> > 2. According to the commit log:
> >     Commit: 67effe8fff32f60bdf51cba484766ba6003005bb
> >     Subject: ACPI: add "acpi_no_auto_ssdt" bootparam
> >    The acpi.no_static_ssdt (originally acpi.no_auto_ssdt) was introduced to
> >    be used for this use case.  And over a long time, there is no other
> >    situation than this requires this boot parameter.
> > This patch thus converts the runtime kernel parameter into the compile-time
> > configuration item.
> > 
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=3774
> > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
> > Original-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > ---
> >  Documentation/kernel-parameters.txt |   10 ----------
> >  drivers/acpi/Kconfig                |   32 ++++++++++++++++++++++++++------
> >  drivers/acpi/internal.h             |    5 +++++
> >  drivers/acpi/osl.c                  |    8 +++-----
> >  drivers/acpi/tables.c               |    2 ++
> >  5 files changed, 36 insertions(+), 21 deletions(-)
> > 
> > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > index 831fb1e..ceba0d6 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -237,16 +237,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> >  			This feature is enabled by default.
> >  			This option allows to turn off the feature.
> >  
> > -	acpi_no_static_ssdt	[HW,ACPI]
> > -			Disable installation of static SSDTs at early boot time
> > -			By default, SSDTs contained in the RSDT/XSDT will be
> > -			installed automatically and they will appear under
> > -			/sys/firmware/acpi/tables.
> > -			This option turns off this feature.
> > -			Note that specifying this option does not affect
> > -			dynamic table installation which will install SSDT
> > -			tables to /sys/firmware/acpi/tables/dynamic.
> > -
> >  	acpica_no_return_repair [HW, ACPI]
> >  			Disable AML predefined validation mechanism
> >  			This mechanism can repair the evaluation result to make
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index c0160bb..51dc3b8 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -216,22 +216,40 @@ config ACPI_NUMA
> >  	depends on (X86 || IA64)
> >  	default y if IA64_GENERIC || IA64_SGI_SN2
> >  
> > -config ACPI_CUSTOM_DSDT_FILE
> > -	string "Custom DSDT Table file to include"
> > -	default ""
> > +menu "Table override"
> > +
> > +menuconfig ACPI_CUSTOM_DSDT
> 
> I would make this a choice, like the "Kernel compression mode" (in init/Kconfig).
> 
> Then ACPI_CUSTOM_DSDT_ONLY and ACPI_CUSTOM_DSDT_WITH_SSDT may be the options
> and they both would select ACPI_CUSTOM_DSDT (otherwise invisible to the user),

With the third (and default) option NO_ACPI_CUSTOM_DSDT that would not
select ACPI_CUSTOM_DSDT.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

RE: [PATCH v3 1/2] ACPI: Cleanup to convert acpi.no_static_ssdt into a compile-time configurable.

[Zheng, Lv]

Hi, Rafael

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Tuesday, May 20, 2014 6:52 AM
> Subject: Re: [PATCH v3 1/2] ACPI: Cleanup to convert acpi.no_static_ssdt into a compile-time configurable.
> 
> On Tuesday, May 20, 2014 12:49:04 AM Rafael J. Wysocki wrote:
> > On Monday, May 12, 2014 03:49:59 PM Lv Zheng wrote:
> > > User can specify a DSDT with SSDT embedded, in which case, no_static_ssdt
> > > must be enforced during compile-time.  If we don't do that and forget to
> > > specify acpi.no_static_ssdt as a boot parameter, then:
> > > 1. The AML executable will be executed twice, the second execution is
> > >    running under a changed environment.
> > > 2. The namespace object conflicts will result in an AE_ALREADY_EXISTS
> > >    exception;
> > > 3. The namespace objects that are deleted from the original SSDT will be
> > >    restored by the auto loading of the original SSDT.
> > >
> > > Note that:
> > > 1. The DSDT customization is a compile-time feature, thus the SSDT
> > >    inclusion indication of the DSDT customization should also be
> > >    implemented as a compile-time configurable.
> > > 2. According to the commit log:
> > >     Commit: 67effe8fff32f60bdf51cba484766ba6003005bb
> > >     Subject: ACPI: add "acpi_no_auto_ssdt" bootparam
> > >    The acpi.no_static_ssdt (originally acpi.no_auto_ssdt) was introduced to
> > >    be used for this use case.  And over a long time, there is no other
> > >    situation than this requires this boot parameter.
> > > This patch thus converts the runtime kernel parameter into the compile-time
> > > configuration item.
> > >
> > > References: https://bugzilla.kernel.org/show_bug.cgi?id=3774
> > > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
> > > Original-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
> > > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > > ---
> > >  Documentation/kernel-parameters.txt |   10 ----------
> > >  drivers/acpi/Kconfig                |   32 ++++++++++++++++++++++++++------
> > >  drivers/acpi/internal.h             |    5 +++++
> > >  drivers/acpi/osl.c                  |    8 +++-----
> > >  drivers/acpi/tables.c               |    2 ++
> > >  5 files changed, 36 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > index 831fb1e..ceba0d6 100644
> > > --- a/Documentation/kernel-parameters.txt
> > > +++ b/Documentation/kernel-parameters.txt
> > > @@ -237,16 +237,6 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> > >  			This feature is enabled by default.
> > >  			This option allows to turn off the feature.
> > >
> > > -	acpi_no_static_ssdt	[HW,ACPI]
> > > -			Disable installation of static SSDTs at early boot time
> > > -			By default, SSDTs contained in the RSDT/XSDT will be
> > > -			installed automatically and they will appear under
> > > -			/sys/firmware/acpi/tables.
> > > -			This option turns off this feature.
> > > -			Note that specifying this option does not affect
> > > -			dynamic table installation which will install SSDT
> > > -			tables to /sys/firmware/acpi/tables/dynamic.
> > > -
> > >  	acpica_no_return_repair [HW, ACPI]
> > >  			Disable AML predefined validation mechanism
> > >  			This mechanism can repair the evaluation result to make
> > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > > index c0160bb..51dc3b8 100644
> > > --- a/drivers/acpi/Kconfig
> > > +++ b/drivers/acpi/Kconfig
> > > @@ -216,22 +216,40 @@ config ACPI_NUMA
> > >  	depends on (X86 || IA64)
> > >  	default y if IA64_GENERIC || IA64_SGI_SN2
> > >
> > > -config ACPI_CUSTOM_DSDT_FILE
> > > -	string "Custom DSDT Table file to include"
> > > -	default ""
> > > +menu "Table override"
> > > +
> > > +menuconfig ACPI_CUSTOM_DSDT
> >
> > I would make this a choice, like the "Kernel compression mode" (in init/Kconfig).
> >
> > Then ACPI_CUSTOM_DSDT_ONLY and ACPI_CUSTOM_DSDT_WITH_SSDT may be the options
> > and they both would select ACPI_CUSTOM_DSDT (otherwise invisible to the user),
> 
> With the third (and default) option NO_ACPI_CUSTOM_DSDT that would not
> select ACPI_CUSTOM_DSDT.

OK.

Thanks
-Lv

> 
> Thanks!
> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH v3 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.

[Lv Zheng]

This patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
Cc: <stable@vger.kernel.org> # 2.6.35.x: aa2110cb: ACPI: add boot option acpi=copy_dsdt to fix corrupt DSDT
---
 drivers/acpi/bus.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 610261a..cfd2318 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -52,6 +52,12 @@ struct proc_dir_entry *acpi_root_dir;
 EXPORT_SYMBOL(acpi_root_dir);
 
 #ifdef CONFIG_X86
+#ifdef CONFIG_ACPI_CUSTOM_DSDT
+static inline int set_copy_dsdt(const struct dmi_system_id *id)
+{
+	return 0;
+}
+#else
 static int set_copy_dsdt(const struct dmi_system_id *id)
 {
 	printk(KERN_NOTICE "%s detected - "
@@ -59,6 +65,7 @@ static int set_copy_dsdt(const struct dmi_system_id *id)
 	acpi_gbl_copy_dsdt_locally = 1;
 	return 0;
 }
+#endif
 
 static struct dmi_system_id dsdt_dmi_table[] __initdata = {
 	/*
-- 
1.7.10

Re: [PATCH v3 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.

[Rafael J. Wysocki]

On Monday, May 12, 2014 03:50:16 PM Lv Zheng wrote:
> This patch fixes the following issue:
> If DSDT is customized, no local DSDT copy is needed.
> 
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=69711
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Signed-off-by: Enrico Etxe Arte <goitizena.generoa@gmail.com>
> Cc: <stable@vger.kernel.org> # 2.6.35.x: aa2110cb: ACPI: add boot option acpi=copy_dsdt to fix corrupt DSDT

Queued up for 3.16, thanks!

> ---
>  drivers/acpi/bus.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 610261a..cfd2318 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -52,6 +52,12 @@ struct proc_dir_entry *acpi_root_dir;
>  EXPORT_SYMBOL(acpi_root_dir);
>  
>  #ifdef CONFIG_X86
> +#ifdef CONFIG_ACPI_CUSTOM_DSDT
> +static inline int set_copy_dsdt(const struct dmi_system_id *id)
> +{
> +	return 0;
> +}
> +#else
>  static int set_copy_dsdt(const struct dmi_system_id *id)
>  {
>  	printk(KERN_NOTICE "%s detected - "
> @@ -59,6 +65,7 @@ static int set_copy_dsdt(const struct dmi_system_id *id)
>  	acpi_gbl_copy_dsdt_locally = 1;
>  	return 0;
>  }
> +#endif
>  
>  static struct dmi_system_id dsdt_dmi_table[] __initdata = {
>  	/*
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.