> -----Original Message----- > From: Wysocki, Rafael J > Sent: Thursday, May 16, 2019 8:36 AM > To: Schmauss, Erik <erik.schmauss@xxxxxxxxx>; Greg Kroah-Hartman > <gregkh@xxxxxxxxxxxxxxxxxxx> > Cc: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>; > stable@xxxxxxxxxxxxxxx > Subject: Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML > interpreter: add region addresses...") > > On 5/16/2019 2:45 AM, Schmauss, Erik wrote: > > > >> -----Original Message----- > >> From: Wysocki, Rafael J > >> Sent: Wednesday, May 15, 2019 1:57 PM > >> To: Schmauss, Erik <erik.schmauss@xxxxxxxxx> > >> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Paul Gortmaker > >> <paul.gortmaker@xxxxxxxxxxxxx>; stable@xxxxxxxxxxxxxxx > >> 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@xxxxxxxxxxxxxxxxxxx] > >>>>> Sent: Monday, May 6, 2019 1:42 AM > >>>>> To: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>; Wysocki, > Rafael > >>>>> J <rafael.j.wysocki@xxxxxxxxx> > >>>>> Cc: stable@xxxxxxxxxxxxxxx; Schmauss, Erik > >>>>> <erik.schmauss@xxxxxxxxx> > >>>>> 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-addres > >>>>>> se > >>>>>> 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@xxxxxxxxx> > > 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@xxxxxxxxxxxxx> > > Signed-off-by: Erik Schmauss <erik.schmauss@xxxxxxxxx> > > --- > > 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 > > Thanks Erik, LGTM. > Thanks for your patience with these patches! Erik