ACPI: Fix issues related to the DSDT customization.
Subsystems (2)
cover ACPI: Fix issues related to the DSDT customization.
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(-)
Signature · 1 line
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.
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.
--- 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(-)
drivers/acpi/Kconfig +8 -0
@@ -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
drivers/acpi/internal.h +5 -0
@@ -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;
drivers/acpi/osl.c +13 -1
@@ -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) {
drivers/acpi/tables.c +2 -0
@@ -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 ;
Signature · 1 line
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.
Quoted 27 lines · Lv Zheng
This patch fixes the following issue:
User can specify a DSDT with SSDT embedded, in which case,no_static_ssdtmust be enforced. If we don't do that, then:
1. The namespace object conflicts will result in anAE_ALREADY_EXISTSexception;
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.
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)drivers/acpi/Kconfig +0
-0
drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 221
Quoted 10 lines
drivers/acpi/Kconfig +7
-0@@ -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?
Quoted 74 lines
drivers/acpi/Kconfig +1
-0+ config ACPI_INITRD_TABLE_OVERRIDE bool "ACPI tables override via initrd" depends on BLK_DEV_INITRD && X86drivers/acpi/internal.h +5
-0@@ -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;drivers/acpi/osl.c +13
-1@@ -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) {drivers/acpi/tables.c +2
-0@@ -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 ;
Signature · 2 lines
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.
Hi, Rafael
Quoted 44 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Tuesday, April 22, 2014 5:14 AMQuoted 27 lines · Lv Zheng
This patch fixes the following issue:
User can specify a DSDT with SSDT embedded, in which case,no_static_ssdtmust be enforced. If we don't do that, then:
1. The namespace object conflicts will result in anAE_ALREADY_EXISTSexception;
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.
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)drivers/acpi/Kconfig +0
-0drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 221
Quoted 10 lines
drivers/acpi/Kconfig +7
-0@@ -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?
Quoted 3 lines
Can't we just use
CONFIG_ACPI_CUSTOM_DSDTinstead ofCONFIG_ACPI_CUSTOM_SSDT_INCLUDEDeverywhere 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
Quoted 79 lines
Quoted 68 lines
drivers/acpi/Kconfig +1
-0+ config ACPI_INITRD_TABLE_OVERRIDE bool "ACPI tables override via initrd" depends on BLK_DEV_INITRD && X86drivers/acpi/internal.h +5
-0@@ -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;drivers/acpi/osl.c +13
-1@@ -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) {drivers/acpi/tables.c +1
-0@@ -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,drivers/acpi/tables.c int __init acpi_table_init(void) · line 340
Quoted 7 lines
drivers/acpi/tables.c +1
-0@@ -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.
Quoted 60 lines · Lv
Hi, Rafael
Quoted 44 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Tuesday, April 22, 2014 5:14 AMQuoted 27 lines · Lv Zheng
This patch fixes the following issue:
User can specify a DSDT with SSDT embedded, in which case,no_static_ssdtmust be enforced. If we don't do that, then:
1. The namespace object conflicts will result in anAE_ALREADY_EXISTSexception;
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.
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)drivers/acpi/Kconfig +0
-0drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 221
Quoted 10 lines
drivers/acpi/Kconfig +7
-0@@ -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?
Quoted 3 lines
Can't we just use
CONFIG_ACPI_CUSTOM_DSDTinstead ofCONFIG_ACPI_CUSTOM_SSDT_INCLUDEDeverywhere instead?It is a different use case.
Normally,ACPI_CUSTOM_DSDTonly 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 forACPI_CUSTOM_DSDT.
So user can:1. customize DSDT without SSDTs copied into it;
2. customize DSDT with SSDTs copied into it.
Quoted 6 lines
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_SSDTto indicate the fact that it is a sub-option that depends on theACPI_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.
Hi,
Quoted 67 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Rafael J. Wysocki
Sent: Tuesday, April 22, 2014 7:20 PMQuoted 63 lines · Lv
Hi, Rafael
Quoted 44 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Tuesday, April 22, 2014 5:14 AMQuoted 27 lines · Lv Zheng
This patch fixes the following issue:
User can specify a DSDT with SSDT embedded, in which case,no_static_ssdtmust be enforced. If we don't do that, then:
1. The namespace object conflicts will result in anAE_ALREADY_EXISTSexception;
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.
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)drivers/acpi/Kconfig +0
-0drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 221
Quoted 10 lines
drivers/acpi/Kconfig +7
-0@@ -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?
Quoted 3 lines
Can't we just use
CONFIG_ACPI_CUSTOM_DSDTinstead ofCONFIG_ACPI_CUSTOM_SSDT_INCLUDEDeverywhere instead?It is a different use case.
Normally,ACPI_CUSTOM_DSDTonly 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 forACPI_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.
Quoted 6 lines
Quoted 3 lines
I think the naming of the new option might be confusing. It can be
ACPI_CUSTOM_DSDT_WITH_SSDTto indicate the fact that it is a sub-option that depends on theACPI_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.
Quoted 2 lines
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.
Quoted 73 lines · Lv
Hi,
Quoted 71 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Rafael J. Wysocki
Sent: Tuesday, April 22, 2014 7:20 PMQuoted 66 lines · Lv
Hi, Rafael
Quoted 44 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Tuesday, April 22, 2014 5:14 AMQuoted 27 lines · Lv Zheng
This patch fixes the following issue:
User can specify a DSDT with SSDT embedded, in which case,no_static_ssdtmust be enforced. If we don't do that, then:
1. The namespace object conflicts will result in anAE_ALREADY_EXISTSexception;
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.
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)drivers/acpi/Kconfig +0
-0drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 221
Quoted 10 lines
drivers/acpi/Kconfig +7
-0@@ -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?
Quoted 3 lines
Can't we just use
CONFIG_ACPI_CUSTOM_DSDTinstead ofCONFIG_ACPI_CUSTOM_SSDT_INCLUDEDeverywhere instead?It is a different use case.
Normally,ACPI_CUSTOM_DSDTonly 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 forACPI_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_SSDTto indicate the fact that it is a sub-option that depends on theACPI_CUSTOM_DSDT.
Quoted 6 lines
Quoted 2 lines
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_DSDTfeature, 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?
Signature · 2 lines
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.
Hi, Rafael
Quoted 81 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Wednesday, April 23, 2014 6:48 PM
To: Zheng, LvQuoted 76 lines · Lv
Hi,
Quoted 73 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Rafael J. Wysocki
Sent: Tuesday, April 22, 2014 7:20 PMQuoted 60 lines · Lv
Hi, Rafael
Quoted 44 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Tuesday, April 22, 2014 5:14 AMQuoted 27 lines · Lv Zheng
This patch fixes the following issue:
User can specify a DSDT with SSDT embedded, in which case,no_static_ssdtmust be enforced. If we don't do that, then:
1. The namespace object conflicts will result in anAE_ALREADY_EXISTSexception;
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.
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)drivers/acpi/Kconfig +0
-0drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 221
Quoted 10 lines
drivers/acpi/Kconfig +7
-0@@ -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?
Quoted 3 lines
Can't we just use
CONFIG_ACPI_CUSTOM_DSDTinstead ofCONFIG_ACPI_CUSTOM_SSDT_INCLUDEDeverywhere instead?It is a different use case.
Normally,ACPI_CUSTOM_DSDTonly 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 forACPI_CUSTOM_DSDT.
So user can:1. customize DSDT without SSDTs copied into it;
2. customize DSDT with SSDTs copied into it.Quoted 6 lines
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_SSDTto indicate the fact that it is a sub-option that depends on theACPI_CUSTOM_DSDT.Well, it looks like the solution would be to allow SSDTs and DSDTs to be customized independently, wouldn't it?
Quoted 7 lines
Quoted 3 lines
First, I'm not talking about initrd table override.
For the
CONFIG_ACPI_CUSTOM_DSDTfeature, I'm afraid the answer is SSDT cannot be independently customized without DSDTcustomization.
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
Quoted 4 lines
--
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.
In reply to Lv ZhengThis patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.
--- drivers/acpi/bus.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/acpi/bus.c +8 -1
@@ -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 = {/*
Signature · 1 line
1.7.10
Re: [PATCH 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.
Quoted 15 lines · Lv Zheng
This patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.Signed-off-by Lv Zheng <[email protected]>Signed-off-by Enrico Etxe Arte <[email protected]>--- drivers/acpi/bus.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/acpi/bus.c +0
-0
drivers/acpi/bus.c struct proc_dir_entry *acpi_root_dir; · line 55
Quoted 4 lines
drivers/acpi/bus.c +1
-0@@ -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.
drivers/acpi/bus.c struct proc_dir_entry *acpi_root_dir; · line 58
Quoted 4 lines
drivers/acpi/bus.c +1
-1static 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?
Quoted 14 lines
drivers/acpi/bus.c +6
-0"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 = {/*
Signature · 2 lines
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.
Hi, Rafael
Quoted 26 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Tuesday, April 22, 2014 5:16 AMQuoted 15 lines · Lv Zheng
This patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.Signed-off-by Lv Zheng <[email protected]>Signed-off-by Enrico Etxe Arte <[email protected]>--- drivers/acpi/bus.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/acpi/bus.c +0
-0drivers/acpi/bus.c struct proc_dir_entry *acpi_root_dir; · line 55
Quoted 4 lines
drivers/acpi/bus.c +1
-0@@ -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?
Quoted 7 lines
So do
#ifdef
CONFIG_ACPI_CUSTOM_DSDT
static inline intset_copy_dsdt(const struct dmi_system_id *id){ return 0; } #elseetc.
OK.
Quoted 7 lines
drivers/acpi/bus.c struct proc_dir_entry *acpi_root_dir; · line 58
Quoted 5 lines
drivers/acpi/bus.c +1
-1static 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
Quoted 19 lines
Quoted 5 lines
drivers/acpi/bus.c +0
-0"force copy of DSDT to local memory\n", id->ident); acpi_gbl_copy_dsdt_locally =1 ;return 0 ;drivers/acpi/bus.c struct proc_dir_entry *acpi_root_dir; · line 71
Quoted 10 lines
drivers/acpi/bus.c +6
-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.
In reply to Lv ZhengThis 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.
--- 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(-)
drivers/acpi/Kconfig +8 -0
@@ -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
drivers/acpi/internal.h +5 -0
@@ -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;
drivers/acpi/osl.c +13 -1
@@ -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) {
drivers/acpi/tables.c +2 -0
@@ -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 ;
Signature · 1 line
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.
Quoted 27 lines · Lv Zheng
This patch fixes the following issue:
User can specify a DSDT with SSDT embedded, in which case,no_static_ssdtmust be enforced. If we don't do that, then:
1. The namespace object conflicts will result in anAE_ALREADY_EXISTSexception;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.
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)drivers/acpi/Kconfig +0
-0
drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 238
Quoted 6 lines
drivers/acpi/Kconfig +3
-0@@ -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.
drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 240
Quoted 2 lines
drivers/acpi/Kconfig +2
-0+ 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.
Quoted 75 lines
drivers/acpi/Kconfig +3
-0+ + 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 && X86drivers/acpi/internal.h +5
-0@@ -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;drivers/acpi/osl.c +13
-1@@ -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) {drivers/acpi/tables.c +2
-0@@ -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.
Hi, Rafael
Thanks for the review.
From: "Rafael J. Wysocki" <[email protected]>
Sent: Friday, May 09, 2014 6:40 PM
Quoted 23 lines · Lv Zheng
Quoted 22 lines
This patch fixes the following issue:
User can specify a DSDT with SSDT embedded, in which case,no_static_ssdtmust be enforced. If we don't do that, then:
1. The namespace object conflicts will result in anAE_ALREADY_EXISTSexception;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.
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)
Quoted 15 lines
drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 237
Quoted 10 lines
drivers/acpi/Kconfig +2
-0@@ -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_DSDTIf you think that we really have to have a separate
CONFIG_option here, I would preferACPI_CUSTOM_DSDTto 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.
Quoted 5 lines
drivers/acpi/Kconfig config ACPI_CUSTOM_DSDT · line 240
Quoted 2 lines
drivers/acpi/Kconfig +2
-0+ 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
Quoted 70 lines
Quoted 69 lines
drivers/acpi/Kconfig +3
-0+ + 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 && X86drivers/acpi/internal.h +5
-0@@ -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.cindex 9aeae41..089e55d 100644
drivers/acpi/osl.c +13
-0@@ -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) {drivers/acpi/tables.c +0
-0@@ -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.
In reply to Lv ZhengThis patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.
--- drivers/acpi/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/acpi/bus.c +7 -0
@@ -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 = {/*
Signature · 1 line
1.7.10
Re: [PATCH v2 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.
Quoted 6 lines · Lv Zheng
This patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.Signed-off-by Lv Zheng <[email protected]>Signed-off-by Enrico Etxe Arte <[email protected]>
This can go in without [1/2], right?
If so, do we need it in -stable?
Quoted 30 lines
--- drivers/acpi/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/acpi/bus.c +7
-0@@ -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 = {/*
Signature · 2 lines
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.
Hi, Rafael
Quoted 4 lines
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Monday, May 12, 2014 8:29 AMOn Friday, May 09, 2014 04:44:19 PM Lv Zheng wrote:
Quoted 8 lines
Quoted 6 lines
This patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.Signed-off-by Lv Zheng <[email protected]>Signed-off-by Enrico Etxe Arte <[email protected]>This can go in without [1/2], right?
Yes.
Quoted 1 line
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: <[email protected]> # 2.6.35.x: aa2110cb: ACPI: add boot option acpi=copy_dsdt to fix corrupt DSDT
Thanks and best regards
-Lv
Quoted 35 lines
Quoted 24 lines
--- drivers/acpi/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/acpi/bus.c +6
-0@@ -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 - "drivers/acpi/bus.c static int set_copy_dsdt(const struct dmi_system_id *id) · line 71
Quoted 7 lines
drivers/acpi/bus.c +1
-0@@ -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.
In reply to Lv ZhengUser 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
--- 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(-)
Documentation/kernel-parameters.txt +0 -10
@@ -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
drivers/acpi/Kconfig +26 -6
@@ -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
drivers/acpi/internal.h +5 -0
@@ -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;
drivers/acpi/osl.c +3 -5
@@ -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) {
drivers/acpi/tables.c +2 -0
@@ -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 ;
Signature · 1 line
1.7.10
Re: [PATCH v3 1/2] ACPI: Cleanup to convert acpi.no_static_ssdt into a compile-time configurable.
Quoted 61 lines · Lv Zheng
User can specify a DSDT with SSDT embedded, in which case,
no_static_ssdtmust 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 anAE_ALREADY_EXISTSexception;
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
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)Documentation/kernel-parameters.txt +0
-10@@ -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 makedrivers/acpi/Kconfig +0
-0
drivers/acpi/Kconfig config ACPI_NUMA · line 221
Quoted 9 lines
drivers/acpi/Kconfig +3
-3@@ -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 ->
drivers/acpi/Kconfig config ACPI_NUMA · line 232
Quoted 11 lines
drivers/acpi/Kconfig +6
-0+ 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?
Quoted 95 lines
drivers/acpi/Kconfig +17
-3+ 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 ndrivers/acpi/internal.h +5
-0@@ -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;drivers/acpi/osl.c +3
-5@@ -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) {drivers/acpi/tables.c +2
-0@@ -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 ;
Signature · 2 lines
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.
Quoted 74 lines · Rafael J. Wysocki
Quoted 61 lines · Lv Zheng
User can specify a DSDT with SSDT embedded, in which case,
no_static_ssdtmust 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 anAE_ALREADY_EXISTSexception;
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
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)Documentation/kernel-parameters.txt +0
-10@@ -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 makedrivers/acpi/Kconfig +0
-0drivers/acpi/Kconfig config ACPI_NUMA · line 221
Quoted 9 lines
drivers/acpi/Kconfig +3
-3@@ -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_DSDTI would make this a choice, like the "Kernel compression mode" (in init/Kconfig).
Quoted 2 lines
Then
ACPI_CUSTOM_DSDT_ONLYandACPI_CUSTOM_DSDT_WITH_SSDTmay be the options and they both would selectACPI_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!
Signature · 2 lines
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.
Hi, Rafael
Quoted 79 lines
From: Rafael J. Wysocki [mailto:[email protected]]
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.Quoted 74 lines · Rafael J. Wysocki
Quoted 61 lines · Lv Zheng
User can specify a DSDT with SSDT embedded, in which case,
no_static_ssdtmust 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 anAE_ALREADY_EXISTSexception;
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
Original-by Enrico Etxe Arte <[email protected]>Signed-off-by Lv Zheng <[email protected]>--- 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(-)Documentation/kernel-parameters.txt +0
-10@@ -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 makedrivers/acpi/Kconfig +0
-0drivers/acpi/Kconfig config ACPI_NUMA · line 221
Quoted 9 lines
drivers/acpi/Kconfig +3
-3@@ -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_DSDTI would make this a choice, like the "Kernel compression mode" (in init/Kconfig).
Quoted 5 lines
Quoted 2 lines
Then
ACPI_CUSTOM_DSDT_ONLYandACPI_CUSTOM_DSDT_WITH_SSDTmay be the options and they both would selectACPI_CUSTOM_DSDT(otherwise invisible to the user),With the third (and default) option
NO_ACPI_CUSTOM_DSDTthat would not selectACPI_CUSTOM_DSDT.
OK.
Thanks
-Lv
Quoted 6 lines
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.
In reply to Lv ZhengThis patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.
--- drivers/acpi/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/acpi/bus.c +7 -0
@@ -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 = {/*
Signature · 1 line
1.7.10
Rafael J. Wysocki
AppliedRe: [PATCH v3 2/2] ACPI: Fix the issue that the DSDT local copy is conflict with the customized DSDT.
Quoted 7 lines · Lv Zheng
This patch fixes the following issue:
If DSDT is customized, no local DSDT copy is needed.Signed-off-by Lv Zheng <[email protected]>Signed-off-by Enrico Etxe Arte <[email protected]>Cc <[email protected]> # 2.6.35.x: aa2110cb: ACPI: add boot option acpi=copy_dsdt to fix corrupt DSDT
Queued up for 3.16, thanks!
Quoted 30 lines
--- drivers/acpi/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/acpi/bus.c +7
-0@@ -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 = {/*
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.