Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1533026 > unrolled thread

[PATCH 00/11] ACPICA: 20161117 Release

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng <lv.zheng@intel.com> - 2016-11-30 08:30 +0100
    [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel Lv Zheng <lv.zheng@intel.com> - 2016-11-30 08:30 +0100
      Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size()  and early_acpi_os_unmap_memory() from Linux kernel Dan Williams <dan.j.williams@intel.com> - 2016-12-08 02:20 +0100
        Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size()  and early_acpi_os_unmap_memory() from Linux kernel "Rafael J. Wysocki" <rafael@kernel.org> - 2016-12-08 14:20 +0100
          Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size()  and early_acpi_os_unmap_memory() from Linux kernel Dan Williams <dan.j.williams@intel.com> - 2016-12-08 20:10 +0100
            RE: [PATCH 08/11] ACPICA: Tables: Back port  acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel "Zheng, Lv" <lv.zheng@intel.com> - 2016-12-09 03:00 +0100
              Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size()  and early_acpi_os_unmap_memory() from Linux kernel "Rafael J. Wysocki" <rafael@kernel.org> - 2016-12-09 03:10 +0100
                RE: [PATCH 08/11] ACPICA: Tables: Back port  acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel "Zheng, Lv" <lv.zheng@intel.com> - 2016-12-09 03:30 +0100
              Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size()  and early_acpi_os_unmap_memory() from Linux kernel Dan Williams <dan.j.williams@intel.com> - 2016-12-09 03:10 +0100
                RE: [PATCH 08/11] ACPICA: Tables: Back port  acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel "Zheng, Lv" <lv.zheng@intel.com> - 2016-12-09 03:20 +0100
                RE: [PATCH 08/11] ACPICA: Tables: Back port  acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel "Zheng, Lv" <lv.zheng@intel.com> - 2016-12-09 03:30 +0100
        Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size()  and early_acpi_os_unmap_memory() from Linux kernel Dan Williams <dan.j.williams@intel.com> - 2016-12-09 03:00 +0100
        RE: [PATCH 08/11] ACPICA: Tables: Back port  acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel "Zheng, Lv" <lv.zheng@intel.com> - 2016-12-09 03:00 +0100
    [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size() Lv Zheng <lv.zheng@intel.com> - 2016-12-09 03:30 +0100
      Re: [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size() "Rafael J. Wysocki" <rafael@kernel.org> - 2016-12-09 04:50 +0100
        RE: [PATCH] ACPI / OSL: Fix a regression by returning table size  via acpi_get_table_with_size() "Zheng, Lv" <lv.zheng@intel.com> - 2016-12-09 07:10 +0100

#1533026 — [PATCH 00/11] ACPICA: 20161117 Release

The 20161117 ACPICA kernel-resident subsystem updates are linuxized based
on the linux-pm/linux-next branch.

The patchset has passed the following build/boot tests.
Build tests are performed as follows:
1. i386 + allyes
2. i386 + allno
3. i386 + default + ACPI_DEBUGGER=y
4. i386 + default + ACPI_DEBUGGER=n + ACPI_DEBUG=y
5. i386 + default + ACPI_DEBUG=n + ACPI=y
6. i386 + default + ACPI=n
7. x86_64 + allyes
8. x86_64 + allno
9. x86_64 + default + ACPI_DEBUGGER=y
10.x86_64 + default + ACPI_DEBUGGER=n + ACPI_DEBUG=y
11.x86_64 + default + ACPI_DEBUG=n + ACPI=y
12.x86_64 + default + ACPI=n
Boot tests are performed as follows:
1. i386 + default + ACPI_DEBUGGER=y
2. x86_64 + default + ACPI_DEBUGGER=y
Where:
1. i386: machine named as "Dell Inspiron Mini 1010"
2. x86_64: machine named as "Microsoft Surface Pro 3"
3. default: kernel configuration with following items enabled:
   All hardware drivers related to the machines of i386/x86_64
   All "drivers/acpi" configurations
   All "drivers/platform" drivers
   All other drivers that link the APIs provided by ACPICA subsystem

The divergences checking result:
Before applying (20160930 Release):
  508 lines
After applying (20161117 Release):
  467 lines

Bob Moore (3):
  ACPICA: Fix for implicit result conversion for the ToXXXX functions
  ACPICA: Utilities: Add new decode function for parser values
  ACPICA: Update version to 20161117

Lv Zheng (8):
  ACPICA: Namespace: Add acpi_ns_handle_to_name()
  ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock
    around AcpiEvInitializeRegion()"
  ACPICA: Events: Fix acpi_ev_initialize_region() return value
  ACPICA: Tables: Cleanup acpi_tb_install_and_load_table()
  ACPICA: Tables: Add acpi_tb_unload_table()
  ACPICA: Tables: Add an error message complaining driver bugs
  ACPICA: Tables: Back port acpi_get_table_with_size() and
    early_acpi_os_unmap_memory() from Linux kernel
  ACPICA: Tables: Allow FADT to be customized with virtual address

 drivers/acpi/acpica/acevents.h    |    4 +-
 drivers/acpi/acpica/acnamesp.h    |    3 +
 drivers/acpi/acpica/acopcode.h    |   14 ++--
 drivers/acpi/acpica/actables.h    |   11 ++-
 drivers/acpi/acpica/acutils.h     |    2 +
 drivers/acpi/acpica/amlcode.h     |   21 +++++-
 drivers/acpi/acpica/dsinit.c      |    4 +-
 drivers/acpi/acpica/dsopcode.c    |    2 +-
 drivers/acpi/acpica/dswload2.c    |   13 +---
 drivers/acpi/acpica/evrgnini.c    |   59 ++++++---------
 drivers/acpi/acpica/exconfig.c    |   42 +----------
 drivers/acpi/acpica/exconvrt.c    |    1 -
 drivers/acpi/acpica/exresop.c     |    1 -
 drivers/acpi/acpica/nsnames.c     |   45 ++++++++++++
 drivers/acpi/acpica/nsxfname.c    |   43 +++--------
 drivers/acpi/acpica/tbdata.c      |   81 +++++++++++---------
 drivers/acpi/acpica/tbfadt.c      |   14 ++--
 drivers/acpi/acpica/tbutils.c     |   85 +++++++++++++++++++++
 drivers/acpi/acpica/tbxface.c     |  146 +++++++++++++++++++++++++------------
 drivers/acpi/acpica/tbxfload.c    |   38 +---------
 drivers/acpi/acpica/utdecode.c    |   49 +++++++++++++
 drivers/acpi/osl.c                |   31 +++++++-
 include/acpi/acpixf.h             |   14 +++-
 include/acpi/actbl.h              |    1 +
 include/acpi/platform/aclinuxex.h |    1 -
 25 files changed, 458 insertions(+), 267 deletions(-)

-- 
1.7.10

[toc] | [next] | [standalone]

#1533027 — [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07

This patch back ports Linux acpi_get_table_with_size() and
early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.

The 2 APIs are used by Linux as table management APIs for long time, it
contains a hidden logic that during the early stage, the mapped tables
should be unmapped before the early stage ends.

During the early stage, tables are handled by the following sequence:
 acpi_get_table_with_size();
 parse the table
 early_acpi_os_unmap_memory();
During the late stage, tables are handled by the following sequence:
 acpi_get_table();
 parse the table
Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
late stage.

The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
remember the early mapped pointer in acpi_get_table() and Linux isn't able to
prevent ACPICA from using the wrong early mapped pointer during the late
stage as there is no API provided from ACPICA to be an inverse of
acpi_get_table() to forget the early mapped pointer.

But how ACPICA can work with the early/late stage requirement? Inside of
ACPICA, tables are ensured to be remained in "INSTALLED" state during the
early stage, and they are carefully not transitioned to "VALIDATED" state
until the late stage. So the same logic is in fact implemented inside of
ACPICA in a different way. The gap is only that the feature is not provided
to the OSPMs in an accessible external API style.

It then is possible to fix the gap by providing an inverse of
acpi_get_table() from ACPICA, so that the two Linux sequences can be
combined:
 acpi_get_table();
 parse the table
 acpi_put_table();
In order to work easier with the current Linux code, acpi_get_table() and
acpi_put_table() is implemented in a usage counting based style:
 1. When the usage count of the table is increased from 0 to 1, table is
    mapped and .Pointer is set with the mapping address (VALIDATED);
 2. When the usage count of the table is decreased from 1 to 0, .Pointer
    is unset and the mapping address is unmapped (INVALIDATED).
So that we can deploy the new APIs to Linux with minimal effort by just
invoking acpi_get_table() in acpi_get_table_with_size() and invoking
acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.

Link: https://github.com/acpica/acpica/commit/cac67909
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
---
 drivers/acpi/acpica/actables.h    |    6 ++
 drivers/acpi/acpica/tbutils.c     |   85 ++++++++++++++++++++++++
 drivers/acpi/acpica/tbxface.c     |  130 +++++++++++++++++++++++--------------
 drivers/acpi/osl.c                |   31 ++++++++-
 include/acpi/acpixf.h             |   12 +++-
 include/acpi/actbl.h              |    1 +
 include/acpi/platform/aclinuxex.h |    1 -
 7 files changed, 212 insertions(+), 54 deletions(-)

diff --git a/drivers/acpi/acpica/actables.h b/drivers/acpi/acpica/actables.h
index 7dd527f..94be8a8 100644
--- a/drivers/acpi/acpica/actables.h
+++ b/drivers/acpi/acpica/actables.h
@@ -166,6 +166,12 @@
 
 acpi_status acpi_tb_parse_root_table(acpi_physical_address rsdp_address);
 
+acpi_status
+acpi_tb_get_table(struct acpi_table_desc *table_desc,
+		  struct acpi_table_header **out_table);
+
+void acpi_tb_put_table(struct acpi_table_desc *table_desc);
+
 /*
  * tbxfload
  */
diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
index 51eb07c..86854e8 100644
--- a/drivers/acpi/acpica/tbutils.c
+++ b/drivers/acpi/acpica/tbutils.c
@@ -381,3 +381,88 @@ struct acpi_table_header *acpi_tb_copy_dsdt(u32 table_index)
 	acpi_os_unmap_memory(table, length);
 	return_ACPI_STATUS(AE_OK);
 }
+
+/*******************************************************************************
+ *
+ * FUNCTION:    acpi_tb_get_table
+ *
+ * PARAMETERS:  table_desc          - Table descriptor
+ *              out_table           - Where the pointer to the table is returned
+ *
+ * RETURN:      Status and pointer to the requested table
+ *
+ * DESCRIPTION: Increase a reference to a table descriptor and return the
+ *              validated table pointer.
+ *              If the table descriptor is an entry of the root table list,
+ *              this API must be invoked with ACPI_MTX_TABLES acquired.
+ *
+ ******************************************************************************/
+
+acpi_status
+acpi_tb_get_table(struct acpi_table_desc *table_desc,
+		  struct acpi_table_header **out_table)
+{
+	acpi_status status;
+
+	ACPI_FUNCTION_TRACE(acpi_tb_get_table);
+
+	if (table_desc->validation_count == 0) {
+
+		/* Table need to be "VALIDATED" */
+
+		status = acpi_tb_validate_table(table_desc);
+		if (ACPI_FAILURE(status)) {
+			return_ACPI_STATUS(status);
+		}
+	}
+
+	table_desc->validation_count++;
+	if (table_desc->validation_count == 0) {
+		ACPI_ERROR((AE_INFO,
+			    "Table %p, Validation count is zero after increment\n",
+			    table_desc));
+		table_desc->validation_count--;
+		return_ACPI_STATUS(AE_LIMIT);
+	}
+
+	*out_table = table_desc->pointer;
+	return_ACPI_STATUS(AE_OK);
+}
+
+/*******************************************************************************
+ *
+ * FUNCTION:    acpi_tb_put_table
+ *
+ * PARAMETERS:  table_desc          - Table descriptor
+ *
+ * RETURN:      None
+ *
+ * DESCRIPTION: Decrease a reference to a table descriptor and release the
+ *              validated table pointer if no references.
+ *              If the table descriptor is an entry of the root table list,
+ *              this API must be invoked with ACPI_MTX_TABLES acquired.
+ *
+ ******************************************************************************/
+
+void acpi_tb_put_table(struct acpi_table_desc *table_desc)
+{
+
+	ACPI_FUNCTION_TRACE(acpi_tb_put_table);
+
+	if (table_desc->validation_count == 0) {
+		ACPI_WARNING((AE_INFO,
+			      "Table %p, Validation count is zero before decrement\n",
+			      table_desc));
+		return_VOID;
+	}
+	table_desc->validation_count--;
+
+	if (table_desc->validation_count == 0) {
+
+		/* Table need to be "INVALIDATED" */
+
+		acpi_tb_invalidate_table(table_desc);
+	}
+
+	return_VOID;
+}
diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
index d5adb7a..7684707 100644
--- a/drivers/acpi/acpica/tbxface.c
+++ b/drivers/acpi/acpica/tbxface.c
@@ -282,7 +282,7 @@ acpi_status ACPI_INIT_FUNCTION acpi_reallocate_root_table(void)
 
 /*******************************************************************************
  *
- * FUNCTION:    acpi_get_table_with_size
+ * FUNCTION:    acpi_get_table
  *
  * PARAMETERS:  signature           - ACPI signature of needed table
  *              instance            - Which instance (for SSDTs)
@@ -292,16 +292,21 @@ acpi_status ACPI_INIT_FUNCTION acpi_reallocate_root_table(void)
  *
  * DESCRIPTION: Finds and verifies an ACPI table. Table must be in the
  *              RSDT/XSDT.
+ *              Note that an early stage acpi_get_table() call must be paired
+ *              with an early stage acpi_put_table() call. otherwise the table
+ *              pointer mapped by the early stage mapping implementation may be
+ *              erroneously unmapped by the late stage unmapping implementation
+ *              in an acpi_put_table() invoked during the late stage.
  *
  ******************************************************************************/
 acpi_status
-acpi_get_table_with_size(char *signature,
-	       u32 instance, struct acpi_table_header **out_table,
-	       acpi_size *tbl_size)
+acpi_get_table(char *signature,
+	       u32 instance, struct acpi_table_header ** out_table)
 {
 	u32 i;
 	u32 j;
-	acpi_status status;
+	acpi_status status = AE_NOT_FOUND;
+	struct acpi_table_desc *table_desc;
 
 	/* Parameter validation */
 
@@ -309,13 +314,22 @@ acpi_status ACPI_INIT_FUNCTION acpi_reallocate_root_table(void)
 		return (AE_BAD_PARAMETER);
 	}
 
+	/*
+	 * Note that the following line is required by some OSPMs, they only
+	 * check if the returned table is NULL instead of the returned status
+	 * to determined if this function is succeeded.
+	 */
+	*out_table = NULL;
+
+	(void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
+
 	/* Walk the root table list */
 
 	for (i = 0, j = 0; i < acpi_gbl_root_table_list.current_table_count;
 	     i++) {
-		if (!ACPI_COMPARE_NAME
-		    (&(acpi_gbl_root_table_list.tables[i].signature),
-		     signature)) {
+		table_desc = &acpi_gbl_root_table_list.tables[i];
+
+		if (!ACPI_COMPARE_NAME(&table_desc->signature, signature)) {
 			continue;
 		}
 
@@ -323,43 +337,65 @@ acpi_status ACPI_INIT_FUNCTION acpi_reallocate_root_table(void)
 			continue;
 		}
 
-		status =
-		    acpi_tb_validate_table(&acpi_gbl_root_table_list.tables[i]);
-		if (ACPI_SUCCESS(status)) {
-			*out_table = acpi_gbl_root_table_list.tables[i].pointer;
-			*tbl_size = acpi_gbl_root_table_list.tables[i].length;
-		}
-
-		if (!acpi_gbl_permanent_mmap) {
-			acpi_gbl_root_table_list.tables[i].pointer = NULL;
-		}
-
-		return (status);
+		status = acpi_tb_get_table(table_desc, out_table);
+		break;
 	}
 
-	return (AE_NOT_FOUND);
+	(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
+	return (status);
 }
 
-ACPI_EXPORT_SYMBOL(acpi_get_table_with_size)
+ACPI_EXPORT_SYMBOL(acpi_get_table)
 
-acpi_status
-acpi_get_table(char *signature,
-	       u32 instance, struct acpi_table_header **out_table)
+/*******************************************************************************
+ *
+ * FUNCTION:    acpi_put_table
+ *
+ * PARAMETERS:  table               - The pointer to the table
+ *
+ * RETURN:      None
+ *
+ * DESCRIPTION: Release a table returned by acpi_get_table() and its clones.
+ *              Note that it is not safe if this function was invoked after an
+ *              uninstallation happened to the original table descriptor.
+ *              Currently there is no OSPMs' requirement to handle such
+ *              situations.
+ *
+ ******************************************************************************/
+void acpi_put_table(struct acpi_table_header *table)
 {
-	acpi_size tbl_size;
+	u32 i;
+	struct acpi_table_desc *table_desc;
+
+	ACPI_FUNCTION_TRACE(acpi_put_table);
+
+	(void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
+
+	/* Walk the root table list */
+
+	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; i++) {
+		table_desc = &acpi_gbl_root_table_list.tables[i];
 
-	return acpi_get_table_with_size(signature,
-		       instance, out_table, &tbl_size);
+		if (table_desc->pointer != table) {
+			continue;
+		}
+
+		acpi_tb_put_table(table_desc);
+		break;
+	}
+
+	(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
+	return_VOID;
 }
 
-ACPI_EXPORT_SYMBOL(acpi_get_table)
+ACPI_EXPORT_SYMBOL(acpi_put_table)
 
 /*******************************************************************************
  *
  * FUNCTION:    acpi_get_table_by_index
  *
  * PARAMETERS:  table_index         - Table index
- *              table               - Where the pointer to the table is returned
+ *              out_table           - Where the pointer to the table is returned
  *
  * RETURN:      Status and pointer to the requested table
  *
@@ -368,7 +404,7 @@ acpi_status ACPI_INIT_FUNCTION acpi_reallocate_root_table(void)
  *
  ******************************************************************************/
 acpi_status
-acpi_get_table_by_index(u32 table_index, struct acpi_table_header **table)
+acpi_get_table_by_index(u32 table_index, struct acpi_table_header **out_table)
 {
 	acpi_status status;
 
@@ -376,35 +412,33 @@ acpi_status ACPI_INIT_FUNCTION acpi_reallocate_root_table(void)
 
 	/* Parameter validation */
 
-	if (!table) {
+	if (!out_table) {
 		return_ACPI_STATUS(AE_BAD_PARAMETER);
 	}
 
+	/*
+	 * Note that the following line is required by some OSPMs, they only
+	 * check if the returned table is NULL instead of the returned status
+	 * to determined if this function is succeeded.
+	 */
+	*out_table = NULL;
+
 	(void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
 
 	/* Validate index */
 
 	if (table_index >= acpi_gbl_root_table_list.current_table_count) {
-		(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
-		return_ACPI_STATUS(AE_BAD_PARAMETER);
+		status = AE_BAD_PARAMETER;
+		goto unlock_and_exit;
 	}
 
-	if (!acpi_gbl_root_table_list.tables[table_index].pointer) {
-
-		/* Table is not mapped, map it */
+	status =
+	    acpi_tb_get_table(&acpi_gbl_root_table_list.tables[table_index],
+			      out_table);
 
-		status =
-		    acpi_tb_validate_table(&acpi_gbl_root_table_list.
-					   tables[table_index]);
-		if (ACPI_FAILURE(status)) {
-			(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
-			return_ACPI_STATUS(status);
-		}
-	}
-
-	*table = acpi_gbl_root_table_list.tables[table_index].pointer;
+unlock_and_exit:
 	(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
-	return_ACPI_STATUS(AE_OK);
+	return_ACPI_STATUS(status);
 }
 
 ACPI_EXPORT_SYMBOL(acpi_get_table_by_index)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 416953a..4d8f2cd 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -433,10 +433,37 @@ void __ref acpi_os_unmap_memory(void *virt, acpi_size size)
 }
 EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
 
+/*******************************************************************************
+ *
+ * acpi_get_table_with_size()/early_acpi_os_unmap_memory():
+ *
+ * These 2 functions are traditionally used by Linux to map/unmap physical
+ * addressed ACPI tables during the early stage.
+ * They are deprectated now. Do not use them in the new code, but use
+ * acpi_get_table()/acpi_put_table() instead.
+ *
+ ******************************************************************************/
+acpi_status
+acpi_get_table_with_size(char *signature,
+	       u32 instance, struct acpi_table_header **out_table,
+	       acpi_size *tbl_size)
+{
+	acpi_status status;
+
+	status = acpi_get_table(signature, instance, out_table);
+	if (ACPI_SUCCESS(status)) {
+		/* No longer used by early_acpi_os_unmap_memory() */
+		*tbl_size = 0;
+	}
+
+	return (status);
+}
+
+ACPI_EXPORT_SYMBOL(acpi_get_table_with_size)
+
 void __init early_acpi_os_unmap_memory(void __iomem *virt, acpi_size size)
 {
-	if (!acpi_gbl_permanent_mmap)
-		__acpi_unmap_table(virt, size);
+	acpi_put_table(ACPI_CAST_PTR(struct acpi_table_header, virt));
 }
 
 int acpi_os_map_generic_address(struct acpi_generic_address *gas)
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 5c7356a..33828dd 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -513,10 +513,12 @@
 			     acpi_get_table(acpi_string signature, u32 instance,
 					    struct acpi_table_header
 					    **out_table))
+ACPI_EXTERNAL_RETURN_VOID(void acpi_put_table(struct acpi_table_header *table))
+
 ACPI_EXTERNAL_RETURN_STATUS(acpi_status
-			     acpi_get_table_by_index(u32 table_index,
-						     struct acpi_table_header
-						     **out_table))
+			    acpi_get_table_by_index(u32 table_index,
+						    struct acpi_table_header
+						    **out_table))
 ACPI_EXTERNAL_RETURN_STATUS(acpi_status
 			     acpi_install_table_handler(acpi_table_handler
 							handler, void *context))
@@ -974,6 +976,10 @@
 						     **out_table,
 						     acpi_size *tbl_size))
 
+ACPI_EXTERNAL_RETURN_VOID(void
+			  early_acpi_os_unmap_memory(void __iomem * virt,
+						     acpi_size size))
+
 ACPI_EXTERNAL_RETURN_STATUS(acpi_status
 			    acpi_get_data_full(acpi_handle object,
 					       acpi_object_handler handler,
diff --git a/include/acpi/actbl.h b/include/acpi/actbl.h
index c19700e..da5708c 100644
--- a/include/acpi/actbl.h
+++ b/include/acpi/actbl.h
@@ -371,6 +371,7 @@ struct acpi_table_desc {
 	union acpi_name_union signature;
 	acpi_owner_id owner_id;
 	u8 flags;
+	u16 validation_count;
 };
 
 /* Masks for Flags field above */
diff --git a/include/acpi/platform/aclinuxex.h b/include/acpi/platform/aclinuxex.h
index a5509d8..7dbb114 100644
--- a/include/acpi/platform/aclinuxex.h
+++ b/include/acpi/platform/aclinuxex.h
@@ -142,7 +142,6 @@ static inline void acpi_os_terminate_command_signals(void)
 /*
  * OSL interfaces added by Linux
  */
-void early_acpi_os_unmap_memory(void __iomem * virt, acpi_size size);
 
 #endif				/* __KERNEL__ */
 
-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

#1538241 — Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
>
> This patch back ports Linux acpi_get_table_with_size() and
> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
>
> The 2 APIs are used by Linux as table management APIs for long time, it
> contains a hidden logic that during the early stage, the mapped tables
> should be unmapped before the early stage ends.
>
> During the early stage, tables are handled by the following sequence:
>  acpi_get_table_with_size();
>  parse the table
>  early_acpi_os_unmap_memory();
> During the late stage, tables are handled by the following sequence:
>  acpi_get_table();
>  parse the table
> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
> late stage.
>
> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
> prevent ACPICA from using the wrong early mapped pointer during the late
> stage as there is no API provided from ACPICA to be an inverse of
> acpi_get_table() to forget the early mapped pointer.
>
> But how ACPICA can work with the early/late stage requirement? Inside of
> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
> early stage, and they are carefully not transitioned to "VALIDATED" state
> until the late stage. So the same logic is in fact implemented inside of
> ACPICA in a different way. The gap is only that the feature is not provided
> to the OSPMs in an accessible external API style.
>
> It then is possible to fix the gap by providing an inverse of
> acpi_get_table() from ACPICA, so that the two Linux sequences can be
> combined:
>  acpi_get_table();
>  parse the table
>  acpi_put_table();
> In order to work easier with the current Linux code, acpi_get_table() and
> acpi_put_table() is implemented in a usage counting based style:
>  1. When the usage count of the table is increased from 0 to 1, table is
>     mapped and .Pointer is set with the mapping address (VALIDATED);
>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
>     is unset and the mapping address is unmapped (INVALIDATED).
> So that we can deploy the new APIs to Linux with minimal effort by just
> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
>
> Link: https://github.com/acpica/acpica/commit/cac67909
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Signed-off-by: Bob Moore <robert.moore@intel.com>

This commit in -next (071b39575679 ACPICA: Tables: Back port
acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
kernel) causes a regression in my nfit/nvdimm test environment. The
nfit produced by QEMU no longer results in a nvdimm bus being created.

I have not root caused it, but I'm using the following command line
options to create an nfit in qemu-2.6.  Reverting the commit leads
compile failures.

qemu=$HOME/git/qemu/build/x86_64-softmmu/qemu-system-x86_64
mem=$HOME/mem
label_size=$((128*1024))
mem_size=$(((3*1024*1024*1024) + (64 * 1024 *1024)))
IMAGE=$HOME/ahci.img

kvm=(
        $qemu
        -enable-kvm
        -cpu kvm64
        -kernel $kernel
        -initrd $initrd
        -m 12G,slots=3,maxmem=40G

        -machine pc-i440fx-2.4,accel=kvm,usb=off,vmport=off,nvdimm
        -cpu SandyBridge
        -smp 2
        -netdev tap,id=hostnet0,ifname=tap0,script=no,downscript=no
        -device
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:b7:a1:ad,bus=pci.0,addr=0x7
        -object
memory-backend-file,id=mem1,share,mem-path=${mem},size=$((label_size +
mem_size))
        -device nvdimm,memdev=mem1,id=nv1,label-size=${label_size}
        -device ahci,id=sata0,bus=pci.0,addr=0x8
        -drive file=$IMAGE,if=none,id=drive-sata0-0-0,format=raw
        -device ide-hd,bus=sata0.0,drive=drive-sata0-0-0,id=sata0-0-0
        -boot order=nc
        -no-reboot
        -watchdog i6300esb
        -rtc base=localtime
        -serial stdio
        -display none
        -monitor null
)

[toc] | [prev] | [next] | [standalone]

#1538541 — Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

On Thu, Dec 8, 2016 at 2:11 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
>> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
>>
>> This patch back ports Linux acpi_get_table_with_size() and
>> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
>>
>> The 2 APIs are used by Linux as table management APIs for long time, it
>> contains a hidden logic that during the early stage, the mapped tables
>> should be unmapped before the early stage ends.
>>
>> During the early stage, tables are handled by the following sequence:
>>  acpi_get_table_with_size();
>>  parse the table
>>  early_acpi_os_unmap_memory();
>> During the late stage, tables are handled by the following sequence:
>>  acpi_get_table();
>>  parse the table
>> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
>> late stage.
>>
>> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
>> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
>> prevent ACPICA from using the wrong early mapped pointer during the late
>> stage as there is no API provided from ACPICA to be an inverse of
>> acpi_get_table() to forget the early mapped pointer.
>>
>> But how ACPICA can work with the early/late stage requirement? Inside of
>> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
>> early stage, and they are carefully not transitioned to "VALIDATED" state
>> until the late stage. So the same logic is in fact implemented inside of
>> ACPICA in a different way. The gap is only that the feature is not provided
>> to the OSPMs in an accessible external API style.
>>
>> It then is possible to fix the gap by providing an inverse of
>> acpi_get_table() from ACPICA, so that the two Linux sequences can be
>> combined:
>>  acpi_get_table();
>>  parse the table
>>  acpi_put_table();
>> In order to work easier with the current Linux code, acpi_get_table() and
>> acpi_put_table() is implemented in a usage counting based style:
>>  1. When the usage count of the table is increased from 0 to 1, table is
>>     mapped and .Pointer is set with the mapping address (VALIDATED);
>>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
>>     is unset and the mapping address is unmapped (INVALIDATED).
>> So that we can deploy the new APIs to Linux with minimal effort by just
>> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
>> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
>>
>> Link: https://github.com/acpica/acpica/commit/cac67909
>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> Signed-off-by: Bob Moore <robert.moore@intel.com>
>
> This commit in -next (071b39575679 ACPICA: Tables: Back port
> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
> kernel) causes a regression in my nfit/nvdimm test environment. The
> nfit produced by QEMU no longer results in a nvdimm bus being created.
>
> I have not root caused it, but I'm using the following command line
> options to create an nfit in qemu-2.6.  Reverting the commit leads
> compile failures.

Would the build problems go away if you reverted "ACPICA: Tables:
Allow FADT to be customized with virtual address" (linux-next commit
cf334d3174f9) in addition to it?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1538818 — Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

On Thu, Dec 8, 2016 at 5:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Thu, Dec 8, 2016 at 2:11 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
>>> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
>>>
>>> This patch back ports Linux acpi_get_table_with_size() and
>>> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
>>>
>>> The 2 APIs are used by Linux as table management APIs for long time, it
>>> contains a hidden logic that during the early stage, the mapped tables
>>> should be unmapped before the early stage ends.
>>>
>>> During the early stage, tables are handled by the following sequence:
>>>  acpi_get_table_with_size();
>>>  parse the table
>>>  early_acpi_os_unmap_memory();
>>> During the late stage, tables are handled by the following sequence:
>>>  acpi_get_table();
>>>  parse the table
>>> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
>>> late stage.
>>>
>>> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
>>> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
>>> prevent ACPICA from using the wrong early mapped pointer during the late
>>> stage as there is no API provided from ACPICA to be an inverse of
>>> acpi_get_table() to forget the early mapped pointer.
>>>
>>> But how ACPICA can work with the early/late stage requirement? Inside of
>>> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
>>> early stage, and they are carefully not transitioned to "VALIDATED" state
>>> until the late stage. So the same logic is in fact implemented inside of
>>> ACPICA in a different way. The gap is only that the feature is not provided
>>> to the OSPMs in an accessible external API style.
>>>
>>> It then is possible to fix the gap by providing an inverse of
>>> acpi_get_table() from ACPICA, so that the two Linux sequences can be
>>> combined:
>>>  acpi_get_table();
>>>  parse the table
>>>  acpi_put_table();
>>> In order to work easier with the current Linux code, acpi_get_table() and
>>> acpi_put_table() is implemented in a usage counting based style:
>>>  1. When the usage count of the table is increased from 0 to 1, table is
>>>     mapped and .Pointer is set with the mapping address (VALIDATED);
>>>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
>>>     is unset and the mapping address is unmapped (INVALIDATED).
>>> So that we can deploy the new APIs to Linux with minimal effort by just
>>> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
>>> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
>>>
>>> Link: https://github.com/acpica/acpica/commit/cac67909
>>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>>> Signed-off-by: Bob Moore <robert.moore@intel.com>
>>
>> This commit in -next (071b39575679 ACPICA: Tables: Back port
>> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
>> kernel) causes a regression in my nfit/nvdimm test environment. The
>> nfit produced by QEMU no longer results in a nvdimm bus being created.
>>
>> I have not root caused it, but I'm using the following command line
>> options to create an nfit in qemu-2.6.  Reverting the commit leads
>> compile failures.
>
> Would the build problems go away if you reverted "ACPICA: Tables:
> Allow FADT to be customized with virtual address" (linux-next commit
> cf334d3174f9) in addition to it?

Yes, reverting those two commits gets me back to a functional environment:

Revert "ACPICA: Tables: Allow FADT to be customized with virtual address"
Revert "ACPICA: Tables: Back port acpi_get_table_with_size() and
early_acpi_os_un

[toc] | [prev] | [next] | [standalone]

#1539001 — RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

Hi, Rafael and Dan

> From: Dan Williams [mailto:dan.j.williams@intel.com]
> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> early_acpi_os_unmap_memory() from Linux kernel
> 
> On Thu, Dec 8, 2016 at 5:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > On Thu, Dec 8, 2016 at 2:11 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> >> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
> >>> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
> >>>
> >>> This patch back ports Linux acpi_get_table_with_size() and
> >>> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
> >>>
> >>> The 2 APIs are used by Linux as table management APIs for long time, it
> >>> contains a hidden logic that during the early stage, the mapped tables
> >>> should be unmapped before the early stage ends.
> >>>
> >>> During the early stage, tables are handled by the following sequence:
> >>>  acpi_get_table_with_size();
> >>>  parse the table
> >>>  early_acpi_os_unmap_memory();
> >>> During the late stage, tables are handled by the following sequence:
> >>>  acpi_get_table();
> >>>  parse the table
> >>> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
> >>> late stage.
> >>>
> >>> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
> >>> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
> >>> prevent ACPICA from using the wrong early mapped pointer during the late
> >>> stage as there is no API provided from ACPICA to be an inverse of
> >>> acpi_get_table() to forget the early mapped pointer.
> >>>
> >>> But how ACPICA can work with the early/late stage requirement? Inside of
> >>> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
> >>> early stage, and they are carefully not transitioned to "VALIDATED" state
> >>> until the late stage. So the same logic is in fact implemented inside of
> >>> ACPICA in a different way. The gap is only that the feature is not provided
> >>> to the OSPMs in an accessible external API style.
> >>>
> >>> It then is possible to fix the gap by providing an inverse of
> >>> acpi_get_table() from ACPICA, so that the two Linux sequences can be
> >>> combined:
> >>>  acpi_get_table();
> >>>  parse the table
> >>>  acpi_put_table();
> >>> In order to work easier with the current Linux code, acpi_get_table() and
> >>> acpi_put_table() is implemented in a usage counting based style:
> >>>  1. When the usage count of the table is increased from 0 to 1, table is
> >>>     mapped and .Pointer is set with the mapping address (VALIDATED);
> >>>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
> >>>     is unset and the mapping address is unmapped (INVALIDATED).
> >>> So that we can deploy the new APIs to Linux with minimal effort by just
> >>> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
> >>> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
> >>>
> >>> Link: https://github.com/acpica/acpica/commit/cac67909
> >>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> >>> Signed-off-by: Bob Moore <robert.moore@intel.com>
> >>
> >> This commit in -next (071b39575679 ACPICA: Tables: Back port
> >> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
> >> kernel) causes a regression in my nfit/nvdimm test environment. The
> >> nfit produced by QEMU no longer results in a nvdimm bus being created.
> >>
> >> I have not root caused it, but I'm using the following command line
> >> options to create an nfit in qemu-2.6.  Reverting the commit leads
> >> compile failures.
> >
> > Would the build problems go away if you reverted "ACPICA: Tables:
> > Allow FADT to be customized with virtual address" (linux-next commit
> > cf334d3174f9) in addition to it?
> 
> Yes, reverting those two commits gets me back to a functional environment:
> 
> Revert "ACPICA: Tables: Allow FADT to be customized with virtual address"
> Revert "ACPICA: Tables: Back port acpi_get_table_with_size() and
> early_acpi_os_un

To Dan:
It seems in drivers/acpi/nfit/core.c.
The returned table size is used by the NFIT code.
I think it should be changed to use table_header->length.

To Rafael:
I can offer a quick fix for this by returning table_header->length from acpi_get_table_with_size().

Thanks and best regards
Lv

[toc] | [prev] | [next] | [standalone]

#1539006 — Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

On Fri, Dec 9, 2016 at 2:59 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
> Hi, Rafael and Dan
>
>> From: Dan Williams [mailto:dan.j.williams@intel.com]
>> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
>> early_acpi_os_unmap_memory() from Linux kernel
>>
>> On Thu, Dec 8, 2016 at 5:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> > On Thu, Dec 8, 2016 at 2:11 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>> >> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
>> >>> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
>> >>>
>> >>> This patch back ports Linux acpi_get_table_with_size() and
>> >>> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
>> >>>
>> >>> The 2 APIs are used by Linux as table management APIs for long time, it
>> >>> contains a hidden logic that during the early stage, the mapped tables
>> >>> should be unmapped before the early stage ends.
>> >>>
>> >>> During the early stage, tables are handled by the following sequence:
>> >>>  acpi_get_table_with_size();
>> >>>  parse the table
>> >>>  early_acpi_os_unmap_memory();
>> >>> During the late stage, tables are handled by the following sequence:
>> >>>  acpi_get_table();
>> >>>  parse the table
>> >>> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
>> >>> late stage.
>> >>>
>> >>> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
>> >>> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
>> >>> prevent ACPICA from using the wrong early mapped pointer during the late
>> >>> stage as there is no API provided from ACPICA to be an inverse of
>> >>> acpi_get_table() to forget the early mapped pointer.
>> >>>
>> >>> But how ACPICA can work with the early/late stage requirement? Inside of
>> >>> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
>> >>> early stage, and they are carefully not transitioned to "VALIDATED" state
>> >>> until the late stage. So the same logic is in fact implemented inside of
>> >>> ACPICA in a different way. The gap is only that the feature is not provided
>> >>> to the OSPMs in an accessible external API style.
>> >>>
>> >>> It then is possible to fix the gap by providing an inverse of
>> >>> acpi_get_table() from ACPICA, so that the two Linux sequences can be
>> >>> combined:
>> >>>  acpi_get_table();
>> >>>  parse the table
>> >>>  acpi_put_table();
>> >>> In order to work easier with the current Linux code, acpi_get_table() and
>> >>> acpi_put_table() is implemented in a usage counting based style:
>> >>>  1. When the usage count of the table is increased from 0 to 1, table is
>> >>>     mapped and .Pointer is set with the mapping address (VALIDATED);
>> >>>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
>> >>>     is unset and the mapping address is unmapped (INVALIDATED).
>> >>> So that we can deploy the new APIs to Linux with minimal effort by just
>> >>> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
>> >>> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
>> >>>
>> >>> Link: https://github.com/acpica/acpica/commit/cac67909
>> >>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> >>> Signed-off-by: Bob Moore <robert.moore@intel.com>
>> >>
>> >> This commit in -next (071b39575679 ACPICA: Tables: Back port
>> >> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
>> >> kernel) causes a regression in my nfit/nvdimm test environment. The
>> >> nfit produced by QEMU no longer results in a nvdimm bus being created.
>> >>
>> >> I have not root caused it, but I'm using the following command line
>> >> options to create an nfit in qemu-2.6.  Reverting the commit leads
>> >> compile failures.
>> >
>> > Would the build problems go away if you reverted "ACPICA: Tables:
>> > Allow FADT to be customized with virtual address" (linux-next commit
>> > cf334d3174f9) in addition to it?
>>
>> Yes, reverting those two commits gets me back to a functional environment:
>>
>> Revert "ACPICA: Tables: Allow FADT to be customized with virtual address"
>> Revert "ACPICA: Tables: Back port acpi_get_table_with_size() and
>> early_acpi_os_un
>
> To Dan:
> It seems in drivers/acpi/nfit/core.c.
> The returned table size is used by the NFIT code.
> I think it should be changed to use table_header->length.
>
> To Rafael:
> I can offer a quick fix for this by returning table_header->length from acpi_get_table_with_size().

OK

I've dropped the problematic ACPICA commit for now (along with the one
that depended on it) to prevent the issue from going in.

The change that you are suggesting would defeat the purpose of what
the NFIT code does.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1539016 — RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

Hi, Rafael

> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J. Wysocki
> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> early_acpi_os_unmap_memory() from Linux kernel
> 
> On Fri, Dec 9, 2016 at 2:59 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
> > Hi, Rafael and Dan
> >
> >> From: Dan Williams [mailto:dan.j.williams@intel.com]
> >> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> >> early_acpi_os_unmap_memory() from Linux kernel
> >>
> >> On Thu, Dec 8, 2016 at 5:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >> > On Thu, Dec 8, 2016 at 2:11 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> >> >> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
> >> >>> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
> >> >>>
> >> >>> This patch back ports Linux acpi_get_table_with_size() and
> >> >>> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
> >> >>>
> >> >>> The 2 APIs are used by Linux as table management APIs for long time, it
> >> >>> contains a hidden logic that during the early stage, the mapped tables
> >> >>> should be unmapped before the early stage ends.
> >> >>>
> >> >>> During the early stage, tables are handled by the following sequence:
> >> >>>  acpi_get_table_with_size();
> >> >>>  parse the table
> >> >>>  early_acpi_os_unmap_memory();
> >> >>> During the late stage, tables are handled by the following sequence:
> >> >>>  acpi_get_table();
> >> >>>  parse the table
> >> >>> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
> >> >>> late stage.
> >> >>>
> >> >>> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
> >> >>> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
> >> >>> prevent ACPICA from using the wrong early mapped pointer during the late
> >> >>> stage as there is no API provided from ACPICA to be an inverse of
> >> >>> acpi_get_table() to forget the early mapped pointer.
> >> >>>
> >> >>> But how ACPICA can work with the early/late stage requirement? Inside of
> >> >>> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
> >> >>> early stage, and they are carefully not transitioned to "VALIDATED" state
> >> >>> until the late stage. So the same logic is in fact implemented inside of
> >> >>> ACPICA in a different way. The gap is only that the feature is not provided
> >> >>> to the OSPMs in an accessible external API style.
> >> >>>
> >> >>> It then is possible to fix the gap by providing an inverse of
> >> >>> acpi_get_table() from ACPICA, so that the two Linux sequences can be
> >> >>> combined:
> >> >>>  acpi_get_table();
> >> >>>  parse the table
> >> >>>  acpi_put_table();
> >> >>> In order to work easier with the current Linux code, acpi_get_table() and
> >> >>> acpi_put_table() is implemented in a usage counting based style:
> >> >>>  1. When the usage count of the table is increased from 0 to 1, table is
> >> >>>     mapped and .Pointer is set with the mapping address (VALIDATED);
> >> >>>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
> >> >>>     is unset and the mapping address is unmapped (INVALIDATED).
> >> >>> So that we can deploy the new APIs to Linux with minimal effort by just
> >> >>> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
> >> >>> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
> >> >>>
> >> >>> Link: https://github.com/acpica/acpica/commit/cac67909
> >> >>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> >> >>> Signed-off-by: Bob Moore <robert.moore@intel.com>
> >> >>
> >> >> This commit in -next (071b39575679 ACPICA: Tables: Back port
> >> >> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
> >> >> kernel) causes a regression in my nfit/nvdimm test environment. The
> >> >> nfit produced by QEMU no longer results in a nvdimm bus being created.
> >> >>
> >> >> I have not root caused it, but I'm using the following command line
> >> >> options to create an nfit in qemu-2.6.  Reverting the commit leads
> >> >> compile failures.
> >> >
> >> > Would the build problems go away if you reverted "ACPICA: Tables:
> >> > Allow FADT to be customized with virtual address" (linux-next commit
> >> > cf334d3174f9) in addition to it?
> >>
> >> Yes, reverting those two commits gets me back to a functional environment:
> >>
> >> Revert "ACPICA: Tables: Allow FADT to be customized with virtual address"
> >> Revert "ACPICA: Tables: Back port acpi_get_table_with_size() and
> >> early_acpi_os_un
> >
> > To Dan:
> > It seems in drivers/acpi/nfit/core.c.
> > The returned table size is used by the NFIT code.
> > I think it should be changed to use table_header->length.
> >
> > To Rafael:
> > I can offer a quick fix for this by returning table_header->length from acpi_get_table_with_size().
> 
> OK
> 
> I've dropped the problematic ACPICA commit for now (along with the one
> that depended on it) to prevent the issue from going in.
> 
> The change that you are suggesting would defeat the purpose of what
> the NFIT code does.

