Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-16 Thread Dan Williams 
On Mon, Apr 16, 2018 at 5:05 PM, Schmauss, Erik <erik.schma...@intel.com> wrote:
>
>> -Original Message-
>> From: linux-acpi-ow...@vger.kernel.org [mailto:linux-acpi-
>> ow...@vger.kernel.org] On Behalf Of Dan Williams
>> Sent: Monday, April 16, 2018 4:22 PM
>> To: Schmauss, Erik <erik.schma...@intel.com>
>> Cc: Rafael J. Wysocki <r...@rjwysocki.net>; Linux ACPI > a...@vger.kernel.org>; Moore, Robert <robert.mo...@intel.com>; linux-
>> nvdimm <linux-nvdimm@lists.01.org>; Qemu Developers > de...@nongnu.org>
>> Subject: Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-
>> level code
>>
>> On Mon, Apr 16, 2018 at 4:15 PM, Schmauss, Erik <erik.schma...@intel.com>
>> wrote:
>> > [ trimming ]
>> >> >> Rafael, we may want to hold back on the module-level code changes
>> >> >> (the patches below) for rc1. Between this and the strange _TSS
>> >> >> issue, it seems like there are a few more things to resolve before
>> >> >> this is ready for kernel upstream.
>> >> >
>> > Hi Rafael,
>> >
>> >> > It looks like you are asking me to queue up reverts as per the
>> >> > Dan's report, is that correct?
>> >
>> > This is indeed what I meant last week. However, I've looked into the
>> > issue and Dan's qemu instance had AML that we no longer support. This
>> > is because the ACPICA commit makes changes to the execution of AML
>> > during table load to match windows AML interpreter behavior so this commit
>> also got rid of support for executing code containing forward references 
>> (except
>> for package elements).
>> >
>> > I've suggested a fix for the firmware in a separate email. So I would
>> > say that this issue is resolved after if Dan can run his test successfully 
>> > with the
>> adjusted firmware.
>> >
>> > If Dan's test is successful, we don’t need to revert these changes
>>
>
> Hi Dan,
>
>> I'm concerned about other qemu-kvm users that do not upgrade their hypervisor
>> at the same pace as their guest kernel. Especially for cloud providers that 
>> may
>> be running latest mainline kernel on older qemu-kvm this will look like a 
>> pure
>> kernel regression. Is there a quick fix we can carry in the kernel to 
>> support these
>> forward references, at least until we know that qemu-kvm is no longer 
>> shipping
>> the broken AML?
>
> This is a very good point. Thanks for bringing this up! One option is for 
> them to set
> the global variable acpi_gbl_execute_tables_as_methods in 
> include/acpi/acpixf.h to false.
> This will effectively revert the new behavior in the AML interpreter and go 
> back to the old way.
> Since this is a global flag, we could have a command line option for Linux 
> kernel to turn this
> feature on.
>
> Out of curiosity, is this ACPI table somehow customized for your work? I have 
> a collection
> of acpi tables and your ACPI tables are the only ones that have an 
> OperationRegion called
> NRAM. What is the chance that others will be running Linux on the same tables 
> as the one
> you sent me?

I don't think there's anything atypical about my particular setup. It
creates two virtual NVDIMMs that each represent a 30GB address space.
I suspect any user of the KVM NVDIMM virtualization would see the same
problem.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

RE: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-16 Thread Schmauss, Erik 

> -Original Message-
> From: linux-acpi-ow...@vger.kernel.org [mailto:linux-acpi-
> ow...@vger.kernel.org] On Behalf Of Dan Williams
> Sent: Monday, April 16, 2018 4:22 PM
> To: Schmauss, Erik <erik.schma...@intel.com>
> Cc: Rafael J. Wysocki <r...@rjwysocki.net>; Linux ACPI  a...@vger.kernel.org>; Moore, Robert <robert.mo...@intel.com>; linux-
> nvdimm <linux-nvdimm@lists.01.org>; Qemu Developers  de...@nongnu.org>
> Subject: Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-
> level code
> 
> On Mon, Apr 16, 2018 at 4:15 PM, Schmauss, Erik <erik.schma...@intel.com>
> wrote:
> > [ trimming ]
> >> >> Rafael, we may want to hold back on the module-level code changes
> >> >> (the patches below) for rc1. Between this and the strange _TSS
> >> >> issue, it seems like there are a few more things to resolve before
> >> >> this is ready for kernel upstream.
> >> >
> > Hi Rafael,
> >
> >> > It looks like you are asking me to queue up reverts as per the
> >> > Dan's report, is that correct?
> >
> > This is indeed what I meant last week. However, I've looked into the
> > issue and Dan's qemu instance had AML that we no longer support. This
> > is because the ACPICA commit makes changes to the execution of AML
> > during table load to match windows AML interpreter behavior so this commit
> also got rid of support for executing code containing forward references 
> (except
> for package elements).
> >
> > I've suggested a fix for the firmware in a separate email. So I would
> > say that this issue is resolved after if Dan can run his test successfully 
> > with the
> adjusted firmware.
> >
> > If Dan's test is successful, we don’t need to revert these changes
> 

