diff mbox series

[v2,1/3] ACPI: Resolve objects on host-directed table loads

Message ID e2a4ddfd93a904b50b7ccc074e00e14dc4661963.1560327219.git.nikolaus.voss@loewensteinmedical.de
State Not Applicable
Headers show
Series PWM framework: add support referencing PWMs from ACPI | expand

Commit Message

Nikolaus Voss June 12, 2019, 8:36 a.m. UTC
If an ACPI SSDT overlay is loaded after built-in tables
have been loaded e.g. via configfs or efivar_ssdt_load()
it is necessary to rewalk the namespace to resolve
references. Without this, relative and absolute paths
like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved
correctly.

Make configfs load use the same method as efivar_ssdt_load().

Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
---
 drivers/acpi/acpi_configfs.c   |  6 +-----
 drivers/acpi/acpica/tbxfload.c | 11 +++++++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

Comments

Rafael J. Wysocki June 14, 2019, 9:12 a.m. UTC | #1
On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
<nikolaus.voss@loewensteinmedical.de> wrote:
>
> If an ACPI SSDT overlay is loaded after built-in tables
> have been loaded e.g. via configfs or efivar_ssdt_load()
> it is necessary to rewalk the namespace to resolve
> references. Without this, relative and absolute paths
> like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved
> correctly.
>
> Make configfs load use the same method as efivar_ssdt_load().
>
> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>

This is fine by me, so

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Or if you want me to take this patch (without the other two in the
series), please let me know.

As for the other two patches, someone else needs to review them for
you as I'm not particularly familiar with the PWM subsystem.