I've just sent the regression fix.
Or we can just wait for Dan to confirm and apply the regression fix if it is working.

Thanks and best regards
Lv

[toc] | [prev] | [next] | [standalone]

#1539008 — Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

On Thu, Dec 8, 2016 at 5:59 PM, Zheng, Lv <lv.zheng@intel.com> wrote:
> Hi, Rafael and Dan
>
>> From: Dan Williams [mailto:dan.j.williams@intel.com]
>> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
>> early_acpi_os_unmap_memory() from Linux kernel
>>
>> On Thu, Dec 8, 2016 at 5:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> > On Thu, Dec 8, 2016 at 2:11 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>> >> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
>> >>> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
>> >>>
>> >>> This patch back ports Linux acpi_get_table_with_size() and
>> >>> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
>> >>>
>> >>> The 2 APIs are used by Linux as table management APIs for long time, it
>> >>> contains a hidden logic that during the early stage, the mapped tables
>> >>> should be unmapped before the early stage ends.
>> >>>
>> >>> During the early stage, tables are handled by the following sequence:
>> >>>  acpi_get_table_with_size();
>> >>>  parse the table
>> >>>  early_acpi_os_unmap_memory();
>> >>> During the late stage, tables are handled by the following sequence:
>> >>>  acpi_get_table();
>> >>>  parse the table
>> >>> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
>> >>> late stage.
>> >>>
>> >>> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
>> >>> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
>> >>> prevent ACPICA from using the wrong early mapped pointer during the late
>> >>> stage as there is no API provided from ACPICA to be an inverse of
>> >>> acpi_get_table() to forget the early mapped pointer.
>> >>>
>> >>> But how ACPICA can work with the early/late stage requirement? Inside of
>> >>> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
>> >>> early stage, and they are carefully not transitioned to "VALIDATED" state
>> >>> until the late stage. So the same logic is in fact implemented inside of
>> >>> ACPICA in a different way. The gap is only that the feature is not provided
>> >>> to the OSPMs in an accessible external API style.
>> >>>
>> >>> It then is possible to fix the gap by providing an inverse of
>> >>> acpi_get_table() from ACPICA, so that the two Linux sequences can be
>> >>> combined:
>> >>>  acpi_get_table();
>> >>>  parse the table
>> >>>  acpi_put_table();
>> >>> In order to work easier with the current Linux code, acpi_get_table() and
>> >>> acpi_put_table() is implemented in a usage counting based style:
>> >>>  1. When the usage count of the table is increased from 0 to 1, table is
>> >>>     mapped and .Pointer is set with the mapping address (VALIDATED);
>> >>>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
>> >>>     is unset and the mapping address is unmapped (INVALIDATED).
>> >>> So that we can deploy the new APIs to Linux with minimal effort by just
>> >>> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
>> >>> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
>> >>>
>> >>> Link: https://github.com/acpica/acpica/commit/cac67909
>> >>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> >>> Signed-off-by: Bob Moore <robert.moore@intel.com>
>> >>
>> >> This commit in -next (071b39575679 ACPICA: Tables: Back port
>> >> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
>> >> kernel) causes a regression in my nfit/nvdimm test environment. The
>> >> nfit produced by QEMU no longer results in a nvdimm bus being created.
>> >>
>> >> I have not root caused it, but I'm using the following command line
>> >> options to create an nfit in qemu-2.6.  Reverting the commit leads
>> >> compile failures.
>> >
>> > Would the build problems go away if you reverted "ACPICA: Tables:
>> > Allow FADT to be customized with virtual address" (linux-next commit
>> > cf334d3174f9) in addition to it?
>>
>> Yes, reverting those two commits gets me back to a functional environment:
>>
>> Revert "ACPICA: Tables: Allow FADT to be customized with virtual address"
>> Revert "ACPICA: Tables: Back port acpi_get_table_with_size() and
>> early_acpi_os_un
>
> To Dan:
> It seems in drivers/acpi/nfit/core.c.
> The returned table size is used by the NFIT code.
> I think it should be changed to use table_header->length.

