[PATCH 0/5] ACPI / sysfs: Cleanup sysfs table handling code

[PATCH 0/5] ACPI / sysfs: Cleanup sysfs table handling code

[Lv Zheng]

There are issues in sysfs table handling code:
1. It cannot handle table loaded by LoadTable opcode executed after
   acpi_sysfs_init();
2. It's signature handling code is not correct.

This patchset fixes these issues.

Lv Zheng (5):
  ACPICA: Tables: Remove acpi_tb_install_fixed_table()
  ACPICA: Tables: Remove wrong table event macros
  ACPICA: Tables: Add new table events indicating table
    installation/uninstallation
  ACPI / sysfs: Fix an issue for LoadTable opcode
  ACPI / sysfs: Update sysfs signature handling code

 drivers/acpi/acpica/actables.h |    7 +---
 drivers/acpi/acpica/tbfadt.c   |   24 +++++++------
 drivers/acpi/acpica/tbinstal.c |   73 +++++-----------------------------------
 drivers/acpi/sysfs.c           |   39 ++++++++++-----------
 include/acpi/actypes.h         |   16 ++++-----
 5 files changed, 51 insertions(+), 108 deletions(-)

-- 
1.7.10

[PATCH 1/5] ACPICA: Tables: Remove acpi_tb_install_fixed_table()

[Lv Zheng]

acpi_tb_install_fixed_table() is now redundant as we've removed the fixed
table indexing mechanism. This patch cleans up the code accordingly.
No functional change. Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/acpica/actables.h |    7 +----
 drivers/acpi/acpica/tbfadt.c   |   24 ++++++++-------
 drivers/acpi/acpica/tbinstal.c |   65 +---------------------------------------
 3 files changed, 16 insertions(+), 80 deletions(-)

diff --git a/drivers/acpi/acpica/actables.h b/drivers/acpi/acpica/actables.h
index 86d4d62..9469cd4 100644
--- a/drivers/acpi/acpica/actables.h
+++ b/drivers/acpi/acpica/actables.h
@@ -155,12 +155,7 @@ void
 acpi_tb_install_table_with_override(struct acpi_table_desc *new_table_desc,
 				    u8 override, u32 *table_index);
 
-acpi_status
-acpi_tb_install_fixed_table(acpi_physical_address address,
-			    char *signature, u32 *table_index);
-
-acpi_status ACPI_INIT_FUNCTION
-acpi_tb_parse_root_table(acpi_physical_address rsdp_address);
+acpi_status acpi_tb_parse_root_table(acpi_physical_address rsdp_address);
 
 /*
  * tbxfload
diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
index 016bcdc..e678d6f 100644
--- a/drivers/acpi/acpica/tbfadt.c
+++ b/drivers/acpi/acpica/tbfadt.c
@@ -344,23 +344,27 @@ void acpi_tb_parse_fadt(void)
 
 	/* Obtain the DSDT and FACS tables via their addresses within the FADT */
 
-	acpi_tb_install_fixed_table((acpi_physical_address)acpi_gbl_FADT.Xdsdt,
-				    ACPI_SIG_DSDT, &acpi_gbl_dsdt_index);
+	acpi_tb_install_standard_table((acpi_physical_address)acpi_gbl_FADT.
+				       Xdsdt,
+				       ACPI_TABLE_ORIGIN_INTERNAL_PHYSICAL,
+				       FALSE, TRUE, &acpi_gbl_dsdt_index);
 
 	/* If Hardware Reduced flag is set, there is no FACS */
 
 	if (!acpi_gbl_reduced_hardware) {
 		if (acpi_gbl_FADT.facs) {
-			acpi_tb_install_fixed_table((acpi_physical_address)
-						    acpi_gbl_FADT.facs,
-						    ACPI_SIG_FACS,
-						    &acpi_gbl_facs_index);
+			acpi_tb_install_standard_table((acpi_physical_address)
+						       acpi_gbl_FADT.facs,
+						       ACPI_TABLE_ORIGIN_INTERNAL_PHYSICAL,
+						       FALSE, TRUE,
+						       &acpi_gbl_facs_index);
 		}
 		if (acpi_gbl_FADT.Xfacs) {
-			acpi_tb_install_fixed_table((acpi_physical_address)
-						    acpi_gbl_FADT.Xfacs,
-						    ACPI_SIG_FACS,
-						    &acpi_gbl_xfacs_index);
+			acpi_tb_install_standard_table((acpi_physical_address)
+						       acpi_gbl_FADT.Xfacs,
+						       ACPI_TABLE_ORIGIN_INTERNAL_PHYSICAL,
+						       FALSE, TRUE,
+						       &acpi_gbl_xfacs_index);
 		}
 	}
 }
