From: Lorenzo Pieralisi <hidden> Date: 2021-09-10 13:54:55
[dropped CC stable, +CC Harb]
On Fri, Sep 10, 2021 at 08:28:20PM +0800, Jia He wrote:
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
From the debugging, we're mapping something which is *not* described by
the EFI memory map, but *does* want PROT_NORMAL_NC.
"Does not" you mean. We are forcing memory semantics mappings to
PROT_NORMAL_NC, which eMAG does not like at all and I'd need to
understand why.
It looks like the issue happen in SystemMemory Opregion handler.
Hence just revert it before everything is clear.
Fixes: 437b38c51162 ("ACPI: Add memory semantics to acpi_os_map_memory()")
Cc: stable@vger.kernel.org
No need, it is not even in an -rc yet (and stable should not be CCed in
the addressees CC list).
Thanks,
Lorenzo
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Fixes: 437b38c51162 ("ACPI: Add memory semantics to acpi_os_map_memory()")
Cc: Lorenzo Pieralisi <redacted>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Hanjun Guo <guohanjun@huawei.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rafael J. Wysocki <redacted>
Cc: Harb Abdulhamid <redacted>
Signed-off-by: Jia He <redacted>
---
v2: Improve the commit message
arch/arm64/include/asm/acpi.h | 3 ---
arch/arm64/kernel/acpi.c | 19 +++----------------
drivers/acpi/osl.c | 23 +++++++----------------
include/acpi/acpi_io.h | 8 --------
4 files changed, 10 insertions(+), 43 deletions(-)
On Fri, 10 Sept 2021 at 16:32, Jia He [off-list ref] wrote:
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Can we try to find the root cause first? -rc1 is not even out yet, and
reverting it now means we can not resubmit it until the next merge
window.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Lorenzo Pieralisi <hidden> Date: 2021-09-11 10:14:29
On Fri, Sep 10, 2021 at 07:28:49PM +0200, Ard Biesheuvel wrote:
On Fri, 10 Sept 2021 at 16:32, Jia He [off-list ref] wrote:
quoted
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Can we try to find the root cause first? -rc1 is not even out yet, and
reverting it now means we can not resubmit it until the next merge
window.
Yes, absolutely. We need to understand where the problem is, because it
looks like we can't map SystemMemory Opregion with NORMAL_NC if the PA
is not in the EFI map, that's a problem (ie how can we determine the
right memory attributes for SystemMemory Operation regions then) but
let's not speculate and find what the issue is first.
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Lorenzo Pieralisi <hidden> Date: 2021-09-16 16:18:49
On Fri, Sep 10, 2021 at 07:28:49PM +0200, Ard Biesheuvel wrote:
On Fri, 10 Sept 2021 at 16:32, Jia He [off-list ref] wrote:
quoted
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Can we try to find the root cause first? -rc1 is not even out yet, and
reverting it now means we can not resubmit it until the next merge
window.
I am waiting to debug this on an eMAG but I noticed something that
I wanted to bring up.
SystemMemory Operation region handler - ie
acpi_ex_system_memory_space_handler()
maps the Operation Region (that AFAICS is MMIO, it is _not_ memory)
with acpi_os_map_memory() and I believe that's what is causing this
bug.
On the other hand, acpi_os_map_generic_address(), to handle spaceid
ACPI_ADR_SPACE_SYSTEM_MEMORY, uses acpi_os_map_iomem() that is more
in line with my expectations.
Question is: is the mapping in acpi_ex_system_memory_space_handler()
wrong (and should be patched with acpi_os_map_iomem() ?)
On x86 this should not change a thing, on ARM it would.
I don't think it is right to map SystemMemory Operation regions with
memory semantics but on the other hand, other than the EFI memory map,
there is nothing we can do to determine what a SystemMemory Operation
region address space actually represents.
Thoughts ? Before embarking on patching
acpi_ex_system_memory_space_handler()
I want to make sure my understanding of the SystemMemory space is
correct, comments welcome.
I will pinpoint the trigger for this bug shortly and before doing
anything else.
Thanks,
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Lorenzo Pieralisi <hidden> Date: 2021-09-20 17:04:12
On Thu, Sep 16, 2021 at 05:08:27PM +0100, Lorenzo Pieralisi wrote:
On Fri, Sep 10, 2021 at 07:28:49PM +0200, Ard Biesheuvel wrote:
quoted
On Fri, 10 Sept 2021 at 16:32, Jia He [off-list ref] wrote:
quoted
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Can we try to find the root cause first? -rc1 is not even out yet, and
reverting it now means we can not resubmit it until the next merge
window.
I am waiting to debug this on an eMAG but I noticed something that
I wanted to bring up.
SystemMemory Operation region handler - ie
acpi_ex_system_memory_space_handler()
maps the Operation Region (that AFAICS is MMIO, it is _not_ memory)
with acpi_os_map_memory() and I believe that's what is causing this
bug.
On the other hand, acpi_os_map_generic_address(), to handle spaceid
ACPI_ADR_SPACE_SYSTEM_MEMORY, uses acpi_os_map_iomem() that is more
in line with my expectations.
Hi Rafael,
I wanted to ask please if you have any insights on why
(1) acpi_ex_system_memory_space_handler()
(2) acpi_os_map_generic_address()
Use two different calls to map memory for the _same_ address space ID
(SystemMemory).
(3) acpi_os_map_memory()
vs
(4) acpi_os_map_iomem()
I am struggling to understand why (1) uses (3) ("memory semantics") when
(2) uses (4) - it is actually unclear how the distinction between
the two mapping APIs is to be drawn and on what basis one should
choose which one to use.
I am still waiting to grab some HW to debug this report but the issue
here is that we are mapping an OpRegion SystemMemory with (3) in the
memory space handler and given the patch we are reverting we end up
mapping the operation region with normal non-cacheable memory attributes
that probably the physical address range behind the OpRegion does not
support.
Thanks a lot,
Lorenzo
Question is: is the mapping in acpi_ex_system_memory_space_handler()
wrong (and should be patched with acpi_os_map_iomem() ?)
On x86 this should not change a thing, on ARM it would.
I don't think it is right to map SystemMemory Operation regions with
memory semantics but on the other hand, other than the EFI memory map,
there is nothing we can do to determine what a SystemMemory Operation
region address space actually represents.
Thoughts ? Before embarking on patching
acpi_ex_system_memory_space_handler()
I want to make sure my understanding of the SystemMemory space is
correct, comments welcome.
I will pinpoint the trigger for this bug shortly and before doing
anything else.
Thanks,
Lorenzo
From: "Rafael J. Wysocki" <rafael@kernel.org> Date: 2021-09-20 17:35:44
On Mon, Sep 20, 2021 at 7:03 PM Lorenzo Pieralisi
[off-list ref] wrote:
On Thu, Sep 16, 2021 at 05:08:27PM +0100, Lorenzo Pieralisi wrote:
quoted
On Fri, Sep 10, 2021 at 07:28:49PM +0200, Ard Biesheuvel wrote:
quoted
On Fri, 10 Sept 2021 at 16:32, Jia He [off-list ref] wrote:
quoted
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Can we try to find the root cause first? -rc1 is not even out yet, and
reverting it now means we can not resubmit it until the next merge
window.
I am waiting to debug this on an eMAG but I noticed something that
I wanted to bring up.
SystemMemory Operation region handler - ie
acpi_ex_system_memory_space_handler()
maps the Operation Region (that AFAICS is MMIO, it is _not_ memory)
with acpi_os_map_memory() and I believe that's what is causing this
bug.
On the other hand, acpi_os_map_generic_address(), to handle spaceid
ACPI_ADR_SPACE_SYSTEM_MEMORY, uses acpi_os_map_iomem() that is more
in line with my expectations.
Hi Rafael,
I wanted to ask please if you have any insights on why
(1) acpi_ex_system_memory_space_handler()
(2) acpi_os_map_generic_address()
Use two different calls to map memory for the _same_ address space ID
(SystemMemory).
(3) acpi_os_map_memory()
vs
(4) acpi_os_map_iomem()
I don't really have a good answer here.
On x86 this doesn't really matter and that's where
acpi_ex_system_memory_space_handler() was first introduced. It is not
only used for IOMEM (there are SystemMemory operation regions in RAM),
but since it may be in IOMEM, it should assume so.
I am struggling to understand why (1) uses (3) ("memory semantics") when
(2) uses (4) - it is actually unclear how the distinction between
the two mapping APIs is to be drawn and on what basis one should
choose which one to use.
I am still waiting to grab some HW to debug this report but the issue
here is that we are mapping an OpRegion SystemMemory with (3) in the
memory space handler and given the patch we are reverting we end up
mapping the operation region with normal non-cacheable memory attributes
that probably the physical address range behind the OpRegion does not
support.
If that is the case, there needs to be a mechanism to decide what kind
of mapping to use for SystemMemory operation regions based on the type
of physical memory the address range in question is located in.
quoted
Question is: is the mapping in acpi_ex_system_memory_space_handler()
wrong (and should be patched with acpi_os_map_iomem() ?)
On x86 this should not change a thing, on ARM it would.
I don't think it is right to map SystemMemory Operation regions with
memory semantics but on the other hand, other than the EFI memory map,
there is nothing we can do to determine what a SystemMemory Operation
region address space actually represents.
Thoughts ? Before embarking on patching
acpi_ex_system_memory_space_handler()
I want to make sure my understanding of the SystemMemory space is
correct, comments welcome.
I will pinpoint the trigger for this bug shortly and before doing
anything else.
Thanks,
Lorenzo
From: Lorenzo Pieralisi <hidden> Date: 2021-09-21 10:05:38
On Mon, Sep 20, 2021 at 07:32:56PM +0200, Rafael J. Wysocki wrote:
On Mon, Sep 20, 2021 at 7:03 PM Lorenzo Pieralisi
[off-list ref] wrote:
quoted
On Thu, Sep 16, 2021 at 05:08:27PM +0100, Lorenzo Pieralisi wrote:
quoted
On Fri, Sep 10, 2021 at 07:28:49PM +0200, Ard Biesheuvel wrote:
quoted
On Fri, 10 Sept 2021 at 16:32, Jia He [off-list ref] wrote:
quoted
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Can we try to find the root cause first? -rc1 is not even out yet, and
reverting it now means we can not resubmit it until the next merge
window.
I am waiting to debug this on an eMAG but I noticed something that
I wanted to bring up.
SystemMemory Operation region handler - ie
acpi_ex_system_memory_space_handler()
maps the Operation Region (that AFAICS is MMIO, it is _not_ memory)
with acpi_os_map_memory() and I believe that's what is causing this
bug.
On the other hand, acpi_os_map_generic_address(), to handle spaceid
ACPI_ADR_SPACE_SYSTEM_MEMORY, uses acpi_os_map_iomem() that is more
in line with my expectations.
Hi Rafael,
I wanted to ask please if you have any insights on why
(1) acpi_ex_system_memory_space_handler()
(2) acpi_os_map_generic_address()
Use two different calls to map memory for the _same_ address space ID
(SystemMemory).
(3) acpi_os_map_memory()
vs
(4) acpi_os_map_iomem()
I don't really have a good answer here.
On x86 this doesn't really matter and that's where
acpi_ex_system_memory_space_handler() was first introduced. It is not
only used for IOMEM (there are SystemMemory operation regions in RAM),
but since it may be in IOMEM, it should assume so.
quoted
I am struggling to understand why (1) uses (3) ("memory semantics") when
(2) uses (4) - it is actually unclear how the distinction between
the two mapping APIs is to be drawn and on what basis one should
choose which one to use.
I am still waiting to grab some HW to debug this report but the issue
here is that we are mapping an OpRegion SystemMemory with (3) in the
memory space handler and given the patch we are reverting we end up
mapping the operation region with normal non-cacheable memory attributes
that probably the physical address range behind the OpRegion does not
support.
If that is the case, there needs to be a mechanism to decide what kind
of mapping to use for SystemMemory operation regions based on the type
of physical memory the address range in question is located in.
Thank you Rafael. The mechanism we are currently relying on is the EFI
memory map but if the Opregion address is not described there then we
are left with a default choice to make (theoretically I may also parse
all _CRS in the namespace to find whether a resource include the
Opregion and I may infer attributes from the _CRS resource entry).
Maybe we should update the ACPI specs to enforce it; with current
firmware the idea of using the OS expected *usage* of memory (ie
memory vs IO) described by the mapping function prototype can't work
as this revert shows (even though it would be better if I manage
to find what the precise issue is).
We can't map something with specific attributes if we don't know
whether the physical address space backing the region supports it.
I am left with little choice: I assume the best thing I could do
to fix the original bug is to use ioremap_* in acpi_data_show()
instead of acpi_os_map/unmap_memory() to map that memory with
specific attributes (for BERT error regions, they must be RAM
so, _hopefully_, we know it can be mapped with eg normal memory
mappings).
Thoughts ?
Thanks a lot,
Lorenzo
quoted
quoted
Question is: is the mapping in acpi_ex_system_memory_space_handler()
wrong (and should be patched with acpi_os_map_iomem() ?)
On x86 this should not change a thing, on ARM it would.
I don't think it is right to map SystemMemory Operation regions with
memory semantics but on the other hand, other than the EFI memory map,
there is nothing we can do to determine what a SystemMemory Operation
region address space actually represents.
Thoughts ? Before embarking on patching
acpi_ex_system_memory_space_handler()
I want to make sure my understanding of the SystemMemory space is
correct, comments welcome.
I will pinpoint the trigger for this bug shortly and before doing
anything else.
Thanks,
Lorenzo
On Tue, 21 Sept 2021 at 12:05, Lorenzo Pieralisi
[off-list ref] wrote:
On Mon, Sep 20, 2021 at 07:32:56PM +0200, Rafael J. Wysocki wrote:
quoted
On Mon, Sep 20, 2021 at 7:03 PM Lorenzo Pieralisi
[off-list ref] wrote:
quoted
On Thu, Sep 16, 2021 at 05:08:27PM +0100, Lorenzo Pieralisi wrote:
quoted
On Fri, Sep 10, 2021 at 07:28:49PM +0200, Ard Biesheuvel wrote:
quoted
On Fri, 10 Sept 2021 at 16:32, Jia He [off-list ref] wrote:
quoted
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Can we try to find the root cause first? -rc1 is not even out yet, and
reverting it now means we can not resubmit it until the next merge
window.
I am waiting to debug this on an eMAG but I noticed something that
I wanted to bring up.
SystemMemory Operation region handler - ie
acpi_ex_system_memory_space_handler()
maps the Operation Region (that AFAICS is MMIO, it is _not_ memory)
with acpi_os_map_memory() and I believe that's what is causing this
bug.
On the other hand, acpi_os_map_generic_address(), to handle spaceid
ACPI_ADR_SPACE_SYSTEM_MEMORY, uses acpi_os_map_iomem() that is more
in line with my expectations.
Hi Rafael,
I wanted to ask please if you have any insights on why
(1) acpi_ex_system_memory_space_handler()
(2) acpi_os_map_generic_address()
Use two different calls to map memory for the _same_ address space ID
(SystemMemory).
(3) acpi_os_map_memory()
vs
(4) acpi_os_map_iomem()
I don't really have a good answer here.
On x86 this doesn't really matter and that's where
acpi_ex_system_memory_space_handler() was first introduced. It is not
only used for IOMEM (there are SystemMemory operation regions in RAM),
but since it may be in IOMEM, it should assume so.
quoted
I am struggling to understand why (1) uses (3) ("memory semantics") when
(2) uses (4) - it is actually unclear how the distinction between
the two mapping APIs is to be drawn and on what basis one should
choose which one to use.
I am still waiting to grab some HW to debug this report but the issue
here is that we are mapping an OpRegion SystemMemory with (3) in the
memory space handler and given the patch we are reverting we end up
mapping the operation region with normal non-cacheable memory attributes
that probably the physical address range behind the OpRegion does not
support.
If that is the case, there needs to be a mechanism to decide what kind
of mapping to use for SystemMemory operation regions based on the type
of physical memory the address range in question is located in.
Thank you Rafael. The mechanism we are currently relying on is the EFI
memory map but if the Opregion address is not described there then we
are left with a default choice to make (theoretically I may also parse
all _CRS in the namespace to find whether a resource include the
Opregion and I may infer attributes from the _CRS resource entry).
I'm not sure that would help, as I would expected the memory described
by _CRS to be mostly mutually exclusive from memory used by OpRegions.
Maybe we should update the ACPI specs to enforce it; with current
firmware the idea of using the OS expected *usage* of memory (ie
memory vs IO) described by the mapping function prototype can't work
as this revert shows (even though it would be better if I manage
to find what the precise issue is).
We can't map something with specific attributes if we don't know
whether the physical address space backing the region supports it.
We don't have a a safe default in either direction, so I agree this is
a hole in the specs.
I am left with little choice: I assume the best thing I could do
to fix the original bug is to use ioremap_* in acpi_data_show()
instead of acpi_os_map/unmap_memory() to map that memory with
specific attributes (for BERT error regions, they must be RAM
so, _hopefully_, we know it can be mapped with eg normal memory
mappings).
Thoughts ?
One thing I just realized is that the EFI memory map is not a complete
solution to begin with, as it may not cover hot/coldplugged memory
regions that are only described via ACPI.
Did you make any progress with the eMAG?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Lorenzo Pieralisi <hidden> Date: 2021-09-22 13:07:11
On Wed, Sep 22, 2021 at 01:11:26PM +0200, Ard Biesheuvel wrote:
On Tue, 21 Sept 2021 at 12:05, Lorenzo Pieralisi
[off-list ref] wrote:
quoted
On Mon, Sep 20, 2021 at 07:32:56PM +0200, Rafael J. Wysocki wrote:
quoted
On Mon, Sep 20, 2021 at 7:03 PM Lorenzo Pieralisi
[off-list ref] wrote:
quoted
On Thu, Sep 16, 2021 at 05:08:27PM +0100, Lorenzo Pieralisi wrote:
quoted
On Fri, Sep 10, 2021 at 07:28:49PM +0200, Ard Biesheuvel wrote:
quoted
On Fri, 10 Sept 2021 at 16:32, Jia He [off-list ref] wrote:
quoted
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Can we try to find the root cause first? -rc1 is not even out yet, and
reverting it now means we can not resubmit it until the next merge
window.
I am waiting to debug this on an eMAG but I noticed something that
I wanted to bring up.
SystemMemory Operation region handler - ie
acpi_ex_system_memory_space_handler()
maps the Operation Region (that AFAICS is MMIO, it is _not_ memory)
with acpi_os_map_memory() and I believe that's what is causing this
bug.
On the other hand, acpi_os_map_generic_address(), to handle spaceid
ACPI_ADR_SPACE_SYSTEM_MEMORY, uses acpi_os_map_iomem() that is more
in line with my expectations.
Hi Rafael,
I wanted to ask please if you have any insights on why
(1) acpi_ex_system_memory_space_handler()
(2) acpi_os_map_generic_address()
Use two different calls to map memory for the _same_ address space ID
(SystemMemory).
(3) acpi_os_map_memory()
vs
(4) acpi_os_map_iomem()
I don't really have a good answer here.
On x86 this doesn't really matter and that's where
acpi_ex_system_memory_space_handler() was first introduced. It is not
only used for IOMEM (there are SystemMemory operation regions in RAM),
but since it may be in IOMEM, it should assume so.
quoted
I am struggling to understand why (1) uses (3) ("memory semantics") when
(2) uses (4) - it is actually unclear how the distinction between
the two mapping APIs is to be drawn and on what basis one should
choose which one to use.
I am still waiting to grab some HW to debug this report but the issue
here is that we are mapping an OpRegion SystemMemory with (3) in the
memory space handler and given the patch we are reverting we end up
mapping the operation region with normal non-cacheable memory attributes
that probably the physical address range behind the OpRegion does not
support.
If that is the case, there needs to be a mechanism to decide what kind
of mapping to use for SystemMemory operation regions based on the type
of physical memory the address range in question is located in.
Thank you Rafael. The mechanism we are currently relying on is the EFI
memory map but if the Opregion address is not described there then we
are left with a default choice to make (theoretically I may also parse
all _CRS in the namespace to find whether a resource include the
Opregion and I may infer attributes from the _CRS resource entry).
I'm not sure that would help, as I would expected the memory described
by _CRS to be mostly mutually exclusive from memory used by OpRegions.
quoted
Maybe we should update the ACPI specs to enforce it; with current
firmware the idea of using the OS expected *usage* of memory (ie
memory vs IO) described by the mapping function prototype can't work
as this revert shows (even though it would be better if I manage
to find what the precise issue is).
We can't map something with specific attributes if we don't know
whether the physical address space backing the region supports it.
We don't have a a safe default in either direction, so I agree this is
a hole in the specs.
quoted
I am left with little choice: I assume the best thing I could do
to fix the original bug is to use ioremap_* in acpi_data_show()
instead of acpi_os_map/unmap_memory() to map that memory with
specific attributes (for BERT error regions, they must be RAM
so, _hopefully_, we know it can be mapped with eg normal memory
mappings).
Thoughts ?
One thing I just realized is that the EFI memory map is not a complete
solution to begin with, as it may not cover hot/coldplugged memory
regions that are only described via ACPI.
Did you make any progress with the eMAG?
I manage to get the ACPI tables dump. The fault is triggered on
a SystemMemory OPregion access (FYI - should be a reset register),
probably (but on this only Ampere can help us) because the MMIO
range in question does not support the AXI attributes assigned
by the NormalNC mapping.
I believe mapping SystemMemory Opregions as NormalNC does not make
much sense anyway.
The UEFI specs seem to hint that the ACPI Op-region cacheability
attributes must be determined through the UEFI memory map, not
sure whether that means that the OpRegion itself _must_ be in
the EFI memory map.
I believe we need to go on with the revert and find a way to fix the
BERT error region mappings, to make them NormalNC so that we can do
unaligned accesses on them.
What to do specs side - to be debated, we have to do something because
it is impossible to handle it sensibly otherwise.
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Lorenzo Pieralisi <hidden> Date: 2021-09-22 16:33:45
On Fri, Sep 10, 2021 at 10:32:23PM +0800, Jia He wrote:
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Fixes: 437b38c51162 ("ACPI: Add memory semantics to acpi_os_map_memory()")
Cc: Lorenzo Pieralisi <redacted>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Hanjun Guo <guohanjun@huawei.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rafael J. Wysocki <redacted>
Cc: Harb Abdulhamid <redacted>
Signed-off-by: Jia He <redacted>
Rewrote the commit log, please take the patch below and repost
it as a v3.
It would still be great if Ampere can help us understand why
the NormalNC attributes trigger a sync abort on the opregion
before merging it.
-- >8 --
Subject: [PATCH] Revert "ACPI: Add memory semantics to acpi_os_map_memory()"
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
The memory semantics added in commit 437b38c51162 causes SystemMemory
Operation region, whose address range is not described in the EFI memory
map to be mapped as NormalNC memory on arm64 platforms (through
acpi_os_map_memory() in acpi_ex_system_memory_space_handler()).
This triggers the following abort on an ARM64 Ampere eMAG machine,
because presumably the physical address range area backing the Opregion
does not support NormalNC memory attributes driven on the bus.
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
If the Opregion address range is not present in the EFI memory map there
is no way for us to determine the memory attributes to use to map it -
defaulting to NormalNC does not work (and it is not correct on a memory
region that may have read side-effects) and therefore commit
437b38c51162 should be reverted, which means reverting back to the
original behavior whereby address ranges that are mapped using
acpi_os_map_memory() default to the safe devicenGnRnE attributes on
ARM64 if the mapped address range is not defined in the EFI memory map.
Fixes: 437b38c51162 ("ACPI: Add memory semantics to acpi_os_map_memory()")
Signed-off-by: Jia He <redacted>
Cc: Lorenzo Pieralisi <redacted>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Hanjun Guo <guohanjun@huawei.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Rafael J. Wysocki <redacted>
Cc: Harb Abdulhamid <redacted>
---
arch/arm64/include/asm/acpi.h | 3 ---
arch/arm64/kernel/acpi.c | 19 +++----------------
drivers/acpi/osl.c | 23 +++++++----------------
include/acpi/acpi_io.h | 8 --------
4 files changed, 10 insertions(+), 43 deletions(-)
From: Jeremy Linton <hidden> Date: 2021-09-22 23:45:44
Hi,
On 9/22/21 6:11 AM, Ard Biesheuvel wrote:
On Tue, 21 Sept 2021 at 12:05, Lorenzo Pieralisi
[off-list ref] wrote:
quoted
On Mon, Sep 20, 2021 at 07:32:56PM +0200, Rafael J. Wysocki wrote:
quoted
On Mon, Sep 20, 2021 at 7:03 PM Lorenzo Pieralisi
[off-list ref] wrote:
quoted
On Thu, Sep 16, 2021 at 05:08:27PM +0100, Lorenzo Pieralisi wrote:
quoted
On Fri, Sep 10, 2021 at 07:28:49PM +0200, Ard Biesheuvel wrote:
quoted
On Fri, 10 Sept 2021 at 16:32, Jia He [off-list ref] wrote:
quoted
This reverts commit 437b38c51162f8b87beb28a833c4d5dc85fa864e.
After this commit, a boot panic is alway hit on an Ampere EMAG server
with call trace as follows:
Internal error: synchronous external abort: 96000410 [#1] SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0+ #462
Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 0.14 02/22/2019
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[...snip...]
Call trace:
acpi_ex_system_memory_space_handler+0x26c/0x2c8
acpi_ev_address_space_dispatch+0x228/0x2c4
acpi_ex_access_region+0x114/0x268
acpi_ex_field_datum_io+0x128/0x1b8
acpi_ex_extract_from_field+0x14c/0x2ac
acpi_ex_read_data_from_field+0x190/0x1b8
acpi_ex_resolve_node_to_value+0x1ec/0x288
acpi_ex_resolve_to_value+0x250/0x274
acpi_ds_evaluate_name_path+0xac/0x124
acpi_ds_exec_end_op+0x90/0x410
acpi_ps_parse_loop+0x4ac/0x5d8
acpi_ps_parse_aml+0xe0/0x2c8
acpi_ps_execute_method+0x19c/0x1ac
acpi_ns_evaluate+0x1f8/0x26c
acpi_ns_init_one_device+0x104/0x140
acpi_ns_walk_namespace+0x158/0x1d0
acpi_ns_initialize_devices+0x194/0x218
acpi_initialize_objects+0x48/0x50
acpi_init+0xe0/0x498
As mentioned by Lorenzo:
"We are forcing memory semantics mappings to PROT_NORMAL_NC, which
eMAG does not like at all and I'd need to understand why. It looks
like the issue happen in SystemMemory Opregion handler."
Hence just revert it before everything is clear.
Can we try to find the root cause first? -rc1 is not even out yet, and
reverting it now means we can not resubmit it until the next merge
window.
I am waiting to debug this on an eMAG but I noticed something that
I wanted to bring up.
SystemMemory Operation region handler - ie
acpi_ex_system_memory_space_handler()
maps the Operation Region (that AFAICS is MMIO, it is _not_ memory)
with acpi_os_map_memory() and I believe that's what is causing this
bug.
On the other hand, acpi_os_map_generic_address(), to handle spaceid
ACPI_ADR_SPACE_SYSTEM_MEMORY, uses acpi_os_map_iomem() that is more
in line with my expectations.
Hi Rafael,
I wanted to ask please if you have any insights on why
(1) acpi_ex_system_memory_space_handler()
(2) acpi_os_map_generic_address()
Use two different calls to map memory for the _same_ address space ID
(SystemMemory).
(3) acpi_os_map_memory()
vs
(4) acpi_os_map_iomem()
I don't really have a good answer here.
On x86 this doesn't really matter and that's where
acpi_ex_system_memory_space_handler() was first introduced. It is not
only used for IOMEM (there are SystemMemory operation regions in RAM),
but since it may be in IOMEM, it should assume so.
quoted
I am struggling to understand why (1) uses (3) ("memory semantics") when
(2) uses (4) - it is actually unclear how the distinction between
the two mapping APIs is to be drawn and on what basis one should
choose which one to use.
I am still waiting to grab some HW to debug this report but the issue
here is that we are mapping an OpRegion SystemMemory with (3) in the
memory space handler and given the patch we are reverting we end up
mapping the operation region with normal non-cacheable memory attributes
that probably the physical address range behind the OpRegion does not
support.
If that is the case, there needs to be a mechanism to decide what kind
of mapping to use for SystemMemory operation regions based on the type
of physical memory the address range in question is located in.
Thank you Rafael. The mechanism we are currently relying on is the EFI
memory map but if the Opregion address is not described there then we
are left with a default choice to make (theoretically I may also parse
all _CRS in the namespace to find whether a resource include the
Opregion and I may infer attributes from the _CRS resource entry).
I'm not sure that would help, as I would expected the memory described
by _CRS to be mostly mutually exclusive from memory used by OpRegions.
quoted
Maybe we should update the ACPI specs to enforce it; with current
firmware the idea of using the OS expected *usage* of memory (ie
memory vs IO) described by the mapping function prototype can't work
as this revert shows (even though it would be better if I manage
to find what the precise issue is).
We can't map something with specific attributes if we don't know
whether the physical address space backing the region supports it.
We don't have a a safe default in either direction, so I agree this is
a hole in the specs.
I just debugged down to this patch because of boot failures with the rpi4. Then I found this thread.
I had always assumed that SystemIO was x86/PIO, and SystemMemory was defined as MMIO but on arm we would have to determine if the memory region was described in the uefi memory map as actual system ram, and if not assume device memory. I was looking at tweaking acpi_map() to check that similar to what is happening on !arm64.
Gotta run, but throwing that out there since it seems a possible fix without creating a new OpregionType to differentiate actual memory vs MMIO.
quoted
I am left with little choice: I assume the best thing I could do
to fix the original bug is to use ioremap_* in acpi_data_show()
instead of acpi_os_map/unmap_memory() to map that memory with
specific attributes (for BERT error regions, they must be RAM
so, _hopefully_, we know it can be mapped with eg normal memory
mappings).
Thoughts ?
One thing I just realized is that the EFI memory map is not a complete
solution to begin with, as it may not cover hot/coldplugged memory
regions that are only described via ACPI.
Did you make any progress with the eMAG?