Does the acpi core already validate that table_header->length is
correct? i.e. is is possible that a broken implementation could have
the wrong length in the header? I was assuming that was the purpose of
the _with_size(), but maybe I was wrong?

[toc] | [prev] | [next] | [standalone]

#1539009 — RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

Hi, Dan

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Dan
> Williams
> Sent: Friday, December 9, 2016 10:05 AM
> To: Zheng, Lv <lv.zheng@intel.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>; Rafael J.
> Wysocki <rjw@rjwysocki.net>; Brown, Len <len.brown@intel.com>; Lv Zheng <zetalog@gmail.com>; Linux
> Kernel Mailing List <linux-kernel@vger.kernel.org>; Linux ACPI <linux-acpi@vger.kernel.org>; Moore,
> Robert <robert.moore@intel.com>; linux-nvdimm@lists.01.org <linux-nvdimm@ml01.01.org>
> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> early_acpi_os_unmap_memory() from Linux kernel
> 
> On Thu, Dec 8, 2016 at 5:59 PM, Zheng, Lv <lv.zheng@intel.com> wrote:
> > Hi, Rafael and Dan
> >
> >> From: Dan Williams [mailto:dan.j.williams@intel.com]
> >> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> >> early_acpi_os_unmap_memory() from Linux kernel
> >>
> >> On Thu, Dec 8, 2016 at 5:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >> > On Thu, Dec 8, 2016 at 2:11 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> >> >> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
> >> >>> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
> >> >>>
> >> >>> This patch back ports Linux acpi_get_table_with_size() and
> >> >>> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
> >> >>>
> >> >>> The 2 APIs are used by Linux as table management APIs for long time, it
> >> >>> contains a hidden logic that during the early stage, the mapped tables
> >> >>> should be unmapped before the early stage ends.
> >> >>>
> >> >>> During the early stage, tables are handled by the following sequence:
> >> >>>  acpi_get_table_with_size();
> >> >>>  parse the table
> >> >>>  early_acpi_os_unmap_memory();
> >> >>> During the late stage, tables are handled by the following sequence:
> >> >>>  acpi_get_table();
> >> >>>  parse the table
> >> >>> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
> >> >>> late stage.
> >> >>>
> >> >>> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
> >> >>> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
> >> >>> prevent ACPICA from using the wrong early mapped pointer during the late
> >> >>> stage as there is no API provided from ACPICA to be an inverse of
> >> >>> acpi_get_table() to forget the early mapped pointer.
> >> >>>
> >> >>> But how ACPICA can work with the early/late stage requirement? Inside of
> >> >>> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
> >> >>> early stage, and they are carefully not transitioned to "VALIDATED" state
> >> >>> until the late stage. So the same logic is in fact implemented inside of
> >> >>> ACPICA in a different way. The gap is only that the feature is not provided
> >> >>> to the OSPMs in an accessible external API style.
> >> >>>
> >> >>> It then is possible to fix the gap by providing an inverse of
> >> >>> acpi_get_table() from ACPICA, so that the two Linux sequences can be
> >> >>> combined:
> >> >>>  acpi_get_table();
> >> >>>  parse the table
> >> >>>  acpi_put_table();
> >> >>> In order to work easier with the current Linux code, acpi_get_table() and
> >> >>> acpi_put_table() is implemented in a usage counting based style:
> >> >>>  1. When the usage count of the table is increased from 0 to 1, table is
> >> >>>     mapped and .Pointer is set with the mapping address (VALIDATED);
> >> >>>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
> >> >>>     is unset and the mapping address is unmapped (INVALIDATED).
> >> >>> So that we can deploy the new APIs to Linux with minimal effort by just
> >> >>> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
> >> >>> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
> >> >>>
> >> >>> Link: https://github.com/acpica/acpica/commit/cac67909
> >> >>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> >> >>> Signed-off-by: Bob Moore <robert.moore@intel.com>
> >> >>
> >> >> This commit in -next (071b39575679 ACPICA: Tables: Back port
> >> >> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
> >> >> kernel) causes a regression in my nfit/nvdimm test environment. The
> >> >> nfit produced by QEMU no longer results in a nvdimm bus being created.
> >> >>
> >> >> I have not root caused it, but I'm using the following command line
> >> >> options to create an nfit in qemu-2.6.  Reverting the commit leads
> >> >> compile failures.
> >> >
> >> > Would the build problems go away if you reverted "ACPICA: Tables:
> >> > Allow FADT to be customized with virtual address" (linux-next commit
> >> > cf334d3174f9) in addition to it?
> >>
> >> Yes, reverting those two commits gets me back to a functional environment:
> >>
> >> Revert "ACPICA: Tables: Allow FADT to be customized with virtual address"
> >> Revert "ACPICA: Tables: Back port acpi_get_table_with_size() and
> >> early_acpi_os_un
> >
> > To Dan:
> > It seems in drivers/acpi/nfit/core.c.
> > The returned table size is used by the NFIT code.
> > I think it should be changed to use table_header->length.
> 
> Does the acpi core already validate that table_header->length is
> correct? i.e. is is possible that a broken implementation could have
> the wrong length in the header? I was assuming that was the purpose of
> the _with_size(), but maybe I was wrong?

