Patchwork [01/15] ACPICA: Remove legacy module-level code support

login
register
mail settings
Submitter Erik Schmauss
Date Feb. 15, 2019, 9:36 p.m.
Message ID <20190215213620.26059-2-erik.schmauss@intel.com>
Download mbox | patch
Permalink /patch/728007/
State New
Headers show

Comments

Erik Schmauss - Feb. 15, 2019, 9:36 p.m.
ACPICA commit 47f5607c204719d9239a12b889df725225098c8f

Module-level code refers to executable ASL code that runs during
table load. This is typically used in ASL to declare named objects
based on a condition evaluated during table load like so:

definition_block(...)
{
  opreation_region (OPR1, system_memory, ...)
  Field (OPR1)
  {
    FLD1, 8 /* Assume that FLD1's value is 0x1 */
  }

  /* The if statement below is referred to as module-level code */

  If (FLD1)
  {
    /* Declare DEV1 conditionally */
    Device (DEV1) {...}
  }

  Device (DEV2)
  {
    ...
  }
}

In legacy module-level code, the execution of the If statement
was deferred after other modules were loaded. The order of
code execution for the table above is the following:

1.) Load OPR1 to the ACPI Namespace
2.) Load FLD1 to the ACPI Namespace (not intended for drivers)
3.) Load DEV2 to the ACPI Namespace
4.) Execute If (FLD1) and load DEV1 if the condition is true

This legacy approach can be problematic for tables that look like the
following:

definition_block(...)
{
  opreation_region (OPR1, system_memory, ...)
  Field (OPR1)
  {
    FLD1, 8 /* Assume that FLD1's value is 0x1 */
  }

  /* The if statement below is referred to as module-level code */

  If (FLD1)
  {
    /* Declare DEV1 conditionally */
    Device (DEV1) {...}
  }

  Scope (DEV1)
  {
    /* Add objects DEV1's scope */
    Name (OBJ1, 0x1234)
  }
}

When loading this in the legacy approach, Scope DEV1 gets evaluated
before the If statement. The following is the order of execution:

1.) Load OPR1 to the ACPI Namespace
2.) Load FLD1 to the ACPI Namespace (not intended for drivers)
3.) Add OBJ1 under DEV1's scope -- ERROR. DEV1 does not exist
4.) Execute If (FLD1) and load DEV1 if the condition is true

The legacy approach can never succeed for tables like this due to the
deferral of the module-level code. Due to this limitation, a new
module-level code was developed. This new approach exeutes if
statements in the order that they appear in the definition block.
With this approach, the order of execution for the above defintion
block is as follows:

1.) Load OPR1 to the ACPI Namespace
2.) Load FLD1 to the ACPI Namespace (not intended for drivers)
3.) Execute If (FLD1) and load DEV1 because the condition is true
4.) Add OBJ1 under DEV1's scope.

Since DEV1 is loaded in the namespace in step 3, step 4 executes
successfully.

This change removes support for the legacy module-level code
execution. From this point onward, the new module-level code
execution will be the official approach.

Link: https://github.com/acpica/acpica/commit/47f5607c
Signed-off-by: Erik Schmauss <erik.schmauss@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
---
 drivers/acpi/acpica/evrgnini.c |  19 ----
 drivers/acpi/acpica/nsparse.c  |  66 ++---------
 drivers/acpi/acpica/psloop.c   | 193 ---------------------------------
 drivers/acpi/acpica/tbxfload.c |  25 ++---
 include/acpi/acpixf.h          |   8 --
 5 files changed, 22 insertions(+), 289 deletions(-)

Patch

diff --git a/drivers/acpi/acpica/evrgnini.c b/drivers/acpi/acpica/evrgnini.c
index 163dd4a142eb..0b47bbcd2a23 100644
--- a/drivers/acpi/acpica/evrgnini.c
+++ b/drivers/acpi/acpica/evrgnini.c
@@ -516,25 +516,6 @@  acpi_status acpi_ev_initialize_region(union acpi_operand_object *region_obj)
 				handler_obj = obj_desc->common_notify.handler;
 				break;
 
