* [PATCH 01/11] ACPICA: Namespace: Add acpi_ns_handle_to_name()
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
@ 2016-11-30 7:20 ` Lv Zheng
2016-11-30 7:20 ` [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()" Lv Zheng
` (9 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:20 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
ACPICA commit f9fe27a68a90c9d32dd3156241a5e788fb6956ea
This patch adds acpi_ns_handle_to_name() so that in the acpi_get_name():
1. Logics can be made simpler,
2. Lock held for acpi_ns_handle_to_name() can also be applied to
acpi_ns_handle_to_pathname().
The lock might be useless (see Link 1 below), but kept as acpi_get_name() is
an external API. Except the lock correction, this patch is a functional
no-op. BZ 1182, Lv Zheng.
Link: https://github.com/acpica/acpica/commit/f9fe27a6
Link: https://bugs.acpica.org/show_bug.cgi?id=1182 [# 1]
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
---
drivers/acpi/acpica/acnamesp.h | 3 +++
drivers/acpi/acpica/nsnames.c | 45 ++++++++++++++++++++++++++++++++++++++++
drivers/acpi/acpica/nsxfname.c | 43 ++++++++++----------------------------
3 files changed, 59 insertions(+), 32 deletions(-)
diff --git a/drivers/acpi/acpica/acnamesp.h b/drivers/acpi/acpica/acnamesp.h
index bb7fca1..7affdcd 100644
--- a/drivers/acpi/acpica/acnamesp.h
+++ b/drivers/acpi/acpica/acnamesp.h
@@ -292,6 +292,9 @@ char *acpi_ns_get_normalized_pathname(struct acpi_namespace_node *node,
char *acpi_ns_name_of_current_scope(struct acpi_walk_state *walk_state);
acpi_status
+acpi_ns_handle_to_name(acpi_handle target_handle, struct acpi_buffer *buffer);
+
+acpi_status
acpi_ns_handle_to_pathname(acpi_handle target_handle,
struct acpi_buffer *buffer, u8 no_trailing);
diff --git a/drivers/acpi/acpica/nsnames.c b/drivers/acpi/acpica/nsnames.c
index f03dd41..94d5d33 100644
--- a/drivers/acpi/acpica/nsnames.c
+++ b/drivers/acpi/acpica/nsnames.c
@@ -97,6 +97,51 @@ acpi_size acpi_ns_get_pathname_length(struct acpi_namespace_node *node)
/*******************************************************************************
*
+ * FUNCTION: acpi_ns_handle_to_name
+ *
+ * PARAMETERS: target_handle - Handle of named object whose name is
+ * to be found
+ * buffer - Where the name is returned
+ *
+ * RETURN: Status, Buffer is filled with name if status is AE_OK
+ *
+ * DESCRIPTION: Build and return a full namespace name
+ *
+ ******************************************************************************/
+
+acpi_status
+acpi_ns_handle_to_name(acpi_handle target_handle, struct acpi_buffer *buffer)
+{
+ acpi_status status;
+ struct acpi_namespace_node *node;
+ const char *node_name;
+
+ ACPI_FUNCTION_TRACE_PTR(ns_handle_to_name, target_handle);
+
+ node = acpi_ns_validate_handle(target_handle);
+ if (!node) {
+ return_ACPI_STATUS(AE_BAD_PARAMETER);
+ }
+
+ /* Validate/Allocate/Clear caller buffer */
+
+ status = acpi_ut_initialize_buffer(buffer, ACPI_PATH_SEGMENT_LENGTH);
+ if (ACPI_FAILURE(status)) {
+ return_ACPI_STATUS(status);
+ }
+
+ /* Just copy the ACPI name from the Node and zero terminate it */
+
+ node_name = acpi_ut_get_node_name(node);
+ ACPI_MOVE_NAME(buffer->pointer, node_name);
+ ((char *)buffer->pointer)[ACPI_NAME_SIZE] = 0;
+
+ ACPI_DEBUG_PRINT((ACPI_DB_EXEC, "%4.4s\n", (char *)buffer->pointer));
+ return_ACPI_STATUS(AE_OK);
+}
+
+/*******************************************************************************
+ *
* FUNCTION: acpi_ns_handle_to_pathname
*
* PARAMETERS: target_handle - Handle of named object whose name is
diff --git a/drivers/acpi/acpica/nsxfname.c b/drivers/acpi/acpica/nsxfname.c
index 76a1bd4..e525cbe 100644
--- a/drivers/acpi/acpica/nsxfname.c
+++ b/drivers/acpi/acpica/nsxfname.c
@@ -158,8 +158,6 @@ static char *acpi_ns_copy_device_id(struct acpi_pnp_device_id *dest,
acpi_get_name(acpi_handle handle, u32 name_type, struct acpi_buffer *buffer)
{
acpi_status status;
- struct acpi_namespace_node *node;
- const char *node_name;
/* Parameter validation */
@@ -172,18 +170,6 @@ static char *acpi_ns_copy_device_id(struct acpi_pnp_device_id *dest,
return (status);
}
- if (name_type == ACPI_FULL_PATHNAME ||
- name_type == ACPI_FULL_PATHNAME_NO_TRAILING) {
-
- /* Get the full pathname (From the namespace root) */
-
- status = acpi_ns_handle_to_pathname(handle, buffer,
- name_type ==
- ACPI_FULL_PATHNAME ? FALSE :
- TRUE);
- return (status);
- }
-
/*
* Wants the single segment ACPI name.
* Validate handle and convert to a namespace Node
@@ -193,27 +179,20 @@ static char *acpi_ns_copy_device_id(struct acpi_pnp_device_id *dest,
return (status);
}
- node = acpi_ns_validate_handle(handle);
- if (!node) {
- status = AE_BAD_PARAMETER;
- goto unlock_and_exit;
- }
-
- /* Validate/Allocate/Clear caller buffer */
-
- status = acpi_ut_initialize_buffer(buffer, ACPI_PATH_SEGMENT_LENGTH);
- if (ACPI_FAILURE(status)) {
- goto unlock_and_exit;
- }
+ if (name_type == ACPI_FULL_PATHNAME ||
+ name_type == ACPI_FULL_PATHNAME_NO_TRAILING) {
- /* Just copy the ACPI name from the Node and zero terminate it */
+ /* Get the full pathname (From the namespace root) */
- node_name = acpi_ut_get_node_name(node);
- ACPI_MOVE_NAME(buffer->pointer, node_name);
- ((char *)buffer->pointer)[ACPI_NAME_SIZE] = 0;
- status = AE_OK;
+ status = acpi_ns_handle_to_pathname(handle, buffer,
+ name_type ==
+ ACPI_FULL_PATHNAME ? FALSE :
+ TRUE);
+ } else {
+ /* Get the single name */
-unlock_and_exit:
+ status = acpi_ns_handle_to_name(handle, buffer);
+ }
(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
return (status);
--
1.7.10
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()"
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
2016-11-30 7:20 ` [PATCH 01/11] ACPICA: Namespace: Add acpi_ns_handle_to_name() Lv Zheng
@ 2016-11-30 7:20 ` Lv Zheng
2016-11-30 22:30 ` Rafael J. Wysocki
2016-11-30 7:21 ` [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value Lv Zheng
` (8 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:20 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
ACPICA commit bc481e758e54f7644fd0b657119ca7763d8b6a9c
This is a back port result of the following commit:
Commit: 8633db6b027952449e155a316f4ae3a530bbe18f
Subject: ACPICA: Dispatcher: Fix interpreter locking around acpi_ev_initialize_region()
Link: https://github.com/acpica/acpica/commit/bc481e75
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
---
drivers/acpi/acpica/dsinit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/acpica/dsinit.c b/drivers/acpi/acpica/dsinit.c
index 54d48b9..5de3f10 100644
--- a/drivers/acpi/acpica/dsinit.c
+++ b/drivers/acpi/acpica/dsinit.c
@@ -221,8 +221,8 @@
*/
status =
acpi_ns_walk_namespace(ACPI_TYPE_ANY, start_node, ACPI_UINT32_MAX,
- 0, acpi_ds_init_one_object, NULL, &info,
- NULL);
+ ACPI_NS_WALK_NO_UNLOCK,
+ acpi_ds_init_one_object, NULL, &info, NULL);
if (ACPI_FAILURE(status)) {
ACPI_EXCEPTION((AE_INFO, status, "During WalkNamespace"));
}
--
1.7.10
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()"
2016-11-30 7:20 ` [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()" Lv Zheng
@ 2016-11-30 22:30 ` Rafael J. Wysocki
2016-12-01 7:50 ` Zheng, Lv
0 siblings, 1 reply; 31+ messages in thread
From: Rafael J. Wysocki @ 2016-11-30 22:30 UTC (permalink / raw)
To: Lv Zheng
Cc: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown, Lv Zheng,
Linux Kernel Mailing List, ACPI Devel Maling List, Bob Moore
On Wed, Nov 30, 2016 at 8:20 AM, Lv Zheng <lv.zheng@intel.com> wrote:
> ACPICA commit bc481e758e54f7644fd0b657119ca7763d8b6a9c
>
> This is a back port result of the following commit:
> Commit: 8633db6b027952449e155a316f4ae3a530bbe18f
> Subject: ACPICA: Dispatcher: Fix interpreter locking around acpi_ev_initialize_region()
>
> Link: https://github.com/acpica/acpica/commit/bc481e75
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Signed-off-by: Bob Moore <robert.moore@intel.com>
> ---
> drivers/acpi/acpica/dsinit.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dsinit.c b/drivers/acpi/acpica/dsinit.c
> index 54d48b9..5de3f10 100644
> --- a/drivers/acpi/acpica/dsinit.c
> +++ b/drivers/acpi/acpica/dsinit.c
> @@ -221,8 +221,8 @@
> */
> status =
> acpi_ns_walk_namespace(ACPI_TYPE_ANY, start_node, ACPI_UINT32_MAX,
> - 0, acpi_ds_init_one_object, NULL, &info,
> - NULL);
> + ACPI_NS_WALK_NO_UNLOCK,
> + acpi_ds_init_one_object, NULL, &info, NULL);
> if (ACPI_FAILURE(status)) {
> ACPI_EXCEPTION((AE_INFO, status, "During WalkNamespace"));
> }
> --
This isn't necessary IMO, the current code linux-next code looks like
the change has been made in there already AFAICS (please double check,
though).
I'm skipping this patch.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 31+ messages in thread* RE: [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()"
2016-11-30 22:30 ` Rafael J. Wysocki
@ 2016-12-01 7:50 ` Zheng, Lv
2016-12-01 13:29 ` Rafael J. Wysocki
0 siblings, 1 reply; 31+ messages in thread
From: Zheng, Lv @ 2016-12-01 7:50 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Wysocki, Rafael J, Rafael J. Wysocki, Brown, Len, Lv Zheng,
Linux Kernel Mailing List, ACPI Devel Maling List, Moore, Robert
Hi, Rafael
> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J. Wysocki
> Subject: Re: [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around
> AcpiEvInitializeRegion()"
>
> On Wed, Nov 30, 2016 at 8:20 AM, Lv Zheng <lv.zheng@intel.com> wrote:
> > ACPICA commit bc481e758e54f7644fd0b657119ca7763d8b6a9c
> >
> > This is a back port result of the following commit:
> > Commit: 8633db6b027952449e155a316f4ae3a530bbe18f
> > Subject: ACPICA: Dispatcher: Fix interpreter locking around acpi_ev_initialize_region()
> >
> > Link: https://github.com/acpica/acpica/commit/bc481e75
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > Signed-off-by: Bob Moore <robert.moore@intel.com>
> > ---
> > drivers/acpi/acpica/dsinit.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/dsinit.c b/drivers/acpi/acpica/dsinit.c
> > index 54d48b9..5de3f10 100644
> > --- a/drivers/acpi/acpica/dsinit.c
> > +++ b/drivers/acpi/acpica/dsinit.c
> > @@ -221,8 +221,8 @@
> > */
> > status =
> > acpi_ns_walk_namespace(ACPI_TYPE_ANY, start_node, ACPI_UINT32_MAX,
> > - 0, acpi_ds_init_one_object, NULL, &info,
> > - NULL);
> > + ACPI_NS_WALK_NO_UNLOCK,
> > + acpi_ds_init_one_object, NULL, &info, NULL);
> > if (ACPI_FAILURE(status)) {
> > ACPI_EXCEPTION((AE_INFO, status, "During WalkNamespace"));
> > }
> > --
>
> This isn't necessary IMO, the current code linux-next code looks like
> the change has been made in there already AFAICS (please double check,
> though).
The fix was in Linux, however, when it is back ported to ACPICA, Bob asked me to do this change.
Using ACPI_NS_WALK_NO_UNLOCK instead of meaningless 0.
So during this release cycle, this change is detected out as the only difference of the back ported commit.
>
> I'm skipping this patch.
If this is skipped, it leaves us 14 lines divergences.
Hope we can have this kind of divergences eliminated.
Thanks and best regards
Lv
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()"
2016-12-01 7:50 ` Zheng, Lv
@ 2016-12-01 13:29 ` Rafael J. Wysocki
0 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2016-12-01 13:29 UTC (permalink / raw)
To: Zheng, Lv
Cc: Rafael J. Wysocki, Wysocki, Rafael J, Rafael J. Wysocki, Brown,
Len, Lv Zheng, Linux Kernel Mailing List, ACPI Devel Maling List,
Moore, Robert
On Thu, Dec 1, 2016 at 8:50 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
> Hi, Rafael
>
>> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of Rafael J. Wysocki
>> Subject: Re: [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around
>> AcpiEvInitializeRegion()"
>>
>> On Wed, Nov 30, 2016 at 8:20 AM, Lv Zheng <lv.zheng@intel.com> wrote:
>> > ACPICA commit bc481e758e54f7644fd0b657119ca7763d8b6a9c
>> >
>> > This is a back port result of the following commit:
>> > Commit: 8633db6b027952449e155a316f4ae3a530bbe18f
>> > Subject: ACPICA: Dispatcher: Fix interpreter locking around acpi_ev_initialize_region()
>> >
>> > Link: https://github.com/acpica/acpica/commit/bc481e75
>> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> > Signed-off-by: Bob Moore <robert.moore@intel.com>
>> > ---
>> > drivers/acpi/acpica/dsinit.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/acpi/acpica/dsinit.c b/drivers/acpi/acpica/dsinit.c
>> > index 54d48b9..5de3f10 100644
>> > --- a/drivers/acpi/acpica/dsinit.c
>> > +++ b/drivers/acpi/acpica/dsinit.c
>> > @@ -221,8 +221,8 @@
>> > */
>> > status =
>> > acpi_ns_walk_namespace(ACPI_TYPE_ANY, start_node, ACPI_UINT32_MAX,
>> > - 0, acpi_ds_init_one_object, NULL, &info,
>> > - NULL);
>> > + ACPI_NS_WALK_NO_UNLOCK,
>> > + acpi_ds_init_one_object, NULL, &info, NULL);
>> > if (ACPI_FAILURE(status)) {
>> > ACPI_EXCEPTION((AE_INFO, status, "During WalkNamespace"));
>> > }
>> > --
>>
>> This isn't necessary IMO, the current code linux-next code looks like
>> the change has been made in there already AFAICS (please double check,
>> though).
>
> The fix was in Linux, however, when it is back ported to ACPICA, Bob asked me to do this change.
> Using ACPI_NS_WALK_NO_UNLOCK instead of meaningless 0.
> So during this release cycle, this change is detected out as the only difference of the back ported commit.
>
>>
>> I'm skipping this patch.
>
> If this is skipped, it leaves us 14 lines divergences.
> Hope we can have this kind of divergences eliminated.
OK, my bad.
I forgot to merge back the revert I made before into the acpica branch.
Sorry for the noise, everything applies now.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
2016-11-30 7:20 ` [PATCH 01/11] ACPICA: Namespace: Add acpi_ns_handle_to_name() Lv Zheng
2016-11-30 7:20 ` [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()" Lv Zheng
@ 2016-11-30 7:21 ` Lv Zheng
2016-11-30 23:07 ` Rafael J. Wysocki
2016-11-30 7:21 ` [PATCH 05/11] ACPICA: Tables: Cleanup acpi_tb_install_and_load_table() Lv Zheng
` (7 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:21 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
ACPICA commit 543342ab7a676f4eb0c9f100d349388a84dff0e8
This patch changes acpi_ev_initialize_region(), stop returning AE_NOT_EXIST
from it so that, not only in acpi_ds_load2_end_op(), but all places invoking
this function won't emit exceptions. The exception can be seen in
acpi_ds_initialize_objects() when certain table loading mode is chosen.
This patch also removes useless acpi_ns_locked from acpi_ev_initialize_region()
as this function will always be invoked with interpreter lock held now, and
the lock granularity has been tuned to lock around _REG execution, thus it
is now handled by acpi_ex_exit_interpreter(). Lv Zheng.
Link: https://github.com/acpica/acpica/commit/543342ab
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
---
drivers/acpi/acpica/acevents.h | 4 +--
drivers/acpi/acpica/dsopcode.c | 2 +-
drivers/acpi/acpica/dswload2.c | 13 +--------
drivers/acpi/acpica/evrgnini.c | 59 ++++++++++++++++------------------------
4 files changed, 27 insertions(+), 51 deletions(-)
diff --git a/drivers/acpi/acpica/acevents.h b/drivers/acpi/acpica/acevents.h
index 92fa47c..8a0049d 100644
--- a/drivers/acpi/acpica/acevents.h
+++ b/drivers/acpi/acpica/acevents.h
@@ -243,9 +243,7 @@ union acpi_operand_object *acpi_ev_find_region_handler(acpi_adr_space_type
u32 function,
void *handler_context, void **region_context);
-acpi_status
-acpi_ev_initialize_region(union acpi_operand_object *region_obj,
- u8 acpi_ns_locked);
+acpi_status acpi_ev_initialize_region(union acpi_operand_object *region_obj);
/*
* evsci - SCI (System Control Interrupt) handling/dispatch
diff --git a/drivers/acpi/acpica/dsopcode.c b/drivers/acpi/acpica/dsopcode.c
index 4cc9d98..77fd7c8 100644
--- a/drivers/acpi/acpica/dsopcode.c
+++ b/drivers/acpi/acpica/dsopcode.c
@@ -84,7 +84,7 @@ acpi_status acpi_ds_initialize_region(acpi_handle obj_handle)
/* Namespace is NOT locked */
- status = acpi_ev_initialize_region(obj_desc, FALSE);
+ status = acpi_ev_initialize_region(obj_desc);
return (status);
}
diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c
index e362182..651f35a 100644
--- a/drivers/acpi/acpica/dswload2.c
+++ b/drivers/acpi/acpica/dswload2.c
@@ -609,18 +609,7 @@ acpi_status acpi_ds_load2_end_op(struct acpi_walk_state *walk_state)
status =
acpi_ev_initialize_region
- (acpi_ns_get_attached_object(node), FALSE);
-
- if (ACPI_FAILURE(status)) {
- /*
- * If AE_NOT_EXIST is returned, it is not fatal
- * because many regions get created before a handler
- * is installed for said region.
- */
- if (AE_NOT_EXIST == status) {
- status = AE_OK;
- }
- }
+ (acpi_ns_get_attached_object(node));
break;
case AML_NAME_OP:
diff --git a/drivers/acpi/acpica/evrgnini.c b/drivers/acpi/acpica/evrgnini.c
index 75ddd16..a909225 100644
--- a/drivers/acpi/acpica/evrgnini.c
+++ b/drivers/acpi/acpica/evrgnini.c
@@ -479,7 +479,6 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
* FUNCTION: acpi_ev_initialize_region
*
* PARAMETERS: region_obj - Region we are initializing
- * acpi_ns_locked - Is namespace locked?
*
* RETURN: Status
*
@@ -497,19 +496,28 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
* MUTEX: Interpreter should be unlocked, because we may run the _REG
* method for this region.
*
+ * NOTE: Possible incompliance:
+ * There is a behavior conflict in automatic _REG execution:
+ * 1. When the interpreter is evaluating a method, we can only
+ * automatically run _REG for the following case:
+ * operation_region (OPR1, 0x80, 0x1000010, 0x4)
+ * 2. When the interpreter is loading a table, we can also
+ * automatically run _REG for the following case:
+ * operation_region (OPR1, 0x80, 0x1000010, 0x4)
+ * Though this may not be compliant to the de-facto standard, the
+ * logic is kept in order not to trigger regressions. And keeping
+ * this logic should be taken care by the caller of this function.
+ *
******************************************************************************/
-acpi_status
-acpi_ev_initialize_region(union acpi_operand_object *region_obj,
- u8 acpi_ns_locked)
+acpi_status acpi_ev_initialize_region(union acpi_operand_object *region_obj)
{
union acpi_operand_object *handler_obj;
union acpi_operand_object *obj_desc;
acpi_adr_space_type space_id;
struct acpi_namespace_node *node;
- acpi_status status;
- ACPI_FUNCTION_TRACE_U32(ev_initialize_region, acpi_ns_locked);
+ ACPI_FUNCTION_TRACE(ev_initialize_region);
if (!region_obj) {
return_ACPI_STATUS(AE_BAD_PARAMETER);
@@ -580,39 +588,17 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
handler_obj, region_obj,
obj_desc));
- status =
- acpi_ev_attach_region(handler_obj,
- region_obj,
- acpi_ns_locked);
+ (void)acpi_ev_attach_region(handler_obj,
+ region_obj, FALSE);
/*
* Tell all users that this region is usable by
* running the _REG method
*/
- if (acpi_ns_locked) {
- status =
- acpi_ut_release_mutex
- (ACPI_MTX_NAMESPACE);
- if (ACPI_FAILURE(status)) {
- return_ACPI_STATUS(status);
- }
- }
-
acpi_ex_exit_interpreter();
- status =
- acpi_ev_execute_reg_method(region_obj,
- ACPI_REG_CONNECT);
+ (void)acpi_ev_execute_reg_method(region_obj,
+ ACPI_REG_CONNECT);
acpi_ex_enter_interpreter();
-
- if (acpi_ns_locked) {
- status =
- acpi_ut_acquire_mutex
- (ACPI_MTX_NAMESPACE);
- if (ACPI_FAILURE(status)) {
- return_ACPI_STATUS(status);
- }
- }
-
return_ACPI_STATUS(AE_OK);
}
}
@@ -622,12 +608,15 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
node = node->parent;
}
- /* If we get here, there is no handler for this region */
-
+ /*
+ * If we get here, there is no handler for this region. This is not
+ * fatal because many regions get created before a handler is installed
+ * for said region.
+ */
ACPI_DEBUG_PRINT((ACPI_DB_OPREGION,
"No handler for RegionType %s(%X) (RegionObj %p)\n",
acpi_ut_get_region_name(space_id), space_id,
region_obj));
- return_ACPI_STATUS(AE_NOT_EXIST);
+ return_ACPI_STATUS(AE_OK);
}
--
1.7.10
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value
2016-11-30 7:21 ` [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value Lv Zheng
@ 2016-11-30 23:07 ` Rafael J. Wysocki
2016-12-01 8:00 ` Zheng, Lv
0 siblings, 1 reply; 31+ messages in thread
From: Rafael J. Wysocki @ 2016-11-30 23:07 UTC (permalink / raw)
To: Lv Zheng
Cc: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown, Lv Zheng,
Linux Kernel Mailing List, ACPI Devel Maling List, Bob Moore
On Wed, Nov 30, 2016 at 8:21 AM, Lv Zheng <lv.zheng@intel.com> wrote:
> ACPICA commit 543342ab7a676f4eb0c9f100d349388a84dff0e8
>
> This patch changes acpi_ev_initialize_region(), stop returning AE_NOT_EXIST
> from it so that, not only in acpi_ds_load2_end_op(), but all places invoking
> this function won't emit exceptions. The exception can be seen in
> acpi_ds_initialize_objects() when certain table loading mode is chosen.
>
> This patch also removes useless acpi_ns_locked from acpi_ev_initialize_region()
> as this function will always be invoked with interpreter lock held now, and
> the lock granularity has been tuned to lock around _REG execution, thus it
> is now handled by acpi_ex_exit_interpreter(). Lv Zheng.
>
> Link: https://github.com/acpica/acpica/commit/543342ab
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Signed-off-by: Bob Moore <robert.moore@intel.com>
> ---
> drivers/acpi/acpica/acevents.h | 4 +--
> drivers/acpi/acpica/dsopcode.c | 2 +-
> drivers/acpi/acpica/dswload2.c | 13 +--------
> drivers/acpi/acpica/evrgnini.c | 59 ++++++++++++++++------------------------
> 4 files changed, 27 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/acpi/acpica/acevents.h b/drivers/acpi/acpica/acevents.h
> index 92fa47c..8a0049d 100644
> --- a/drivers/acpi/acpica/acevents.h
> +++ b/drivers/acpi/acpica/acevents.h
> @@ -243,9 +243,7 @@ union acpi_operand_object *acpi_ev_find_region_handler(acpi_adr_space_type
> u32 function,
> void *handler_context, void **region_context);
>
> -acpi_status
> -acpi_ev_initialize_region(union acpi_operand_object *region_obj,
> - u8 acpi_ns_locked);
> +acpi_status acpi_ev_initialize_region(union acpi_operand_object *region_obj);
>
> /*
> * evsci - SCI (System Control Interrupt) handling/dispatch
> diff --git a/drivers/acpi/acpica/dsopcode.c b/drivers/acpi/acpica/dsopcode.c
> index 4cc9d98..77fd7c8 100644
> --- a/drivers/acpi/acpica/dsopcode.c
> +++ b/drivers/acpi/acpica/dsopcode.c
> @@ -84,7 +84,7 @@ acpi_status acpi_ds_initialize_region(acpi_handle obj_handle)
>
> /* Namespace is NOT locked */
>
> - status = acpi_ev_initialize_region(obj_desc, FALSE);
> + status = acpi_ev_initialize_region(obj_desc);
> return (status);
> }
>
> diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c
> index e362182..651f35a 100644
> --- a/drivers/acpi/acpica/dswload2.c
> +++ b/drivers/acpi/acpica/dswload2.c
> @@ -609,18 +609,7 @@ acpi_status acpi_ds_load2_end_op(struct acpi_walk_state *walk_state)
>
> status =
> acpi_ev_initialize_region
> - (acpi_ns_get_attached_object(node), FALSE);
> -
> - if (ACPI_FAILURE(status)) {
> - /*
> - * If AE_NOT_EXIST is returned, it is not fatal
> - * because many regions get created before a handler
> - * is installed for said region.
> - */
> - if (AE_NOT_EXIST == status) {
> - status = AE_OK;
> - }
> - }
> + (acpi_ns_get_attached_object(node));
This hunk doesn't apply for me.
We have acpi_ex_exit_interpreter() / acpi_ex_enter_interpreter()
around the acpi_ev_initialize_region() in linux-next.
> break;
>
> case AML_NAME_OP:
> diff --git a/drivers/acpi/acpica/evrgnini.c b/drivers/acpi/acpica/evrgnini.c
> index 75ddd16..a909225 100644
> --- a/drivers/acpi/acpica/evrgnini.c
> +++ b/drivers/acpi/acpica/evrgnini.c
> @@ -479,7 +479,6 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
> * FUNCTION: acpi_ev_initialize_region
> *
> * PARAMETERS: region_obj - Region we are initializing
> - * acpi_ns_locked - Is namespace locked?
> *
> * RETURN: Status
> *
> @@ -497,19 +496,28 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
> * MUTEX: Interpreter should be unlocked, because we may run the _REG
> * method for this region.
> *
> + * NOTE: Possible incompliance:
> + * There is a behavior conflict in automatic _REG execution:
> + * 1. When the interpreter is evaluating a method, we can only
> + * automatically run _REG for the following case:
> + * operation_region (OPR1, 0x80, 0x1000010, 0x4)
> + * 2. When the interpreter is loading a table, we can also
> + * automatically run _REG for the following case:
> + * operation_region (OPR1, 0x80, 0x1000010, 0x4)
> + * Though this may not be compliant to the de-facto standard, the
> + * logic is kept in order not to trigger regressions. And keeping
> + * this logic should be taken care by the caller of this function.
> + *
> ******************************************************************************/
>
> -acpi_status
> -acpi_ev_initialize_region(union acpi_operand_object *region_obj,
> - u8 acpi_ns_locked)
> +acpi_status acpi_ev_initialize_region(union acpi_operand_object *region_obj)
> {
> union acpi_operand_object *handler_obj;
> union acpi_operand_object *obj_desc;
> acpi_adr_space_type space_id;
> struct acpi_namespace_node *node;
> - acpi_status status;
>
> - ACPI_FUNCTION_TRACE_U32(ev_initialize_region, acpi_ns_locked);
> + ACPI_FUNCTION_TRACE(ev_initialize_region);
>
> if (!region_obj) {
> return_ACPI_STATUS(AE_BAD_PARAMETER);
> @@ -580,39 +588,17 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
> handler_obj, region_obj,
> obj_desc));
>
> - status =
> - acpi_ev_attach_region(handler_obj,
> - region_obj,
> - acpi_ns_locked);
> + (void)acpi_ev_attach_region(handler_obj,
> + region_obj, FALSE);
>
> /*
> * Tell all users that this region is usable by
> * running the _REG method
> */
> - if (acpi_ns_locked) {
> - status =
> - acpi_ut_release_mutex
> - (ACPI_MTX_NAMESPACE);
> - if (ACPI_FAILURE(status)) {
> - return_ACPI_STATUS(status);
> - }
> - }
> -
> acpi_ex_exit_interpreter();
> - status =
> - acpi_ev_execute_reg_method(region_obj,
> - ACPI_REG_CONNECT);
> + (void)acpi_ev_execute_reg_method(region_obj,
> + ACPI_REG_CONNECT);
> acpi_ex_enter_interpreter();
And this also doesn't apply, because we don't invoke
acpi_ex_exit_interpreter() / acpi_ex_enter_interpreter() around the
acpi_ev_execute_reg_method() call in linux-next.
> -
> - if (acpi_ns_locked) {
> - status =
> - acpi_ut_acquire_mutex
> - (ACPI_MTX_NAMESPACE);
> - if (ACPI_FAILURE(status)) {
> - return_ACPI_STATUS(status);
> - }
> - }
> -
> return_ACPI_STATUS(AE_OK);
> }
> }
> @@ -622,12 +608,15 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
> node = node->parent;
> }
>
> - /* If we get here, there is no handler for this region */
> -
> + /*
> + * If we get here, there is no handler for this region. This is not
> + * fatal because many regions get created before a handler is installed
> + * for said region.
> + */
> ACPI_DEBUG_PRINT((ACPI_DB_OPREGION,
> "No handler for RegionType %s(%X) (RegionObj %p)\n",
> acpi_ut_get_region_name(space_id), space_id,
> region_obj));
>
> - return_ACPI_STATUS(AE_NOT_EXIST);
> + return_ACPI_STATUS(AE_OK);
> }
> --
Thanks,
Rafael
^ permalink raw reply [flat|nested] 31+ messages in thread* RE: [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value
2016-11-30 23:07 ` Rafael J. Wysocki
@ 2016-12-01 8:00 ` Zheng, Lv
2016-12-01 13:30 ` Rafael J. Wysocki
0 siblings, 1 reply; 31+ messages in thread
From: Zheng, Lv @ 2016-12-01 8:00 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Wysocki, Rafael J, Rafael J. Wysocki, Brown, Len, Lv Zheng,
Linux Kernel Mailing List, ACPI Devel Maling List, Moore, Robert
Hi, Rafael
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J.
> Wysocki
> Subject: Re: [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value
>
> On Wed, Nov 30, 2016 at 8:21 AM, Lv Zheng <lv.zheng@intel.com> wrote:
> > ACPICA commit 543342ab7a676f4eb0c9f100d349388a84dff0e8
> >
> > This patch changes acpi_ev_initialize_region(), stop returning AE_NOT_EXIST
> > from it so that, not only in acpi_ds_load2_end_op(), but all places invoking
> > this function won't emit exceptions. The exception can be seen in
> > acpi_ds_initialize_objects() when certain table loading mode is chosen.
> >
> > This patch also removes useless acpi_ns_locked from acpi_ev_initialize_region()
> > as this function will always be invoked with interpreter lock held now, and
> > the lock granularity has been tuned to lock around _REG execution, thus it
> > is now handled by acpi_ex_exit_interpreter(). Lv Zheng.
> >
> > Link: https://github.com/acpica/acpica/commit/543342ab
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > Signed-off-by: Bob Moore <robert.moore@intel.com>
> > ---
> > drivers/acpi/acpica/acevents.h | 4 +--
> > drivers/acpi/acpica/dsopcode.c | 2 +-
> > drivers/acpi/acpica/dswload2.c | 13 +--------
> > drivers/acpi/acpica/evrgnini.c | 59 ++++++++++++++++------------------------
> > 4 files changed, 27 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/acevents.h b/drivers/acpi/acpica/acevents.h
> > index 92fa47c..8a0049d 100644
> > --- a/drivers/acpi/acpica/acevents.h
> > +++ b/drivers/acpi/acpica/acevents.h
> > @@ -243,9 +243,7 @@ union acpi_operand_object *acpi_ev_find_region_handler(acpi_adr_space_type
> > u32 function,
> > void *handler_context, void **region_context);
> >
> > -acpi_status
> > -acpi_ev_initialize_region(union acpi_operand_object *region_obj,
> > - u8 acpi_ns_locked);
> > +acpi_status acpi_ev_initialize_region(union acpi_operand_object *region_obj);
> >
> > /*
> > * evsci - SCI (System Control Interrupt) handling/dispatch
> > diff --git a/drivers/acpi/acpica/dsopcode.c b/drivers/acpi/acpica/dsopcode.c
> > index 4cc9d98..77fd7c8 100644
> > --- a/drivers/acpi/acpica/dsopcode.c
> > +++ b/drivers/acpi/acpica/dsopcode.c
> > @@ -84,7 +84,7 @@ acpi_status acpi_ds_initialize_region(acpi_handle obj_handle)
> >
> > /* Namespace is NOT locked */
> >
> > - status = acpi_ev_initialize_region(obj_desc, FALSE);
> > + status = acpi_ev_initialize_region(obj_desc);
> > return (status);
> > }
> >
> > diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c
> > index e362182..651f35a 100644
> > --- a/drivers/acpi/acpica/dswload2.c
> > +++ b/drivers/acpi/acpica/dswload2.c
> > @@ -609,18 +609,7 @@ acpi_status acpi_ds_load2_end_op(struct acpi_walk_state *walk_state)
> >
> > status =
> > acpi_ev_initialize_region
> > - (acpi_ns_get_attached_object(node), FALSE);
> > -
> > - if (ACPI_FAILURE(status)) {
> > - /*
> > - * If AE_NOT_EXIST is returned, it is not fatal
> > - * because many regions get created before a handler
> > - * is installed for said region.
> > - */
> > - if (AE_NOT_EXIST == status) {
> > - status = AE_OK;
> > - }
> > - }
> > + (acpi_ns_get_attached_object(node));
>
> This hunk doesn't apply for me.
>
> We have acpi_ex_exit_interpreter() / acpi_ex_enter_interpreter()
> around the acpi_ev_initialize_region() in linux-next.
This commit only changes returning value.
Lock changes are not included.
We never invokes acpi_ev_initialize_region with acpi_ns_locked=true.
So all if (acpi_ns_locked) code pieces are useless and deleted in this commit.
>
> > break;
> >
> > case AML_NAME_OP:
> > diff --git a/drivers/acpi/acpica/evrgnini.c b/drivers/acpi/acpica/evrgnini.c
> > index 75ddd16..a909225 100644
> > --- a/drivers/acpi/acpica/evrgnini.c
> > +++ b/drivers/acpi/acpica/evrgnini.c
> > @@ -479,7 +479,6 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
> > * FUNCTION: acpi_ev_initialize_region
> > *
> > * PARAMETERS: region_obj - Region we are initializing
> > - * acpi_ns_locked - Is namespace locked?
> > *
> > * RETURN: Status
> > *
> > @@ -497,19 +496,28 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
> > * MUTEX: Interpreter should be unlocked, because we may run the _REG
> > * method for this region.
> > *
> > + * NOTE: Possible incompliance:
> > + * There is a behavior conflict in automatic _REG execution:
> > + * 1. When the interpreter is evaluating a method, we can only
> > + * automatically run _REG for the following case:
> > + * operation_region (OPR1, 0x80, 0x1000010, 0x4)
> > + * 2. When the interpreter is loading a table, we can also
> > + * automatically run _REG for the following case:
> > + * operation_region (OPR1, 0x80, 0x1000010, 0x4)
> > + * Though this may not be compliant to the de-facto standard, the
> > + * logic is kept in order not to trigger regressions. And keeping
> > + * this logic should be taken care by the caller of this function.
> > + *
> > ******************************************************************************/
> >
> > -acpi_status
> > -acpi_ev_initialize_region(union acpi_operand_object *region_obj,
> > - u8 acpi_ns_locked)
> > +acpi_status acpi_ev_initialize_region(union acpi_operand_object *region_obj)
> > {
> > union acpi_operand_object *handler_obj;
> > union acpi_operand_object *obj_desc;
> > acpi_adr_space_type space_id;
> > struct acpi_namespace_node *node;
> > - acpi_status status;
> >
> > - ACPI_FUNCTION_TRACE_U32(ev_initialize_region, acpi_ns_locked);
> > + ACPI_FUNCTION_TRACE(ev_initialize_region);
> >
> > if (!region_obj) {
> > return_ACPI_STATUS(AE_BAD_PARAMETER);
> > @@ -580,39 +588,17 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
> > handler_obj, region_obj,
> > obj_desc));
> >
> > - status =
> > - acpi_ev_attach_region(handler_obj,
> > - region_obj,
> > - acpi_ns_locked);
> > + (void)acpi_ev_attach_region(handler_obj,
> > + region_obj, FALSE);
> >
> > /*
> > * Tell all users that this region is usable by
> > * running the _REG method
> > */
> > - if (acpi_ns_locked) {
> > - status =
> > - acpi_ut_release_mutex
> > - (ACPI_MTX_NAMESPACE);
> > - if (ACPI_FAILURE(status)) {
> > - return_ACPI_STATUS(status);
> > - }
> > - }
> > -
> > acpi_ex_exit_interpreter();
> > - status =
> > - acpi_ev_execute_reg_method(region_obj,
> > - ACPI_REG_CONNECT);
> > + (void)acpi_ev_execute_reg_method(region_obj,
> > + ACPI_REG_CONNECT);
> > acpi_ex_enter_interpreter();
>
> And this also doesn't apply, because we don't invoke
> acpi_ex_exit_interpreter() / acpi_ex_enter_interpreter() around the
> acpi_ev_execute_reg_method() call in linux-next.
>
acpi_ex_exit_interpreter() / acpi_ex_enter_interpreter() are not-modified-lines.
Please check again.
Unlock before _REG is the minimum requirement as ACPICA will reacquire interpreter lock when _REG is evaluated.
Before applying the following commit (which is in the upstream):
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8633db6b02
ACPICA unlocks the entire walk namespace process.
However this change did nothing to the locks.
Only changed the returning value of acpi_ev_initialize_region().
Always returning AE_OK instead of AE_NOT_EXIST.
Other than this, changes are no-ops.
Thanks and best regards
Lv
>
> > -
> > - if (acpi_ns_locked) {
> > - status =
> > - acpi_ut_acquire_mutex
> > - (ACPI_MTX_NAMESPACE);
> > - if (ACPI_FAILURE(status)) {
> > - return_ACPI_STATUS(status);
> > - }
> > - }
> > -
> > return_ACPI_STATUS(AE_OK);
> > }
> > }
> > @@ -622,12 +608,15 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
> > node = node->parent;
> > }
> >
> > - /* If we get here, there is no handler for this region */
> > -
> > + /*
> > + * If we get here, there is no handler for this region. This is not
> > + * fatal because many regions get created before a handler is installed
> > + * for said region.
> > + */
> > ACPI_DEBUG_PRINT((ACPI_DB_OPREGION,
> > "No handler for RegionType %s(%X) (RegionObj %p)\n",
> > acpi_ut_get_region_name(space_id), space_id,
> > region_obj));
> >
> > - return_ACPI_STATUS(AE_NOT_EXIST);
> > + return_ACPI_STATUS(AE_OK);
> > }
> > --
>
> Thanks,
> Rafael
> --
> 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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value
2016-12-01 8:00 ` Zheng, Lv
@ 2016-12-01 13:30 ` Rafael J. Wysocki
0 siblings, 0 replies; 31+ messages in thread
From: Rafael J. Wysocki @ 2016-12-01 13:30 UTC (permalink / raw)
To: Zheng, Lv
Cc: Rafael J. Wysocki, Wysocki, Rafael J, Rafael J. Wysocki, Brown,
Len, Lv Zheng, Linux Kernel Mailing List, ACPI Devel Maling List,
Moore, Robert
On Thu, Dec 1, 2016 at 9:00 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
> Hi, Rafael
>
>> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J.
>> Wysocki
>> Subject: Re: [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value
>>
>> On Wed, Nov 30, 2016 at 8:21 AM, Lv Zheng <lv.zheng@intel.com> wrote:
>> > ACPICA commit 543342ab7a676f4eb0c9f100d349388a84dff0e8
>> >
>> > This patch changes acpi_ev_initialize_region(), stop returning AE_NOT_EXIST
>> > from it so that, not only in acpi_ds_load2_end_op(), but all places invoking
>> > this function won't emit exceptions. The exception can be seen in
>> > acpi_ds_initialize_objects() when certain table loading mode is chosen.
>> >
>> > This patch also removes useless acpi_ns_locked from acpi_ev_initialize_region()
>> > as this function will always be invoked with interpreter lock held now, and
>> > the lock granularity has been tuned to lock around _REG execution, thus it
>> > is now handled by acpi_ex_exit_interpreter(). Lv Zheng.
>> >
>> > Link: https://github.com/acpica/acpica/commit/543342ab
>> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> > Signed-off-by: Bob Moore <robert.moore@intel.com>
>> > ---
>> > drivers/acpi/acpica/acevents.h | 4 +--
>> > drivers/acpi/acpica/dsopcode.c | 2 +-
>> > drivers/acpi/acpica/dswload2.c | 13 +--------
>> > drivers/acpi/acpica/evrgnini.c | 59 ++++++++++++++++------------------------
>> > 4 files changed, 27 insertions(+), 51 deletions(-)
>> >
>> > diff --git a/drivers/acpi/acpica/acevents.h b/drivers/acpi/acpica/acevents.h
>> > index 92fa47c..8a0049d 100644
>> > --- a/drivers/acpi/acpica/acevents.h
>> > +++ b/drivers/acpi/acpica/acevents.h
>> > @@ -243,9 +243,7 @@ union acpi_operand_object *acpi_ev_find_region_handler(acpi_adr_space_type
>> > u32 function,
>> > void *handler_context, void **region_context);
>> >
>> > -acpi_status
>> > -acpi_ev_initialize_region(union acpi_operand_object *region_obj,
>> > - u8 acpi_ns_locked);
>> > +acpi_status acpi_ev_initialize_region(union acpi_operand_object *region_obj);
>> >
>> > /*
>> > * evsci - SCI (System Control Interrupt) handling/dispatch
>> > diff --git a/drivers/acpi/acpica/dsopcode.c b/drivers/acpi/acpica/dsopcode.c
>> > index 4cc9d98..77fd7c8 100644
>> > --- a/drivers/acpi/acpica/dsopcode.c
>> > +++ b/drivers/acpi/acpica/dsopcode.c
>> > @@ -84,7 +84,7 @@ acpi_status acpi_ds_initialize_region(acpi_handle obj_handle)
>> >
>> > /* Namespace is NOT locked */
>> >
>> > - status = acpi_ev_initialize_region(obj_desc, FALSE);
>> > + status = acpi_ev_initialize_region(obj_desc);
>> > return (status);
>> > }
>> >
>> > diff --git a/drivers/acpi/acpica/dswload2.c b/drivers/acpi/acpica/dswload2.c
>> > index e362182..651f35a 100644
>> > --- a/drivers/acpi/acpica/dswload2.c
>> > +++ b/drivers/acpi/acpica/dswload2.c
>> > @@ -609,18 +609,7 @@ acpi_status acpi_ds_load2_end_op(struct acpi_walk_state *walk_state)
>> >
>> > status =
>> > acpi_ev_initialize_region
>> > - (acpi_ns_get_attached_object(node), FALSE);
>> > -
>> > - if (ACPI_FAILURE(status)) {
>> > - /*
>> > - * If AE_NOT_EXIST is returned, it is not fatal
>> > - * because many regions get created before a handler
>> > - * is installed for said region.
>> > - */
>> > - if (AE_NOT_EXIST == status) {
>> > - status = AE_OK;
>> > - }
>> > - }
>> > + (acpi_ns_get_attached_object(node));
>>
>> This hunk doesn't apply for me.
>>
>> We have acpi_ex_exit_interpreter() / acpi_ex_enter_interpreter()
>> around the acpi_ev_initialize_region() in linux-next.
>
> This commit only changes returning value.
> Lock changes are not included.
>
> We never invokes acpi_ev_initialize_region with acpi_ns_locked=true.
> So all if (acpi_ns_locked) code pieces are useless and deleted in this commit.
>
>>
>> > break;
>> >
>> > case AML_NAME_OP:
>> > diff --git a/drivers/acpi/acpica/evrgnini.c b/drivers/acpi/acpica/evrgnini.c
>> > index 75ddd16..a909225 100644
>> > --- a/drivers/acpi/acpica/evrgnini.c
>> > +++ b/drivers/acpi/acpica/evrgnini.c
>> > @@ -479,7 +479,6 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
>> > * FUNCTION: acpi_ev_initialize_region
>> > *
>> > * PARAMETERS: region_obj - Region we are initializing
>> > - * acpi_ns_locked - Is namespace locked?
>> > *
>> > * RETURN: Status
>> > *
>> > @@ -497,19 +496,28 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
>> > * MUTEX: Interpreter should be unlocked, because we may run the _REG
>> > * method for this region.
>> > *
>> > + * NOTE: Possible incompliance:
>> > + * There is a behavior conflict in automatic _REG execution:
>> > + * 1. When the interpreter is evaluating a method, we can only
>> > + * automatically run _REG for the following case:
>> > + * operation_region (OPR1, 0x80, 0x1000010, 0x4)
>> > + * 2. When the interpreter is loading a table, we can also
>> > + * automatically run _REG for the following case:
>> > + * operation_region (OPR1, 0x80, 0x1000010, 0x4)
>> > + * Though this may not be compliant to the de-facto standard, the
>> > + * logic is kept in order not to trigger regressions. And keeping
>> > + * this logic should be taken care by the caller of this function.
>> > + *
>> > ******************************************************************************/
>> >
>> > -acpi_status
>> > -acpi_ev_initialize_region(union acpi_operand_object *region_obj,
>> > - u8 acpi_ns_locked)
>> > +acpi_status acpi_ev_initialize_region(union acpi_operand_object *region_obj)
>> > {
>> > union acpi_operand_object *handler_obj;
>> > union acpi_operand_object *obj_desc;
>> > acpi_adr_space_type space_id;
>> > struct acpi_namespace_node *node;
>> > - acpi_status status;
>> >
>> > - ACPI_FUNCTION_TRACE_U32(ev_initialize_region, acpi_ns_locked);
>> > + ACPI_FUNCTION_TRACE(ev_initialize_region);
>> >
>> > if (!region_obj) {
>> > return_ACPI_STATUS(AE_BAD_PARAMETER);
>> > @@ -580,39 +588,17 @@ static u8 acpi_ev_is_pci_root_bridge(struct acpi_namespace_node *node)
>> > handler_obj, region_obj,
>> > obj_desc));
>> >
>> > - status =
>> > - acpi_ev_attach_region(handler_obj,
>> > - region_obj,
>> > - acpi_ns_locked);
>> > + (void)acpi_ev_attach_region(handler_obj,
>> > + region_obj, FALSE);
>> >
>> > /*
>> > * Tell all users that this region is usable by
>> > * running the _REG method
>> > */
>> > - if (acpi_ns_locked) {
>> > - status =
>> > - acpi_ut_release_mutex
>> > - (ACPI_MTX_NAMESPACE);
>> > - if (ACPI_FAILURE(status)) {
>> > - return_ACPI_STATUS(status);
>> > - }
>> > - }
>> > -
>> > acpi_ex_exit_interpreter();
>> > - status =
>> > - acpi_ev_execute_reg_method(region_obj,
>> > - ACPI_REG_CONNECT);
>> > + (void)acpi_ev_execute_reg_method(region_obj,
>> > + ACPI_REG_CONNECT);
>> > acpi_ex_enter_interpreter();
>>
>> And this also doesn't apply, because we don't invoke
>> acpi_ex_exit_interpreter() / acpi_ex_enter_interpreter() around the
>> acpi_ev_execute_reg_method() call in linux-next.
>>
>
> acpi_ex_exit_interpreter() / acpi_ex_enter_interpreter() are not-modified-lines.
> Please check again.
As I said in the previous message, this was a result of my mistake.
Fixed now and everything applies as it should.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 05/11] ACPICA: Tables: Cleanup acpi_tb_install_and_load_table()
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
` (2 preceding siblings ...)
2016-11-30 7:21 ` [PATCH 04/11] ACPICA: Events: Fix acpi_ev_initialize_region() return value Lv Zheng
@ 2016-11-30 7:21 ` Lv Zheng
2016-11-30 7:21 ` [PATCH 06/11] ACPICA: Tables: Add acpi_tb_unload_table() Lv Zheng
` (6 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:21 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
ACPICA commit 7fdac0289faa1c28b91413c8e394e87372aa69e6
acpi_tb_install_and_load_table() can invoke acpi_tb_load_table() to eliminate
redundant code.
No functional change. Lv Zheng.
Link: https://github.com/acpica/acpica/commit/7fdac028
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
---
drivers/acpi/acpica/actables.h | 3 +--
drivers/acpi/acpica/exconfig.c | 7 +++----
drivers/acpi/acpica/tbdata.c | 45 +++++-----------------------------------
drivers/acpi/acpica/tbxfload.c | 7 +++----
4 files changed, 12 insertions(+), 50 deletions(-)
diff --git a/drivers/acpi/acpica/actables.h b/drivers/acpi/acpica/actables.h
index e85953b..1850005 100644
--- a/drivers/acpi/acpica/actables.h
+++ b/drivers/acpi/acpica/actables.h
@@ -127,8 +127,7 @@
acpi_tb_load_table(u32 table_index, struct acpi_namespace_node *parent_node);
acpi_status
-acpi_tb_install_and_load_table(struct acpi_table_header *table,
- acpi_physical_address address,
+acpi_tb_install_and_load_table(acpi_physical_address address,
u8 flags, u8 override, u32 *table_index);
void acpi_tb_terminate(void);
diff --git a/drivers/acpi/acpica/exconfig.c b/drivers/acpi/acpica/exconfig.c
index 718428b..8b8d620 100644
--- a/drivers/acpi/acpica/exconfig.c
+++ b/drivers/acpi/acpica/exconfig.c
@@ -437,10 +437,9 @@
ACPI_INFO(("Dynamic OEM Table Load:"));
acpi_ex_exit_interpreter();
- status =
- acpi_tb_install_and_load_table(table, ACPI_PTR_TO_PHYSADDR(table),
- ACPI_TABLE_ORIGIN_INTERNAL_VIRTUAL,
- TRUE, &table_index);
+ status = acpi_tb_install_and_load_table(ACPI_PTR_TO_PHYSADDR(table),
+ ACPI_TABLE_ORIGIN_INTERNAL_VIRTUAL,
+ TRUE, &table_index);
acpi_ex_enter_interpreter();
if (ACPI_FAILURE(status)) {
diff --git a/drivers/acpi/acpica/tbdata.c b/drivers/acpi/acpica/tbdata.c
index d9ca8c2..4cbfa30 100644
--- a/drivers/acpi/acpica/tbdata.c
+++ b/drivers/acpi/acpica/tbdata.c
@@ -832,9 +832,9 @@ void acpi_tb_set_table_loaded_flag(u32 table_index, u8 is_loaded)
*
* FUNCTION: acpi_tb_install_and_load_table
*
- * PARAMETERS: table - Pointer to the table
- * address - Physical address of the table
+ * PARAMETERS: address - Physical address of the table
* flags - Allocation flags of the table
+ * override - Whether override should be performed
* table_index - Where table index is returned
*
* RETURN: Status
@@ -844,15 +844,13 @@ void acpi_tb_set_table_loaded_flag(u32 table_index, u8 is_loaded)
******************************************************************************/
acpi_status
-acpi_tb_install_and_load_table(struct acpi_table_header *table,
- acpi_physical_address address,
+acpi_tb_install_and_load_table(acpi_physical_address address,
u8 flags, u8 override, u32 *table_index)
{
acpi_status status;
u32 i;
- acpi_owner_id owner_id;
- ACPI_FUNCTION_TRACE(acpi_load_table);
+ ACPI_FUNCTION_TRACE(tb_install_and_load_table);
(void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
@@ -864,41 +862,8 @@ void acpi_tb_set_table_loaded_flag(u32 table_index, u8 is_loaded)
goto unlock_and_exit;
}
- /*
- * Note: Now table is "INSTALLED", it must be validated before
- * using.
- */
- status = acpi_tb_validate_table(&acpi_gbl_root_table_list.tables[i]);
- if (ACPI_FAILURE(status)) {
- goto unlock_and_exit;
- }
-
(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
- status = acpi_ns_load_table(i, acpi_gbl_root_node);
-
- /* Execute any module-level code that was found in the table */
-
- if (!acpi_gbl_parse_table_as_term_list
- && acpi_gbl_group_module_level_code) {
- acpi_ns_exec_module_code_list();
- }
-
- /*
- * Update GPEs for any new _Lxx/_Exx methods. Ignore errors. The host is
- * responsible for discovering any new wake GPEs by running _PRW methods
- * that may have been loaded by this table.
- */
- status = acpi_tb_get_owner_id(i, &owner_id);
- if (ACPI_SUCCESS(status)) {
- acpi_ev_update_gpes(owner_id);
- }
-
- /* Invoke table handler if present */
-
- if (acpi_gbl_table_handler) {
- (void)acpi_gbl_table_handler(ACPI_TABLE_EVENT_LOAD, table,
- acpi_gbl_table_handler_context);
- }
+ status = acpi_tb_load_table(i, acpi_gbl_root_node);
(void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
unlock_and_exit:
diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index 2599e831..77de33b 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -330,10 +330,9 @@ acpi_status acpi_load_table(struct acpi_table_header *table)
/* Install the table and load it into the namespace */
ACPI_INFO(("Host-directed Dynamic ACPI Table Load:"));
- status =
- acpi_tb_install_and_load_table(table, ACPI_PTR_TO_PHYSADDR(table),
- ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL,
- FALSE, &table_index);
+ status = acpi_tb_install_and_load_table(ACPI_PTR_TO_PHYSADDR(table),
+ ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL,
+ FALSE, &table_index);
return_ACPI_STATUS(status);
}
--
1.7.10
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 06/11] ACPICA: Tables: Add acpi_tb_unload_table()
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
` (3 preceding siblings ...)
2016-11-30 7:21 ` [PATCH 05/11] ACPICA: Tables: Cleanup acpi_tb_install_and_load_table() Lv Zheng
@ 2016-11-30 7:21 ` Lv Zheng
2016-11-30 7:21 ` [PATCH 07/11] ACPICA: Tables: Add an error message complaining driver bugs Lv Zheng
` (5 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:21 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
ACPICA commit 80e24663b212daac0c32767fdbd8a46892292f1f
This patch introduces acpi_tb_unload_table() to eliminate redundant code from
acpi_ex_unload_table() and acpi_unload_parent_table().
No functional change. Lv Zheng.
Link: https://github.com/acpica/acpica/commit/80e24663
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
---
drivers/acpi/acpica/actables.h | 2 ++
drivers/acpi/acpica/exconfig.c | 35 +----------------------------
drivers/acpi/acpica/tbdata.c | 48 ++++++++++++++++++++++++++++++++++++++++
drivers/acpi/acpica/tbxfload.c | 31 +-------------------------
4 files changed, 52 insertions(+), 64 deletions(-)
diff --git a/drivers/acpi/acpica/actables.h b/drivers/acpi/acpica/actables.h
index 1850005..7dd527f 100644
--- a/drivers/acpi/acpica/actables.h
+++ b/drivers/acpi/acpica/actables.h
@@ -130,6 +130,8 @@
acpi_tb_install_and_load_table(acpi_physical_address address,
u8 flags, u8 override, u32 *table_index);
+acpi_status acpi_tb_unload_table(u32 table_index);
+
void acpi_tb_terminate(void);
acpi_status acpi_tb_delete_namespace_by_owner(u32 table_index);
diff --git a/drivers/acpi/acpica/exconfig.c b/drivers/acpi/acpica/exconfig.c
index 8b8d620..c32c782 100644
--- a/drivers/acpi/acpica/exconfig.c
+++ b/drivers/acpi/acpica/exconfig.c
@@ -499,7 +499,6 @@ acpi_status acpi_ex_unload_table(union acpi_operand_object *ddb_handle)
acpi_status status = AE_OK;
union acpi_operand_object *table_desc = ddb_handle;
u32 table_index;
- struct acpi_table_header *table;
ACPI_FUNCTION_TRACE(ex_unload_table);
@@ -536,39 +535,7 @@ acpi_status acpi_ex_unload_table(union acpi_operand_object *ddb_handle)
* strict order requirement against it.
*/
acpi_ex_exit_interpreter();
-
- /* Ensure the table is still loaded */
-
- if (!acpi_tb_is_table_loaded(table_index)) {
- status = AE_NOT_EXIST;
- goto lock_and_exit;
- }
-
- /* Invoke table handler if present */
-
- if (acpi_gbl_table_handler) {
- status = acpi_get_table_by_index(table_index, &table);
- if (ACPI_SUCCESS(status)) {
- (void)acpi_gbl_table_handler(ACPI_TABLE_EVENT_UNLOAD,
- table,
- acpi_gbl_table_handler_context);
- }
- }
-
- /* Delete the portion of the namespace owned by this table */
-
- status = acpi_tb_delete_namespace_by_owner(table_index);
- if (ACPI_FAILURE(status)) {
- goto lock_and_exit;
- }
-
- (void)acpi_tb_release_owner_id(table_index);
- acpi_tb_set_table_loaded_flag(table_index, FALSE);
-
-lock_and_exit:
-
- /* Re-acquire the interpreter lock */
-
+ status = acpi_tb_unload_table(table_index);
acpi_ex_enter_interpreter();
/*
diff --git a/drivers/acpi/acpica/tbdata.c b/drivers/acpi/acpica/tbdata.c
index 4cbfa30..82b0b57 100644
--- a/drivers/acpi/acpica/tbdata.c
+++ b/drivers/acpi/acpica/tbdata.c
@@ -871,3 +871,51 @@ void acpi_tb_set_table_loaded_flag(u32 table_index, u8 is_loaded)
(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
return_ACPI_STATUS(status);
}
+
+/*******************************************************************************
+ *
+ * FUNCTION: acpi_tb_unload_table
+ *
+ * PARAMETERS: table_index - Table index
+ *
+ * RETURN: Status
+ *
+ * DESCRIPTION: Unload an ACPI table
+ *
+ ******************************************************************************/
+
+acpi_status acpi_tb_unload_table(u32 table_index)
+{
+ acpi_status status = AE_OK;
+ struct acpi_table_header *table;
+
+ ACPI_FUNCTION_TRACE(tb_unload_table);
+
+ /* Ensure the table is still loaded */
+
+ if (!acpi_tb_is_table_loaded(table_index)) {
+ return_ACPI_STATUS(AE_NOT_EXIST);
+ }
+
+ /* Invoke table handler if present */
+
+ if (acpi_gbl_table_handler) {
+ status = acpi_get_table_by_index(table_index, &table);
+ if (ACPI_SUCCESS(status)) {
+ (void)acpi_gbl_table_handler(ACPI_TABLE_EVENT_UNLOAD,
+ table,
+ acpi_gbl_table_handler_context);
+ }
+ }
+
+ /* Delete the portion of the namespace owned by this table */
+
+ status = acpi_tb_delete_namespace_by_owner(table_index);
+ if (ACPI_FAILURE(status)) {
+ return_ACPI_STATUS(status);
+ }
+
+ (void)acpi_tb_release_owner_id(table_index);
+ acpi_tb_set_table_loaded_flag(table_index, FALSE);
+ return_ACPI_STATUS(status);
+}
diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index 77de33b..82019c0 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -408,37 +408,8 @@ acpi_status acpi_unload_parent_table(acpi_handle object)
break;
}
- /* Ensure the table is actually loaded */
-
(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
- if (!acpi_tb_is_table_loaded(i)) {
- status = AE_NOT_EXIST;
- (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
- break;
- }
-
- /* Invoke table handler if present */
-
- if (acpi_gbl_table_handler) {
- (void)acpi_gbl_table_handler(ACPI_TABLE_EVENT_UNLOAD,
- acpi_gbl_root_table_list.
- tables[i].pointer,
- acpi_gbl_table_handler_context);
- }
-
- /*
- * Delete all namespace objects owned by this table. Note that
- * these objects can appear anywhere in the namespace by virtue
- * of the AML "Scope" operator. Thus, we need to track ownership
- * by an ID, not simply a position within the hierarchy.
- */
- status = acpi_tb_delete_namespace_by_owner(i);
- if (ACPI_FAILURE(status)) {
- break;
- }
-
- status = acpi_tb_release_owner_id(i);
- acpi_tb_set_table_loaded_flag(i, FALSE);
+ status = acpi_tb_unload_table(i);
(void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
break;
}
--
1.7.10
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 07/11] ACPICA: Tables: Add an error message complaining driver bugs
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
` (4 preceding siblings ...)
2016-11-30 7:21 ` [PATCH 06/11] ACPICA: Tables: Add acpi_tb_unload_table() Lv Zheng
@ 2016-11-30 7:21 ` Lv Zheng
2016-11-30 7:21 ` [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel Lv Zheng
` (4 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:21 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
ACPICA commit 68af3c3aa238dd8040e846ac6b4827a016434d8d
During early OS boot stage, drivers that have mapped system memory should
unmap it during the same stage. Linux kernel has an error message
indicating the unbalanced early memory mappings.
This patch back ports such error message into ACPICA for the early table
mappings, so that ACPICA development environment is also aware of this OS
specific requirement and thus is able to ensure the consistent quality
locally. Lv Zheng.
Link: https://github.com/acpica/acpica/commit/68af3c3a
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
---
drivers/acpi/acpica/tbxface.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
index 4ab6b9c..d5adb7a 100644
--- a/drivers/acpi/acpica/tbxface.c
+++ b/drivers/acpi/acpica/tbxface.c
@@ -167,6 +167,7 @@ acpi_status acpi_allocate_root_table(u32 initial_table_count)
acpi_status ACPI_INIT_FUNCTION acpi_reallocate_root_table(void)
{
acpi_status status;
+ u32 i;
ACPI_FUNCTION_TRACE(acpi_reallocate_root_table);
@@ -178,6 +179,21 @@ acpi_status ACPI_INIT_FUNCTION acpi_reallocate_root_table(void)
return_ACPI_STATUS(AE_SUPPORT);
}
+ /*
+ * Ensure OS early boot logic, which is required by some hosts. If the
+ * table state is reported to be wrong, developers should fix the
+ * issue by invoking acpi_put_table() for the reported table during the
+ * early stage.
+ */
+ for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
+ if (acpi_gbl_root_table_list.tables[i].pointer) {
+ ACPI_ERROR((AE_INFO,
+ "Table [%4.4s] is not invalidated during early boot stage",
+ acpi_gbl_root_table_list.tables[i].
+ signature.ascii));
+ }
+ }
+
acpi_gbl_root_table_list.flags |= ACPI_ROOT_ALLOW_RESIZE;
status = acpi_tb_resize_root_table_list();
--
1.7.10
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
` (5 preceding siblings ...)
2016-11-30 7:21 ` [PATCH 07/11] ACPICA: Tables: Add an error message complaining driver bugs Lv Zheng
@ 2016-11-30 7:21 ` Lv Zheng
2016-12-08 1:11 ` Dan Williams
2016-11-30 7:21 ` [PATCH 09/11] ACPICA: Tables: Allow FADT to be customized with virtual address Lv Zheng
` (3 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:21 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-11-30 7:21 ` [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel Lv Zheng
@ 2016-12-08 1:11 ` Dan Williams
2016-12-08 13:18 ` Rafael J. Wysocki
2016-12-09 1:49 ` Zheng, Lv
0 siblings, 2 replies; 31+ messages in thread
From: Dan Williams @ 2016-12-08 1:11 UTC (permalink / raw)
To: Lv Zheng
Cc: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown, Lv Zheng,
Linux Kernel Mailing List, Linux ACPI, Bob Moore,
linux-nvdimm@lists.01.org
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
)
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-08 1:11 ` Dan Williams
@ 2016-12-08 13:18 ` Rafael J. Wysocki
2016-12-08 19:04 ` Dan Williams
2016-12-09 1:49 ` Zheng, Lv
1 sibling, 1 reply; 31+ messages in thread
From: Rafael J. Wysocki @ 2016-12-08 13:18 UTC (permalink / raw)
To: Dan Williams
Cc: Lv Zheng, Rafael J. Wysocki, Rafael J. Wysocki, Len Brown,
Lv Zheng, Linux Kernel Mailing List, Linux ACPI, Bob Moore,
linux-nvdimm@lists.01.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-08 13:18 ` Rafael J. Wysocki
@ 2016-12-08 19:04 ` Dan Williams
2016-12-09 1:59 ` Zheng, Lv
0 siblings, 1 reply; 31+ messages in thread
From: Dan Williams @ 2016-12-08 19:04 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Lv Zheng, Rafael J. Wysocki, Rafael J. Wysocki, Len Brown,
Lv Zheng, Linux Kernel Mailing List, Linux ACPI, Bob Moore,
linux-nvdimm@lists.01.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-08 19:04 ` Dan Williams
@ 2016-12-09 1:59 ` Zheng, Lv
2016-12-09 2:04 ` Dan Williams
2016-12-09 2:05 ` Rafael J. Wysocki
0 siblings, 2 replies; 31+ messages in thread
From: Zheng, Lv @ 2016-12-09 1:59 UTC (permalink / raw)
To: Williams, Dan J, Rafael J. Wysocki
Cc: Wysocki, Rafael J, Rafael J. Wysocki, Brown, Len, Lv Zheng,
Linux Kernel Mailing List, Linux ACPI, Moore, Robert,
linux-nvdimm@lists.01.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-09 1:59 ` Zheng, Lv
@ 2016-12-09 2:04 ` Dan Williams
2016-12-09 2:15 ` Zheng, Lv
2016-12-09 2:27 ` Zheng, Lv
2016-12-09 2:05 ` Rafael J. Wysocki
1 sibling, 2 replies; 31+ messages in thread
From: Dan Williams @ 2016-12-09 2:04 UTC (permalink / raw)
To: Zheng, Lv
Cc: Rafael J. Wysocki, Wysocki, Rafael J, Rafael J. Wysocki, Brown,
Len, Lv Zheng, Linux Kernel Mailing List, Linux ACPI, Moore,
Robert, linux-nvdimm@lists.01.org
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?
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-09 2:04 ` Dan Williams
@ 2016-12-09 2:15 ` Zheng, Lv
2016-12-09 2:27 ` Zheng, Lv
1 sibling, 0 replies; 31+ messages in thread
From: Zheng, Lv @ 2016-12-09 2:15 UTC (permalink / raw)
To: Williams, Dan J
Cc: Rafael J. Wysocki, Wysocki, Rafael J, Rafael J. Wysocki, Brown,
Len, Lv Zheng, Linux Kernel Mailing List, Linux ACPI, Moore,
Robert, linux-nvdimm@lists.01.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-09 2:04 ` Dan Williams
2016-12-09 2:15 ` Zheng, Lv
@ 2016-12-09 2:27 ` Zheng, Lv
1 sibling, 0 replies; 31+ messages in thread
From: Zheng, Lv @ 2016-12-09 2:27 UTC (permalink / raw)
To: Williams, Dan J
Cc: Rafael J. Wysocki, Wysocki, Rafael J, Rafael J. Wysocki, Brown,
Len, Lv Zheng, Linux Kernel Mailing List, Linux ACPI, Moore,
Robert, linux-nvdimm@lists.01.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-09 1:59 ` Zheng, Lv
2016-12-09 2:04 ` Dan Williams
@ 2016-12-09 2:05 ` Rafael J. Wysocki
2016-12-09 2:23 ` Zheng, Lv
1 sibling, 1 reply; 31+ messages in thread
From: Rafael J. Wysocki @ 2016-12-09 2:05 UTC (permalink / raw)
To: Zheng, Lv
Cc: Williams, Dan J, Rafael J. Wysocki, Wysocki, Rafael J,
Rafael J. Wysocki, Brown, Len, Lv Zheng,
Linux Kernel Mailing List, Linux ACPI, Moore, Robert,
linux-nvdimm@lists.01.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-09 2:05 ` Rafael J. Wysocki
@ 2016-12-09 2:23 ` Zheng, Lv
0 siblings, 0 replies; 31+ messages in thread
From: Zheng, Lv @ 2016-12-09 2:23 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Williams, Dan J, Wysocki, Rafael J, Rafael J. Wysocki, Brown,
Len, Lv Zheng, Linux Kernel Mailing List, Linux ACPI, Moore,
Robert, linux-nvdimm@lists.01.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-08 1:11 ` Dan Williams
2016-12-08 13:18 ` Rafael J. Wysocki
@ 2016-12-09 1:49 ` Zheng, Lv
2016-12-09 1:57 ` Dan Williams
1 sibling, 1 reply; 31+ messages in thread
From: Zheng, Lv @ 2016-12-09 1:49 UTC (permalink / raw)
To: Williams, Dan J
Cc: Wysocki, Rafael J, Rafael J. Wysocki, Brown, Len, Lv Zheng,
Linux Kernel Mailing List, Linux ACPI, Moore, Robert,
linux-nvdimm@lists.01.org
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
2016-12-09 1:49 ` Zheng, Lv
@ 2016-12-09 1:57 ` Dan Williams
0 siblings, 0 replies; 31+ messages in thread
From: Dan Williams @ 2016-12-09 1:57 UTC (permalink / raw)
To: Zheng, Lv
Cc: Wysocki, Rafael J, Rafael J. Wysocki, Brown, Len, Lv Zheng,
Linux Kernel Mailing List, Linux ACPI, Moore, Robert,
linux-nvdimm@lists.01.org
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().
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 09/11] ACPICA: Tables: Allow FADT to be customized with virtual address
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
` (6 preceding siblings ...)
2016-11-30 7:21 ` [PATCH 08/11] ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel Lv Zheng
@ 2016-11-30 7:21 ` Lv Zheng
2016-11-30 7:21 ` [PATCH 10/11] ACPICA: Utilities: Add new decode function for parser values Lv Zheng
` (2 subsequent siblings)
10 siblings, 0 replies; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:21 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
ACPICA commit d98de9ca14891130efc5dcdc871b97eb27b4b0f5
FADT parsing code requires FADT to be installed as
ACPI_TABLE_ORIGIN_INTERNAL_PHYSICAL, using new
acpi_tb_get_table()/acpi_tb_put_table(), other address types can also be allowed,
thus facilitates FADT customization with virtual address. Lv Zheng.
Link: https://github.com/acpica/acpica/commit/d98de9ca
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
---
drivers/acpi/acpica/tbfadt.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/acpica/tbfadt.c b/drivers/acpi/acpica/tbfadt.c
index 5fb838e..81473a4 100644
--- a/drivers/acpi/acpica/tbfadt.c
+++ b/drivers/acpi/acpica/tbfadt.c
@@ -311,6 +311,8 @@ void acpi_tb_parse_fadt(void)
{
u32 length;
struct acpi_table_header *table;
+ struct acpi_table_desc *fadt_desc;
+ acpi_status status;
/*
* The FADT has multiple versions with different lengths,
@@ -319,14 +321,12 @@ void acpi_tb_parse_fadt(void)
* Get a local copy of the FADT and convert it to a common format
* Map entire FADT, assumed to be smaller than one page.
*/
- length = acpi_gbl_root_table_list.tables[acpi_gbl_fadt_index].length;
-
- table =
- acpi_os_map_memory(acpi_gbl_root_table_list.
- tables[acpi_gbl_fadt_index].address, length);
- if (!table) {
+ fadt_desc = &acpi_gbl_root_table_list.tables[acpi_gbl_fadt_index];
+ status = acpi_tb_get_table(fadt_desc, &table);
+ if (ACPI_FAILURE(status)) {
return;
}
+ length = fadt_desc->length;
/*
* Validate the FADT checksum before we copy the table. Ignore
@@ -340,7 +340,7 @@ void acpi_tb_parse_fadt(void)
/* All done with the real FADT, unmap it */
- acpi_os_unmap_memory(table, length);
+ acpi_tb_put_table(fadt_desc);
/* Obtain the DSDT and FACS tables via their addresses within the FADT */
--
1.7.10
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 10/11] ACPICA: Utilities: Add new decode function for parser values
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
` (7 preceding siblings ...)
2016-11-30 7:21 ` [PATCH 09/11] ACPICA: Tables: Allow FADT to be customized with virtual address Lv Zheng
@ 2016-11-30 7:21 ` Lv Zheng
2016-11-30 7:22 ` [PATCH 11/11] ACPICA: Update version to 20161117 Lv Zheng
2016-12-09 2:21 ` [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size() Lv Zheng
10 siblings, 0 replies; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:21 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
From: Bob Moore <robert.moore@intel.com>
ACPICA commit 198fde8a061ac77357bcf1752e3c988fbe59f128
Implements a decode function for the ARGP_* parser info values
for all AML opcodes.
Link: https://github.com/acpica/acpica/commit/198fde8a
Signed-off-by: Bob Moore <robert.moore@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/acpi/acpica/acutils.h | 2 ++
drivers/acpi/acpica/amlcode.h | 1 +
drivers/acpi/acpica/utdecode.c | 49 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 52 insertions(+)
diff --git a/drivers/acpi/acpica/acutils.h b/drivers/acpi/acpica/acutils.h
index 0a1b53c..845afb1 100644
--- a/drivers/acpi/acpica/acutils.h
+++ b/drivers/acpi/acpica/acutils.h
@@ -232,6 +232,8 @@ struct acpi_pkg_info {
const char *acpi_ut_get_event_name(u32 event_id);
+const char *acpi_ut_get_argument_type_name(u32 arg_type);
+
char acpi_ut_hex_to_ascii_char(u64 integer, u32 position);
acpi_status acpi_ut_ascii_to_hex_byte(char *two_ascii_chars, u8 *return_byte);
diff --git a/drivers/acpi/acpica/amlcode.h b/drivers/acpi/acpica/amlcode.h
index 04fbd88..8a0f959 100644
--- a/drivers/acpi/acpica/amlcode.h
+++ b/drivers/acpi/acpica/amlcode.h
@@ -240,6 +240,7 @@
#define ARGP_QWORDDATA 0x11
#define ARGP_SIMPLENAME 0x12 /* name_string | local_term | arg_term */
#define ARGP_NAME_OR_REF 0x13 /* For object_type only */
+#define ARGP_MAX 0x13
/*
* Resolved argument types for the AML Interpreter
diff --git a/drivers/acpi/acpica/utdecode.c b/drivers/acpi/acpica/utdecode.c
index 15728ad..b3d8421 100644
--- a/drivers/acpi/acpica/utdecode.c
+++ b/drivers/acpi/acpica/utdecode.c
@@ -44,6 +44,7 @@
#include <acpi/acpi.h>
#include "accommon.h"
#include "acnamesp.h"
+#include "amlcode.h"
#define _COMPONENT ACPI_UTILITIES
ACPI_MODULE_NAME("utdecode")
@@ -532,6 +533,54 @@ const char *acpi_ut_get_notify_name(u32 notify_value, acpi_object_type type)
return ("Hardware-Specific");
}
+
+/*******************************************************************************
+ *
+ * FUNCTION: acpi_ut_get_argument_type_name
+ *
+ * PARAMETERS: arg_type - an ARGP_* parser argument type
+ *
+ * RETURN: Decoded ARGP_* type
+ *
+ * DESCRIPTION: Decode an ARGP_* parser type, as defined in the amlcode.h file,
+ * and used in the acopcode.h file. For example, ARGP_TERMARG.
+ * Used for debug only.
+ *
+ ******************************************************************************/
+
+static const char *acpi_gbl_argument_type[20] = {
+ /* 00 */ "Unknown ARGP",
+ /* 01 */ "ByteData",
+ /* 02 */ "ByteList",
+ /* 03 */ "CharList",
+ /* 04 */ "DataObject",
+ /* 05 */ "DataObjectList",
+ /* 06 */ "DWordData",
+ /* 07 */ "FieldList",
+ /* 08 */ "Name",
+ /* 09 */ "NameString",
+ /* 0A */ "ObjectList",
+ /* 0B */ "PackageLength",
+ /* 0C */ "SuperName",
+ /* 0D */ "Target",
+ /* 0E */ "TermArg",
+ /* 0F */ "TermList",
+ /* 10 */ "WordData",
+ /* 11 */ "QWordData",
+ /* 12 */ "SimpleName",
+ /* 13 */ "NameOrRef"
+};
+
+const char *acpi_ut_get_argument_type_name(u32 arg_type)
+{
+
+ if (arg_type > ARGP_MAX) {
+ return ("Unknown ARGP");
+ }
+
+ return (acpi_gbl_argument_type[arg_type]);
+}
+
#endif
/*******************************************************************************
--
1.7.10
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 11/11] ACPICA: Update version to 20161117
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
` (8 preceding siblings ...)
2016-11-30 7:21 ` [PATCH 10/11] ACPICA: Utilities: Add new decode function for parser values Lv Zheng
@ 2016-11-30 7:22 ` Lv Zheng
2016-12-09 2:21 ` [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size() Lv Zheng
10 siblings, 0 replies; 31+ messages in thread
From: Lv Zheng @ 2016-11-30 7:22 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Bob Moore
From: Bob Moore <robert.moore@intel.com>
ACPICA commit 0d5a056877c2e37e0bfce8d262cec339dc8d55fd
Version 20161117.
Link: https://github.com/acpica/acpica/commit/0d5a0568
Signed-off-by: Bob Moore <robert.moore@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
include/acpi/acpixf.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 33828dd..0d20c53 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -46,7 +46,7 @@
/* Current ACPICA subsystem version in YYYYMMDD format */
-#define ACPI_CA_VERSION 0x20160930
+#define ACPI_CA_VERSION 0x20161117
#include <acpi/acconfig.h>
#include <acpi/actypes.h>
--
1.7.10
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size()
2016-11-30 7:20 [PATCH 00/11] ACPICA: 20161117 Release Lv Zheng
` (9 preceding siblings ...)
2016-11-30 7:22 ` [PATCH 11/11] ACPICA: Update version to 20161117 Lv Zheng
@ 2016-12-09 2:21 ` Lv Zheng
2016-12-09 3:48 ` Rafael J. Wysocki
10 siblings, 1 reply; 31+ messages in thread
From: Lv Zheng @ 2016-12-09 2:21 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Dan Williams
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size()
2016-12-09 2:21 ` [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size() Lv Zheng
@ 2016-12-09 3:48 ` Rafael J. Wysocki
2016-12-09 6:09 ` Zheng, Lv
0 siblings, 1 reply; 31+ messages in thread
From: Rafael J. Wysocki @ 2016-12-09 3:48 UTC (permalink / raw)
To: Lv Zheng
Cc: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown, Lv Zheng,
Linux Kernel Mailing List, ACPI Devel Maling List, Dan Williams
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
^ permalink raw reply [flat|nested] 31+ messages in thread* RE: [PATCH] ACPI / OSL: Fix a regression by returning table size via acpi_get_table_with_size()
2016-12-09 3:48 ` Rafael J. Wysocki
@ 2016-12-09 6:09 ` Zheng, Lv
0 siblings, 0 replies; 31+ messages in thread
From: Zheng, Lv @ 2016-12-09 6:09 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Wysocki, Rafael J, Rafael J. Wysocki, Brown, Len, Lv Zheng,
Linux Kernel Mailing List, ACPI Devel Maling List, Williams,
Dan J
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
^ permalink raw reply [flat|nested] 31+ messages in thread