That should always be correct.
In acpi_tb_init_table_descriptor(), table_desc->length is set by table_header->length.
In acpi_tb_validate_table(), which calls acpi_tb_acquire_table(), acpi_os_map_memory() always uses table_desc->length.

Thanks and best regards
Lv

> --
> 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

[toc] | [prev] | [next] | [standalone]

#1539013 — RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

Hi, Dan

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Dan
> Williams
> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> early_acpi_os_unmap_memory() from Linux kernel
> 
> On Thu, Dec 8, 2016 at 5:59 PM, Zheng, Lv <lv.zheng@intel.com> wrote:
> > Hi, Rafael and Dan
> >
> >> From: Dan Williams [mailto:dan.j.williams@intel.com]
> >> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> >> early_acpi_os_unmap_memory() from Linux kernel
> >>
> >> On Thu, Dec 8, 2016 at 5:18 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >> > On Thu, Dec 8, 2016 at 2:11 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> >> >> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
> >> >>> ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
> >> >>>
> >> >>> This patch back ports Linux acpi_get_table_with_size() and
> >> >>> early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
> >> >>>
> >> >>> The 2 APIs are used by Linux as table management APIs for long time, it
> >> >>> contains a hidden logic that during the early stage, the mapped tables
> >> >>> should be unmapped before the early stage ends.
> >> >>>
> >> >>> During the early stage, tables are handled by the following sequence:
> >> >>>  acpi_get_table_with_size();
> >> >>>  parse the table
> >> >>>  early_acpi_os_unmap_memory();
> >> >>> During the late stage, tables are handled by the following sequence:
> >> >>>  acpi_get_table();
> >> >>>  parse the table
> >> >>> Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
> >> >>> late stage.
> >> >>>
> >> >>> The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
> >> >>> remember the early mapped pointer in acpi_get_table() and Linux isn't able to
> >> >>> prevent ACPICA from using the wrong early mapped pointer during the late
> >> >>> stage as there is no API provided from ACPICA to be an inverse of
> >> >>> acpi_get_table() to forget the early mapped pointer.
> >> >>>
> >> >>> But how ACPICA can work with the early/late stage requirement? Inside of
> >> >>> ACPICA, tables are ensured to be remained in "INSTALLED" state during the
> >> >>> early stage, and they are carefully not transitioned to "VALIDATED" state
> >> >>> until the late stage. So the same logic is in fact implemented inside of
> >> >>> ACPICA in a different way. The gap is only that the feature is not provided
> >> >>> to the OSPMs in an accessible external API style.
> >> >>>
> >> >>> It then is possible to fix the gap by providing an inverse of
> >> >>> acpi_get_table() from ACPICA, so that the two Linux sequences can be
> >> >>> combined:
> >> >>>  acpi_get_table();
> >> >>>  parse the table
> >> >>>  acpi_put_table();
> >> >>> In order to work easier with the current Linux code, acpi_get_table() and
> >> >>> acpi_put_table() is implemented in a usage counting based style:
> >> >>>  1. When the usage count of the table is increased from 0 to 1, table is
> >> >>>     mapped and .Pointer is set with the mapping address (VALIDATED);
> >> >>>  2. When the usage count of the table is decreased from 1 to 0, .Pointer
> >> >>>     is unset and the mapping address is unmapped (INVALIDATED).
> >> >>> So that we can deploy the new APIs to Linux with minimal effort by just
> >> >>> invoking acpi_get_table() in acpi_get_table_with_size() and invoking
> >> >>> acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
> >> >>>
> >> >>> Link: https://github.com/acpica/acpica/commit/cac67909
> >> >>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> >> >>> Signed-off-by: Bob Moore <robert.moore@intel.com>
> >> >>
> >> >> This commit in -next (071b39575679 ACPICA: Tables: Back port
> >> >> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
> >> >> kernel) causes a regression in my nfit/nvdimm test environment. The
> >> >> nfit produced by QEMU no longer results in a nvdimm bus being created.
> >> >>
> >> >> I have not root caused it, but I'm using the following command line
> >> >> options to create an nfit in qemu-2.6.  Reverting the commit leads
> >> >> compile failures.
> >> >
> >> > Would the build problems go away if you reverted "ACPICA: Tables:
> >> > Allow FADT to be customized with virtual address" (linux-next commit
> >> > cf334d3174f9) in addition to it?
> >>
> >> Yes, reverting those two commits gets me back to a functional environment:
> >>
> >> Revert "ACPICA: Tables: Allow FADT to be customized with virtual address"
> >> Revert "ACPICA: Tables: Back port acpi_get_table_with_size() and
> >> early_acpi_os_un
> >
> > To Dan:
> > It seems in drivers/acpi/nfit/core.c.
> > The returned table size is used by the NFIT code.
> > I think it should be changed to use table_header->length.
> 
> Does the acpi core already validate that table_header->length is
> correct? i.e. is is possible that a broken implementation could have
> the wrong length in the header? I was assuming that was the purpose of
> the _with_size(), but maybe I was wrong?