-			case ACPI_TYPE_METHOD:
-				/*
-				 * If we are executing module level code, the original
-				 * Node's object was replaced by this Method object and we
-				 * saved the handler in the method object.
-				 *
-				 * Note: Only used for the legacy MLC support. Will
-				 * be removed in the future.
-				 *
-				 * See acpi_ns_exec_module_code
-				 */
-				if (!acpi_gbl_execute_tables_as_methods &&
-				    obj_desc->method.
-				    info_flags & ACPI_METHOD_MODULE_LEVEL) {
-					handler_obj =
-					    obj_desc->method.dispatch.handler;
-				}
-				break;
-
 			default:
 
 				/* Ignore other objects */
diff --git a/drivers/acpi/acpica/nsparse.c b/drivers/acpi/acpica/nsparse.c
index bfa408f7fd39..c0b4f7bedfab 100644
--- a/drivers/acpi/acpica/nsparse.c
+++ b/drivers/acpi/acpica/nsparse.c
@@ -253,61 +253,19 @@  acpi_ns_parse_table(u32 table_index, struct acpi_namespace_node *start_node)
 
 	ACPI_FUNCTION_TRACE(ns_parse_table);
 
-	if (acpi_gbl_execute_tables_as_methods) {
-		/*
-		 * This case executes the AML table as one large control method.
-		 * The point of this is to execute any module-level code in-place
-		 * as the table is parsed. Some AML code depends on this behavior.
-		 *
-		 * It is a run-time option at this time, but will eventually become
-		 * the default.
-		 *
-		 * Note: This causes the table to only have a single-pass parse.
-		 * However, this is compatible with other ACPI implementations.
-		 */
-		ACPI_DEBUG_PRINT_RAW((ACPI_DB_PARSE,
-				      "%s: **** Start table execution pass\n",
-				      ACPI_GET_FUNCTION_NAME));
-
-		status = acpi_ns_execute_table(table_index, start_node);
-		if (ACPI_FAILURE(status)) {
-			return_ACPI_STATUS(status);
-		}
-	} 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)) {
-			return_ACPI_STATUS(status);
-		}
+	/*
+	 * Executes the AML table as one large control method.
+	 * The point of this is to execute any module-level code in-place
+	 * as the table is parsed. Some AML code depends on this behavior.
+	 *
+	 * Note: This causes the table to only have a single-pass parse.
+	 * However, this is compatible with other ACPI implementations.
+	 */
+	ACPI_DEBUG_PRINT_RAW((ACPI_DB_PARSE,
+			      "%s: **** Start table execution pass\n",
+			      ACPI_GET_FUNCTION_NAME));
 