diff --git a/drivers/acpi/acpica/tbinstal.c b/drivers/acpi/acpica/tbinstal.c
index 8b13052..d461864 100644
--- a/drivers/acpi/acpica/tbinstal.c
+++ b/drivers/acpi/acpica/tbinstal.c
@@ -157,68 +157,6 @@ acpi_tb_install_table_with_override(struct acpi_table_desc *new_table_desc,
 
 /*******************************************************************************
  *
- * FUNCTION:    acpi_tb_install_fixed_table
- *
- * PARAMETERS:  address                 - Physical address of DSDT or FACS
- *              signature               - Table signature, NULL if no need to
- *                                        match
- *              table_index             - Where the table index is returned
- *
- * RETURN:      Status
- *
- * DESCRIPTION: Install a fixed ACPI table (DSDT/FACS) into the global data
- *              structure.
- *
- ******************************************************************************/
-
-acpi_status
-acpi_tb_install_fixed_table(acpi_physical_address address,
-			    char *signature, u32 *table_index)
-{
-	struct acpi_table_desc new_table_desc;
-	acpi_status status;
-
-	ACPI_FUNCTION_TRACE(tb_install_fixed_table);
-
-	if (!address) {
-		ACPI_ERROR((AE_INFO,
-			    "Null physical address for ACPI table [%s]",
-			    signature));
-		return (AE_NO_MEMORY);
-	}
-
-	/* Fill a table descriptor for validation */
-
-	status = acpi_tb_acquire_temp_table(&new_table_desc, address,
-					    ACPI_TABLE_ORIGIN_INTERNAL_PHYSICAL);
-	if (ACPI_FAILURE(status)) {
-		ACPI_ERROR((AE_INFO,
-			    "Could not acquire table length at %8.8X%8.8X",
-			    ACPI_FORMAT_UINT64(address)));
-		return_ACPI_STATUS(status);
-	}
-
-	/* Validate and verify a table before installation */
-
-	status = acpi_tb_verify_temp_table(&new_table_desc, signature);
-	if (ACPI_FAILURE(status)) {
-		goto release_and_exit;
-	}
-
-	/* Add the table to the global root table list */
-
-	acpi_tb_install_table_with_override(&new_table_desc, TRUE, table_index);
-
-release_and_exit:
-
-	/* Release the temporary table descriptor */
-
-	acpi_tb_release_temp_table(&new_table_desc);
-	return_ACPI_STATUS(status);
-}
-
-/*******************************************************************************
- *
  * FUNCTION:    acpi_tb_install_standard_table
  *
  * PARAMETERS:  address             - Address of the table (might be a virtual
@@ -230,8 +168,7 @@ release_and_exit:
  *
  * RETURN:      Status
  *
- * DESCRIPTION: This function is called to install an ACPI table that is
- *              neither DSDT nor FACS (a "standard" table.)
+ * DESCRIPTION: This function is called to verify and install an ACPI table.
  *              When this function is called by "Load" or "LoadTable" opcodes,
  *              or by acpi_load_table() API, the "Reload" parameter is set.
  *              After sucessfully returning from this function, table is
-- 
1.7.10

[PATCH 2/5] ACPICA: Tables: Remove wrong table event macros

[Lv Zheng]

There are wrong table event macros, this patch cleans them up.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 include/acpi/actypes.h |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index e96907b..892595f 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -1034,12 +1034,6 @@ struct acpi_statistics {
 	u32 method_count;
 };
 
-/* Table Event Types */
-
-#define ACPI_TABLE_EVENT_LOAD           0x0
-#define ACPI_TABLE_EVENT_UNLOAD         0x1
-#define ACPI_NUM_TABLE_EVENTS           2
-
 /*
  * Types specific to the OS service interfaces
  */
@@ -1091,9 +1085,11 @@ acpi_status (*acpi_exception_handler) (acpi_status aml_status,
 typedef
 acpi_status (*acpi_table_handler) (u32 event, void *table, void *context);
 
-#define ACPI_TABLE_LOAD             0x0
-#define ACPI_TABLE_UNLOAD           0x1
-#define ACPI_NUM_TABLE_EVENTS       2
+/* Table Event Types */
+
+#define ACPI_TABLE_EVENT_LOAD           0x0
+#define ACPI_TABLE_EVENT_UNLOAD         0x1
+#define ACPI_NUM_TABLE_EVENTS           2
 
 /* Address Spaces (For Operation Regions) */
 
-- 
1.7.10

[PATCH 3/5] ACPICA: Tables: Add new table events indicating table installation/uninstallation

[Lv Zheng]

This patch adds 2 new table events to indicate table
installation/uninstallation.

Currently, as ACPICA never uninstalls tables, this patch thus only add
table handler invocation for the table installation event. Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/acpica/tbinstal.c |    8 ++++++++
 include/acpi/actypes.h         |    4 +++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpica/tbinstal.c b/drivers/acpi/acpica/tbinstal.c
index d461864..5fdf251 100644
--- a/drivers/acpi/acpica/tbinstal.c
+++ b/drivers/acpi/acpica/tbinstal.c
@@ -301,6 +301,14 @@ acpi_tb_install_standard_table(acpi_physical_address address,
 	acpi_tb_install_table_with_override(&new_table_desc, override,
 					    table_index);
 
+	/* Invoke table handler if present */
+
+	if (acpi_gbl_table_handler) {
+		(void)acpi_gbl_table_handler(ACPI_TABLE_EVENT_INSTALL,
+					     new_table_desc.pointer,
+					     acpi_gbl_table_handler_context);
+	}
+
 release_and_exit:
 
 	/* Release the temporary table descriptor */
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index 892595f..1d798ab 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -1089,7 +1089,9 @@ acpi_status (*acpi_table_handler) (u32 event, void *table, void *context);
 
 #define ACPI_TABLE_EVENT_LOAD           0x0
 #define ACPI_TABLE_EVENT_UNLOAD         0x1
-#define ACPI_NUM_TABLE_EVENTS           2
+#define ACPI_TABLE_EVENT_INSTALL        0x2
+#define ACPI_TABLE_EVENT_UNINSTALL      0x3
+#define ACPI_NUM_TABLE_EVENTS           4
 
 /* Address Spaces (For Operation Regions) */
 
-- 
1.7.10

[PATCH 4/5] ACPI / sysfs: Fix an issue for LoadTable opcode

[Lv Zheng]

OEM tables can be installed via RSDT/XSDT, in this case, they have already
been created under /sys/firmware/acpi/tables.

For this kind of tables, normally LoadTable opcode will be executed to load
them. If LoadTable opcode is executed after acpi_sysfs_init(),
acpi_sysfs_table_handler() will be invoked, thus a redundant table file
will be created under /sys/firmware/acpi/tables/dynamic. Then running
"acpidump" on such platform results in an error.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=150841
Reported-by: Jason Voelz <jason.voelz@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/sysfs.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 358165e..f2fa363 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -383,7 +383,7 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
 	struct acpi_table_attr *table_attr;
 
 	switch (event) {
-	case ACPI_TABLE_EVENT_LOAD:
+	case ACPI_TABLE_EVENT_INSTALL:
 		table_attr =
 		    kzalloc(sizeof(struct acpi_table_attr), GFP_KERNEL);
 		if (!table_attr)
@@ -397,7 +397,9 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
 		} else
 			list_add_tail(&table_attr->node, &acpi_table_attr_list);
 		break;
+	case ACPI_TABLE_EVENT_LOAD:
 	case ACPI_TABLE_EVENT_UNLOAD:
+	case ACPI_TABLE_EVENT_UNINSTALL:
 		/*
 		 * we do not need to do anything right now
 		 * because the table is not deleted from the
-- 
1.7.10

[PATCH 5/5] ACPI / sysfs: Update sysfs signature handling code

[Lv Zheng]

This is a cleanup to covert sysfs signature handling code using ACPICA
APIs, so that sysfs code can automatically be benefit from any future
implementation in the name tag handling macros.
Additional filename attribute is added in order to handle both BE/LE name
tag cases.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/sysfs.c |   35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index f2fa363..974eb64 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -314,10 +314,13 @@ static struct kobject *tables_kobj;
 static struct kobject *dynamic_tables_kobj;
 static struct kobject *hotplug_kobj;
 
+#define ACPI_INST_SIZE		4	/* including trailing 0 */
+
 struct acpi_table_attr {
 	struct bin_attribute attr;
-	char name[8];
+	char name[ACPI_NAME_SIZE];
 	int instance;
+	char filename[ACPI_NAME_SIZE+ACPI_INST_SIZE];
 	struct list_head node;
 };
 
@@ -329,14 +332,9 @@ static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj,
 	    container_of(bin_attr, struct acpi_table_attr, attr);
 	struct acpi_table_header *table_header = NULL;
 	acpi_status status;
-	char name[ACPI_NAME_SIZE];
 
-	if (strncmp(table_attr->name, "NULL", 4))
-		memcpy(name, table_attr->name, ACPI_NAME_SIZE);
-	else
-		memcpy(name, "\0\0\0\0", 4);
-
-	status = acpi_get_table(name, table_attr->instance, &table_header);
+	status = acpi_get_table(table_attr->name, table_attr->instance,
+				&table_header);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
@@ -349,30 +347,31 @@ static void acpi_table_attr_init(struct acpi_table_attr *table_attr,
 {
 	struct acpi_table_header *header = NULL;
 	struct acpi_table_attr *attr = NULL;
+	char instance_str[ACPI_INST_SIZE];
 
 	sysfs_attr_init(&table_attr->attr.attr);
-	if (table_header->signature[0] != '\0')
-		memcpy(table_attr->name, table_header->signature,
-		       ACPI_NAME_SIZE);
-	else
-		memcpy(table_attr->name, "NULL", 4);
+	ACPI_MOVE_NAME(table_attr->name, table_header->signature);
 
 	list_for_each_entry(attr, &acpi_table_attr_list, node) {
-		if (!memcmp(table_attr->name, attr->name, ACPI_NAME_SIZE))
+		if (ACPI_COMPARE_NAME(table_attr->name, attr->name))
 			if (table_attr->instance < attr->instance)
 				table_attr->instance = attr->instance;
 	}
 	table_attr->instance++;
 
+	ACPI_MOVE_NAME(table_attr->filename, table_header->signature);
+	table_attr->filename[ACPI_NAME_SIZE] = '\0';
 	if (table_attr->instance > 1 || (table_attr->instance == 1 &&
 					 !acpi_get_table
-					 (table_header->signature, 2, &header)))
-		sprintf(table_attr->name + ACPI_NAME_SIZE, "%d",
-			table_attr->instance);
+					 (table_header->signature, 2, &header))) {
+		snprintf(instance_str, sizeof(instance_str), "%u",
+			 table_attr->instance);
+		strcat(table_attr->filename, instance_str);
+	}
 
 	table_attr->attr.size = table_header->length;
 	table_attr->attr.read = acpi_table_show;
-	table_attr->attr.attr.name = table_attr->name;
+	table_attr->attr.attr.name = table_attr->filename;
 	table_attr->attr.attr.mode = 0400;
 
 	return;
-- 
1.7.10

Re: [PATCH 0/5] ACPI / sysfs: Cleanup sysfs table handling code

[Rafael J. Wysocki]

On Tue, Aug 16, 2016 at 10:52 AM, Lv Zheng <lv.zheng@intel.com> wrote:
> There are issues in sysfs table handling code:
> 1. It cannot handle table loaded by LoadTable opcode executed after
>    acpi_sysfs_init();
> 2. It's signature handling code is not correct.
>
> This patchset fixes these issues.
>
> Lv Zheng (5):
>   ACPICA: Tables: Remove acpi_tb_install_fixed_table()
>   ACPICA: Tables: Remove wrong table event macros
>   ACPICA: Tables: Add new table events indicating table
>     installation/uninstallation
>   ACPI / sysfs: Fix an issue for LoadTable opcode
>   ACPI / sysfs: Update sysfs signature handling code
>
>  drivers/acpi/acpica/actables.h |    7 +---
>  drivers/acpi/acpica/tbfadt.c   |   24 +++++++------
>  drivers/acpi/acpica/tbinstal.c |   73 +++++-----------------------------------
>  drivers/acpi/sysfs.c           |   39 ++++++++++-----------
>  include/acpi/actypes.h         |   16 ++++-----
>  5 files changed, 51 insertions(+), 108 deletions(-)

Patches [1-3/5] clearly modify ACPICA.  Have they gone into the
upstream already?

Thanks,
Rafael

RE: [PATCH 0/5] ACPI / sysfs: Cleanup sysfs table handling code

[Zheng, Lv]

Hi,

> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J. Wysocki
> Subject: Re: [PATCH 0/5] ACPI / sysfs: Cleanup sysfs table handling code
> 
> On Tue, Aug 16, 2016 at 10:52 AM, Lv Zheng <lv.zheng@intel.com> wrote:
> > There are issues in sysfs table handling code:
> > 1. It cannot handle table loaded by LoadTable opcode executed after
> >    acpi_sysfs_init();
> > 2. It's signature handling code is not correct.
> >
> > This patchset fixes these issues.
> >
> > Lv Zheng (5):
> >   ACPICA: Tables: Remove acpi_tb_install_fixed_table()
> >   ACPICA: Tables: Remove wrong table event macros
> >   ACPICA: Tables: Add new table events indicating table
> >     installation/uninstallation
> >   ACPI / sysfs: Fix an issue for LoadTable opcode
> >   ACPI / sysfs: Update sysfs signature handling code
> >
> >  drivers/acpi/acpica/actables.h |    7 +---
> >  drivers/acpi/acpica/tbfadt.c   |   24 +++++++------
> >  drivers/acpi/acpica/tbinstal.c |   73 +++++-----------------------------------
> >  drivers/acpi/sysfs.c           |   39 ++++++++++-----------
> >  include/acpi/actypes.h         |   16 ++++-----
> >  5 files changed, 51 insertions(+), 108 deletions(-)
> 
> Patches [1-3/5] clearly modify ACPICA.  Have they gone into the
> upstream already?

No, they are fixing an urgent user reported bug.
So I posted them for the reporters.
We can wait until PATCH 1-3 merged in the ACPICA upstream.

Thanks
Lv

Re: [PATCH 0/5] ACPI / sysfs: Cleanup sysfs table handling code

[Rafael J. Wysocki]

On Wed, Aug 17, 2016 at 3:46 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
> Hi,
>
>> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J. Wysocki
>> Subject: Re: [PATCH 0/5] ACPI / sysfs: Cleanup sysfs table handling code
>>
>> On Tue, Aug 16, 2016 at 10:52 AM, Lv Zheng <lv.zheng@intel.com> wrote:
>> > There are issues in sysfs table handling code:
>> > 1. It cannot handle table loaded by LoadTable opcode executed after
>> >    acpi_sysfs_init();
>> > 2. It's signature handling code is not correct.
>> >
>> > This patchset fixes these issues.
>> >
>> > Lv Zheng (5):
>> >   ACPICA: Tables: Remove acpi_tb_install_fixed_table()
>> >   ACPICA: Tables: Remove wrong table event macros
>> >   ACPICA: Tables: Add new table events indicating table
>> >     installation/uninstallation
>> >   ACPI / sysfs: Fix an issue for LoadTable opcode
>> >   ACPI / sysfs: Update sysfs signature handling code
>> >
>> >  drivers/acpi/acpica/actables.h |    7 +---
>> >  drivers/acpi/acpica/tbfadt.c   |   24 +++++++------
>> >  drivers/acpi/acpica/tbinstal.c |   73 +++++-----------------------------------
>> >  drivers/acpi/sysfs.c           |   39 ++++++++++-----------
>> >  include/acpi/actypes.h         |   16 ++++-----
>> >  5 files changed, 51 insertions(+), 108 deletions(-)
>>
>> Patches [1-3/5] clearly modify ACPICA.  Have they gone into the
>> upstream already?
>
> No, they are fixing an urgent user reported bug.
> So I posted them for the reporters.
> We can wait until PATCH 1-3 merged in the ACPICA upstream.

OK

Thanks,
Rafael

[PATCH v2 0/3] ACPI / sysfs: Cleanup sysfs table handling code

[Lv Zheng]

There are issues in sysfs table handling code:
1. It cannot handle table loaded by LoadTable opcode executed after
   acpi_sysfs_init();
2. Its signature handling code is not correct.
This patchset fixes these issues.

This patchset also contains a regression fix, fixing a mistake caused in
ACPICA upstream due to a rebase problem, the wrong commit is released in
the ACPICA 20160831 release cycle.

Lv Zheng (3):
  ACPICA: Tables: Fix a regression in acpi_tb_find_table()
  ACPI / sysfs: Fix an issue for LoadTable opcode
  ACPI / sysfs: Update sysfs signature handling code

 drivers/acpi/acpica/tbfind.c |    2 +-
 drivers/acpi/sysfs.c         |   66 +++++++++++++++++++++++-------------------
 2 files changed, 37 insertions(+), 31 deletions(-)

-- 
1.7.10

[PATCH v2 1/3] ACPICA: Tables: Fix a regression in acpi_tb_find_table()

[Lv Zheng]

In the following commit, returning value of acpi_tb_find_table() is wrong:
 Subject: ACPICA: Tables: Tune table mutex to be a leaf lock
This causes LoadTable opcode to fail. This patch fixes this mistake.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/acpica/tbfind.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpica/tbfind.c b/drivers/acpi/acpica/tbfind.c
index a3f7b37..f6b9b4e 100644
--- a/drivers/acpi/acpica/tbfind.c
+++ b/drivers/acpi/acpica/tbfind.c
@@ -152,5 +152,5 @@ acpi_tb_find_table(char *signature,
 
 unlock_and_exit:
 	(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
-	return_ACPI_STATUS(AE_NOT_FOUND);
+	return_ACPI_STATUS(status);
 }
-- 
1.7.10

[PATCH v2 2/3] ACPI / sysfs: Fix an issue for LoadTable opcode

[Lv Zheng]

OEM tables can be installed via RSDT/XSDT, in this case, they have already
been created under /sys/firmware/acpi/tables.

For this kind of tables, normally LoadTable opcode will be executed to load
them. If LoadTable opcode is executed after acpi_sysfs_init(),
acpi_sysfs_table_handler() will be invoked, thus a redundant table file
will be created under /sys/firmware/acpi/tables/dynamic. Then running
"acpidump" on such platform results in an error, complaining blank empty
table (see Link 1 below).

The bug can be reproduced by customizing an OEM1 table, allowing it to be
overridden via 'table_sigs' (drivers/acpi/tables.c), adding the following
code to the customized DSDT to load it:
  Name (OEMH, Zero)
  Name (OEMF, One)
  If (LEqual (OEMF, One)) {
      Store (LoadTable ("OEM1", "Intel", "Test"), OEMH)
      Store (Zero, OEMF)
  }
In order to make sure that the OEM1 table is installed after
acpi_sysfs_init(), acpi_sysfs_init() can be moved before invoking
acpi_load_tables(). Then the following command execution result can be
seen:
 # acpidump > acpidump.txt
 Could not read table header: /sysfs/firmware/acpi/tables/dynamic/OEM12
 Could not get ACPI table at index 17, AE_BAD_HEADER

Link: https://bugzilla.kernel.org/show_bug.cgi?id=150841 # [1]
Link: https://github.com/acpica/acpica/commit/ed6a5fbc
Reported-by: Jason Voelz <jason.voelz@intel.com>
Reported-by: Francisco Leoner <francisco.j.lenoer.soto@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/sysfs.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 703c993..c88d4bd 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -383,7 +383,7 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
 	struct acpi_table_attr *table_attr;
 
 	switch (event) {
-	case ACPI_TABLE_EVENT_LOAD:
+	case ACPI_TABLE_EVENT_INSTALL:
 		table_attr =
 		    kzalloc(sizeof(struct acpi_table_attr), GFP_KERNEL);
 		if (!table_attr)
@@ -397,7 +397,9 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
 		} else
 			list_add_tail(&table_attr->node, &acpi_table_attr_list);
 		break;
+	case ACPI_TABLE_EVENT_LOAD:
 	case ACPI_TABLE_EVENT_UNLOAD:
+	case ACPI_TABLE_EVENT_UNINSTALL:
 		/*
 		 * we do not need to do anything right now
 		 * because the table is not deleted from the
-- 
1.7.10

[PATCH v2 3/3] ACPI / sysfs: Update sysfs signature handling code

[Lv Zheng]

This patch cleans up sysfs table signature handling code:
1. Convert the signature handling code to use the ACPICA APIs in order to
   be benefit from the future improvements of the APIs;
2. Add 'filename' attribute in order to handle both BE/LE name tags;
3. Add instance check in order to avoid the possible buffer overflow
   occurred to the table file name.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/sysfs.c |   62 +++++++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index c88d4bd..703c26e 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -314,10 +314,14 @@ static struct kobject *tables_kobj;
 static struct kobject *dynamic_tables_kobj;
 static struct kobject *hotplug_kobj;
 
+#define ACPI_MAX_TABLE_INSTANCES	999
+#define ACPI_INST_SIZE			4 /* including trailing 0 */
+
 struct acpi_table_attr {
 	struct bin_attribute attr;
-	char name[8];
+	char name[ACPI_NAME_SIZE];
 	int instance;
+	char filename[ACPI_NAME_SIZE+ACPI_INST_SIZE];
 	struct list_head node;
 };
 
@@ -329,14 +333,9 @@ static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj,
 	    container_of(bin_attr, struct acpi_table_attr, attr);
 	struct acpi_table_header *table_header = NULL;
 	acpi_status status;
-	char name[ACPI_NAME_SIZE];
 
-	if (strncmp(table_attr->name, "NULL", 4))
-		memcpy(name, table_attr->name, ACPI_NAME_SIZE);
-	else
-		memcpy(name, "\0\0\0\0", 4);
-
-	status = acpi_get_table(name, table_attr->instance, &table_header);
+	status = acpi_get_table(table_attr->name, table_attr->instance,
+				&table_header);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
@@ -344,38 +343,45 @@ static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj,
 				       table_header, table_header->length);
 }
 
-static void acpi_table_attr_init(struct acpi_table_attr *table_attr,
-				 struct acpi_table_header *table_header)
+static int acpi_table_attr_init(struct kobject *tables_obj,
+				struct acpi_table_attr *table_attr,
+				struct acpi_table_header *table_header)
 {
 	struct acpi_table_header *header = NULL;
 	struct acpi_table_attr *attr = NULL;
+	char instance_str[ACPI_INST_SIZE];
 
 	sysfs_attr_init(&table_attr->attr.attr);
-	if (table_header->signature[0] != '\0')
-		memcpy(table_attr->name, table_header->signature,
-		       ACPI_NAME_SIZE);
-	else
-		memcpy(table_attr->name, "NULL", 4);
+	ACPI_MOVE_NAME(table_attr->name, table_header->signature);
 
 	list_for_each_entry(attr, &acpi_table_attr_list, node) {
-		if (!memcmp(table_attr->name, attr->name, ACPI_NAME_SIZE))
+		if (ACPI_COMPARE_NAME(table_attr->name, attr->name))
 			if (table_attr->instance < attr->instance)
 				table_attr->instance = attr->instance;
 	}
 	table_attr->instance++;
+	if (table_attr->instance > ACPI_MAX_TABLE_INSTANCES) {
+		pr_warn("%4.4s: too many table instances\n",
+			table_attr->name);
+		return -ERANGE;
+	}
 
+	ACPI_MOVE_NAME(table_attr->filename, table_header->signature);
+	table_attr->filename[ACPI_NAME_SIZE] = '\0';
 	if (table_attr->instance > 1 || (table_attr->instance == 1 &&
 					 !acpi_get_table
-					 (table_header->signature, 2, &header)))
-		sprintf(table_attr->name + ACPI_NAME_SIZE, "%d",
-			table_attr->instance);
+					 (table_header->signature, 2, &header))) {
+		snprintf(instance_str, sizeof(instance_str), "%u",
+			 table_attr->instance);
+		strcat(table_attr->filename, instance_str);
+	}
 
 	table_attr->attr.size = table_header->length;
 	table_attr->attr.read = acpi_table_show;
-	table_attr->attr.attr.name = table_attr->name;
+	table_attr->attr.attr.name = table_attr->filename;
 	table_attr->attr.attr.mode = 0400;
 
-	return;
+	return sysfs_create_bin_file(tables_obj, &table_attr->attr);
 }
 
 acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