There is no such breakage.
The purpose is not that.

The purpose of the API is:
During the Linux early stage, it is required to unmap the early maps as such pointer cannot be used by late stage.
While ACPICA keeps such pointer in table_desc if acpi_get_table() is called.

So Linux guys provided acpi_get_table_with_size(), in which table_desc will not remember the mapped pointer.
The size must be returned to the caller as the caller need to use it as a parameter to unmap the memory during early stage.

Thanks and best regards
Lv



> --
> 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

[toc] | [prev] | [next] | [standalone]

#1539003 — Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

On Thu, Dec 8, 2016 at 5:49 PM, Zheng, Lv <lv.zheng@intel.com> wrote:
> Hi, Dan
>
> Good to see you here!
>
>> From: dan.j.williams@gmail.com [mailto:dan.j.williams@gmail.com] On Behalf Of Dan Williams
>> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
>> early_acpi_os_unmap_memory() from Linux kernel
>>
>> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
>> > ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
>> >
>> > This patch back ports Linux acpi_get_table_with_size() and
>> > early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
>> >
>> > The 2 APIs are used by Linux as table management APIs for long time, it
>> > contains a hidden logic that during the early stage, the mapped tables
>> > should be unmapped before the early stage ends.
>> >
>> > During the early stage, tables are handled by the following sequence:
>> >  acpi_get_table_with_size();
>> >  parse the table
>> >  early_acpi_os_unmap_memory();
>> > During the late stage, tables are handled by the following sequence:
>> >  acpi_get_table();
>> >  parse the table
>> > Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
>> > late stage.
>> >
>> > The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
>> > remember the early mapped pointer in acpi_get_table() and Linux isn't able to
>> > prevent ACPICA from using the wrong early mapped pointer during the late
>> > stage as there is no API provided from ACPICA to be an inverse of
>> > acpi_get_table() to forget the early mapped pointer.
>> >
>> > But how ACPICA can work with the early/late stage requirement? Inside of
>> > ACPICA, tables are ensured to be remained in "INSTALLED" state during the
>> > early stage, and they are carefully not transitioned to "VALIDATED" state
>> > until the late stage. So the same logic is in fact implemented inside of
>> > ACPICA in a different way. The gap is only that the feature is not provided
>> > to the OSPMs in an accessible external API style.
>> >
>> > It then is possible to fix the gap by providing an inverse of
>> > acpi_get_table() from ACPICA, so that the two Linux sequences can be
>> > combined:
>> >  acpi_get_table();
>> >  parse the table
>> >  acpi_put_table();
>> > In order to work easier with the current Linux code, acpi_get_table() and
>> > acpi_put_table() is implemented in a usage counting based style:
>> >  1. When the usage count of the table is increased from 0 to 1, table is
>> >     mapped and .Pointer is set with the mapping address (VALIDATED);
>> >  2. When the usage count of the table is decreased from 1 to 0, .Pointer
>> >     is unset and the mapping address is unmapped (INVALIDATED).
>> > So that we can deploy the new APIs to Linux with minimal effort by just
>> > invoking acpi_get_table() in acpi_get_table_with_size() and invoking
>> > acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
>> >
>> > Link: https://github.com/acpica/acpica/commit/cac67909
>> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> > Signed-off-by: Bob Moore <robert.moore@intel.com>
>>
>> This commit in -next (071b39575679 ACPICA: Tables: Back port
>> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
>> kernel) causes a regression in my nfit/nvdimm test environment. The
>> nfit produced by QEMU no longer results in a nvdimm bus being created.
>
> This commit is almost a no-op, unless some code in kernel is using the length field returned by old acpi_get_table_with_size().
>
>>
>> I have not root caused it, but I'm using the following command line
>> options to create an nfit in qemu-2.6.  Reverting the commit leads
>> compile failures.
>>
>> qemu=$HOME/git/qemu/build/x86_64-softmmu/qemu-system-x86_64
>> mem=$HOME/mem
>> label_size=$((128*1024))
>> mem_size=$(((3*1024*1024*1024) + (64 * 1024 *1024)))
>> IMAGE=$HOME/ahci.img
>>
>> kvm=(
>>         $qemu
>>         -enable-kvm
>>         -cpu kvm64
>>         -kernel $kernel
>>         -initrd $initrd
>>         -m 12G,slots=3,maxmem=40G
>>
>>         -machine pc-i440fx-2.4,accel=kvm,usb=off,vmport=off,nvdimm
>>         -cpu SandyBridge
>>         -smp 2
>>         -netdev tap,id=hostnet0,ifname=tap0,script=no,downscript=no
>>         -device
>> virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:b7:a1:ad,bus=pci.0,addr=0x7
>>         -object
>> memory-backend-file,id=mem1,share,mem-path=${mem},size=$((label_size +
>> mem_size))
>>         -device nvdimm,memdev=mem1,id=nv1,label-size=${label_size}
>>         -device ahci,id=sata0,bus=pci.0,addr=0x8
>>         -drive file=$IMAGE,if=none,id=drive-sata0-0-0,format=raw
>>         -device ide-hd,bus=sata0.0,drive=drive-sata0-0-0,id=sata0-0-0
>>         -boot order=nc
>>         -no-reboot
>>         -watchdog i6300esb
>>         -rtc base=localtime
>>         -serial stdio
>>         -display none
>>         -monitor null
>> )
>
> Let me file a kernel Bugzilla bug to track this issue:
> https://bugzilla.kernel.org/show_bug.cgi?id=189891
> And see if we can quickly fix it.
>
> Could you also point me the NFIT code that I should take a look at.
> Thanks in advance.
>