-		/*
-		 * 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)) {
-			return_ACPI_STATUS(status);
-		}
-	}
+	status = acpi_ns_execute_table(table_index, start_node);
 
 	return_ACPI_STATUS(status);
 }
diff --git a/drivers/acpi/acpica/psloop.c b/drivers/acpi/acpica/psloop.c
index 4796f17805ac..207805047bc4 100644
--- a/drivers/acpi/acpica/psloop.c
+++ b/drivers/acpi/acpica/psloop.c
@@ -32,10 +32,6 @@  static acpi_status
 acpi_ps_get_arguments(struct acpi_walk_state *walk_state,
 		      u8 * aml_op_start, union acpi_parse_object *op);
 
-static void
-acpi_ps_link_module_code(union acpi_parse_object *parent_op,
-			 u8 *aml_start, u32 aml_length, acpi_owner_id owner_id);
-
 /*******************************************************************************
  *
  * FUNCTION:    acpi_ps_get_arguments
@@ -56,7 +52,6 @@  acpi_ps_get_arguments(struct acpi_walk_state *walk_state,
 {
 	acpi_status status = AE_OK;
 	union acpi_parse_object *arg = NULL;
-	const struct acpi_opcode_info *op_info;
 
 	ACPI_FUNCTION_TRACE_PTR(ps_get_arguments, walk_state);
 
@@ -136,96 +131,6 @@  acpi_ps_get_arguments(struct acpi_walk_state *walk_state,
 				  walk_state->arg_count,
 				  walk_state->pass_number));
 
-		/*
-		 * This case handles the legacy option that groups all module-level
-		 * code blocks together and defers execution until all of the tables
-		 * are loaded. Execute all of these blocks at this time.
-		 * Execute any module-level code that was detected during the table
-		 * load phase.
-		 *
-		 * Note: this option is deprecated and will be eliminated in the
-		 * future. Use of this option can cause problems with AML code that
-		 * depends upon in-order immediate execution of module-level code.
-		 */
-		if (!acpi_gbl_execute_tables_as_methods &&
-		    (walk_state->pass_number <= ACPI_IMODE_LOAD_PASS2) &&
-		    ((walk_state->parse_flags & ACPI_PARSE_DISASSEMBLE) == 0)) {
-			/*
-			 * We want to skip If/Else/While constructs during Pass1 because we
-			 * want to actually conditionally execute the code during Pass2.
-			 *
-			 * Except for disassembly, where we always want to walk the
-			 * If/Else/While packages
-			 */
-			switch (op->common.aml_opcode) {
-			case AML_IF_OP:
-			case AML_ELSE_OP:
-			case AML_WHILE_OP:
-				/*
-				 * Currently supported module-level opcodes are:
-				 * IF/ELSE/WHILE. These appear to be the most common,
-				 * and easiest to support since they open an AML
-				 * package.
-				 */
-				if (walk_state->pass_number ==
-				    ACPI_IMODE_LOAD_PASS1) {
-					acpi_ps_link_module_code(op->common.
-								 parent,
-								 aml_op_start,
-								 (u32)
-								 (walk_state->
-								 parser_state.
-								 pkg_end -
-								 aml_op_start),
-								 walk_state->
-								 owner_id);
-				}
-
-				ACPI_DEBUG_PRINT((ACPI_DB_PARSE,
-						  "Pass1: Skipping an If/Else/While body\n"));
-
-				/* Skip body of if/else/while in pass 1 */
-
-				walk_state->parser_state.aml =
-				    walk_state->parser_state.pkg_end;
-				walk_state->arg_count = 0;
-				break;
-
-			default:
-				/*
-				 * Check for an unsupported executable opcode at module
-				 * level. We must be in PASS1, the parent must be a SCOPE,
-				 * The opcode class must be EXECUTE, and the opcode must
-				 * not be an argument to another opcode.
-				 */
-				if ((walk_state->pass_number ==
-				     ACPI_IMODE_LOAD_PASS1)
-				    && (op->common.parent->common.aml_opcode ==
-					AML_SCOPE_OP)) {
-					op_info =
-					    acpi_ps_get_opcode_info(op->common.
-								    aml_opcode);
-					if ((op_info->class ==
-					     AML_CLASS_EXECUTE) && (!arg)) {
-						ACPI_WARNING((AE_INFO,
-							      "Unsupported module-level executable opcode "
-							      "0x%.2X at table offset 0x%.4X",
-							      op->common.
-							      aml_opcode,
-							      (u32)
-							      (ACPI_PTR_DIFF
-							       (aml_op_start,
-								walk_state->
-								parser_state.
-								aml_start) +
-							       sizeof(struct
-								      acpi_table_header))));
-					}
-				}
-				break;
-			}
-		}
-
 		/* Special processing for certain opcodes */
 
 		switch (op->common.aml_opcode) {
@@ -300,104 +205,6 @@  acpi_ps_get_arguments(struct acpi_walk_state *walk_state,
 	return_ACPI_STATUS(AE_OK);
 }
 
-/*******************************************************************************
- *
- * FUNCTION:    acpi_ps_link_module_code
- *
- * PARAMETERS:  parent_op           - Parent parser op
- *              aml_start           - Pointer to the AML
- *              aml_length          - Length of executable AML
- *              owner_id            - owner_id of module level code
- *
- * RETURN:      None.
- *
- * DESCRIPTION: Wrap the module-level code with a method object and link the
- *              object to the global list. Note, the mutex field of the method
- *              object is used to link multiple module-level code objects.
- *
- * NOTE: In this legacy option, each block of detected executable AML
- * code that is outside of any control method is wrapped with a temporary
- * control method object and placed on a global list below.
- *
- * This function executes the module-level code for all tables only after
- * 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.
- *
- * This function will be removed when the legacy option is removed.
- *
- ******************************************************************************/
-
-static void
-acpi_ps_link_module_code(union acpi_parse_object *parent_op,
-			 u8 *aml_start, u32 aml_length, acpi_owner_id owner_id)
-{
-	union acpi_operand_object *prev;
-	union acpi_operand_object *next;
-	union acpi_operand_object *method_obj;
-	struct acpi_namespace_node *parent_node;
-
-	ACPI_FUNCTION_TRACE(ps_link_module_code);
-
-	/* Get the tail of the list */
-
-	prev = next = acpi_gbl_module_code_list;
-	while (next) {
-		prev = next;
-		next = next->method.mutex;
-	}
-
-	/*
-	 * Insert the module level code into the list. Merge it if it is
-	 * adjacent to the previous element.
-	 */
-	if (!prev ||
-	    ((prev->method.aml_start + prev->method.aml_length) != aml_start)) {
-
-		/* Create, initialize, and link a new temporary method object */
-
-		method_obj = acpi_ut_create_internal_object(ACPI_TYPE_METHOD);
-		if (!method_obj) {
-			return_VOID;
-		}
-
-		ACPI_DEBUG_PRINT((ACPI_DB_PARSE,
-				  "Create/Link new code block: %p\n",
-				  method_obj));
-
-		if (parent_op->common.node) {
-			parent_node = parent_op->common.node;
-		} else {
-			parent_node = acpi_gbl_root_node;
-		}
-
-		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;
-
-		/*
-		 * Save the parent node in next_object. This is cheating, but we
-		 * don't want to expand the method object.
-		 */
-		method_obj->method.next_object =
-		    ACPI_CAST_PTR(union acpi_operand_object, parent_node);
-
-		if (!prev) {
-			acpi_gbl_module_code_list = method_obj;
-		} else {
-			prev->method.mutex = method_obj;
-		}
-	} else {
-		ACPI_DEBUG_PRINT((ACPI_DB_PARSE,
-				  "Appending to existing code block: %p\n",
-				  prev));
-
-		prev->method.aml_length += aml_length;
-	}
-
-	return_VOID;
-}
-
 /*******************************************************************************
  *
  * FUNCTION:    acpi_ps_parse_loop
diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index 2c573fcc8c70..1a2592cc3245 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -69,23 +69,18 @@  acpi_status ACPI_INIT_FUNCTION acpi_load_tables(void)
 				"While loading namespace from ACPI tables"));
 	}
 
-	if (acpi_gbl_execute_tables_as_methods) {
-		/*
-		 * If the module-level code support is enabled, initialize the objects
-		 * in the namespace that remain uninitialized. This runs the executable
-		 * AML that may be part of the declaration of these name objects:
-		 *     operation_regions, buffer_fields, Buffers, and Packages.
-		 *
-		 * Note: The module-level code is optional at this time, but will
-		 * become the default in the future.
-		 */
-		status = acpi_ns_initialize_objects();
-		if (ACPI_FAILURE(status)) {
-			return_ACPI_STATUS(status);
-		}
+	/*
+	 * Initialize the objects in the namespace that remain uninitialized.
+	 * This runs the executable AML that may be part of the declaration of
+	 * these name objects:
+	 *     operation_regions, buffer_fields, Buffers, and Packages.
+	 *
+	 */
+	status = acpi_ns_initialize_objects();
+	if (ACPI_SUCCESS(status)) {
+		acpi_gbl_namespace_initialized = TRUE;
 	}
 
-	acpi_gbl_namespace_initialized = TRUE;
 	return_ACPI_STATUS(status);
 }
 
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 4cc74954fea3..bcf3bc06eb94 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -156,14 +156,6 @@  ACPI_INIT_GLOBAL(u8, acpi_gbl_copy_dsdt_locally, FALSE);
  */
 ACPI_INIT_GLOBAL(u8, acpi_gbl_do_not_use_xsdt, FALSE);
 
-/*
- * Optionally support module level code by parsing an entire table as
- * a method as it is loaded. Default is TRUE.
- * NOTE, this is essentially obsolete and will be removed soon
- * (01/2018).
- */
-ACPI_INIT_GLOBAL(u8, acpi_gbl_execute_tables_as_methods, TRUE);
-
 /*
  * 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