RE: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...")

["Schmauss, Erik" <erik.schmauss@intel.com>]

> -----Original Message-----
> From: Wysocki, Rafael J
> Sent: Wednesday, May 15, 2019 1:57 PM
> To: Schmauss, Erik <erik.schmauss@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Paul Gortmaker
> <paul.gortmaker@windriver.com>; stable@vger.kernel.org
> Subject: Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML
> interpreter: add region addresses...")
> 
> On 5/15/2019 6:57 AM, Greg Kroah-Hartman wrote:
> > On Wed, May 15, 2019 at 01:17:28AM +0000, Schmauss, Erik wrote:
> >>
> >>> -----Original Message-----
> >>> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> >>> Sent: Monday, May 6, 2019 1:42 AM
> >>> To: Paul Gortmaker <paul.gortmaker@windriver.com>; Wysocki, Rafael J
> >>> <rafael.j.wysocki@intel.com>
> >>> Cc: stable@vger.kernel.org; Schmauss, Erik <erik.schmauss@intel.com>
> >>> Subject: Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA:
> >>> AML
> >>> interpreter: add region addresses...")
> >>>
> >>> On Sun, May 05, 2019 at 03:44:48PM -0400, Paul Gortmaker wrote:
> >>>> I noticed 4.19.35 got a backport of mainline 4abb951b, but it
> >>>> appears to be a duplicate backport that landed in the wrong
> >>>> function.  We can see this in the stable-queue repo:
> >>>>
> >>>> stable-queue$ find . -name '*acpica-aml-interpreter-add-region-addr*'
> >>>> |grep 4.19
> >>>> ./releases/4.19.6/acpica-aml-interpreter-add-region-addresses-in-gl
> >>>> oba
> >>>> l-list-during-initialization.patch
> >>>> ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresse
> >>>> s-i
> >>>> n.patch
> >>>> ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-g
> >>>> lob al-list-during-initialization.patch
> >>>> ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-gl
> >>>> oba
> >>>> l-list-during-initialization.patch
> >>>>
> >>>> So it was added to 4.19.2, reverted in .3, re-added in .6, and then
> >>>> finally patched into a similar looking but wrong function in .35
> >>>>
> >>>> If we diff the .6 and .35 versions, we see the function difference:
> >>>>
> >>>> -@@ -417,6 +417,10 @@ acpi_ds_eval_region_operands(struct acpi
> >>>> +@@ -523,6 +523,10 @@ acpi_ds_eval_table_region_operands(struc
> >>>>
> >>>> I don't know what the history is/was around the 2/3/6 churn, but
> >>>> the re-addition in 4.19.35 to a different function sure looks wrong.
> >>>>
> >>>> The commit adds a call "status = acpi_ut_add_address_range(..." and
> >>>> if we check mainline, there is only one in that file, but in
> >>>> 4.19.35+ there now are two calls - since the two functions had
> >>>> similar context and comments, it isn't hard to see how patch
> >>>> could/would apply it a 2nd time in the wrong place.
> >>>>
> >>>> I didn't check if any of the other currently maintained
> >>>> linux-stable versions also had this possible issue.
> >>>>
> >> Hi Greg,
> >>
> >>> Ugh, Rafael, did I mess this up again?  Can you check to see if I
> >>> need to fix this somehow?
> >> It should be called in acpi_ds_eval_region_operands rather than
> acpi_ds_eval_table_region_operands.
> >> Please remove the call from acpi_ds_eval_table_region_operands.
> > Great, can someone please send me a patch for this so that I don't get
> > it wrong myself?
> 
> Erik, can you please cut a patch for that against 4.19.35 and send it to Greg?
> 

I'm not sure what the process is for this case but here's the patch...
Let me know if you need me to send it some other way...

From a738f1c452c0762d3c0a1b1a9a12c78bd97b0a23 Mon Sep 17 00:00:00 2001
From: Erik Schmauss <erik.schmauss@intel.com>
Date: Wed, 15 May 2019 17:25:31 -0700
Subject: [PATCH] Revert "ACPICA: AML interpreter: add region addresses in
 global list during initialization"

This reverts commit f8053df634d40c733f26ca49c2c3835002e61b77 that was
unintentionally included as a part of the stable branch.

Reported-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Erik Schmauss <erik.schmauss@intel.com>
---
 drivers/acpi/acpica/dsopcode.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/acpi/acpica/dsopcode.c b/drivers/acpi/acpica/dsopcode.c
index 2f4641e5ecde..78f9de260d5f 100644
--- a/drivers/acpi/acpica/dsopcode.c
+++ b/drivers/acpi/acpica/dsopcode.c
@@ -523,10 +523,6 @@ acpi_ds_eval_table_region_operands(struct acpi_walk_state *walk_state,
                          ACPI_FORMAT_UINT64(obj_desc->region.address),
                          obj_desc->region.length));

-       status = acpi_ut_add_address_range(obj_desc->region.space_id,
-                                          obj_desc->region.address,
-                                          obj_desc->region.length, node);
-
        /* Now the address and length are valid for this opregion */

        obj_desc->region.flags |= AOPOBJ_DATA_VALID;
--
2.17.2