Yes, the nfit code is here:

drivers/acpi/nfit/core.c

...and yes it does currently use the size returned from
acpi_get_table_with_size() as a double-check against the size supplied
in the header. See acpi_nfit_add().

[toc] | [prev] | [next] | [standalone]

#1539005 — RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

Hi, Dan

Good to see you here!

> From: dan.j.williams@gmail.com [mailto:dan.j.williams@gmail.com] On Behalf Of Dan Williams
> Subject: Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and
> early_acpi_os_unmap_memory() from Linux kernel
> 
> On Tue, Nov 29, 2016 at 11:21 PM, Lv Zheng <lv.zheng@intel.com> wrote:
> > ACPICA commit cac6790954d4d752a083e6122220b8a22febcd07
> >
> > This patch back ports Linux acpi_get_table_with_size() and
> > early_acpi_os_unmap_memory() into ACPICA upstream to reduce divergences.
> >
> > The 2 APIs are used by Linux as table management APIs for long time, it
> > contains a hidden logic that during the early stage, the mapped tables
> > should be unmapped before the early stage ends.
> >
> > During the early stage, tables are handled by the following sequence:
> >  acpi_get_table_with_size();
> >  parse the table
> >  early_acpi_os_unmap_memory();
> > During the late stage, tables are handled by the following sequence:
> >  acpi_get_table();
> >  parse the table
> > Linux uses acpi_gbl_permanent_mmap to distinguish the early stage and the
> > late stage.
> >
> > The reasoning of introducing acpi_get_table_with_size() is: ACPICA will
> > remember the early mapped pointer in acpi_get_table() and Linux isn't able to
> > prevent ACPICA from using the wrong early mapped pointer during the late
> > stage as there is no API provided from ACPICA to be an inverse of
> > acpi_get_table() to forget the early mapped pointer.
> >
> > But how ACPICA can work with the early/late stage requirement? Inside of
> > ACPICA, tables are ensured to be remained in "INSTALLED" state during the
> > early stage, and they are carefully not transitioned to "VALIDATED" state
> > until the late stage. So the same logic is in fact implemented inside of
> > ACPICA in a different way. The gap is only that the feature is not provided
> > to the OSPMs in an accessible external API style.
> >
> > It then is possible to fix the gap by providing an inverse of
> > acpi_get_table() from ACPICA, so that the two Linux sequences can be
> > combined:
> >  acpi_get_table();
> >  parse the table
> >  acpi_put_table();
> > In order to work easier with the current Linux code, acpi_get_table() and
> > acpi_put_table() is implemented in a usage counting based style:
> >  1. When the usage count of the table is increased from 0 to 1, table is
> >     mapped and .Pointer is set with the mapping address (VALIDATED);
> >  2. When the usage count of the table is decreased from 1 to 0, .Pointer
> >     is unset and the mapping address is unmapped (INVALIDATED).
> > So that we can deploy the new APIs to Linux with minimal effort by just
> > invoking acpi_get_table() in acpi_get_table_with_size() and invoking
> > acpi_put_table() in early_acpi_os_unmap_memory(). Lv Zheng.
> >
> > Link: https://github.com/acpica/acpica/commit/cac67909
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > Signed-off-by: Bob Moore <robert.moore@intel.com>
> 
> This commit in -next (071b39575679 ACPICA: Tables: Back port
> acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux
> kernel) causes a regression in my nfit/nvdimm test environment. The
> nfit produced by QEMU no longer results in a nvdimm bus being created.

