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

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

[Paul Gortmaker]

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-global-list-during-initialization.patch
./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-in.patch
./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-global-list-during-initialization.patch
./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-global-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.

Paul.

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

[Greg Kroah-Hartman]

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-global-list-during-initialization.patch
> ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-in.patch
> ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-global-list-during-initialization.patch
> ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-global-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.
> 

Ugh, Rafael, did I mess this up again?  Can you check to see if I need
to fix this somehow?

thanks,

greg k-h

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

[Schmauss, Erik]

> -----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-globa
> > l-list-during-initialization.patch
> > ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-i
> > n.patch
> > ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-glob
> > al-list-during-initialization.patch
> > ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-globa
> > 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.

Thanks,
Erik
> 
> thanks,
> 
> greg k-h

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

[Greg Kroah-Hartman]

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-globa
> > > l-list-during-initialization.patch
> > > ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-i
> > > n.patch
> > > ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-glob
> > > al-list-during-initialization.patch
> > > ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-globa
> > > 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?

thanks,

greg k-h

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

[Rafael J. Wysocki]

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-globa
>>>> l-list-during-initialization.patch
>>>> ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-i
>>>> n.patch
>>>> ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-glob
>>>> al-list-during-initialization.patch
>>>> ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-globa
>>>> 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?

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

[Schmauss, Erik]

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

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

[Rafael J. Wysocki]

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

Thanks Erik, LGTM.

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

[Schmauss, Erik]

> -----Original Message-----
> From: Wysocki, Rafael J
> Sent: Thursday, May 16, 2019 8:36 AM
> To: Schmauss, Erik <erik.schmauss@intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Cc: 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/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@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-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@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
> 
> Thanks Erik, LGTM.
> 

Thanks for your patience with these patches!

Erik

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

[Sasha Levin]

On Thu, May 16, 2019 at 12:45:54AM +0000, Schmauss, Erik wrote:
>
>
>> -----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.

So to clarify, we're now just going to revert it? It's not needed in
-stable?

--
Thanks,
Sasha

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

[Schmauss, Erik]

> -----Original Message-----
> From: Sasha Levin [mailto:sashal@kernel.org]
> Sent: Thursday, May 16, 2019 5:52 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; Wysocki, Rafael
> J <rafael.j.wysocki@intel.com>
> Subject: Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML
> interpreter: add region addresses...")
> 
> On Thu, May 16, 2019 at 12:45:54AM +0000, Schmauss, Erik wrote:
> >
> >
> >> -----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-addre
> >> >>>> sse
> >> >>>> s-i
> >> >>>> n.patch
> >> >>>> ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-i
> >> >>>> n-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.
> 
> So to clarify, we're now just going to revert it? It's not needed in -stable?
Yes, we're going to revert a commit that got added by accident to the 4.19.y branch.
It may not apply to all stable branches because the reverted commit might not exist in all stable branches.

I hope this helps,

Erik
> 
> --
> Thanks,
> Sasha