Hi Dan,

> I'm concerned about other qemu-kvm users that do not upgrade their hypervisor
> at the same pace as their guest kernel. Especially for cloud providers that 
> may
> be running latest mainline kernel on older qemu-kvm this will look like a pure
> kernel regression. Is there a quick fix we can carry in the kernel to support 
> these
> forward references, at least until we know that qemu-kvm is no longer shipping
> the broken AML?

This is a very good point. Thanks for bringing this up! One option is for them 
to set
the global variable acpi_gbl_execute_tables_as_methods in include/acpi/acpixf.h 
to false.
This will effectively revert the new behavior in the AML interpreter and go 
back to the old way.
Since this is a global flag, we could have a command line option for Linux 
kernel to turn this
feature on.

Out of curiosity, is this ACPI table somehow customized for your work? I have a 
collection
of acpi tables and your ACPI tables are the only ones that have an 
OperationRegion called
NRAM. What is the chance that others will be running Linux on the same tables 
as the one
you sent me?

Erik

> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the 
> body of
> a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-16 Thread Dan Williams 
On Mon, Apr 16, 2018 at 4:15 PM, Schmauss, Erik  wrote:
> [ trimming ]
>> >> Rafael, we may want to hold back on the module-level code changes
>> >> (the patches below) for rc1. Between this and the strange _TSS issue,
>> >> it seems like there are a few more things to resolve before this is
>> >> ready for kernel upstream.
>> >
> Hi Rafael,
>
>> > It looks like you are asking me to queue up reverts as per the Dan's
>> > report, is that correct?
>
> This is indeed what I meant last week. However, I've looked into the issue 
> and Dan's qemu
> instance had AML that we no longer support. This is because the ACPICA commit 
> makes changes to the execution of AML
> during table load to match windows AML interpreter behavior so this commit 
> also got rid of support for executing code
> containing forward references (except for package elements).
>
> I've suggested a fix for the firmware in a separate email. So I would say 
> that this issue is resolved after if Dan can run
> his test successfully with the adjusted firmware.
>
> If Dan's test is successful, we don’t need to revert these changes

I'm concerned about other qemu-kvm users that do not upgrade their
hypervisor at the same pace as their guest kernel. Especially for
cloud providers that may be running latest mainline kernel on older
qemu-kvm this will look like a pure kernel regression. Is there a
quick fix we can carry in the kernel to support these forward
references, at least until we know that qemu-kvm is no longer shipping
the broken AML?
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

RE: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-16 Thread Schmauss, Erik 
[ trimming ]
> >> Rafael, we may want to hold back on the module-level code changes
> >> (the patches below) for rc1. Between this and the strange _TSS issue,
> >> it seems like there are a few more things to resolve before this is
> >> ready for kernel upstream.
> >
Hi Rafael,

> > It looks like you are asking me to queue up reverts as per the Dan's
> > report, is that correct?

This is indeed what I meant last week. However, I've looked into the issue and 
Dan's qemu
instance had AML that we no longer support. This is because the ACPICA commit 
makes changes to the execution of AML
during table load to match windows AML interpreter behavior so this commit also 
got rid of support for executing code
containing forward references (except for package elements).

I've suggested a fix for the firmware in a separate email. So I would say that 
this issue is resolved after if Dan can run
his test successfully with the adjusted firmware.

If Dan's test is successful, we don’t need to revert these changes

Erik

> 
> Note, that I'm willing to try any proposed fix patches on top. I just wanted 
> to
> clarify that we have a simple fallback position if the debug starts dragging 
> out.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