This commit is almost a no-op, unless some code in kernel is using the length field returned by old acpi_get_table_with_size().

> 
> I have not root caused it, but I'm using the following command line
> options to create an nfit in qemu-2.6.  Reverting the commit leads
> compile failures.
> 
> qemu=$HOME/git/qemu/build/x86_64-softmmu/qemu-system-x86_64
> mem=$HOME/mem
> label_size=$((128*1024))
> mem_size=$(((3*1024*1024*1024) + (64 * 1024 *1024)))
> IMAGE=$HOME/ahci.img
> 
> kvm=(
>         $qemu
>         -enable-kvm
>         -cpu kvm64
>         -kernel $kernel
>         -initrd $initrd
>         -m 12G,slots=3,maxmem=40G
> 
>         -machine pc-i440fx-2.4,accel=kvm,usb=off,vmport=off,nvdimm
>         -cpu SandyBridge
>         -smp 2
>         -netdev tap,id=hostnet0,ifname=tap0,script=no,downscript=no
>         -device
> virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:b7:a1:ad,bus=pci.0,addr=0x7
>         -object
> memory-backend-file,id=mem1,share,mem-path=${mem},size=$((label_size +
> mem_size))
>         -device nvdimm,memdev=mem1,id=nv1,label-size=${label_size}
>         -device ahci,id=sata0,bus=pci.0,addr=0x8
>         -drive file=$IMAGE,if=none,id=drive-sata0-0-0,format=raw
>         -device ide-hd,bus=sata0.0,drive=drive-sata0-0-0,id=sata0-0-0
>         -boot order=nc
>         -no-reboot
>         -watchdog i6300esb
>         -rtc base=localtime
>         -serial stdio
>         -display none
>         -monitor null
> )