@@ -389,13 +395,12 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)
 		if (!table_attr)
 			return AE_NO_MEMORY;
 
-		acpi_table_attr_init(table_attr, table);
-		if (sysfs_create_bin_file(dynamic_tables_kobj,
-					  &table_attr->attr)) {
+		if (acpi_table_attr_init(dynamic_tables_kobj,
+					 table_attr, table)) {
 			kfree(table_attr);
 			return AE_ERROR;
-		} else
-			list_add_tail(&table_attr->node, &acpi_table_attr_list);
+		}
+		list_add_tail(&table_attr->node, &acpi_table_attr_list);
 		break;
 	case ACPI_TABLE_EVENT_LOAD:
 	case ACPI_TABLE_EVENT_UNLOAD:
@@ -437,13 +442,12 @@ static int acpi_tables_sysfs_init(void)
 		if (ACPI_FAILURE(status))
 			continue;
 
-		table_attr = NULL;
 		table_attr = kzalloc(sizeof(*table_attr), GFP_KERNEL);
 		if (!table_attr)
 			return -ENOMEM;
 
-		acpi_table_attr_init(table_attr, table_header);
-		ret = sysfs_create_bin_file(tables_kobj, &table_attr->attr);
+		ret = acpi_table_attr_init(tables_kobj,
+					   table_attr, table_header);
 		if (ret) {
 			kfree(table_attr);
 			return ret;
-- 
1.7.10