| Message ID | e481a3b182657e907fb1db6f6f71144c36228ddc.1466479787.git.lv.zheng@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
Hi, Rafael > From: Zheng, Lv > Subject: [PATCH v4 3/5] ACPICA: ACPI 2.0, Interpreter: Fix MLC issues by > switching to new TermList grammar for table loading > > The MLC (Module Level Code) is an ACPICA terminology describing the > AML > code out of any control method, its support is the main contention of the > interpreter behavior during the table loading. > > The original implementation of MLC in ACPICA had several issues: > 1. Out of any control method, besides of the object creating opcodes, only > the code blocks wrapped by "If/Else/While" opcodes were supported. > 2. The supported MLC code blocks were executed after loading the table > rather than being executed right in place. > > ========================================================== > == > The demo of this order issue is as follows: > Name (OBJ1, 1) > If (CND1 == 1) > { > Name (OBJ2, 2) > } > Name (OBJ3, 3) > The original MLC support created OBJ2 after OBJ3's creation. > > ========================================================== > == > Other than these limitations, MLC support in ACPICA looks correct. And > supporting this should be easy/natural for ACPICA, but enabling of this > was > blocked by some ACPICA internal and OSPM specific initialization order > issues we've fixed recently. The wrong support started from the following > false bug fixing commit: > Commit: 80d7951177315f70b5ffd8663985fbf725d07799 > Subject: Add support for module-level executable AML code. > > We can confirm Windows interpreter behavior via reverse engineering > means. > It can be proven that not only If/Else/While wrapped code blocks, all > opcodes can be executed at the module level, including operation region > accesses. And it can be proven that the MLC should be executed right in > place, not in such a deferred way executed after loading the table. > > And the above facts indeed reflect the spec words around ACPI definition > block tables (DSDT/SSDT/...), the entire table and the Scope object is > defined by the AML specification in BNF style as: > AMLCode := DefBlockHeader TermList > DefScope := ScopeOp PkgLength NameString TermList > The bodies of the scope opening terms (AMLCode/Scope) are all TermList, > thus the table loading should be no difference than the control method > evaluations as the body of the Method is also defined by the AML > specification as TermList: > DefMethod := MethodOp PkgLength NameString MethodFlags TermList > The only difference is: after evaluating control method, created named > objects may be freed due to no reference, while named objects created by > the table loading should only be freed after unloading the table. > > So this patch follows the spec and the de-facto standard behavior, enables > the new grammar (TermList) for the table loading. > > By doing so, beyond the fixes to the above issues, we can see additional > differences comparing to the old grammar based table loading: > 1. Originally, beyond the scope opening terms (AMLCode/Scope), > If/Else/While wrapped code blocks under the scope creating terms > (Device/PowerResource/Processor/ThermalZone) are also supported as > deferred MLC, which violates the spec defined grammar where ObjectList > is enforced. With MLC support improved as non-deferred, the interpreter > parses such scope creating terms as TermList rather ObjectList like the > scope opening terms. > After probing the Windows behavior and proving that it also parses > these > terms as TermList, we submitted an ECR (Engineering Change Request) to > the ASWG (ACPI Specification Working Group) to clarify this. The ECR is > titled as "ASL Grammar Clarification for Executable AML Opcodes" and > has > been accepted by the ASWG. The new grammar will appear in ACPI > specification 6.2. > 2. Originally, Buffer/Package/OperationRegion/CreateXXXField/BankField > arguments are evaluated in a deferred way after loading the table. With > MLC support improved, they are also parsed right in place during the > table loading. > This is also Windows compliant and the only difference is the removal > of the debugging messages implemented before > acpi_ds_execute_arguments(), > see Link 1 for the details. A previous commit should have ensured that > acpi_check_address_range() won't regress. > > Note that enabling this feature may cause regressions due to long term > Linux ACPI support on top of the wrong grammar. So this patch also > prepares > a global option to be used to roll back to the old grammar during the > period between a regression is reported and the regression is > root-cause-fixed. ACPICA BZ 963, fixed by Lv Zheng. > > Link 1: https://bugzilla.kernel.org/show_bug.cgi?id=112911 > Link 2: https://bugs.acpica.org/show_bug.cgi?id=963 > Tested-by: Chris Bainbridge <chris.bainbridge@gmail.com> > Signed-off-by: Lv Zheng <lv.zheng@intel.com> > --- > drivers/acpi/acpica/acnamesp.h | 3 + > drivers/acpi/acpica/acparser.h | 2 + > drivers/acpi/acpica/evrgnini.c | 3 +- > drivers/acpi/acpica/exconfig.c | 3 +- > drivers/acpi/acpica/nsload.c | 3 +- > drivers/acpi/acpica/nsparse.c | 166 > ++++++++++++++++++++++++++++++++-------- > drivers/acpi/acpica/psparse.c | 4 +- > drivers/acpi/acpica/psxface.c | 71 +++++++++++++++++ > drivers/acpi/acpica/tbxfload.c | 3 +- > drivers/acpi/acpica/utxfinit.c | 3 +- > include/acpi/acpixf.h | 6 ++ > 11 files changed, 229 insertions(+), 38 deletions(-) > > diff --git a/drivers/acpi/acpica/acnamesp.h > b/drivers/acpi/acpica/acnamesp.h > index f33a4ba..829672a 100644 > --- a/drivers/acpi/acpica/acnamesp.h > +++ b/drivers/acpi/acpica/acnamesp.h > @@ -130,6 +130,9 @@ acpi_status > acpi_ns_parse_table(u32 table_index, struct acpi_namespace_node > *start_node); > > acpi_status > +acpi_ns_execute_table(u32 table_index, struct acpi_namespace_node > *start_node); > + > +acpi_status > acpi_ns_one_complete_parse(u32 pass_number, > u32 table_index, > struct acpi_namespace_node *start_node); > diff --git a/drivers/acpi/acpica/acparser.h b/drivers/acpi/acpica/acparser.h > index fc30577..939d411 100644 > --- a/drivers/acpi/acpica/acparser.h > +++ b/drivers/acpi/acpica/acparser.h > @@ -78,6 +78,8 @@ extern const u8 acpi_gbl_long_op_index[]; > */ > acpi_status acpi_ps_execute_method(struct acpi_evaluate_info *info); > > +acpi_status acpi_ps_execute_table(struct acpi_evaluate_info *info); > + > /* > * psargs - Parse AML opcode arguments > */ > diff --git a/drivers/acpi/acpica/evrgnini.c b/drivers/acpi/acpica/evrgnini.c > index b6ea9c0..3843f1f 100644 > --- a/drivers/acpi/acpica/evrgnini.c > +++ b/drivers/acpi/acpica/evrgnini.c > @@ -553,7 +553,8 @@ acpi_ev_initialize_region(union > acpi_operand_object *region_obj, > * > * See acpi_ns_exec_module_code > */ > - if (obj_desc->method. > + if (!acpi_gbl_parse_table_as_term_list && > + obj_desc->method. > info_flags & > ACPI_METHOD_MODULE_LEVEL) { > handler_obj = > obj_desc- > >method.dispatch.handler; > diff --git a/drivers/acpi/acpica/exconfig.c b/drivers/acpi/acpica/exconfig.c > index 21932d6..75679bc 100644 > --- a/drivers/acpi/acpica/exconfig.c > +++ b/drivers/acpi/acpica/exconfig.c > @@ -120,7 +120,8 @@ acpi_ex_add_table(u32 table_index, > /* Execute any module-level code that was found in the table */ > > acpi_ex_exit_interpreter(); > - if (acpi_gbl_group_module_level_code) { > + if (!acpi_gbl_parse_table_as_term_list > + && acpi_gbl_group_module_level_code) { > acpi_ns_exec_module_code_list(); > } > acpi_ex_enter_interpreter(); > diff --git a/drivers/acpi/acpica/nsload.c b/drivers/acpi/acpica/nsload.c > index b5e2b0a..2daa9a09 100644 > --- a/drivers/acpi/acpica/nsload.c > +++ b/drivers/acpi/acpica/nsload.c > @@ -162,7 +162,8 @@ unlock: > * other ACPI implementations. Optionally, the execution can be > deferred > * until later, see acpi_initialize_objects. > */ > - if (!acpi_gbl_group_module_level_code) { > + if (!acpi_gbl_parse_table_as_term_list > + && !acpi_gbl_group_module_level_code) { > acpi_ns_exec_module_code_list(); > } > > diff --git a/drivers/acpi/acpica/nsparse.c b/drivers/acpi/acpica/nsparse.c > index 1783cd7..f475889 100644 > --- a/drivers/acpi/acpica/nsparse.c > +++ b/drivers/acpi/acpica/nsparse.c > @@ -54,6 +54,98 @@ ACPI_MODULE_NAME("nsparse") > > > /********************************************************* > ********************** > * > + * FUNCTION: ns_execute_table > + * > + * PARAMETERS: table_desc - An ACPI table descriptor for table to > parse > + * start_node - Where to enter the table into the namespace > + * > + * RETURN: Status > + * > + * DESCRIPTION: Load ACPI/AML table by executing the entire table as a > + * term_list. > + * > + > ********************************************************** > ********************/ > +acpi_status > +acpi_ns_execute_table(u32 table_index, struct acpi_namespace_node > *start_node) > +{ > + acpi_status status; > + struct acpi_table_header *table; > + acpi_owner_id owner_id; > + struct acpi_evaluate_info *info = NULL; > + u32 aml_length; > + u8 *aml_start; > + union acpi_operand_object *method_obj = NULL; > + > + ACPI_FUNCTION_TRACE(ns_execute_table); > + > + status = acpi_get_table_by_index(table_index, &table); > + if (ACPI_FAILURE(status)) { > + return_ACPI_STATUS(status); > + } > + > + /* Table must consist of at least a complete header */ > + > + if (table->length < sizeof(struct acpi_table_header)) { > + return_ACPI_STATUS(AE_BAD_HEADER); > + } > + > + aml_start = (u8 *)table + sizeof(struct acpi_table_header); > + aml_length = table->length - sizeof(struct acpi_table_header); > + > + status = acpi_tb_get_owner_id(table_index, &owner_id); > + if (ACPI_FAILURE(status)) { > + return_ACPI_STATUS(status); > + } > + > + /* Create, initialize, and link a new temporary method object */ > + > + method_obj = > acpi_ut_create_internal_object(ACPI_TYPE_METHOD); > + if (!method_obj) { > + return_ACPI_STATUS(AE_NO_MEMORY); > + } > + > + /* Allocate the evaluation information block */ > + > + info = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_evaluate_info)); > + if (!info) { > + status = AE_NO_MEMORY; > + goto cleanup; > + } > + > + ACPI_DEBUG_PRINT((ACPI_DB_PARSE, > + "Create table code block: %p\n", method_obj)); > + > + method_obj->method.aml_start = aml_start; > + method_obj->method.aml_length = aml_length; > + method_obj->method.owner_id = owner_id; > + method_obj->method.info_flags |= > ACPI_METHOD_MODULE_LEVEL; > + > + info->pass_number = ACPI_IMODE_EXECUTE; > + info->node = start_node; > + info->obj_desc = method_obj; > + info->node_flags = info->node->flags; > + info->full_pathname = acpi_ns_get_normalized_pathname(info- > >node, TRUE); > + if (!info->full_pathname) { > + status = AE_NO_MEMORY; > + goto cleanup; > + } > + > + (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); > + status = acpi_ps_execute_table(info); > + (void)acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE); [Lv Zheng] I got a report that we have lock order issue in the previous bug fix: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2f38b1b1 And I'll send a quick fix for that later today. After investigation, we found that ACPICA namespace lock design actually requires a big change in order to support this grammar change. This means that this patch still need to be improved. And the above 3 lines are currently known to be not compliant to the current ACPICA namespace lock design. So please drop this series, and I'll re-send this series after improving the ACPICA namespace lock design. Sorry for the issues. Thanks and best regards -Lv > + > +cleanup: > + if (info) { > + ACPI_FREE(info->full_pathname); > + info->full_pathname = NULL; > + } > + ACPI_FREE(info); > + acpi_ut_remove_reference(method_obj); > + return_ACPI_STATUS(status); > +} > + > +/******************************************************** > *********************** > + * > * FUNCTION: ns_one_complete_parse > * > * PARAMETERS: pass_number - 1 or 2 > @@ -64,6 +156,7 @@ ACPI_MODULE_NAME("nsparse") > * DESCRIPTION: Perform one complete parse of an ACPI/AML table. > * > > ********************************************************** > ********************/ > + > acpi_status > acpi_ns_one_complete_parse(u32 pass_number, > u32 table_index, > @@ -173,38 +266,47 @@ acpi_ns_parse_table(u32 table_index, struct > acpi_namespace_node *start_node) > > acpi_ex_enter_interpreter(); > > - /* > - * AML Parse, pass 1 > - * > - * In this pass, we load most of the namespace. Control methods > - * are not parsed until later. A parse tree is not created. Instead, > - * each Parser Op subtree is deleted when it is finished. This saves > - * a great deal of memory, and allows a small cache of parse > objects > - * to service the entire parse. The second pass of the parse then > - * performs another complete parse of the AML. > - */ > - ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start pass 1\n")); > - > - status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS1, > - table_index, start_node); > - if (ACPI_FAILURE(status)) { > - goto error_exit; > - } > - > - /* > - * AML Parse, pass 2 > - * > - * In this pass, we resolve forward references and other things > - * that could not be completed during the first pass. > - * Another complete parse of the AML is performed, but the > - * overhead of this is compensated for by the fact that the > - * parse objects are all cached. > - */ > - ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start pass 2\n")); > - status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS2, > - table_index, start_node); > - if (ACPI_FAILURE(status)) { > - goto error_exit; > + if (acpi_gbl_parse_table_as_term_list) { > + ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start load > pass\n")); > + > + status = acpi_ns_execute_table(table_index, start_node); > + if (ACPI_FAILURE(status)) { > + goto error_exit; > + } > + } else { > + /* > + * AML Parse, pass 1 > + * > + * In this pass, we load most of the namespace. Control > methods > + * are not parsed until later. A parse tree is not created. > + * Instead, each Parser Op subtree is deleted when it is > finished. > + * This saves a great deal of memory, and allows a small > cache of > + * parse objects to service the entire parse. The second > pass of > + * the parse then performs another complete parse of the > AML. > + */ > + ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start pass > 1\n")); > + > + status = > acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS1, > + table_index, start_node); > + if (ACPI_FAILURE(status)) { > + goto error_exit; > + } > + > + /* > + * AML Parse, pass 2 > + * > + * In this pass, we resolve forward references and other > things > + * that could not be completed during the first pass. > + * Another complete parse of the AML is performed, but > the > + * overhead of this is compensated for by the fact that the > + * parse objects are all cached. > + */ > + ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start pass > 2\n")); > + status = > acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS2, > + table_index, start_node); > + if (ACPI_FAILURE(status)) { > + goto error_exit; > + } > } > > error_exit: > diff --git a/drivers/acpi/acpica/psparse.c b/drivers/acpi/acpica/psparse.c > index 0a23897..3aa9162 100644 > --- a/drivers/acpi/acpica/psparse.c > +++ b/drivers/acpi/acpica/psparse.c > @@ -571,7 +571,9 @@ acpi_status acpi_ps_parse_aml(struct > acpi_walk_state *walk_state) > * cleanup to do > */ > if (((walk_state->parse_flags & ACPI_PARSE_MODE_MASK) > == > - ACPI_PARSE_EXECUTE) || (ACPI_FAILURE(status))) { > + ACPI_PARSE_EXECUTE && > + !(walk_state->parse_flags & > ACPI_PARSE_MODULE_LEVEL)) || > + (ACPI_FAILURE(status))) { > acpi_ds_terminate_control_method(walk_state-> > method_desc, > walk_state); > diff --git a/drivers/acpi/acpica/psxface.c b/drivers/acpi/acpica/psxface.c > index cf30cd82..fccc0e5 100644 > --- a/drivers/acpi/acpica/psxface.c > +++ b/drivers/acpi/acpica/psxface.c > @@ -252,6 +252,77 @@ cleanup: > > > /********************************************************* > ********************** > * > + * FUNCTION: acpi_ps_execute_table > + * > + * PARAMETERS: info - Method info block, contains: > + * node - Node to where the is entered into the > + * namespace > + * obj_desc - Pseudo method object describing the AML > + * code of the entire table > + * pass_number - Parse or execute pass > + * > + * RETURN: Status > + * > + * DESCRIPTION: Execute a table > + * > + > ********************************************************** > ********************/ > + > +acpi_status acpi_ps_execute_table(struct acpi_evaluate_info *info) > +{ > + acpi_status status; > + union acpi_parse_object *op = NULL; > + struct acpi_walk_state *walk_state = NULL; > + > + ACPI_FUNCTION_TRACE(ps_execute_table); > + > + /* Create and init a Root Node */ > + > + op = acpi_ps_create_scope_op(info->obj_desc- > >method.aml_start); > + if (!op) { > + status = AE_NO_MEMORY; > + goto cleanup; > + } > + > + /* Create and initialize a new walk state */ > + > + walk_state = > + acpi_ds_create_walk_state(info->obj_desc->method.owner_id, > NULL, > + NULL, NULL); > + if (!walk_state) { > + status = AE_NO_MEMORY; > + goto cleanup; > + } > + > + status = acpi_ds_init_aml_walk(walk_state, op, info->node, > + info->obj_desc->method.aml_start, > + info->obj_desc->method.aml_length, > info, > + info->pass_number); > + if (ACPI_FAILURE(status)) { > + goto cleanup; > + } > + > + if (info->obj_desc->method.info_flags & > ACPI_METHOD_MODULE_LEVEL) { > + walk_state->parse_flags |= ACPI_PARSE_MODULE_LEVEL; > + } > + > + /* > + * Parse the AML, walk_state will be deleted by parse_aml > + */ > + status = acpi_ps_parse_aml(walk_state); > + walk_state = NULL; > + > +cleanup: > + if (op) { > + acpi_ps_delete_parse_tree(op); > + } > + if (walk_state) { > + acpi_ds_delete_walk_state(walk_state); > + } > + return_ACPI_STATUS(status); > +} > + > +/******************************************************** > *********************** > + * > * FUNCTION: acpi_ps_update_parameter_list > * > * PARAMETERS: info - See struct acpi_evaluate_info > diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c > index ac71abc..2b9e87f 100644 > --- a/drivers/acpi/acpica/tbxfload.c > +++ b/drivers/acpi/acpica/tbxfload.c > @@ -103,7 +103,8 @@ acpi_status __init acpi_load_tables(void) > "While loading namespace from ACPI > tables")); > } > > - if (!acpi_gbl_group_module_level_code) { > + if (acpi_gbl_parse_table_as_term_list > + || !acpi_gbl_group_module_level_code) { > /* > * Initialize the objects that remain uninitialized. This > * runs the executable AML that may be part of the > diff --git a/drivers/acpi/acpica/utxfinit.c b/drivers/acpi/acpica/utxfinit.c > index 75b5f27..e41424a 100644 > --- a/drivers/acpi/acpica/utxfinit.c > +++ b/drivers/acpi/acpica/utxfinit.c > @@ -265,7 +265,8 @@ acpi_status __init acpi_initialize_objects(u32 flags) > * all of the tables have been loaded. It is a legacy option and is > * not compatible with other ACPI implementations. See > acpi_ns_load_table. > */ > - if (acpi_gbl_group_module_level_code) { > + if (!acpi_gbl_parse_table_as_term_list > + && acpi_gbl_group_module_level_code) { > acpi_ns_exec_module_code_list(); > > /* > diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h > index 4e4c214..22b0397 100644 > --- a/include/acpi/acpixf.h > +++ b/include/acpi/acpixf.h > @@ -195,6 +195,12 @@ ACPI_INIT_GLOBAL(u8, > acpi_gbl_do_not_use_xsdt, FALSE); > ACPI_INIT_GLOBAL(u8, acpi_gbl_group_module_level_code, FALSE); > > /* > + * Optionally support module level code by parsing the entire table as > + * a term_list. Default is FALSE, do not execute entire table. > + */ > +ACPI_INIT_GLOBAL(u8, acpi_gbl_parse_table_as_term_list, FALSE); > + > +/* > * Optionally use 32-bit FADT addresses if and when there is a conflict > * (address mismatch) between the 32-bit and 64-bit versions of the > * address. Although ACPICA adheres to the ACPI specification which > -- > 1.7.10 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 5, 2016 at 1:42 AM, Zheng, Lv <lv.zheng@intel.com> wrote: > Hi, Rafael > >> From: Zheng, Lv >> Subject: [PATCH v4 3/5] ACPICA: ACPI 2.0, Interpreter: Fix MLC issues by >> switching to new TermList grammar for table loading >> >> The MLC (Module Level Code) is an ACPICA terminology describing the >> AML >> code out of any control method, its support is the main contention of the >> interpreter behavior during the table loading. >> >> The original implementation of MLC in ACPICA had several issues: >> 1. Out of any control method, besides of the object creating opcodes, only >> the code blocks wrapped by "If/Else/While" opcodes were supported. >> 2. The supported MLC code blocks were executed after loading the table >> rather than being executed right in place. >> >> ========================================================== >> == >> The demo of this order issue is as follows: >> Name (OBJ1, 1) >> If (CND1 == 1) >> { >> Name (OBJ2, 2) >> } >> Name (OBJ3, 3) >> The original MLC support created OBJ2 after OBJ3's creation. >> >> ========================================================== >> == >> Other than these limitations, MLC support in ACPICA looks correct. And >> supporting this should be easy/natural for ACPICA, but enabling of this >> was >> blocked by some ACPICA internal and OSPM specific initialization order >> issues we've fixed recently. The wrong support started from the following >> false bug fixing commit: >> Commit: 80d7951177315f70b5ffd8663985fbf725d07799 >> Subject: Add support for module-level executable AML code. >> >> We can confirm Windows interpreter behavior via reverse engineering >> means. >> It can be proven that not only If/Else/While wrapped code blocks, all >> opcodes can be executed at the module level, including operation region >> accesses. And it can be proven that the MLC should be executed right in >> place, not in such a deferred way executed after loading the table. >> >> And the above facts indeed reflect the spec words around ACPI definition >> block tables (DSDT/SSDT/...), the entire table and the Scope object is >> defined by the AML specification in BNF style as: >> AMLCode := DefBlockHeader TermList >> DefScope := ScopeOp PkgLength NameString TermList >> The bodies of the scope opening terms (AMLCode/Scope) are all TermList, >> thus the table loading should be no difference than the control method >> evaluations as the body of the Method is also defined by the AML >> specification as TermList: >> DefMethod := MethodOp PkgLength NameString MethodFlags TermList >> The only difference is: after evaluating control method, created named >> objects may be freed due to no reference, while named objects created by >> the table loading should only be freed after unloading the table. >> >> So this patch follows the spec and the de-facto standard behavior, enables >> the new grammar (TermList) for the table loading. >> >> By doing so, beyond the fixes to the above issues, we can see additional >> differences comparing to the old grammar based table loading: >> 1. Originally, beyond the scope opening terms (AMLCode/Scope), >> If/Else/While wrapped code blocks under the scope creating terms >> (Device/PowerResource/Processor/ThermalZone) are also supported as >> deferred MLC, which violates the spec defined grammar where ObjectList >> is enforced. With MLC support improved as non-deferred, the interpreter >> parses such scope creating terms as TermList rather ObjectList like the >> scope opening terms. >> After probing the Windows behavior and proving that it also parses >> these >> terms as TermList, we submitted an ECR (Engineering Change Request) to >> the ASWG (ACPI Specification Working Group) to clarify this. The ECR is >> titled as "ASL Grammar Clarification for Executable AML Opcodes" and >> has >> been accepted by the ASWG. The new grammar will appear in ACPI >> specification 6.2. >> 2. Originally, Buffer/Package/OperationRegion/CreateXXXField/BankField >> arguments are evaluated in a deferred way after loading the table. With >> MLC support improved, they are also parsed right in place during the >> table loading. >> This is also Windows compliant and the only difference is the removal >> of the debugging messages implemented before >> acpi_ds_execute_arguments(), >> see Link 1 for the details. A previous commit should have ensured that >> acpi_check_address_range() won't regress. >> >> Note that enabling this feature may cause regressions due to long term >> Linux ACPI support on top of the wrong grammar. So this patch also >> prepares >> a global option to be used to roll back to the old grammar during the >> period between a regression is reported and the regression is >> root-cause-fixed. ACPICA BZ 963, fixed by Lv Zheng. >> >> Link 1: https://bugzilla.kernel.org/show_bug.cgi?id=112911 >> Link 2: https://bugs.acpica.org/show_bug.cgi?id=963 >> Tested-by: Chris Bainbridge <chris.bainbridge@gmail.com> >> Signed-off-by: Lv Zheng <lv.zheng@intel.com> >> --- >> drivers/acpi/acpica/acnamesp.h | 3 + >> drivers/acpi/acpica/acparser.h | 2 + >> drivers/acpi/acpica/evrgnini.c | 3 +- >> drivers/acpi/acpica/exconfig.c | 3 +- >> drivers/acpi/acpica/nsload.c | 3 +- >> drivers/acpi/acpica/nsparse.c | 166 >> ++++++++++++++++++++++++++++++++-------- >> drivers/acpi/acpica/psparse.c | 4 +- >> drivers/acpi/acpica/psxface.c | 71 +++++++++++++++++ >> drivers/acpi/acpica/tbxfload.c | 3 +- >> drivers/acpi/acpica/utxfinit.c | 3 +- >> include/acpi/acpixf.h | 6 ++ >> 11 files changed, 229 insertions(+), 38 deletions(-) >> >> diff --git a/drivers/acpi/acpica/acnamesp.h >> b/drivers/acpi/acpica/acnamesp.h >> index f33a4ba..829672a 100644 >> --- a/drivers/acpi/acpica/acnamesp.h >> +++ b/drivers/acpi/acpica/acnamesp.h >> @@ -130,6 +130,9 @@ acpi_status >> acpi_ns_parse_table(u32 table_index, struct acpi_namespace_node >> *start_node); >> >> acpi_status >> +acpi_ns_execute_table(u32 table_index, struct acpi_namespace_node >> *start_node); >> + >> +acpi_status >> acpi_ns_one_complete_parse(u32 pass_number, >> u32 table_index, >> struct acpi_namespace_node *start_node); >> diff --git a/drivers/acpi/acpica/acparser.h b/drivers/acpi/acpica/acparser.h >> index fc30577..939d411 100644 >> --- a/drivers/acpi/acpica/acparser.h >> +++ b/drivers/acpi/acpica/acparser.h >> @@ -78,6 +78,8 @@ extern const u8 acpi_gbl_long_op_index[]; >> */ >> acpi_status acpi_ps_execute_method(struct acpi_evaluate_info *info); >> >> +acpi_status acpi_ps_execute_table(struct acpi_evaluate_info *info); >> + >> /* >> * psargs - Parse AML opcode arguments >> */ >> diff --git a/drivers/acpi/acpica/evrgnini.c b/drivers/acpi/acpica/evrgnini.c >> index b6ea9c0..3843f1f 100644 >> --- a/drivers/acpi/acpica/evrgnini.c >> +++ b/drivers/acpi/acpica/evrgnini.c >> @@ -553,7 +553,8 @@ acpi_ev_initialize_region(union >> acpi_operand_object *region_obj, >> * >> * See acpi_ns_exec_module_code >> */ >> - if (obj_desc->method. >> + if (!acpi_gbl_parse_table_as_term_list && >> + obj_desc->method. >> info_flags & >> ACPI_METHOD_MODULE_LEVEL) { >> handler_obj = >> obj_desc- >> >method.dispatch.handler; >> diff --git a/drivers/acpi/acpica/exconfig.c b/drivers/acpi/acpica/exconfig.c >> index 21932d6..75679bc 100644 >> --- a/drivers/acpi/acpica/exconfig.c >> +++ b/drivers/acpi/acpica/exconfig.c >> @@ -120,7 +120,8 @@ acpi_ex_add_table(u32 table_index, >> /* Execute any module-level code that was found in the table */ >> >> acpi_ex_exit_interpreter(); >> - if (acpi_gbl_group_module_level_code) { >> + if (!acpi_gbl_parse_table_as_term_list >> + && acpi_gbl_group_module_level_code) { >> acpi_ns_exec_module_code_list(); >> } >> acpi_ex_enter_interpreter(); >> diff --git a/drivers/acpi/acpica/nsload.c b/drivers/acpi/acpica/nsload.c >> index b5e2b0a..2daa9a09 100644 >> --- a/drivers/acpi/acpica/nsload.c >> +++ b/drivers/acpi/acpica/nsload.c >> @@ -162,7 +162,8 @@ unlock: >> * other ACPI implementations. Optionally, the execution can be >> deferred >> * until later, see acpi_initialize_objects. >> */ >> - if (!acpi_gbl_group_module_level_code) { >> + if (!acpi_gbl_parse_table_as_term_list >> + && !acpi_gbl_group_module_level_code) { >> acpi_ns_exec_module_code_list(); >> } >> >> diff --git a/drivers/acpi/acpica/nsparse.c b/drivers/acpi/acpica/nsparse.c >> index 1783cd7..f475889 100644 >> --- a/drivers/acpi/acpica/nsparse.c >> +++ b/drivers/acpi/acpica/nsparse.c >> @@ -54,6 +54,98 @@ ACPI_MODULE_NAME("nsparse") >> >> >> /********************************************************* >> ********************** >> * >> + * FUNCTION: ns_execute_table >> + * >> + * PARAMETERS: table_desc - An ACPI table descriptor for table to >> parse >> + * start_node - Where to enter the table into the namespace >> + * >> + * RETURN: Status >> + * >> + * DESCRIPTION: Load ACPI/AML table by executing the entire table as a >> + * term_list. >> + * >> + >> ********************************************************** >> ********************/ >> +acpi_status >> +acpi_ns_execute_table(u32 table_index, struct acpi_namespace_node >> *start_node) >> +{ >> + acpi_status status; >> + struct acpi_table_header *table; >> + acpi_owner_id owner_id; >> + struct acpi_evaluate_info *info = NULL; >> + u32 aml_length; >> + u8 *aml_start; >> + union acpi_operand_object *method_obj = NULL; >> + >> + ACPI_FUNCTION_TRACE(ns_execute_table); >> + >> + status = acpi_get_table_by_index(table_index, &table); >> + if (ACPI_FAILURE(status)) { >> + return_ACPI_STATUS(status); >> + } >> + >> + /* Table must consist of at least a complete header */ >> + >> + if (table->length < sizeof(struct acpi_table_header)) { >> + return_ACPI_STATUS(AE_BAD_HEADER); >> + } >> + >> + aml_start = (u8 *)table + sizeof(struct acpi_table_header); >> + aml_length = table->length - sizeof(struct acpi_table_header); >> + >> + status = acpi_tb_get_owner_id(table_index, &owner_id); >> + if (ACPI_FAILURE(status)) { >> + return_ACPI_STATUS(status); >> + } >> + >> + /* Create, initialize, and link a new temporary method object */ >> + >> + method_obj = >> acpi_ut_create_internal_object(ACPI_TYPE_METHOD); >> + if (!method_obj) { >> + return_ACPI_STATUS(AE_NO_MEMORY); >> + } >> + >> + /* Allocate the evaluation information block */ >> + >> + info = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_evaluate_info)); >> + if (!info) { >> + status = AE_NO_MEMORY; >> + goto cleanup; >> + } >> + >> + ACPI_DEBUG_PRINT((ACPI_DB_PARSE, >> + "Create table code block: %p\n", method_obj)); >> + >> + method_obj->method.aml_start = aml_start; >> + method_obj->method.aml_length = aml_length; >> + method_obj->method.owner_id = owner_id; >> + method_obj->method.info_flags |= >> ACPI_METHOD_MODULE_LEVEL; >> + >> + info->pass_number = ACPI_IMODE_EXECUTE; >> + info->node = start_node; >> + info->obj_desc = method_obj; >> + info->node_flags = info->node->flags; >> + info->full_pathname = acpi_ns_get_normalized_pathname(info- >> >node, TRUE); >> + if (!info->full_pathname) { >> + status = AE_NO_MEMORY; >> + goto cleanup; >> + } >> + >> + (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); >> + status = acpi_ps_execute_table(info); >> + (void)acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE); > [Lv Zheng] > I got a report that we have lock order issue in the previous bug fix: > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2f38b1b1 > And I'll send a quick fix for that later today. > > After investigation, we found that ACPICA namespace lock design actually requires a big change in order to support this grammar change. > This means that this patch still need to be improved. > And the above 3 lines are currently known to be not compliant to the current ACPICA namespace lock design. > > So please drop this series, and I'll re-send this series after improving the ACPICA namespace lock design. > Dropped, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/acpica/acnamesp.h b/drivers/acpi/acpica/acnamesp.h index f33a4ba..829672a 100644 --- a/drivers/acpi/acpica/acnamesp.h +++ b/drivers/acpi/acpica/acnamesp.h @@ -130,6 +130,9 @@ acpi_status acpi_ns_parse_table(u32 table_index, struct acpi_namespace_node *start_node); acpi_status +acpi_ns_execute_table(u32 table_index, struct acpi_namespace_node *start_node); + +acpi_status acpi_ns_one_complete_parse(u32 pass_number, u32 table_index, struct acpi_namespace_node *start_node); diff --git a/drivers/acpi/acpica/acparser.h b/drivers/acpi/acpica/acparser.h index fc30577..939d411 100644 --- a/drivers/acpi/acpica/acparser.h +++ b/drivers/acpi/acpica/acparser.h @@ -78,6 +78,8 @@ extern const u8 acpi_gbl_long_op_index[]; */ acpi_status acpi_ps_execute_method(struct acpi_evaluate_info *info); +acpi_status acpi_ps_execute_table(struct acpi_evaluate_info *info); + /* * psargs - Parse AML opcode arguments */ diff --git a/drivers/acpi/acpica/evrgnini.c b/drivers/acpi/acpica/evrgnini.c index b6ea9c0..3843f1f 100644 --- a/drivers/acpi/acpica/evrgnini.c +++ b/drivers/acpi/acpica/evrgnini.c @@ -553,7 +553,8 @@ acpi_ev_initialize_region(union acpi_operand_object *region_obj, * * See acpi_ns_exec_module_code */ - if (obj_desc->method. + if (!acpi_gbl_parse_table_as_term_list && + obj_desc->method. info_flags & ACPI_METHOD_MODULE_LEVEL) { handler_obj = obj_desc->method.dispatch.handler; diff --git a/drivers/acpi/acpica/exconfig.c b/drivers/acpi/acpica/exconfig.c index 21932d6..75679bc 100644 --- a/drivers/acpi/acpica/exconfig.c +++ b/drivers/acpi/acpica/exconfig.c @@ -120,7 +120,8 @@ acpi_ex_add_table(u32 table_index, /* Execute any module-level code that was found in the table */ acpi_ex_exit_interpreter(); - if (acpi_gbl_group_module_level_code) { + if (!acpi_gbl_parse_table_as_term_list + && acpi_gbl_group_module_level_code) { acpi_ns_exec_module_code_list(); } acpi_ex_enter_interpreter(); diff --git a/drivers/acpi/acpica/nsload.c b/drivers/acpi/acpica/nsload.c index b5e2b0a..2daa9a09 100644 --- a/drivers/acpi/acpica/nsload.c +++ b/drivers/acpi/acpica/nsload.c @@ -162,7 +162,8 @@ unlock: * other ACPI implementations. Optionally, the execution can be deferred * until later, see acpi_initialize_objects. */ - if (!acpi_gbl_group_module_level_code) { + if (!acpi_gbl_parse_table_as_term_list + && !acpi_gbl_group_module_level_code) { acpi_ns_exec_module_code_list(); } diff --git a/drivers/acpi/acpica/nsparse.c b/drivers/acpi/acpica/nsparse.c index 1783cd7..f475889 100644 --- a/drivers/acpi/acpica/nsparse.c +++ b/drivers/acpi/acpica/nsparse.c @@ -54,6 +54,98 @@ ACPI_MODULE_NAME("nsparse") /******************************************************************************* * + * FUNCTION: ns_execute_table + * + * PARAMETERS: table_desc - An ACPI table descriptor for table to parse + * start_node - Where to enter the table into the namespace + * + * RETURN: Status + * + * DESCRIPTION: Load ACPI/AML table by executing the entire table as a + * term_list. + * + ******************************************************************************/ +acpi_status +acpi_ns_execute_table(u32 table_index, struct acpi_namespace_node *start_node) +{ + acpi_status status; + struct acpi_table_header *table; + acpi_owner_id owner_id; + struct acpi_evaluate_info *info = NULL; + u32 aml_length; + u8 *aml_start; + union acpi_operand_object *method_obj = NULL; + + ACPI_FUNCTION_TRACE(ns_execute_table); + + status = acpi_get_table_by_index(table_index, &table); + if (ACPI_FAILURE(status)) { + return_ACPI_STATUS(status); + } + + /* Table must consist of at least a complete header */ + + if (table->length < sizeof(struct acpi_table_header)) { + return_ACPI_STATUS(AE_BAD_HEADER); + } + + aml_start = (u8 *)table + sizeof(struct acpi_table_header); + aml_length = table->length - sizeof(struct acpi_table_header); + + status = acpi_tb_get_owner_id(table_index, &owner_id); + if (ACPI_FAILURE(status)) { + return_ACPI_STATUS(status); + } + + /* Create, initialize, and link a new temporary method object */ + + method_obj = acpi_ut_create_internal_object(ACPI_TYPE_METHOD); + if (!method_obj) { + return_ACPI_STATUS(AE_NO_MEMORY); + } + + /* Allocate the evaluation information block */ + + info = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_evaluate_info)); + if (!info) { + status = AE_NO_MEMORY; + goto cleanup; + } + + ACPI_DEBUG_PRINT((ACPI_DB_PARSE, + "Create table code block: %p\n", method_obj)); + + method_obj->method.aml_start = aml_start; + method_obj->method.aml_length = aml_length; + method_obj->method.owner_id = owner_id; + method_obj->method.info_flags |= ACPI_METHOD_MODULE_LEVEL; + + info->pass_number = ACPI_IMODE_EXECUTE; + info->node = start_node; + info->obj_desc = method_obj; + info->node_flags = info->node->flags; + info->full_pathname = acpi_ns_get_normalized_pathname(info->node, TRUE); + if (!info->full_pathname) { + status = AE_NO_MEMORY; + goto cleanup; + } + + (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); + status = acpi_ps_execute_table(info); + (void)acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE); + +cleanup: + if (info) { + ACPI_FREE(info->full_pathname); + info->full_pathname = NULL; + } + ACPI_FREE(info); + acpi_ut_remove_reference(method_obj); + return_ACPI_STATUS(status); +} + +/******************************************************************************* + * * FUNCTION: ns_one_complete_parse * * PARAMETERS: pass_number - 1 or 2 @@ -64,6 +156,7 @@ ACPI_MODULE_NAME("nsparse") * DESCRIPTION: Perform one complete parse of an ACPI/AML table. * ******************************************************************************/ + acpi_status acpi_ns_one_complete_parse(u32 pass_number, u32 table_index, @@ -173,38 +266,47 @@ acpi_ns_parse_table(u32 table_index, struct acpi_namespace_node *start_node) acpi_ex_enter_interpreter(); - /* - * AML Parse, pass 1 - * - * In this pass, we load most of the namespace. Control methods - * are not parsed until later. A parse tree is not created. Instead, - * each Parser Op subtree is deleted when it is finished. This saves - * a great deal of memory, and allows a small cache of parse objects - * to service the entire parse. The second pass of the parse then - * performs another complete parse of the AML. - */ - ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start pass 1\n")); - - status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS1, - table_index, start_node); - if (ACPI_FAILURE(status)) { - goto error_exit; - } - - /* - * AML Parse, pass 2 - * - * In this pass, we resolve forward references and other things - * that could not be completed during the first pass. - * Another complete parse of the AML is performed, but the - * overhead of this is compensated for by the fact that the - * parse objects are all cached. - */ - ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start pass 2\n")); - status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS2, - table_index, start_node); - if (ACPI_FAILURE(status)) { - goto error_exit; + if (acpi_gbl_parse_table_as_term_list) { + ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start load pass\n")); + + status = acpi_ns_execute_table(table_index, start_node); + if (ACPI_FAILURE(status)) { + goto error_exit; + } + } else { + /* + * AML Parse, pass 1 + * + * In this pass, we load most of the namespace. Control methods + * are not parsed until later. A parse tree is not created. + * Instead, each Parser Op subtree is deleted when it is finished. + * This saves a great deal of memory, and allows a small cache of + * parse objects to service the entire parse. The second pass of + * the parse then performs another complete parse of the AML. + */ + ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start pass 1\n")); + + status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS1, + table_index, start_node); + if (ACPI_FAILURE(status)) { + goto error_exit; + } + + /* + * AML Parse, pass 2 + * + * In this pass, we resolve forward references and other things + * that could not be completed during the first pass. + * Another complete parse of the AML is performed, but the + * overhead of this is compensated for by the fact that the + * parse objects are all cached. + */ + ACPI_DEBUG_PRINT((ACPI_DB_PARSE, "**** Start pass 2\n")); + status = acpi_ns_one_complete_parse(ACPI_IMODE_LOAD_PASS2, + table_index, start_node); + if (ACPI_FAILURE(status)) { + goto error_exit; + } } error_exit: diff --git a/drivers/acpi/acpica/psparse.c b/drivers/acpi/acpica/psparse.c index 0a23897..3aa9162 100644 --- a/drivers/acpi/acpica/psparse.c +++ b/drivers/acpi/acpica/psparse.c @@ -571,7 +571,9 @@ acpi_status acpi_ps_parse_aml(struct acpi_walk_state *walk_state) * cleanup to do */ if (((walk_state->parse_flags & ACPI_PARSE_MODE_MASK) == - ACPI_PARSE_EXECUTE) || (ACPI_FAILURE(status))) { + ACPI_PARSE_EXECUTE && + !(walk_state->parse_flags & ACPI_PARSE_MODULE_LEVEL)) || + (ACPI_FAILURE(status))) { acpi_ds_terminate_control_method(walk_state-> method_desc, walk_state); diff --git a/drivers/acpi/acpica/psxface.c b/drivers/acpi/acpica/psxface.c index cf30cd82..fccc0e5 100644 --- a/drivers/acpi/acpica/psxface.c +++ b/drivers/acpi/acpica/psxface.c @@ -252,6 +252,77 @@ cleanup: /******************************************************************************* * + * FUNCTION: acpi_ps_execute_table + * + * PARAMETERS: info - Method info block, contains: + * node - Node to where the is entered into the + * namespace + * obj_desc - Pseudo method object describing the AML + * code of the entire table + * pass_number - Parse or execute pass + * + * RETURN: Status + * + * DESCRIPTION: Execute a table + * + ******************************************************************************/ + +acpi_status acpi_ps_execute_table(struct acpi_evaluate_info *info) +{ + acpi_status status; + union acpi_parse_object *op = NULL; + struct acpi_walk_state *walk_state = NULL; + + ACPI_FUNCTION_TRACE(ps_execute_table); + + /* Create and init a Root Node */ + + op = acpi_ps_create_scope_op(info->obj_desc->method.aml_start); + if (!op) { + status = AE_NO_MEMORY; + goto cleanup; + } + + /* Create and initialize a new walk state */ + + walk_state = + acpi_ds_create_walk_state(info->obj_desc->method.owner_id, NULL, + NULL, NULL); + if (!walk_state) { + status = AE_NO_MEMORY; + goto cleanup; + } + + status = acpi_ds_init_aml_walk(walk_state, op, info->node, + info->obj_desc->method.aml_start, + info->obj_desc->method.aml_length, info, + info->pass_number); + if (ACPI_FAILURE(status)) { + goto cleanup; + } + + if (info->obj_desc->method.info_flags & ACPI_METHOD_MODULE_LEVEL) { + walk_state->parse_flags |= ACPI_PARSE_MODULE_LEVEL; + } + + /* + * Parse the AML, walk_state will be deleted by parse_aml + */ + status = acpi_ps_parse_aml(walk_state); + walk_state = NULL; + +cleanup: + if (op) { + acpi_ps_delete_parse_tree(op); + } + if (walk_state) { + acpi_ds_delete_walk_state(walk_state); + } + return_ACPI_STATUS(status); +} + +/******************************************************************************* + * * FUNCTION: acpi_ps_update_parameter_list * * PARAMETERS: info - See struct acpi_evaluate_info diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c index ac71abc..2b9e87f 100644 --- a/drivers/acpi/acpica/tbxfload.c +++ b/drivers/acpi/acpica/tbxfload.c @@ -103,7 +103,8 @@ acpi_status __init acpi_load_tables(void) "While loading namespace from ACPI tables")); } - if (!acpi_gbl_group_module_level_code) { + if (acpi_gbl_parse_table_as_term_list + || !acpi_gbl_group_module_level_code) { /* * Initialize the objects that remain uninitialized. This * runs the executable AML that may be part of the diff --git a/drivers/acpi/acpica/utxfinit.c b/drivers/acpi/acpica/utxfinit.c index 75b5f27..e41424a 100644 --- a/drivers/acpi/acpica/utxfinit.c +++ b/drivers/acpi/acpica/utxfinit.c @@ -265,7 +265,8 @@ acpi_status __init acpi_initialize_objects(u32 flags) * all of the tables have been loaded. It is a legacy option and is * not compatible with other ACPI implementations. See acpi_ns_load_table. */ - if (acpi_gbl_group_module_level_code) { + if (!acpi_gbl_parse_table_as_term_list + && acpi_gbl_group_module_level_code) { acpi_ns_exec_module_code_list(); /* diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index 4e4c214..22b0397 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -195,6 +195,12 @@ ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use_xsdt, FALSE); ACPI_INIT_GLOBAL(u8, acpi_gbl_group_module_level_code, FALSE); /* + * Optionally support module level code by parsing the entire table as + * a term_list. Default is FALSE, do not execute entire table. + */ +ACPI_INIT_GLOBAL(u8, acpi_gbl_parse_table_as_term_list, FALSE); + +/* * Optionally use 32-bit FADT addresses if and when there is a conflict * (address mismatch) between the 32-bit and 64-bit versions of the * address. Although ACPICA adheres to the ACPI specification which
The MLC (Module Level Code) is an ACPICA terminology describing the AML code out of any control method, its support is the main contention of the interpreter behavior during the table loading. The original implementation of MLC in ACPICA had several issues: 1. Out of any control method, besides of the object creating opcodes, only the code blocks wrapped by "If/Else/While" opcodes were supported. 2. The supported MLC code blocks were executed after loading the table rather than being executed right in place. ============================================================ The demo of this order issue is as follows: Name (OBJ1, 1) If (CND1 == 1) { Name (OBJ2, 2) } Name (OBJ3, 3) The original MLC support created OBJ2 after OBJ3's creation. ============================================================ Other than these limitations, MLC support in ACPICA looks correct. And supporting this should be easy/natural for ACPICA, but enabling of this was blocked by some ACPICA internal and OSPM specific initialization order issues we've fixed recently. The wrong support started from the following false bug fixing commit: Commit: 80d7951177315f70b5ffd8663985fbf725d07799 Subject: Add support for module-level executable AML code. We can confirm Windows interpreter behavior via reverse engineering means. It can be proven that not only If/Else/While wrapped code blocks, all opcodes can be executed at the module level, including operation region accesses. And it can be proven that the MLC should be executed right in place, not in such a deferred way executed after loading the table. And the above facts indeed reflect the spec words around ACPI definition block tables (DSDT/SSDT/...), the entire table and the Scope object is defined by the AML specification in BNF style as: AMLCode := DefBlockHeader TermList DefScope := ScopeOp PkgLength NameString TermList The bodies of the scope opening terms (AMLCode/Scope) are all TermList, thus the table loading should be no difference than the control method evaluations as the body of the Method is also defined by the AML specification as TermList: DefMethod := MethodOp PkgLength NameString MethodFlags TermList The only difference is: after evaluating control method, created named objects may be freed due to no reference, while named objects created by the table loading should only be freed after unloading the table. So this patch follows the spec and the de-facto standard behavior, enables the new grammar (TermList) for the table loading. By doing so, beyond the fixes to the above issues, we can see additional differences comparing to the old grammar based table loading: 1. Originally, beyond the scope opening terms (AMLCode/Scope), If/Else/While wrapped code blocks under the scope creating terms (Device/PowerResource/Processor/ThermalZone) are also supported as deferred MLC, which violates the spec defined grammar where ObjectList is enforced. With MLC support improved as non-deferred, the interpreter parses such scope creating terms as TermList rather ObjectList like the scope opening terms. After probing the Windows behavior and proving that it also parses these terms as TermList, we submitted an ECR (Engineering Change Request) to the ASWG (ACPI Specification Working Group) to clarify this. The ECR is titled as "ASL Grammar Clarification for Executable AML Opcodes" and has been accepted by the ASWG. The new grammar will appear in ACPI specification 6.2. 2. Originally, Buffer/Package/OperationRegion/CreateXXXField/BankField arguments are evaluated in a deferred way after loading the table. With MLC support improved, they are also parsed right in place during the table loading. This is also Windows compliant and the only difference is the removal of the debugging messages implemented before acpi_ds_execute_arguments(), see Link 1 for the details. A previous commit should have ensured that acpi_check_address_range() won't regress. Note that enabling this feature may cause regressions due to long term Linux ACPI support on top of the wrong grammar. So this patch also prepares a global option to be used to roll back to the old grammar during the period between a regression is reported and the regression is root-cause-fixed. ACPICA BZ 963, fixed by Lv Zheng. Link 1: https://bugzilla.kernel.org/show_bug.cgi?id=112911 Link 2: https://bugs.acpica.org/show_bug.cgi?id=963 Tested-by: Chris Bainbridge <chris.bainbridge@gmail.com> Signed-off-by: Lv Zheng <lv.zheng@intel.com> --- drivers/acpi/acpica/acnamesp.h | 3 + drivers/acpi/acpica/acparser.h | 2 + drivers/acpi/acpica/evrgnini.c | 3 +- drivers/acpi/acpica/exconfig.c | 3 +- drivers/acpi/acpica/nsload.c | 3 +- drivers/acpi/acpica/nsparse.c | 166 ++++++++++++++++++++++++++++++++-------- drivers/acpi/acpica/psparse.c | 4 +- drivers/acpi/acpica/psxface.c | 71 +++++++++++++++++ drivers/acpi/acpica/tbxfload.c | 3 +- drivers/acpi/acpica/utxfinit.c | 3 +- include/acpi/acpixf.h | 6 ++ 11 files changed, 229 insertions(+), 38 deletions(-)