Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Acpica tables #121

Closed
wants to merge 10 commits into from
Closed

Acpica tables #121

wants to merge 10 commits into from

Conversation

zetalog
Copy link
Contributor

@zetalog zetalog commented Feb 19, 2016 •
edited

New table materials, generated to fix AcpiGetTable()/AcpiPutTable() regressions.

The first 2 patches are Linux materials, and the follow-ups are based on the Linux upstream code base as the feature is requested by Linux kernel developers, they need them to be able to apply on top of current code base.

Reported by Hans de Geode from redhat
Fixed by Lv Zheng

@zetalog zetalog force-pushed the acpica-tables branch 6 times, most recently from 8a0321d to 27262c8 Compare February 26, 2016 07:04
@zetalog zetalog force-pushed the acpica-tables branch 11 times, most recently from 51d22b2 to d29aa2f Compare March 16, 2017 06:43
zetalog pushed a commit to zetalog/acpica that referenced this pull request Apr 14, 2017
Tables: Replace AcpiInstallMethod() with OSDT override
970c875 
This patch converts AcpiInstallMethod() into new OSDT override mechanism.
If this is on top of the table sanity check mechanism (link #2), the change
in tbinstal.c is not needed. Reported by Zhang Rui, fixed by Lv Zheng.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=195325 [#1]
Link: acpica#121
Reported-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
@jwrdegoede
Copy link
Contributor

Hi,

I see that this is not merged yet, what is the status of this ?

Regards,

Hans

@zetalog
Copy link
Contributor Author

zetalog commented May 9, 2017

Bob is waiting for me to validate if Windows does ignore table signature when a table is dynamically loaded via Load/LoadTable opcodes.
I'll try to find a way to poke this out this week.

@zetalog
Copy link
Contributor Author

zetalog commented May 9, 2017

If that's true, ASLTS need to be cleaned up accordingly.

@zetalog zetalog force-pushed the acpica-tables branch 2 times, most recently from f1ef388 to a605a76 Compare May 9, 2017 03:12
@zetalog
Copy link
Contributor Author

zetalog commented May 10, 2017 •
edited

I completed the following test:

  1. prepare an OEM table, signature is ABCD, putting it into XSDT using -acpitable option in qemu
  2. prepare a LoadTable call from another SSDT to load "ABCD" table into "\", and check if the object belonging to that table exists
  3. Invoke the LoadTable call from _SB._INI

Table has been successfully loaded, and you can see screen capture in the below attachment.
This result looks correct in theory.
As it's really useless to validate a table signature.
The AML interpreter should just trust what the BIOS asks it to do.
The signature should only be used to locate the table from XSDT.

Forgot to mention:
Same result for Load, I tried to load "SSDT", "OEM1", "ABCD", all worked, no exceptions could be observed. And the object could be found in namespace after loading.

zetalog pushed a commit to zetalog/acpica that referenced this pull request Jun 9, 2017
Tables: Do not validate signature for dynamic table load
170dc74 
Windows seems to allow arbitrary table signatures for Load/LoadTable
opcodes:
  ACPI BIOS Error (bug): Table has invalid signature [PRAD] (0x44415250)
So this patch removes dynamic load signature checks. However we need to
find a way to avoid table loading against tables like MADT. This is not
covered by this commit.

This Windows behavior has been validated on link #1. An end user bug
report can also be found on link #2.

This patch also includes simple cleanup for static load signature check
code. Reported by Ye Xiaolong, Fixed by Lv Zheng.

Link: acpica#121 [#1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=118601 [#2]
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
@zetalog
Copy link
Contributor Author

zetalog commented Jun 9, 2017

Rebased to contain linux upstreamed materials:
Tables: Fix a hidden logic related to AcpiTbInstallStandardTable() …
Tables: Add mechanism to allow to balance late stage AcpiGetT… …

And simple changes:
Tables: Cleanup table handler invokers …
Tables: Add sanity check in AcpiPutTable() …

And table signature changes according to the Windows validation result:
Tables: Do not validate signature for dynamic table load …
ASLTS: exc_tbl: Fix TLD1 tests …

Thanks

zetalog pushed a commit to zetalog/acpica that referenced this pull request Jun 14, 2017
Tables: Do not validate signature for dynamic table load
4961a26 
Windows seems to allow arbitrary table signatures for Load/LoadTable
opcodes:
  ACPI BIOS Error (bug): Table has invalid signature [PRAD] (0x44415250)
So this patch removes dynamic load signature checks. However we need to
find a way to avoid table loading against tables like MADT. This is not
covered by this commit.

This Windows behavior has been validated on link #1. An end user bug
report can also be found on link #2.

This patch also includes simple cleanup for static load signature check
code. Reported by Ye Xiaolong, Fixed by Lv Zheng.

Link: acpica#121 [#1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=118601 [#2]
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
zetalog pushed a commit to zetalog/acpica that referenced this pull request Jun 16, 2017
Tables: Do not validate signature for dynamic table load
674e420 
Windows seems to allow arbitrary table signatures for Load/LoadTable
opcodes:
  ACPI BIOS Error (bug): Table has invalid signature [PRAD] (0x44415250)
So this patch removes dynamic load signature checks. However we need to
find a way to avoid table loading against tables like MADT. This is not
covered by this commit.

This Windows behavior has been validated on link #1. An end user bug
report can also be found on link #2.

This patch also includes simple cleanup for static load signature check
code. Reported by Ye Xiaolong, Fixed by Lv Zheng.

Link: acpica#121 [#1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=118601 [#2]
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
@zetalog zetalog mentioned this pull request Jun 16, 2017
Closed
zetalog pushed a commit to zetalog/acpica that referenced this pull request Jun 16, 2017
Tables: Do not validate signature for dynamic table load
a863e47 
Windows seems to allow arbitrary table signatures for Load/LoadTable
opcodes:
  ACPI BIOS Error (bug): Table has invalid signature [PRAD] (0x44415250)
So this patch removes dynamic load signature checks. However we need to
find a way to avoid table loading against tables like MADT. This is not
covered by this commit.

This Windows behavior has been validated on link #1. An end user bug
report can also be found on link #2.

This patch also includes simple cleanup for static load signature check
code. Reported by Ye Xiaolong, Fixed by Lv Zheng.

Link: acpica#121 [#1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=118601 [#2]
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Lv Zheng added 10 commits June 16, 2017 11:58
Tables: Fix regression introduced by a too early mechanism enabling
8269b79 
On linux, 2 ioremap() mechanisms are implemented for 2 stages:
1. During early boot, before memory manager is fully initialized.
2. For late boot and runtime, after memory manager is fully
   initialized.
Maps mapped in the early stage can not be used by late stage.

So for early stage, linux implements get/put like table APIs to free table
mappings, and for late stage, linux only invokes get table APIs.

We merged 2 implementations into single style API set:
AcpiGetTable() and AcpiPutTable(). During early stage, we want the
"ValidationCount" based mechanism to fully work to release table mappings,
while for late stages, if get/put invocations are balanced, we can free
mappings, but if not (we'll reach the warnings in
AcpiTbGetTable()/AcpiTbPutTable()), we should stop unmapping tables.

The design should work but unfortunately, "return_ACPI_STAUS(AE_LIMIT)"
prevents AcpiGetTable() from returning mapped table pointer to the
callers and the error message can flood kernel logs.

Thus this patch removes the error value returned by AcpiTbGetTable()
in that case along with the accompanying error message to fix the
issue.

Reported-by: Anush Seetharaman <anush.seetharaman@intel.com>
Reported-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tables: Mechanism to handle AcpiGetTable()/AcpiPutTable() imbalance b…
5cf2e1f 
…etter

On linux, 2 ioremap() mechanisms are implemented for 2 stages:
1. During early boot, before memory manager is fully initialized.
2. For late boot and runtime, after memory manager is fully
   initialized.
Maps mapped in the early stage cannot be used by late stage.

So for early stage, linux invokes AcpiGetTable()/AcpiPutTable() APIs to
free table mappings (means there is no imbalance), and for late stage,
linux mostly only invokes AcpiGetTable() table API (means there are
imbalances, and balances and imbalances co-exist).

The original mechanism is designed to allow:
1. early stage balances;
2. late stage co-existence of balances/imbalances.
But for the latter, it has a limitation, it doesn't allow balances to be
implemented for users who can easily increment ValidationCount to its
maximum value.

Considering this case:
 1. A program opens a sysfs table file 65535 times, it can increase
    ValidationCount and first increment causes the table to be mapped:
     ValidationCount = 65535
 2. AML execution causes "Load" to be executed on the same
    table, this time it cannot increase ValidationCount, so
    ValidationCount remains:
     ValidationCount = 65535
 3. The program closes sysfs table file 65535 times, it can decrease
    ValidationCount and the last decrement cause the table to be
    unmapped:
     ValidationCount = 0
 4. AML code still accessing the loaded table, kernel crash can be
    observed.
This is because the original mechanism is only prepared for late stage
users that won't invoke AcpiPutTable() so often (means can only handle
co-existence of balanced non-frequent AcpiPutTable() invocations and
imbalanced AcpiGetTable() invocations), and cannot handle the
co-existence of balanced frequent AcpiPutTable() invocations and
imbalanced AcpiGetTable() invocations.

To prevent that from happening, add a ValidationCount threashold. When it
is reached, the ValidationCount can no longer be incremented/decremented to
invalidate the table descriptor (means preventing table unmappings). Now
the improved mechanism can handle co-existence of any balances/imbalances.

Note that code added in AcpiTbPutTable() is actually a no-op but changes
the warning message into a "warn once" one. Since all warning messages can
only appear once for one table descriptor, it is safe now to add them back
without worrying about log flooding. Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tables: Add sanity check in AcpiPutTable()
feded25 
To avoid caller to trigger unexpected warning messages (Link #1):
  ACPI Warning: Table ffffffffbb461d20, Validation count is zero before decrement
Which is reported from AcpiTbPutTable(). When the table is validated, the
pointer must be non-zero. Thus the message is not suitable for invalidated
tables. This patch fixes the callee side based on this fact. Reported by
Cristian Aravena Romero, Fixed by Lv Zheng.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=191221 [#1]
Reported-by: Cristian Aravena Romero <caravena@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tables: Fix a hidden logic related to AcpiTbInstallStandardTable()
dd2f5cb 
There is a hidden logic for AcpiTbInstallStandardTable():
1. When it is invoked from the OS boot stage, ACPICA mutex may not be
   available, and thus no AcpiUtAcquireMutex()/AcpiUtReleaseMutex() are
   invoked in these code paths:
   AcpiInitializeTables
     AcpiTbParseRootTable
       AcpiTbInstallStandardTable (4 invocations)
   AcpiInstallTable
       AcpiTbInstallStandardTable
2. When it is invoked during the runtime, ACPICA mutex is correctly used:
   AcpiExLoadOp
     AcpiTbInstallAndLoadTable
       Lock(TABLES)
       AcpiTbInstallStandardTable
       UnLock(TABLES)
   AcpiLoadTable
     AcpiTbInstallAndLoadTable
       Lock(TABLES)
       AcpiTbInstallStandardTable
       UnLock(TABLES)
So the hidden logic is: AcpiTbInstallStandardTable() tries not to hold
mutexes to handle the difference of the boot stage and the runtime, but
leaves the mutexes held from some calling runtime APIs.

This introduces another problem in AcpiTbInstallStandardTable() where
AcpiGbl_TableHandler is invoked from and the lock contexts are thus not
consistent for the table handlers. Linux registers such a handler to track
the table installation and the handler is actually invoked from both
contexts. Since the handler also invokes AcpiGetTable(), when the following
commit corrected AcpiGetTable() to make it start to hold the table lock,
a regression is then triggered.
  Commit: cac6790
  Subject: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

The regression is noticed by LKP as new errors reported by ACPICA mutex
debugging facility.
[    2.043693] ACPI Error: Mutex [ACPI_MTX_Tables] already acquired by this thread [497483776] (20160930/utmutex-254)
[    2.054084] ACPI Error: Mutex [0x2] is not acquired, cannot release (20160930/utmutex-326)

And it triggers a dead lock:
[  247.066214] INFO: task swapper/0:1 blocked for more than 120 seconds.
...
[  247.091271] Call Trace:
...
[  247.121523]  down_timeout+0x47/0x50
[  247.125065]  acpi_os_wait_semaphore+0x47/0x62
[  247.129475]  acpi_ut_acquire_mutex+0x43/0x81
[  247.133798]  acpi_get_table+0x2d/0x84
[  247.137513]  acpi_table_attr_init+0xcd/0x100
[  247.146590]  acpi_sysfs_table_handler+0x5d/0xb8
[  247.151174]  acpi_bus_table_handler+0x23/0x2a
[  247.155583]  acpi_tb_install_standard_table+0xe0/0x213
[  247.164489]  acpi_tb_install_and_load_table+0x3a/0x82
[  247.169592]  acpi_ex_load_op+0x194/0x201
...
[  247.200108]  acpi_ns_evaluate+0x1bb/0x247
[  247.204170]  acpi_evaluate_object+0x178/0x274
[  247.213249]  acpi_processor_set_pdc+0x154/0x17b
...
The table mutex held in AcpiTbInstallAndLoadTable() is re-visited by
AcpiGetTable().

Noticing that the early mutex requirement actually belongs to the OSL layer
and has already been handled in Linux
acpi_os_wait_semaphore()/acpi_os_signal_semaphore(). This patch then can
fix the regression by removing this hidden logic from ACPICA core and
leaving it to OSPMs.

Tested-by: Tomi Sarvela <tomi.p.sarvela@intel.com>
Tested-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tables: Cleanup table handler invokers
c9bf71a 
Recently, we allows the table mutex to be held in both early and late stage
APIs. This patch further cleans up the related code to reduce redundant
code related to AcpiGbl_TableHandler. Lv Zheng.

Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tables: Do not validate signature for dynamic table load
cfcf68f 
Windows seems to allow arbitrary table signatures for Load/LoadTable
opcodes:
  ACPI BIOS Error (bug): Table has invalid signature [PRAD] (0x44415250)
So this patch removes dynamic load signature checks. However we need to
find a way to avoid table loading against tables like MADT. This is not
covered by this commit.

This Windows behavior has been validated on link #1. An end user bug
report can also be found on link #2.

This patch also includes simple cleanup for static load signature check
code. Reported by Ye Xiaolong, Fixed by Lv Zheng.

Link: acpica#121 [#1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=118601 [#2]
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
ASLTS: exc_tbl: Fix TLD1 tests
10ff3a0 
Validated windows behavior shows it allows to load tables via Load and
LoadTable opcode without putting any limitations to the table signature.

This patch corrects exc_tbl:TLD1 cases to reflect this validated Windows
behavior. Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tables: Change table duplication check to be related to AcpiGbl_Verif…
21c6c96 
…yTableChecksum

AcpiGbl_VerifyTableChecksum is used to avoid validating (mapping) an entire
table in OS boot stage. 2nd "Reload" check in AcpiTbInstallStandardTable()
is prepared for the same purpose. So this patch combines them together
using a renamed AcpiGbl_EnableTableValidation flag. Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tables: Combine checksum/duplication verification together
5f6ad5b 
They are all mechanisms used to verify if a table is qualified to be
installed and controlled by AcpiGbl_EnableTableValidation, so combine them
together. By doing so, table duplication check is applied to the statically
loaded tables (however whether it is actually enabled is still determined
by AcpiGbl_EnableTableValidation). Lv Zheng.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tables: Add deferred table verification support
33d47c5 
This patch allows tables not verified in early stage verfied in
AcpiReallocateRootTable(). This is useful for OSPMs like linux where tables
cannot be verified in early stage due to early ioremp limitations on some
architectures. Reported by Hans de Geode, fixed by Lv Zheng.

Reported-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
zetalog pushed a commit to zetalog/acpica that referenced this pull request Jun 26, 2017
Tables: Do not validate signature for dynamic table load
d3c944f 
Windows seems to allow arbitrary table signatures for Load/LoadTable
opcodes:
  ACPI BIOS Error (bug): Table has invalid signature [PRAD] (0x44415250)
So this patch removes dynamic load signature checks. However we need to
find a way to avoid table loading against tables like MADT. This is not
covered by this commit.

This Windows behavior has been validated on link #1. An end user bug
report can also be found on link #2.

This patch also includes simple cleanup for static load signature check
code. Reported by Ye Xiaolong, Fixed by Lv Zheng.

Link: acpica#121 [#1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=118601 [#2]
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
@zetalog zetalog mentioned this pull request Jun 26, 2017
Merged
@zetalog
Copy link
Contributor Author

zetalog commented Jun 30, 2017

Closing as #281 has been merged.
Will communicate and upstream ASLTS fix in a different way.

@zetalog zetalog closed this Jun 30, 2017
fengguang pushed a commit to 0day-ci/linux that referenced this pull request Jul 10, 2017
ACPICA: Tables: Do not validate signature for dynamic table load
9d39f7f 
ACPICA commit d3c944f2cdc8c7e847b7942b1864f285189f7bce

Windows seems to allow arbitrary table signatures for Load/load_table
opcodes:
  ACPI BIOS Error (bug): Table has invalid signature [PRAD] (0x44415250)
So this patch removes dynamic load signature checks. However we need to
find a way to avoid table loading against tables like MADT. This is not
covered by this commit.

This Windows behavior has been validated on link #1. An end user bug
report can also be found on link #2.

This patch also includes simple cleanup for static load signature check
code. Reported by Ye Xiaolong, Fixed by Lv Zheng.

Link: acpica/acpica@d3c944f2
Link: acpica/acpica#121 [#1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=118601 [#2]
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Reported-by: Olga Uhina <olga.uhina@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
fengguang pushed a commit to 0day-ci/linux that referenced this pull request Jul 23, 2017
@rafaeljw
ACPICA: Tables: Do not validate signature for dynamic table load
04ba15a 
ACPICA commit d3c944f2cdc8c7e847b7942b1864f285189f7bce

Windows seems to allow arbitrary table signatures for Load/load_table
opcodes:
  ACPI BIOS Error (bug): Table has invalid signature [PRAD] (0x44415250)
So this patch removes dynamic load signature checks. However we need to
find a way to avoid table loading against tables like MADT. This is not
covered by this commit.

This Windows behavior has been validated on link #1. An end user bug
report can also be found on link #2.

This patch also includes simple cleanup for static load signature check
code. Reported by Ye Xiaolong, Fixed by Lv Zheng.

Link: acpica/acpica@d3c944f2
Link: acpica/acpica#121 [#1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=118601 [#2]
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Reported-by: Olga Uhina <olga.uhina@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
kk-infra pushed a commit to kernkonzept/acpica that referenced this pull request Nov 6, 2019
(source)Tables: Do not validate signature for dynamic table load
c50c539 
Windows seems to allow arbitrary table signatures for Load/LoadTable
opcodes:
  ACPI BIOS Error (bug): Table has invalid signature [PRAD] (0x44415250)
So this patch removes dynamic load signature checks. However we need to
find a way to avoid table loading against tables like MADT. This is not
covered by this commit.

This Windows behavior has been validated on link #1. An end user bug
report can also be found on link #2.

This patch also includes simple cleanup for static load signature check
code. Reported by Ye Xiaolong, Fixed by Lv Zheng.

Link: acpica/acpica#121 [#1]
Link: https://bugzilla.kernel.org/show_bug.cgi?id=118601 [#2]
Reported-by: Ye Xiaolong <xiaolong.ye@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@zetalog @jwrdegoede