Let me file a kernel Bugzilla bug to track this issue:
https://bugzilla.kernel.org/show_bug.cgi?id=189891
And see if we can quickly fix it.

Could you also point me the NFIT code that I should take a look at.
Thanks in advance.

Best regards
Lv

[toc] | [prev] | [next] | [standalone]

#1539012 — [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size()

The returned size is still used by the drivers.

Reported-by: Dan Williams <dan.j.williams@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/osl.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 5bef0f65..adf1ec4 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -445,8 +445,12 @@ void __ref acpi_os_unmap_memory(void *virt, acpi_size size)
 
 	status = acpi_get_table(signature, instance, out_table);
 	if (ACPI_SUCCESS(status)) {
-		/* No longer used by early_acpi_os_unmap_memory() */
-		*tbl_size = 0;
+		/*
+		 * No longer used by early_acpi_os_unmap_memory(), but still
+		 * used by the ACPI table drivers.
+		 */
+		if (*out_table)
+			*tbl_size = (*out_table)->length;
 	}
 
 	return (status);
-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

#1539052 — Re: [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size()

On Fri, Dec 9, 2016 at 3:21 AM, Lv Zheng <lv.zheng@intel.com> wrote:
> The returned size is still used by the drivers.
>
> Reported-by: Dan Williams <dan.j.williams@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> ---
>  drivers/acpi/osl.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 5bef0f65..adf1ec4 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -445,8 +445,12 @@ void __ref acpi_os_unmap_memory(void *virt, acpi_size size)
>
>         status = acpi_get_table(signature, instance, out_table);
>         if (ACPI_SUCCESS(status)) {
> -               /* No longer used by early_acpi_os_unmap_memory() */
> -               *tbl_size = 0;
> +               /*
> +                * No longer used by early_acpi_os_unmap_memory(), but still
> +                * used by the ACPI table drivers.
> +                */
> +               if (*out_table)
> +                       *tbl_size = (*out_table)->length;
>         }
>
>         return (status);
> --

The changelog doesn't explain anything.  Please say (a) what the
problem is and (b) how it is being addressed by your patch.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1539118 — RE: [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size()

Hi, Rafael

> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J. Wysocki
> Subject: Re: [PATCH] ACPI / OSL: Fix a regression by returning table size via
> acpi_get_table_with_size()
> 
> On Fri, Dec 9, 2016 at 3:21 AM, Lv Zheng <lv.zheng@intel.com> wrote:
> > The returned size is still used by the drivers.
> >
> > Reported-by: Dan Williams <dan.j.williams@intel.com>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > ---
> >  drivers/acpi/osl.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index 5bef0f65..adf1ec4 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -445,8 +445,12 @@ void __ref acpi_os_unmap_memory(void *virt, acpi_size size)
> >
> >         status = acpi_get_table(signature, instance, out_table);
> >         if (ACPI_SUCCESS(status)) {
> > -               /* No longer used by early_acpi_os_unmap_memory() */
> > -               *tbl_size = 0;
> > +               /*
> > +                * No longer used by early_acpi_os_unmap_memory(), but still
> > +                * used by the ACPI table drivers.
> > +                */
> > +               if (*out_table)
> > +                       *tbl_size = (*out_table)->length;
> >         }
> >
> >         return (status);
> > --
> 
> The changelog doesn't explain anything.  Please say (a) what the
> problem is and (b) how it is being addressed by your patch.

OK, I'll also add fixes tag to it.

Thanks
Lv

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web