> ---
>  drivers/acpi/acpi_configfs.c   |  6 +-----
>  drivers/acpi/acpica/tbxfload.c | 11 +++++++++++
>  2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/acpi_configfs.c b/drivers/acpi/acpi_configfs.c
> index f92033661239..663f0d88f912 100644
> --- a/drivers/acpi/acpi_configfs.c
> +++ b/drivers/acpi/acpi_configfs.c
> @@ -56,11 +56,7 @@ static ssize_t acpi_table_aml_write(struct config_item *cfg,
>         if (!table->header)
>                 return -ENOMEM;
>
> -       ACPI_INFO(("Host-directed Dynamic ACPI Table Load:"));
> -       ret = acpi_tb_install_and_load_table(
> -                       ACPI_PTR_TO_PHYSADDR(table->header),
> -                       ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE,
> -                       &table->index);
> +       ret = acpi_load_table(table->header);
>         if (ret) {
>                 kfree(table->header);
>                 table->header = NULL;
> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> index 4f30f06a6f78..ef8f8a9f3c9c 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -297,6 +297,17 @@ acpi_status acpi_load_table(struct acpi_table_header *table)
>         status = acpi_tb_install_and_load_table(ACPI_PTR_TO_PHYSADDR(table),
>                                                 ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL,
>                                                 FALSE, &table_index);
> +
> +       if (ACPI_SUCCESS(status)) {
> +               /* Complete the initialization/resolution of package objects */
> +
> +               status = acpi_ns_walk_namespace(ACPI_TYPE_PACKAGE,
> +                                               ACPI_ROOT_OBJECT,
> +                                               ACPI_UINT32_MAX, 0,
> +                                               acpi_ns_init_one_package,
> +                                               NULL, NULL, NULL);
> +       }
> +
>         return_ACPI_STATUS(status);
>  }
>
> --
> 2.17.1
>
Nikolaus Voss June 14, 2019, 9:25 a.m. UTC | #2
Hi Rafael,

On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
> <nikolaus.voss@loewensteinmedical.de> wrote:
>>
>> If an ACPI SSDT overlay is loaded after built-in tables
>> have been loaded e.g. via configfs or efivar_ssdt_load()
>> it is necessary to rewalk the namespace to resolve
>> references. Without this, relative and absolute paths
>> like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved
>> correctly.
>>
>> Make configfs load use the same method as efivar_ssdt_load().
>>
>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>
> This is fine by me, so
>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Or if you want me to take this patch (without the other two in the
> series), please let me know.

thanks. I think it would be the best if you take up this patch as it is an 
independent topic. In retrospect it wasn't a good idea to put it into this 
series.

Kind regards,
Niko

[...]
Moore, Robert June 14, 2019, 3:35 p.m. UTC | #3
-----Original Message-----
From: Nikolaus Voss [mailto:nv@vosn.de] 
Sent: Friday, June 14, 2019 2:26 AM
To: Rafael J. Wysocki <rafael@kernel.org>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Moore, Robert <robert.moore@intel.com>; Schmauss, Erik <erik.schmauss@intel.com>; Jacek Anaszewski <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI Devel Maling List <linux-acpi@vger.kernel.org>; open list:ACPI COMPONENT ARCHITECTURE (ACPICA) <devel@acpica.org>; linux-leds@vger.kernel.org; Linux PWM List <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table loads

Hi Rafael,

On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss 
> <nikolaus.voss@loewensteinmedical.de> wrote:
>>
>> If an ACPI SSDT overlay is loaded after built-in tables have been 
>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to 
>> rewalk the namespace to resolve references. Without this, relative 
>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved 
>> correctly.
>>
>> Make configfs load use the same method as efivar_ssdt_load().
>>
>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>
> This is fine by me, so
>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Or if you want me to take this patch (without the other two in the 
> series), please let me know.

thanks. I think it would be the best if you take up this patch as it is an independent topic. In retrospect it wasn't a good idea to put it into this series.

Kind regards,
Niko

I would have to ask, why is additional code needed for package initialization/resolution? It already happens elsewhere in acpica.
Bob

[...]
Nikolaus Voss June 17, 2019, 6:24 a.m. UTC | #4
Bob,

On Fri, 14 Jun 2019, Moore, Robert wrote:
>
>
> -----Original Message-----
> From: Nikolaus Voss [mailto:nv@vosn.de]
> Sent: Friday, June 14, 2019 2:26 AM
> To: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Moore, Robert <robert.moore@intel.com>; Schmauss, Erik <erik.schmauss@intel.com>; Jacek Anaszewski <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI Devel Maling List <linux-acpi@vger.kernel.org>; open list:ACPI COMPONENT ARCHITECTURE (ACPICA) <devel@acpica.org>; linux-leds@vger.kernel.org; Linux PWM List <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table loads
>
> Hi Rafael,
>
> On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
>> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
>> <nikolaus.voss@loewensteinmedical.de> wrote:
>>>
>>> If an ACPI SSDT overlay is loaded after built-in tables have been
>>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to
>>> rewalk the namespace to resolve references. Without this, relative
>>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not resolved
>>> correctly.
>>>
>>> Make configfs load use the same method as efivar_ssdt_load().
>>>
>>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>>
>> This is fine by me, so
>>
>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> Or if you want me to take this patch (without the other two in the
>> series), please let me know.
>
> thanks. I think it would be the best if you take up this patch as it is 
> an independent topic. In retrospect it wasn't a good idea to put it into 
> this series.
>
> Kind regards,
> Niko
>
> I would have to ask, why is additional code needed for package 
> initialization/resolution? It already happens elsewhere in acpica. Bob

for built-in tables loaded via acpi_ex_load_table_op() everything is fine, 
because after acpi_tb_load_table() acpi_ns_walk_namespace() is called to 
resolve references.

My fix only affects tables loaded dynamically via either 
acpi_configfs driver (for debugging purposes) or efivar_ssdt_load() (to 
specify a table on the kernel's command line). They use acpi_load_table() 
to load the table from a caller-owned buffer. To resolve the references, 
it is again necessary to rewalk the namespace, which was simply missing in 
acpi_load_table().

Niko
Moore, Robert June 17, 2019, 9:18 p.m. UTC | #5
> -----Original Message-----
> From: Nikolaus Voss [mailto:nv@vosn.de]
> Sent: Sunday, June 16, 2019 11:24 PM
> To: Moore, Robert <robert.moore@intel.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; Rafael J. Wysocki
> <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Schmauss, Erik
> <erik.schmauss@intel.com>; Jacek Anaszewski
> <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy
> <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI Devel
> Maling List <linux-acpi@vger.kernel.org>; open list:ACPI COMPONENT
> ARCHITECTURE (ACPICA) <devel@acpica.org>; linux-leds@vger.kernel.org;
> Linux PWM List <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; nikolaus.voss@loewensteinmedical.de
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
> 
> Bob,
> 
> On Fri, 14 Jun 2019, Moore, Robert wrote:
> >
> >
> > -----Original Message-----
> > From: Nikolaus Voss [mailto:nv@vosn.de]
> > Sent: Friday, June 14, 2019 2:26 AM
> > To: Rafael J. Wysocki <rafael@kernel.org>
> > Cc: Rafael J. Wysocki <rjw@rjwysocki.net>; Len Brown
> > <lenb@kernel.org>; Moore, Robert <robert.moore@intel.com>; Schmauss,
> > Erik <erik.schmauss@intel.com>; Jacek Anaszewski
> > <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy
> > <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI
> > Devel Maling List <linux-acpi@vger.kernel.org>; open list:ACPI
> > COMPONENT ARCHITECTURE (ACPICA) <devel@acpica.org>;
> > linux-leds@vger.kernel.org; Linux PWM List
> > <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
> > <linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> > table loads
> >
> > Hi Rafael,
> >
> > On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
> >> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
> >> <nikolaus.voss@loewensteinmedical.de> wrote:
> >>>
> >>> If an ACPI SSDT overlay is loaded after built-in tables have been
> >>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to
> >>> rewalk the namespace to resolve references. Without this, relative
> >>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not
> >>> resolved correctly.
> >>>
> >>> Make configfs load use the same method as efivar_ssdt_load().
> >>>
> >>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> >>
> >> This is fine by me, so
> >>
> >> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>
> >> Or if you want me to take this patch (without the other two in the
> >> series), please let me know.
> >
> > thanks. I think it would be the best if you take up this patch as it
> > is an independent topic. In retrospect it wasn't a good idea to put it
> > into this series.
> >
> > Kind regards,
> > Niko
> >
> > I would have to ask, why is additional code needed for package
> > initialization/resolution? It already happens elsewhere in acpica. Bob
> 
> for built-in tables loaded via acpi_ex_load_table_op() everything is
> fine, because after acpi_tb_load_table() acpi_ns_walk_namespace() is
> called to resolve references.
> 
> My fix only affects tables loaded dynamically via either acpi_configfs
> driver (for debugging purposes) or efivar_ssdt_load() (to specify a
> table on the kernel's command line). They use acpi_load_table() to load
> the table from a caller-owned buffer. To resolve the references, it is
> again necessary to rewalk the namespace, which was simply missing in
> acpi_load_table().
> 
[Moore, Robert] 

Perhaps you should call AcpiInitializeObjects after the call to AcpiLoadTable, but I will check.

> Niko
Nikolaus Voss June 18, 2019, 9:21 a.m. UTC | #6
On Mon, 17 Jun 2019, Moore, Robert wrote:
>> -----Original Message-----
>> From: Nikolaus Voss [mailto:nv@vosn.de]
>> Sent: Sunday, June 16, 2019 11:24 PM
>> To: Moore, Robert <robert.moore@intel.com>
>> Cc: Rafael J. Wysocki <rafael@kernel.org>; Rafael J. Wysocki
>> <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Schmauss, Erik
>> <erik.schmauss@intel.com>; Jacek Anaszewski
>> <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy
>> <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI Devel
>> Maling List <linux-acpi@vger.kernel.org>; open list:ACPI COMPONENT
>> ARCHITECTURE (ACPICA) <devel@acpica.org>; linux-leds@vger.kernel.org;
>> Linux PWM List <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
>> <linux-kernel@vger.kernel.org>; nikolaus.voss@loewensteinmedical.de
>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
>> loads
>>
>> Bob,
>>
>> On Fri, 14 Jun 2019, Moore, Robert wrote:
>>>
>>>
>>> -----Original Message-----
>>> From: Nikolaus Voss [mailto:nv@vosn.de]
>>> Sent: Friday, June 14, 2019 2:26 AM
>>> To: Rafael J. Wysocki <rafael@kernel.org>
>>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>; Len Brown
>>> <lenb@kernel.org>; Moore, Robert <robert.moore@intel.com>; Schmauss,
>>> Erik <erik.schmauss@intel.com>; Jacek Anaszewski
>>> <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy
>>> <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI
>>> Devel Maling List <linux-acpi@vger.kernel.org>; open list:ACPI
>>> COMPONENT ARCHITECTURE (ACPICA) <devel@acpica.org>;
>>> linux-leds@vger.kernel.org; Linux PWM List
>>> <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
>>> <linux-kernel@vger.kernel.org>
>>> Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
>>> table loads
>>>
>>> Hi Rafael,
>>>
>>> On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
>>>> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
>>>> <nikolaus.voss@loewensteinmedical.de> wrote:
>>>>>
>>>>> If an ACPI SSDT overlay is loaded after built-in tables have been
>>>>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to
>>>>> rewalk the namespace to resolve references. Without this, relative
>>>>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not
>>>>> resolved correctly.
>>>>>
>>>>> Make configfs load use the same method as efivar_ssdt_load().
>>>>>
>>>>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
>>>>
>>>> This is fine by me, so
>>>>
>>>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>
>>>> Or if you want me to take this patch (without the other two in the
>>>> series), please let me know.
>>>
>>> thanks. I think it would be the best if you take up this patch as it
>>> is an independent topic. In retrospect it wasn't a good idea to put it
>>> into this series.
>>>
>>> Kind regards,
>>> Niko
>>>
>>> I would have to ask, why is additional code needed for package
>>> initialization/resolution? It already happens elsewhere in acpica. Bob
>>
>> for built-in tables loaded via acpi_ex_load_table_op() everything is
>> fine, because after acpi_tb_load_table() acpi_ns_walk_namespace() is
>> called to resolve references.
>>
>> My fix only affects tables loaded dynamically via either acpi_configfs
>> driver (for debugging purposes) or efivar_ssdt_load() (to specify a
>> table on the kernel's command line). They use acpi_load_table() to load
>> the table from a caller-owned buffer. To resolve the references, it is
>> again necessary to rewalk the namespace, which was simply missing in
>> acpi_load_table().
>>
> [Moore, Robert]
>
> Perhaps you should call AcpiInitializeObjects after the call to 
> AcpiLoadTable, but I will check.

My usage of acpi_load_table() is to load a SSDT which is the intended use 
of this method according to its description. And my expectation is that 
the package objects of the loaded table are initialized when this function 
successfully returns so it aligns with the behavior of 
acpi_ex_load_table_op() for built-in SSDTs. Otherwise there would be no 
point in having this function at all, because 
acpi_tb_install_and_load_table() could be called directly without any 
difference.

Niko
Moore, Robert June 18, 2019, 7:35 p.m. UTC | #7
I'm looking at this. We've changed the initialization of objects in the namespace recently, so I'm checking this out.


> -----Original Message-----
> From: Nikolaus Voss [mailto:nv@vosn.de]
> Sent: Tuesday, June 18, 2019 2:22 AM
> To: Moore, Robert <robert.moore@intel.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; Rafael J. Wysocki
> <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Schmauss, Erik
> <erik.schmauss@intel.com>; Jacek Anaszewski
> <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy
> <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI Devel
> Maling List <linux-acpi@vger.kernel.org>; open list:ACPI COMPONENT
> ARCHITECTURE (ACPICA) <devel@acpica.org>; linux-leds@vger.kernel.org;
> Linux PWM List <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
> 
> On Mon, 17 Jun 2019, Moore, Robert wrote:
> >> -----Original Message-----
> >> From: Nikolaus Voss [mailto:nv@vosn.de]
> >> Sent: Sunday, June 16, 2019 11:24 PM
> >> To: Moore, Robert <robert.moore@intel.com>
> >> Cc: Rafael J. Wysocki <rafael@kernel.org>; Rafael J. Wysocki
> >> <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Schmauss, Erik
> >> <erik.schmauss@intel.com>; Jacek Anaszewski
> >> <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy
> >> <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI
> >> Devel Maling List <linux-acpi@vger.kernel.org>; open list:ACPI
> >> COMPONENT ARCHITECTURE (ACPICA) <devel@acpica.org>;
> >> linux-leds@vger.kernel.org; Linux PWM List
> >> <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
> >> <linux-kernel@vger.kernel.org>; nikolaus.voss@loewensteinmedical.de
> >> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> >> table loads
> >>
> >> Bob,
> >>
> >> On Fri, 14 Jun 2019, Moore, Robert wrote:
> >>>
> >>>
> >>> -----Original Message-----
> >>> From: Nikolaus Voss [mailto:nv@vosn.de]
> >>> Sent: Friday, June 14, 2019 2:26 AM
> >>> To: Rafael J. Wysocki <rafael@kernel.org>
> >>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>; Len Brown
> >>> <lenb@kernel.org>; Moore, Robert <robert.moore@intel.com>; Schmauss,
> >>> Erik <erik.schmauss@intel.com>; Jacek Anaszewski
> >>> <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan
> >>> Murphy <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>;
> >>> ACPI Devel Maling List <linux-acpi@vger.kernel.org>; open list:ACPI
> >>> COMPONENT ARCHITECTURE (ACPICA) <devel@acpica.org>;
> >>> linux-leds@vger.kernel.org; Linux PWM List
> >>> <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
> >>> <linux-kernel@vger.kernel.org>
> >>> Subject: Re: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> >>> table loads
> >>>
> >>> Hi Rafael,
> >>>
> >>> On Fri, 14 Jun 2019, Rafael J. Wysocki wrote:
> >>>> On Wed, Jun 12, 2019 at 10:36 AM Nikolaus Voss
> >>>> <nikolaus.voss@loewensteinmedical.de> wrote:
> >>>>>
> >>>>> If an ACPI SSDT overlay is loaded after built-in tables have been
> >>>>> loaded e.g. via configfs or efivar_ssdt_load() it is necessary to
> >>>>> rewalk the namespace to resolve references. Without this, relative
> >>>>> and absolute paths like ^PCI0.SBUS or \_SB.PCI0.SBUS are not
> >>>>> resolved correctly.
> >>>>>
> >>>>> Make configfs load use the same method as efivar_ssdt_load().
> >>>>>
> >>>>> Signed-off-by: Nikolaus Voss <nikolaus.voss@loewensteinmedical.de>
> >>>>
> >>>> This is fine by me, so
> >>>>
> >>>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>>>
> >>>> Or if you want me to take this patch (without the other two in the
> >>>> series), please let me know.
> >>>
> >>> thanks. I think it would be the best if you take up this patch as it
> >>> is an independent topic. In retrospect it wasn't a good idea to put
> >>> it into this series.
> >>>
> >>> Kind regards,
> >>> Niko
> >>>
> >>> I would have to ask, why is additional code needed for package
> >>> initialization/resolution? It already happens elsewhere in acpica.
> >>> Bob
> >>
> >> for built-in tables loaded via acpi_ex_load_table_op() everything is
> >> fine, because after acpi_tb_load_table() acpi_ns_walk_namespace() is
> >> called to resolve references.
> >>
> >> My fix only affects tables loaded dynamically via either
> >> acpi_configfs driver (for debugging purposes) or efivar_ssdt_load()
> >> (to specify a table on the kernel's command line). They use
> >> acpi_load_table() to load the table from a caller-owned buffer. To
> >> resolve the references, it is again necessary to rewalk the
> >> namespace, which was simply missing in acpi_load_table().
> >>
> > [Moore, Robert]
> >
> > Perhaps you should call AcpiInitializeObjects after the call to
> > AcpiLoadTable, but I will check.
> 
> My usage of acpi_load_table() is to load a SSDT which is the intended
> use of this method according to its description. And my expectation is
> that the package objects of the loaded table are initialized when this
> function successfully returns so it aligns with the behavior of
> acpi_ex_load_table_op() for built-in SSDTs. Otherwise there would be no
> point in having this function at all, because
> acpi_tb_install_and_load_table() could be called directly without any
> difference.
> 
> Niko
Moore, Robert June 18, 2019, 8:22 p.m. UTC | #8
It looks to me that the package objects are being initialized properly already, unless I'm missing something. Please check the examples below and in the attached files.

Attached is a small test case that dynamically loads an SSDT which contains a package object which in turn contains references to other objects.


Main DSDT:
    Method (LD1)
    {
        Load (BUF1, HNDL)      // SSDT is in BUF1
        Store (HNDL, Debug)
        Return
    }   

Loaded table:
    External (DEV1, DeviceObj)
    Name (PKG1, Package() {
        1,2, DEV2, DEV1, 4})
    Device (DEV2) {}


AcpiExec Output:
- ev ld1
Evaluating \LD1
ACPI: Dynamic OEM Table Load:
ACPI: SSDT 0x00000000006DEEB8 000051 (v02 Intel  Load     00000001 INTL 20190509)
ACPI Exec: Table Event INSTALL, [SSDT] 006DEEB8
Table [SSDT: Load    ] (id 06) -    5 Objects with   1 Devices,   0 Regions,    1 Methods
ACPI Exec: Table Event LOAD, [SSDT] 006DEEB8
ACPI Debug:  Reference  [DdbHandle] Table Index 0x3
0x7 Outstanding allocations after evaluation of \LD1
Evaluation of \LD1 returned object 006D2FE8, external buffer length 18
  [Integer] = 0000000000000000

- ev pkg1
Evaluating \PKG1
Evaluation of \PKG1 returned object 006D2FE8, external buffer length 90
  [Package] Contains 5 Elements:
    [Integer] = 0000000000000001
    [Integer] = 0000000000000002
    [Object Reference] = 006DDF88 <Node>            Name DEV2 Device
    [Object Reference] = 006DD608 <Node>            Name DEV1 Device
    [Integer] = 0000000000000004
Moore, Robert June 18, 2019, 8:24 p.m. UTC | #9
If it is in fact the AcpiLoadTable interface that is incorrect, that of course is different. I'll check that out next.


> -----Original Message-----
> From: Moore, Robert
> Sent: Tuesday, June 18, 2019 1:23 PM
> To: 'Nikolaus Voss' <nv@vosn.de>
> Cc: 'Rafael J. Wysocki' <rafael@kernel.org>; 'Rafael J. Wysocki'
> <rjw@rjwysocki.net>; 'Len Brown' <lenb@kernel.org>; Schmauss, Erik
> <erik.schmauss@intel.com>; 'Jacek Anaszewski'
> <jacek.anaszewski@gmail.com>; 'Pavel Machek' <pavel@ucw.cz>; 'Dan
> Murphy' <dmurphy@ti.com>; 'Thierry Reding' <thierry.reding@gmail.com>;
> 'ACPI Devel Maling List' <linux-acpi@vger.kernel.org>; 'open list:ACPI
> COMPONENT ARCHITECTURE (ACPICA)' <devel@acpica.org>; 'linux-
> leds@vger.kernel.org' <linux-leds@vger.kernel.org>; 'Linux PWM List'
> <linux-pwm@vger.kernel.org>; 'Linux Kernel Mailing List' <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
> 
> It looks to me that the package objects are being initialized properly
> already, unless I'm missing something. Please check the examples below
> and in the attached files.
> 
> Attached is a small test case that dynamically loads an SSDT which
> contains a package object which in turn contains references to other
> objects.
> 
> 
> Main DSDT:
>     Method (LD1)
>     {
>         Load (BUF1, HNDL)      // SSDT is in BUF1
>         Store (HNDL, Debug)
>         Return
>     }
> 
> Loaded table:
>     External (DEV1, DeviceObj)
>     Name (PKG1, Package() {
>         1,2, DEV2, DEV1, 4})
>     Device (DEV2) {}
> 
> 
> AcpiExec Output:
> - ev ld1
> Evaluating \LD1
> ACPI: Dynamic OEM Table Load:
> ACPI: SSDT 0x00000000006DEEB8 000051 (v02 Intel  Load     00000001 INTL
> 20190509)
> ACPI Exec: Table Event INSTALL, [SSDT] 006DEEB8
> Table [SSDT: Load    ] (id 06) -    5 Objects with   1 Devices,   0
> Regions,    1 Methods
> ACPI Exec: Table Event LOAD, [SSDT] 006DEEB8 ACPI Debug:  Reference
> [DdbHandle] Table Index 0x3
> 0x7 Outstanding allocations after evaluation of \LD1 Evaluation of \LD1
> returned object 006D2FE8, external buffer length 18
>   [Integer] = 0000000000000000
> 
> - ev pkg1
> Evaluating \PKG1
> Evaluation of \PKG1 returned object 006D2FE8, external buffer length 90
>   [Package] Contains 5 Elements:
>     [Integer] = 0000000000000001
>     [Integer] = 0000000000000002
>     [Object Reference] = 006DDF88 <Node>            Name DEV2 Device
>     [Object Reference] = 006DD608 <Node>            Name DEV1 Device
>     [Integer] = 0000000000000004
Moore, Robert June 18, 2019, 8:31 p.m. UTC | #10
> -----Original Message-----
> From: Moore, Robert
> Sent: Tuesday, June 18, 2019 1:25 PM
> To: 'Nikolaus Voss' <nv@vosn.de>
> Cc: 'Rafael J. Wysocki' <rafael@kernel.org>; 'Rafael J. Wysocki'
> <rjw@rjwysocki.net>; 'Len Brown' <lenb@kernel.org>; Schmauss, Erik
> <erik.schmauss@intel.com>; 'Jacek Anaszewski'
> <jacek.anaszewski@gmail.com>; 'Pavel Machek' <pavel@ucw.cz>; 'Dan
> Murphy' <dmurphy@ti.com>; 'Thierry Reding' <thierry.reding@gmail.com>;
> 'ACPI Devel Maling List' <linux-acpi@vger.kernel.org>; 'open list:ACPI
> COMPONENT ARCHITECTURE (ACPICA)' <devel@acpica.org>; 'linux-
> leds@vger.kernel.org' <linux-leds@vger.kernel.org>; 'Linux PWM List'
> <linux-pwm@vger.kernel.org>; 'Linux Kernel Mailing List' <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
> 
> If it is in fact the AcpiLoadTable interface that is incorrect, that of
> course is different. I'll check that out next.
> 
[Moore, Robert] 

Yes, this is the issue, not specifically the Load() operator, but the AcpiLoadTable interface only.

> 
> > -----Original Message-----
> > From: Moore, Robert
> > Sent: Tuesday, June 18, 2019 1:23 PM
> > To: 'Nikolaus Voss' <nv@vosn.de>
> > Cc: 'Rafael J. Wysocki' <rafael@kernel.org>; 'Rafael J. Wysocki'
> > <rjw@rjwysocki.net>; 'Len Brown' <lenb@kernel.org>; Schmauss, Erik
> > <erik.schmauss@intel.com>; 'Jacek Anaszewski'
> > <jacek.anaszewski@gmail.com>; 'Pavel Machek' <pavel@ucw.cz>; 'Dan
> > Murphy' <dmurphy@ti.com>; 'Thierry Reding' <thierry.reding@gmail.com>;
> > 'ACPI Devel Maling List' <linux-acpi@vger.kernel.org>; 'open list:ACPI
> > COMPONENT ARCHITECTURE (ACPICA)' <devel@acpica.org>; 'linux-
> > leds@vger.kernel.org' <linux-leds@vger.kernel.org>; 'Linux PWM List'
> > <linux-pwm@vger.kernel.org>; 'Linux Kernel Mailing List' <linux-
> > kernel@vger.kernel.org>
> > Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> > table loads
> >
> > It looks to me that the package objects are being initialized properly
> > already, unless I'm missing something. Please check the examples below
> > and in the attached files.
> >
> > Attached is a small test case that dynamically loads an SSDT which
> > contains a package object which in turn contains references to other
> > objects.
> >
> >
> > Main DSDT:
> >     Method (LD1)
> >     {
> >         Load (BUF1, HNDL)      // SSDT is in BUF1
> >         Store (HNDL, Debug)
> >         Return
> >     }
> >
> > Loaded table:
> >     External (DEV1, DeviceObj)
> >     Name (PKG1, Package() {
> >         1,2, DEV2, DEV1, 4})
> >     Device (DEV2) {}
> >
> >
> > AcpiExec Output:
> > - ev ld1
> > Evaluating \LD1
> > ACPI: Dynamic OEM Table Load:
> > ACPI: SSDT 0x00000000006DEEB8 000051 (v02 Intel  Load     00000001
> INTL
> > 20190509)
> > ACPI Exec: Table Event INSTALL, [SSDT] 006DEEB8
> > Table [SSDT: Load    ] (id 06) -    5 Objects with   1 Devices,   0
> > Regions,    1 Methods
> > ACPI Exec: Table Event LOAD, [SSDT] 006DEEB8 ACPI Debug:  Reference
> > [DdbHandle] Table Index 0x3
> > 0x7 Outstanding allocations after evaluation of \LD1 Evaluation of
> > \LD1 returned object 006D2FE8, external buffer length 18
> >   [Integer] = 0000000000000000
> >
> > - ev pkg1
> > Evaluating \PKG1
> > Evaluation of \PKG1 returned object 006D2FE8, external buffer length
> 90
> >   [Package] Contains 5 Elements:
> >     [Integer] = 0000000000000001
> >     [Integer] = 0000000000000002
> >     [Object Reference] = 006DDF88 <Node>            Name DEV2 Device
> >     [Object Reference] = 006DD608 <Node>            Name DEV1 Device
> >     [Integer] = 0000000000000004
Nikolaus Voss June 19, 2019, 9:31 a.m. UTC | #11
Hi Bob,

On Tue, 18 Jun 2019, Moore, Robert wrote:
>
>
>> -----Original Message-----
>> From: Moore, Robert
>> Sent: Tuesday, June 18, 2019 1:25 PM
>> To: 'Nikolaus Voss' <nv@vosn.de>
>> Cc: 'Rafael J. Wysocki' <rafael@kernel.org>; 'Rafael J. Wysocki'
>> <rjw@rjwysocki.net>; 'Len Brown' <lenb@kernel.org>; Schmauss, Erik
>> <erik.schmauss@intel.com>; 'Jacek Anaszewski'
>> <jacek.anaszewski@gmail.com>; 'Pavel Machek' <pavel@ucw.cz>; 'Dan
>> Murphy' <dmurphy@ti.com>; 'Thierry Reding' <thierry.reding@gmail.com>;
>> 'ACPI Devel Maling List' <linux-acpi@vger.kernel.org>; 'open list:ACPI
>> COMPONENT ARCHITECTURE (ACPICA)' <devel@acpica.org>; 'linux-
>> leds@vger.kernel.org' <linux-leds@vger.kernel.org>; 'Linux PWM List'
>> <linux-pwm@vger.kernel.org>; 'Linux Kernel Mailing List' <linux-
>> kernel@vger.kernel.org>
>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
>> loads
>>
>> If it is in fact the AcpiLoadTable interface that is incorrect, that of
>> course is different. I'll check that out next.
>>
> [Moore, Robert]
>
> Yes, this is the issue, not specifically the Load() operator, but the 
> AcpiLoadTable interface only.

thanks for checking this out. So what is the conclusion? Is my fix 
of AcpiLoadTable() sufficient or do we need a different solution?

Niko

[...]
Moore, Robert June 19, 2019, 3:59 p.m. UTC | #12
> -----Original Message-----
> From: Nikolaus Voss [mailto:nv@vosn.de]
> Sent: Wednesday, June 19, 2019 2:31 AM
> To: Moore, Robert <robert.moore@intel.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; Rafael J. Wysocki
> <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Schmauss, Erik
> <erik.schmauss@intel.com>; Jacek Anaszewski
> <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy
> <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI Devel
> Maling List <linux-acpi@vger.kernel.org>; open list:ACPI COMPONENT
> ARCHITECTURE (ACPICA) <devel@acpica.org>; linux-leds@vger.kernel.org;
> Linux PWM List <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; nikolaus.voss@loewensteinmedical.de
> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> loads
> 
> Hi Bob,
> 
> On Tue, 18 Jun 2019, Moore, Robert wrote:
> >
> >
> >> -----Original Message-----
> >> From: Moore, Robert
> >> Sent: Tuesday, June 18, 2019 1:25 PM
> >> To: 'Nikolaus Voss' <nv@vosn.de>
> >> Cc: 'Rafael J. Wysocki' <rafael@kernel.org>; 'Rafael J. Wysocki'
> >> <rjw@rjwysocki.net>; 'Len Brown' <lenb@kernel.org>; Schmauss, Erik
> >> <erik.schmauss@intel.com>; 'Jacek Anaszewski'
> >> <jacek.anaszewski@gmail.com>; 'Pavel Machek' <pavel@ucw.cz>; 'Dan
> >> Murphy' <dmurphy@ti.com>; 'Thierry Reding'
> >> <thierry.reding@gmail.com>; 'ACPI Devel Maling List'
> >> <linux-acpi@vger.kernel.org>; 'open list:ACPI COMPONENT ARCHITECTURE
> >> (ACPICA)' <devel@acpica.org>; 'linux- leds@vger.kernel.org' <linux-
> leds@vger.kernel.org>; 'Linux PWM List'
> >> <linux-pwm@vger.kernel.org>; 'Linux Kernel Mailing List' <linux-
> >> kernel@vger.kernel.org>
> >> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> >> table loads
> >>
> >> If it is in fact the AcpiLoadTable interface that is incorrect, that
> >> of course is different. I'll check that out next.
> >>
> > [Moore, Robert]
> >
> > Yes, this is the issue, not specifically the Load() operator, but the
> > AcpiLoadTable interface only.
> 
> thanks for checking this out. So what is the conclusion? Is my fix of
> AcpiLoadTable() sufficient or do we need a different solution?
> 
> Niko
> 


Your change is in the correct area. We want to do something like this, however:

diff --git a/source/components/executer/exconfig.c b/source/components/executer/exconfig.c
index 84a058ada..eba1a6d28 100644
--- a/source/components/executer/exconfig.c
+++ b/source/components/executer/exconfig.c
@@ -342,10 +342,9 @@ AcpiExLoadTableOp (
         return_ACPI_STATUS (Status);
     }
 
-    /* Complete the initialization/resolution of package objects */
+    /* Complete the initialization/resolution of new objects */
 
-    Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
-        ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
+    AcpiNsInitializeObjects ();
 
     /* Parameter Data (optional) */
 
@@ -620,10 +619,11 @@ AcpiExLoadOp (
         return_ACPI_STATUS (Status);
     }
 
-    /* Complete the initialization/resolution of package objects */
+    /* Complete the initialization/resolution of new objects */
 
-    Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
-        ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
+    AcpiExExitInterpreter ();
+    AcpiNsInitializeObjects ();
+    AcpiExEnterInterpreter ();
 
     /* Store the DdbHandle into the Target operand */
 
diff --git a/source/components/tables/tbxfload.c b/source/components/tables/tbxfload.c
index 217d54bf0..1e17db6c8 100644
--- a/source/components/tables/tbxfload.c
+++ b/source/components/tables/tbxfload.c
@@ -479,6 +479,13 @@ AcpiLoadTable (
     ACPI_INFO (("Host-directed Dynamic ACPI Table Load:"));
     Status = AcpiTbInstallAndLoadTable (ACPI_PTR_TO_PHYSADDR (Table),
         ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE, &TableIndex);
+    if (ACPI_SUCCESS (Status))
+    {
+        /* Complete the initialization/resolution of new objects */
+
+        AcpiNsInitializeObjects ();
+    }
+
     return_ACPI_STATUS (Status);
 }



> [...]
Nikolaus Voss June 20, 2019, 6:49 a.m. UTC | #13
On Wed, 19 Jun 2019, Moore, Robert wrote:
>
>
>> -----Original Message-----
>> From: Nikolaus Voss [mailto:nv@vosn.de]
>> Sent: Wednesday, June 19, 2019 2:31 AM
>> To: Moore, Robert <robert.moore@intel.com>
>> Cc: Rafael J. Wysocki <rafael@kernel.org>; Rafael J. Wysocki
>> <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Schmauss, Erik
>> <erik.schmauss@intel.com>; Jacek Anaszewski
>> <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy
>> <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI Devel
>> Maling List <linux-acpi@vger.kernel.org>; open list:ACPI COMPONENT
>> ARCHITECTURE (ACPICA) <devel@acpica.org>; linux-leds@vger.kernel.org;
>> Linux PWM List <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
>> <linux-kernel@vger.kernel.org>; nikolaus.voss@loewensteinmedical.de
>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
>> loads
>>
>> Hi Bob,
>>
>> On Tue, 18 Jun 2019, Moore, Robert wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Moore, Robert
>>>> Sent: Tuesday, June 18, 2019 1:25 PM
>>>> To: 'Nikolaus Voss' <nv@vosn.de>
>>>> Cc: 'Rafael J. Wysocki' <rafael@kernel.org>; 'Rafael J. Wysocki'
>>>> <rjw@rjwysocki.net>; 'Len Brown' <lenb@kernel.org>; Schmauss, Erik
>>>> <erik.schmauss@intel.com>; 'Jacek Anaszewski'
>>>> <jacek.anaszewski@gmail.com>; 'Pavel Machek' <pavel@ucw.cz>; 'Dan
>>>> Murphy' <dmurphy@ti.com>; 'Thierry Reding'
>>>> <thierry.reding@gmail.com>; 'ACPI Devel Maling List'
>>>> <linux-acpi@vger.kernel.org>; 'open list:ACPI COMPONENT ARCHITECTURE
>>>> (ACPICA)' <devel@acpica.org>; 'linux- leds@vger.kernel.org' <linux-
>> leds@vger.kernel.org>; 'Linux PWM List'
>>>> <linux-pwm@vger.kernel.org>; 'Linux Kernel Mailing List' <linux-
>>>> kernel@vger.kernel.org>
>>>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
>>>> table loads
>>>>
>>>> If it is in fact the AcpiLoadTable interface that is incorrect, that
>>>> of course is different. I'll check that out next.
>>>>
>>> [Moore, Robert]
>>>
>>> Yes, this is the issue, not specifically the Load() operator, but the
>>> AcpiLoadTable interface only.
>>
>> thanks for checking this out. So what is the conclusion? Is my fix of
>> AcpiLoadTable() sufficient or do we need a different solution?
>>
>> Niko
>>
>
>
> Your change is in the correct area. We want to do something like this, however:
>
> diff --git a/source/components/executer/exconfig.c b/source/components/executer/exconfig.c
> index 84a058ada..eba1a6d28 100644
> --- a/source/components/executer/exconfig.c
> +++ b/source/components/executer/exconfig.c
> @@ -342,10 +342,9 @@ AcpiExLoadTableOp (
>         return_ACPI_STATUS (Status);
>     }
>
> -    /* Complete the initialization/resolution of package objects */
> +    /* Complete the initialization/resolution of new objects */
>
> -    Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
> -        ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
> +    AcpiNsInitializeObjects ();
>
>     /* Parameter Data (optional) */
>
> @@ -620,10 +619,11 @@ AcpiExLoadOp (
>         return_ACPI_STATUS (Status);
>     }
>
> -    /* Complete the initialization/resolution of package objects */
> +    /* Complete the initialization/resolution of new objects */
>
> -    Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
> -        ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
> +    AcpiExExitInterpreter ();
> +    AcpiNsInitializeObjects ();
> +    AcpiExEnterInterpreter ();
>
>     /* Store the DdbHandle into the Target operand */
>
> diff --git a/source/components/tables/tbxfload.c b/source/components/tables/tbxfload.c
> index 217d54bf0..1e17db6c8 100644
> --- a/source/components/tables/tbxfload.c
> +++ b/source/components/tables/tbxfload.c
> @@ -479,6 +479,13 @@ AcpiLoadTable (
>     ACPI_INFO (("Host-directed Dynamic ACPI Table Load:"));
>     Status = AcpiTbInstallAndLoadTable (ACPI_PTR_TO_PHYSADDR (Table),
>         ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE, &TableIndex);
> +    if (ACPI_SUCCESS (Status))
> +    {
> +        /* Complete the initialization/resolution of new objects */
> +
> +        AcpiNsInitializeObjects ();
> +    }
> +
>     return_ACPI_STATUS (Status);
> }

Ok, I see your are taking this up (I was a bit unsure after your previous 
post). Thanks,
Niko
Rafael J. Wysocki June 22, 2019, 9:04 a.m. UTC | #14
On Thu, Jun 20, 2019 at 8:49 AM Nikolaus Voss <nv@vosn.de> wrote:
>
> On Wed, 19 Jun 2019, Moore, Robert wrote:
> >
> >
> >> -----Original Message-----
> >> From: Nikolaus Voss [mailto:nv@vosn.de]
> >> Sent: Wednesday, June 19, 2019 2:31 AM
> >> To: Moore, Robert <robert.moore@intel.com>
> >> Cc: Rafael J. Wysocki <rafael@kernel.org>; Rafael J. Wysocki
> >> <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Schmauss, Erik
> >> <erik.schmauss@intel.com>; Jacek Anaszewski
> >> <jacek.anaszewski@gmail.com>; Pavel Machek <pavel@ucw.cz>; Dan Murphy
> >> <dmurphy@ti.com>; Thierry Reding <thierry.reding@gmail.com>; ACPI Devel
> >> Maling List <linux-acpi@vger.kernel.org>; open list:ACPI COMPONENT
> >> ARCHITECTURE (ACPICA) <devel@acpica.org>; linux-leds@vger.kernel.org;
> >> Linux PWM List <linux-pwm@vger.kernel.org>; Linux Kernel Mailing List
> >> <linux-kernel@vger.kernel.org>; nikolaus.voss@loewensteinmedical.de
> >> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed table
> >> loads
> >>
> >> Hi Bob,
> >>
> >> On Tue, 18 Jun 2019, Moore, Robert wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Moore, Robert
> >>>> Sent: Tuesday, June 18, 2019 1:25 PM
> >>>> To: 'Nikolaus Voss' <nv@vosn.de>
> >>>> Cc: 'Rafael J. Wysocki' <rafael@kernel.org>; 'Rafael J. Wysocki'
> >>>> <rjw@rjwysocki.net>; 'Len Brown' <lenb@kernel.org>; Schmauss, Erik
> >>>> <erik.schmauss@intel.com>; 'Jacek Anaszewski'
> >>>> <jacek.anaszewski@gmail.com>; 'Pavel Machek' <pavel@ucw.cz>; 'Dan
> >>>> Murphy' <dmurphy@ti.com>; 'Thierry Reding'
> >>>> <thierry.reding@gmail.com>; 'ACPI Devel Maling List'
> >>>> <linux-acpi@vger.kernel.org>; 'open list:ACPI COMPONENT ARCHITECTURE
> >>>> (ACPICA)' <devel@acpica.org>; 'linux- leds@vger.kernel.org' <linux-
> >> leds@vger.kernel.org>; 'Linux PWM List'
> >>>> <linux-pwm@vger.kernel.org>; 'Linux Kernel Mailing List' <linux-
> >>>> kernel@vger.kernel.org>
> >>>> Subject: RE: [PATCH v2 1/3] ACPI: Resolve objects on host-directed
> >>>> table loads
> >>>>
> >>>> If it is in fact the AcpiLoadTable interface that is incorrect, that
> >>>> of course is different. I'll check that out next.
> >>>>
> >>> [Moore, Robert]
> >>>
> >>> Yes, this is the issue, not specifically the Load() operator, but the
> >>> AcpiLoadTable interface only.
> >>
> >> thanks for checking this out. So what is the conclusion? Is my fix of
> >> AcpiLoadTable() sufficient or do we need a different solution?
> >>
> >> Niko
> >>
> >
> >
> > Your change is in the correct area. We want to do something like this, however:
> >
> > diff --git a/source/components/executer/exconfig.c b/source/components/executer/exconfig.c
> > index 84a058ada..eba1a6d28 100644
> > --- a/source/components/executer/exconfig.c
> > +++ b/source/components/executer/exconfig.c
> > @@ -342,10 +342,9 @@ AcpiExLoadTableOp (
> >         return_ACPI_STATUS (Status);
> >     }
> >
> > -    /* Complete the initialization/resolution of package objects */
> > +    /* Complete the initialization/resolution of new objects */
> >
> > -    Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
> > -        ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
> > +    AcpiNsInitializeObjects ();
> >
> >     /* Parameter Data (optional) */
> >
> > @@ -620,10 +619,11 @@ AcpiExLoadOp (
> >         return_ACPI_STATUS (Status);
> >     }
> >
> > -    /* Complete the initialization/resolution of package objects */
> > +    /* Complete the initialization/resolution of new objects */
> >
> > -    Status = AcpiNsWalkNamespace (ACPI_TYPE_PACKAGE, ACPI_ROOT_OBJECT,
> > -        ACPI_UINT32_MAX, 0, AcpiNsInitOnePackage, NULL, NULL, NULL);
> > +    AcpiExExitInterpreter ();
> > +    AcpiNsInitializeObjects ();
> > +    AcpiExEnterInterpreter ();
> >
> >     /* Store the DdbHandle into the Target operand */
> >
> > diff --git a/source/components/tables/tbxfload.c b/source/components/tables/tbxfload.c
> > index 217d54bf0..1e17db6c8 100644
> > --- a/source/components/tables/tbxfload.c
> > +++ b/source/components/tables/tbxfload.c
> > @@ -479,6 +479,13 @@ AcpiLoadTable (
> >     ACPI_INFO (("Host-directed Dynamic ACPI Table Load:"));
> >     Status = AcpiTbInstallAndLoadTable (ACPI_PTR_TO_PHYSADDR (Table),
> >         ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE, &TableIndex);
> > +    if (ACPI_SUCCESS (Status))
> > +    {
> > +        /* Complete the initialization/resolution of new objects */
> > +
> > +        AcpiNsInitializeObjects ();
> > +    }
> > +
> >     return_ACPI_STATUS (Status);
> > }
>
> Ok, I see your are taking this up (I was a bit unsure after your previous
> post). Thanks,

The $subject patch has been queued for 5.3.  If I should drop it,
please let me know.

Thanks!
diff mbox series

Patch

diff --git a/drivers/acpi/acpi_configfs.c b/drivers/acpi/acpi_configfs.c
index f92033661239..663f0d88f912 100644
--- a/drivers/acpi/acpi_configfs.c
+++ b/drivers/acpi/acpi_configfs.c
@@ -56,11 +56,7 @@  static ssize_t acpi_table_aml_write(struct config_item *cfg,
 	if (!table->header)
 		return -ENOMEM;
 
-	ACPI_INFO(("Host-directed Dynamic ACPI Table Load:"));
-	ret = acpi_tb_install_and_load_table(
-			ACPI_PTR_TO_PHYSADDR(table->header),
-			ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL, FALSE,
-			&table->index);
+	ret = acpi_load_table(table->header);
 	if (ret) {
 		kfree(table->header);
 		table->header = NULL;
diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
index 4f30f06a6f78..ef8f8a9f3c9c 100644
--- a/drivers/acpi/acpica/tbxfload.c
+++ b/drivers/acpi/acpica/tbxfload.c
@@ -297,6 +297,17 @@  acpi_status acpi_load_table(struct acpi_table_header *table)
 	status = acpi_tb_install_and_load_table(ACPI_PTR_TO_PHYSADDR(table),
 						ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL,
 						FALSE, &table_index);
+
+	if (ACPI_SUCCESS(status)) {
+		/* Complete the initialization/resolution of package objects */
+
+		status = acpi_ns_walk_namespace(ACPI_TYPE_PACKAGE,
+						ACPI_ROOT_OBJECT,
+						ACPI_UINT32_MAX, 0,
+						acpi_ns_init_one_package,
+						NULL, NULL, NULL);
+	}
+
 	return_ACPI_STATUS(status);
 }