RE: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-13 Thread Moore, Robert 
> -Original Message-
> From: Rafael J. Wysocki [mailto:r...@rjwysocki.net]
> Sent: Friday, April 13, 2018 12:43 AM
> To: Schmauss, Erik <erik.schma...@intel.com>
> Cc: Williams, Dan J <dan.j.willi...@intel.com>; Linux ACPI  a...@vger.kernel.org>; Moore, Robert <robert.mo...@intel.com>; linux-
> nvdimm <linux-nvdimm@lists.01.org>
> Subject: Re: [PATCH v2 5/7] ACPICA: Integrate package handling with
> module-level code
> 
> On Friday, April 13, 2018 6:54:04 AM CEST Schmauss, Erik wrote:
> >
> > > -Original Message-
> > > From: Dan Williams [mailto:dan.j.willi...@intel.com]
> > > Sent: Thursday, April 12, 2018 5:57 PM
> > > To: Schmauss, Erik <erik.schma...@intel.com>
> > > Cc: Linux ACPI <linux-a...@vger.kernel.org>; Rafael J. Wysocki
> > > <r...@rjwysocki.net>; Moore, Robert <robert.mo...@intel.com>;
> > > linux-nvdimm <linux-nvdimm@lists.01.org>
> > > Subject: Re: [PATCH v2 5/7] ACPICA: Integrate package handling with
> > > module- level code
> > >
> > > On Thu, Apr 12, 2018 at 3:50 PM, Dan Williams
> > > <dan.j.willi...@intel.com>
> > > wrote:
> > > > [ adding linux-nvdimm ]
> > > >
> > > > On Fri, Feb 16, 2018 at 7:20 AM Erik Schmauss
> > > > <erik.schma...@intel.com>
> > > > wrote:
> > > >
> > > >> ACPICA commit 8faf6fca445eb7219963d80543fb802302a7a8c7
> > > >
> > > >> This change completes the integration of the recent changes to
> > > >> package object handling with the module-level code support.
> > > >
> > > >> For acpi_exec, the -ep flag is removed.
> > > >
> > > >> This change allows table load to behave as if it were a method
> > > >> invocation. Before this, the definition block definition below
> > > >> would have loaded all named objects at the root scope. After
> > > >> loading, it would execute the if statements at the root scope.
> > > >
> > > >> DefinitionBlock (...)
> > > >> {
> > > >>Name(OBJ1, 0)
> > > >
> > > >>if (1)
> > > >>{
> > > >>  Device (DEV1)
> > > >>  {
> > > >>Name (_HID,0x0)
> > > >>  }
> > > >>}
> > > >>Scope (DEV1)
> > > >>{
> > > >>  Name (OBJ2)
> > > >>}
> > > >> }
> > > >
> > > >> The above code would load OBJ1 to the namespace, defer the
> > > >> execution of the if statement and attempt to add OBJ2 within the
> scope of DEV1.
> > > >> Since DEV1 is not in scope, this would incur an AE_NOT_FOUND
> error.
> > > >> After this error is emitted, the if block is invoked and DEV1 and
> > > >> its _HID is added to the namespace.
> > > >
> > > >> This commit changes the behavior to execute the if block in place
> > > >> rather than deferring it until all tables are loaded. The new
> > > >> behavior is as follows: insert OBJ1 in the namespace, invoke the
> > > >> if statement and add DEV1 and its _HID to the namespace, add OBJ2
> > > >> to the scope of DEV1.
> > > >
> > > >> Bug report links:
> > > >> Link: https://bugs.acpica.org/show_bug.cgi?id=963
> > > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153541
> > > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196165
> > > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=192621
> > > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197207
> > > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198051
> > > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198515
> > > >
> > > >> ACPICA repo:
> > > >> Link: https://github.com/acpica/acpica/commit/8faf6fca
> > > >
> > > >> Tested-by: Kai-Heng Feng <kai.heng.f...@canonical.com>
> > > >> Signed-off-by: Bob Moore <robert.mo...@intel.com>
> > > >> Signed-off-by: Erik Schmauss <erik.schma...@intel.com>
> > > >
> > > > Hi,
> > > >
> > > > This commit 5a8361f7ecce ("ACPICA: Integrate package handling with
> > > > module-level code") regresses the detection of persistent memory
> > > > in my qemu-k

Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-13 Thread Dan Williams 
On Fri, Apr 13, 2018 at 12:42 AM, Rafael J. Wysocki <r...@rjwysocki.net> wrote:
> On Friday, April 13, 2018 6:54:04 AM CEST Schmauss, Erik wrote:
>>
>> > -Original Message-
>> > From: Dan Williams [mailto:dan.j.willi...@intel.com]
>> > Sent: Thursday, April 12, 2018 5:57 PM
>> > To: Schmauss, Erik <erik.schma...@intel.com>
>> > Cc: Linux ACPI <linux-a...@vger.kernel.org>; Rafael J. Wysocki
>> > <r...@rjwysocki.net>; Moore, Robert <robert.mo...@intel.com>; linux-nvdimm
>> > <linux-nvdimm@lists.01.org>
>> > Subject: Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-
>> > level code
>> >
>> > On Thu, Apr 12, 2018 at 3:50 PM, Dan Williams <dan.j.willi...@intel.com>
>> > wrote:
>> > > [ adding linux-nvdimm ]
>> > >
>> > > On Fri, Feb 16, 2018 at 7:20 AM Erik Schmauss
>> > > <erik.schma...@intel.com>
>> > > wrote:
>> > >
>> > >> ACPICA commit 8faf6fca445eb7219963d80543fb802302a7a8c7
>> > >
>> > >> This change completes the integration of the recent changes to
>> > >> package object handling with the module-level code support.
>> > >
>> > >> For acpi_exec, the -ep flag is removed.
>> > >
>> > >> This change allows table load to behave as if it were a method
>> > >> invocation. Before this, the definition block definition below would
>> > >> have loaded all named objects at the root scope. After loading, it
>> > >> would execute the if statements at the root scope.
>> > >
>> > >> DefinitionBlock (...)
>> > >> {
>> > >>Name(OBJ1, 0)
>> > >
>> > >>if (1)
>> > >>{
>> > >>  Device (DEV1)
>> > >>  {
>> > >>Name (_HID,0x0)
>> > >>  }
>> > >>}
>> > >>Scope (DEV1)
>> > >>{
>> > >>  Name (OBJ2)
>> > >>}
>> > >> }
>> > >
>> > >> The above code would load OBJ1 to the namespace, defer the execution
>> > >> of the if statement and attempt to add OBJ2 within the scope of DEV1.
>> > >> Since DEV1 is not in scope, this would incur an AE_NOT_FOUND error.
>> > >> After this error is emitted, the if block is invoked and DEV1 and its
>> > >> _HID is added to the namespace.
>> > >
>> > >> This commit changes the behavior to execute the if block in place
>> > >> rather than deferring it until all tables are loaded. The new
>> > >> behavior is as follows: insert OBJ1 in the namespace, invoke the if
>> > >> statement and add DEV1 and its _HID to the namespace, add OBJ2 to the
>> > >> scope of DEV1.
>> > >
>> > >> Bug report links:
>> > >> Link: https://bugs.acpica.org/show_bug.cgi?id=963
>> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153541
>> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196165
>> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=192621
>> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197207
>> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198051
>> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198515
>> > >
>> > >> ACPICA repo:
>> > >> Link: https://github.com/acpica/acpica/commit/8faf6fca
>> > >
>> > >> Tested-by: Kai-Heng Feng <kai.heng.f...@canonical.com>
>> > >> Signed-off-by: Bob Moore <robert.mo...@intel.com>
>> > >> Signed-off-by: Erik Schmauss <erik.schma...@intel.com>
>> > >
>> > > Hi,
>> > >
>> > > This commit 5a8361f7ecce ("ACPICA: Integrate package handling with
>> > > module-level code") regresses the detection of persistent memory in my
>> > > qemu-kvm setup.
>> > >
>> >
>> Hi Dan,
>>
>> Thanks for figuring this out. Do you think it's possible for you to send me 
>> a full acpidump or some sort hexdump of the QEMU environment?
>>
>> > With the following set of clean reverts on top of latest-Linus I'm back to 
>> > a
>> > working configuration:
>> >
>> >  Revert "ACPICA: Change a compile-time option to a runtime option"
>> >  Revert "ACPICA: Cleanup/simplify module-level code support"
>> >  Revert "ACPICA: Rename a global for clarity, no functional change"
>> >  Revert "ACPICA: Add option to disable Package object name resolution 
>> > errors"
>> >  Revert "ACPICA: Integrate package handling with module-level code"
>> >
>>
>> Rafael, we may want to hold back on the module-level code changes
>> (the patches below) for rc1. Between this and the strange _TSS issue,
>> it seems like there are a few more things to resolve before this is ready
>> for kernel upstream.
>
> It looks like you are asking me to queue up reverts as per the Dan's report,
> is that correct?

Note, that I'm willing to try any proposed fix patches on top. I just
wanted to clarify that we have a simple fallback position if the debug
starts dragging out.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-13 Thread Rafael J. Wysocki 
On Friday, April 13, 2018 6:54:04 AM CEST Schmauss, Erik wrote:
> 
> > -Original Message-
> > From: Dan Williams [mailto:dan.j.willi...@intel.com]
> > Sent: Thursday, April 12, 2018 5:57 PM
> > To: Schmauss, Erik <erik.schma...@intel.com>
> > Cc: Linux ACPI <linux-a...@vger.kernel.org>; Rafael J. Wysocki
> > <r...@rjwysocki.net>; Moore, Robert <robert.mo...@intel.com>; linux-nvdimm
> > <linux-nvdimm@lists.01.org>
> > Subject: Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-
> > level code
> > 
> > On Thu, Apr 12, 2018 at 3:50 PM, Dan Williams <dan.j.willi...@intel.com>
> > wrote:
> > > [ adding linux-nvdimm ]
> > >
> > > On Fri, Feb 16, 2018 at 7:20 AM Erik Schmauss
> > > <erik.schma...@intel.com>
> > > wrote:
> > >
> > >> ACPICA commit 8faf6fca445eb7219963d80543fb802302a7a8c7
> > >
> > >> This change completes the integration of the recent changes to
> > >> package object handling with the module-level code support.
> > >
> > >> For acpi_exec, the -ep flag is removed.
> > >
> > >> This change allows table load to behave as if it were a method
> > >> invocation. Before this, the definition block definition below would
> > >> have loaded all named objects at the root scope. After loading, it
> > >> would execute the if statements at the root scope.
> > >
> > >> DefinitionBlock (...)
> > >> {
> > >>Name(OBJ1, 0)
> > >
> > >>if (1)
> > >>{
> > >>  Device (DEV1)
> > >>  {
> > >>Name (_HID,0x0)
> > >>  }
> > >>}
> > >>Scope (DEV1)
> > >>{
> > >>  Name (OBJ2)
> > >>}
> > >> }
> > >
> > >> The above code would load OBJ1 to the namespace, defer the execution
> > >> of the if statement and attempt to add OBJ2 within the scope of DEV1.
> > >> Since DEV1 is not in scope, this would incur an AE_NOT_FOUND error.
> > >> After this error is emitted, the if block is invoked and DEV1 and its
> > >> _HID is added to the namespace.
> > >
> > >> This commit changes the behavior to execute the if block in place
> > >> rather than deferring it until all tables are loaded. The new
> > >> behavior is as follows: insert OBJ1 in the namespace, invoke the if
> > >> statement and add DEV1 and its _HID to the namespace, add OBJ2 to the
> > >> scope of DEV1.
> > >
> > >> Bug report links:
> > >> Link: https://bugs.acpica.org/show_bug.cgi?id=963
> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153541
> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196165
> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=192621
> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197207
> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198051
> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198515
> > >
> > >> ACPICA repo:
> > >> Link: https://github.com/acpica/acpica/commit/8faf6fca
> > >
> > >> Tested-by: Kai-Heng Feng <kai.heng.f...@canonical.com>
> > >> Signed-off-by: Bob Moore <robert.mo...@intel.com>
> > >> Signed-off-by: Erik Schmauss <erik.schma...@intel.com>
> > >
> > > Hi,
> > >
> > > This commit 5a8361f7ecce ("ACPICA: Integrate package handling with
> > > module-level code") regresses the detection of persistent memory in my
> > > qemu-kvm setup.
> > >
> > 
> Hi Dan,
> 
> Thanks for figuring this out. Do you think it's possible for you to send me a 
> full acpidump or some sort hexdump of the QEMU environment?
> 
> > With the following set of clean reverts on top of latest-Linus I'm back to a
> > working configuration:
> > 
> >  Revert "ACPICA: Change a compile-time option to a runtime option"
> >  Revert "ACPICA: Cleanup/simplify module-level code support"
> >  Revert "ACPICA: Rename a global for clarity, no functional change"
> >  Revert "ACPICA: Add option to disable Package object name resolution 
> > errors"
> >  Revert "ACPICA: Integrate package handling with module-level code"
> > 
> 
> Rafael, we may want to hold back on the module-level code changes
> (the patches below) for rc1. Between this and the strange _TSS issue,
> it seems like there are a few more things to resolve before this is ready
> for kernel upstream.

It looks like you are asking me to queue up reverts as per the Dan's report,
is that correct?

> If this is the case, I wonder if we could change the
> ACPICA version number so that it is one digit lower or append something
> to the end of the version number. This would be helpful in reminding us
> that it's not the full release in ACPICA upstream. Either that or remove
> the ACPICA release all together for this rc..

I can revert the commit updating the ACPICA version number easily enough. :-)

> > ...i.e.
> > 
> > git revert 34f206fd757c
> > git revert a406dea82af8
> > git revert e7d970f6fca8
> > git revert 959c38a7e128
> > git revert 5a8361f7ecce
> 


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

RE: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-12 Thread Schmauss, Erik 


> -Original Message-
> From: Dan Williams [mailto:dan.j.willi...@intel.com]
> Sent: Thursday, April 12, 2018 5:57 PM
> To: Schmauss, Erik <erik.schma...@intel.com>
> Cc: Linux ACPI <linux-a...@vger.kernel.org>; Rafael J. Wysocki
> <r...@rjwysocki.net>; Moore, Robert <robert.mo...@intel.com>; linux-nvdimm
> <linux-nvdimm@lists.01.org>
> Subject: Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-
> level code
> 
> On Thu, Apr 12, 2018 at 3:50 PM, Dan Williams <dan.j.willi...@intel.com>
> wrote:
> > [ adding linux-nvdimm ]
> >
> > On Fri, Feb 16, 2018 at 7:20 AM Erik Schmauss
> > <erik.schma...@intel.com>
> > wrote:
> >
> >> ACPICA commit 8faf6fca445eb7219963d80543fb802302a7a8c7
> >
> >> This change completes the integration of the recent changes to
> >> package object handling with the module-level code support.
> >
> >> For acpi_exec, the -ep flag is removed.
> >
> >> This change allows table load to behave as if it were a method
> >> invocation. Before this, the definition block definition below would
> >> have loaded all named objects at the root scope. After loading, it
> >> would execute the if statements at the root scope.
> >
> >> DefinitionBlock (...)
> >> {
> >>Name(OBJ1, 0)
> >
> >>if (1)
> >>{
> >>  Device (DEV1)
> >>  {
> >>Name (_HID,0x0)
> >>  }
> >>}
> >>Scope (DEV1)
> >>{
> >>  Name (OBJ2)
> >>}
> >> }
> >
> >> The above code would load OBJ1 to the namespace, defer the execution
> >> of the if statement and attempt to add OBJ2 within the scope of DEV1.
> >> Since DEV1 is not in scope, this would incur an AE_NOT_FOUND error.
> >> After this error is emitted, the if block is invoked and DEV1 and its
> >> _HID is added to the namespace.
> >
> >> This commit changes the behavior to execute the if block in place
> >> rather than deferring it until all tables are loaded. The new
> >> behavior is as follows: insert OBJ1 in the namespace, invoke the if
> >> statement and add DEV1 and its _HID to the namespace, add OBJ2 to the
> >> scope of DEV1.
> >
> >> Bug report links:
> >> Link: https://bugs.acpica.org/show_bug.cgi?id=963
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153541
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196165
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=192621
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197207
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198051
> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198515
> >
> >> ACPICA repo:
> >> Link: https://github.com/acpica/acpica/commit/8faf6fca
> >
> >> Tested-by: Kai-Heng Feng <kai.heng.f...@canonical.com>
> >> Signed-off-by: Bob Moore <robert.mo...@intel.com>
> >> Signed-off-by: Erik Schmauss <erik.schma...@intel.com>
> >
> > Hi,
> >
> > This commit 5a8361f7ecce ("ACPICA: Integrate package handling with
> > module-level code") regresses the detection of persistent memory in my
> > qemu-kvm setup.
> >
> 
Hi Dan,

Thanks for figuring this out. Do you think it's possible for you to send me a 
full acpidump or some sort hexdump of the QEMU environment?

> With the following set of clean reverts on top of latest-Linus I'm back to a
> working configuration:
> 
>  Revert "ACPICA: Change a compile-time option to a runtime option"
>  Revert "ACPICA: Cleanup/simplify module-level code support"
>  Revert "ACPICA: Rename a global for clarity, no functional change"
>  Revert "ACPICA: Add option to disable Package object name resolution errors"
>  Revert "ACPICA: Integrate package handling with module-level code"
> 

Rafael, we may want to hold back on the module-level code changes (the patches 
below) for rc1. Between this and the strange _TSS issue, it seems like there 
are a few more things to resolve before this is ready for kernel upstream. If 
this is the case, I wonder if we could change the ACPICA version number so that 
it is one digit lower or append something to the end of the version number. 
This would be helpful in reminding us that it's not the full release in ACPICA 
upstream. Either that or remove the ACPICA release all together for this rc..

Thanks,

Erik
> ...i.e.
> 
> git revert 34f206fd757c
> git revert a406dea82af8
> git revert e7d970f6fca8
> git revert 959c38a7e128
> git revert 5a8361f7ecce
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-12 Thread Dan Williams 
On Thu, Apr 12, 2018 at 3:50 PM, Dan Williams  wrote:
> [ adding linux-nvdimm ]
>
> On Fri, Feb 16, 2018 at 7:20 AM Erik Schmauss 
> wrote:
>
>> ACPICA commit 8faf6fca445eb7219963d80543fb802302a7a8c7
>
>> This change completes the integration of the recent changes to
>> package object handling with the module-level code support.
>
>> For acpi_exec, the -ep flag is removed.
>
>> This change allows table load to behave as if it were a method
>> invocation. Before this, the definition block definition below would
>> have loaded all named objects at the root scope. After loading, it
>> would execute the if statements at the root scope.
>
>> DefinitionBlock (...)
>> {
>>Name(OBJ1, 0)
>
>>if (1)
>>{
>>  Device (DEV1)
>>  {
>>Name (_HID,0x0)
>>  }
>>}
>>Scope (DEV1)
>>{
>>  Name (OBJ2)
>>}
>> }
>
>> The above code would load OBJ1 to the namespace, defer the execution
>> of the if statement and attempt to add OBJ2 within the scope of DEV1.
>> Since DEV1 is not in scope, this would incur an AE_NOT_FOUND error.
>> After this error is emitted, the if block is invoked and DEV1 and its
>> _HID is added to the namespace.
>
>> This commit changes the behavior to execute the if block in place
>> rather than deferring it until all tables are loaded. The new
>> behavior is as follows: insert OBJ1 in the namespace, invoke the if
>> statement and add DEV1 and its _HID to the namespace, add OBJ2 to the
>> scope of DEV1.
>
>> Bug report links:
>> Link: https://bugs.acpica.org/show_bug.cgi?id=963
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153541
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196165
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=192621
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197207
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198051
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198515
>
>> ACPICA repo:
>> Link: https://github.com/acpica/acpica/commit/8faf6fca
>
>> Tested-by: Kai-Heng Feng 
>> Signed-off-by: Bob Moore 
>> Signed-off-by: Erik Schmauss 
>
> Hi,
>
> This commit 5a8361f7ecce ("ACPICA: Integrate package handling with
> module-level code") regresses the detection of persistent memory in my
> qemu-kvm setup.
>

With the following set of clean reverts on top of latest-Linus I'm
back to a working configuration:

 Revert "ACPICA: Change a compile-time option to a runtime option"
 Revert "ACPICA: Cleanup/simplify module-level code support"
 Revert "ACPICA: Rename a global for clarity, no functional change"
 Revert "ACPICA: Add option to disable Package object name resolution errors"
 Revert "ACPICA: Integrate package handling with module-level code"

...i.e.

git revert 34f206fd757c
git revert a406dea82af8
git revert e7d970f6fca8
git revert 959c38a7e128
git revert 5a8361f7ecce
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 5/7] ACPICA: Integrate package handling with module-level code

2018-04-12 Thread Dan Williams 
[ adding linux-nvdimm ]

On Fri, Feb 16, 2018 at 7:20 AM Erik Schmauss 
wrote:

> ACPICA commit 8faf6fca445eb7219963d80543fb802302a7a8c7

> This change completes the integration of the recent changes to
> package object handling with the module-level code support.

> For acpi_exec, the -ep flag is removed.

> This change allows table load to behave as if it were a method
> invocation. Before this, the definition block definition below would
> have loaded all named objects at the root scope. After loading, it
> would execute the if statements at the root scope.

> DefinitionBlock (...)
> {
>Name(OBJ1, 0)

>if (1)
>{
>  Device (DEV1)
>  {
>Name (_HID,0x0)
>  }
>}
>Scope (DEV1)
>{
>  Name (OBJ2)
>}
> }

> The above code would load OBJ1 to the namespace, defer the execution
> of the if statement and attempt to add OBJ2 within the scope of DEV1.
> Since DEV1 is not in scope, this would incur an AE_NOT_FOUND error.
> After this error is emitted, the if block is invoked and DEV1 and its
> _HID is added to the namespace.

> This commit changes the behavior to execute the if block in place
> rather than deferring it until all tables are loaded. The new
> behavior is as follows: insert OBJ1 in the namespace, invoke the if
> statement and add DEV1 and its _HID to the namespace, add OBJ2 to the
> scope of DEV1.

> Bug report links:
> Link: https://bugs.acpica.org/show_bug.cgi?id=963
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=153541
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196165
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=192621
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197207
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198051
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198515

> ACPICA repo:
> Link: https://github.com/acpica/acpica/commit/8faf6fca

> Tested-by: Kai-Heng Feng 
> Signed-off-by: Bob Moore 
> Signed-off-by: Erik Schmauss 

Hi,

This commit 5a8361f7ecce ("ACPICA: Integrate package handling with
module-level code") regresses the detection of persistent memory in my
qemu-kvm setup.

Expect:

# ndctl list -BR
[
   {
 "provider":"ACPI.NFIT",
 "dev":"ndbus1",
 "regions":[
   {
 "dev":"region1",
 "size":33285996544,
 "available_size":0,
 "type":"pmem",
 "numa_node":0,
 "iset_id":52512752653219036,
 "persistence_domain":"unknown"
   },
   {
 "dev":"region2",
 "size":33285996544,
 "available_size":0,
 "type":"pmem",
 "numa_node":0,
 "iset_id":52512795602891997,
 "persistence_domain":"unknown"
   }
 ]
   },
   {
 "provider":"e820",
 "dev":"ndbus0",
 "regions":[
   {
 "dev":"region0",
 "size":4294967296,
 "available_size":0,
 "type":"pmem",
 "persistence_domain":"unknown"
   }
 ]
   }
]

Actual:

# ndctl list -BR
{
   "provider":"e820",
   "dev":"ndbus0",
   "regions":[
 {
   "dev":"region0",
   "size":4294967296,
   "available_size":0,
   "type":"pmem",
   "persistence_domain":"unknown"
 }
   ]
}

The immediately preceding commit 7decc66df940 ("ACPICA: Revert "Fix for
implicit result conversion for the To functions"") works fine.

Commit 5a8361f7ecce does not revert cleanly. Here is my qemu-kvm command
line:

label_size=$((128*1024))
mem_size=$((31 << 30))

kvm=(
 $qemu
 -enable-kvm
 -cpu kvm64
 -kernel $kernel
 -initrd $initrd
 -m 16G,slots=4,maxmem=128G

 -machine pc-i440fx-2.4,accel=kvm,usb=off,vmport=off,nvdimm
 -cpu SandyBridge
 -smp 40
 -netdev tap,id=hostnet0,ifname=tap0,script=no,downscript=no
 -device
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:b7:a1:ad,bus=pci.0,addr=0x7
 -object
memory-backend-file,id=mem1,share,mem-path=${mem}1,size=$((label_size +
mem_size))
 -device nvdimm,memdev=mem1,id=nv1,label-size=${label_size}

 -object
memory-backend-file,id=mem2,share,mem-path=${mem}2,size=$((label_size +
mem_size))
 -device nvdimm,memdev=mem2,id=nv2,label-size=${label_size}
 -numa node
 -numa node
 -device ahci,id=sata0,bus=pci.0,addr=0x8
 -drive file=$IMAGE,if=none,id=drive-sata0-0-0,format=raw
 -device ide-hd,bus=sata0.0,drive=drive-sata0-0-0,id=sata0-0-0
 -boot order=nc
 -no-reboot
 -watchdog i6300esb
 -rtc base=localtime
 -serial stdio
 -display none
 -monitor null
)

...where the important lines that setup the pmem emulation are:

 -object
memory-backend-file,id=mem1,share,mem-path=${mem}1,size=$((label_size +
mem_size))
 -device nvdimm,memdev=mem1,id=nv1,label-size=${label_size}

 -object