Bug 18012

Summary:	PM1a registers in MMIO space causes panic
Product:	ACPI	Reporter:	Bjorn Helgaas (bjorn.helgaas)
Component:	Config-Other	Assignee:	Myron Stowe (myron.stowe)
Status:	CLOSED CODE_FIX
Severity:	normal	CC:	lenb, ming.m.lin, myron.stowe, Robert.Moore, rui.zhang, yakui.zhao
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	2.6.36-rc3	Subsystem:
Regression:	No	Bisected commit-id:

Description Bjorn Helgaas 2010-09-07 20:30:08 UTC

The acpi_irq() path reads the PM1a registers.  These registers are in
I/O port space on most machines, and there's no problem in that case.

But these registers may be in MMIO space instead, and in that case,
we use acpi_os_read_memory(), which is not safe in interrupt context
because it depends on ioremap(), which may allocate memory.

Here's a stacktrace from such a machine (this is a prototype, and I
can't supply any details about it):

kernel BUG at .../mm/vmalloc.c:1226!
invalid opcode: 0000 [#1] SMP 
Pid: 1, comm: swapper Tainted: G   M        2.6.35-06116-g73f44ba-dirty #9

Call Trace:
 [<ffffffff810cb575>] get_vm_area_caller+0x39/0x3b
 [<ffffffff81026f76>] __ioremap_caller+0x250/0x2ee
 [<ffffffff8102709d>] ioremap_nocache+0x12/0x14
 [<ffffffff8121b003>] acpi_os_read_memory+0x17/0x61
 [<ffffffff81237775>] acpi_hw_read+0x4c/0xc3
 [<ffffffff81237816>] acpi_hw_read_multiple+0x2a/0x63
 [<ffffffff81237a40>] acpi_hw_register_read+0x53/0xe3
 [<ffffffff8122bda9>] acpi_ev_fixed_event_detect+0x31/0xfe
 [<ffffffff8122cb1c>] acpi_ev_sci_xrupt_handler+0xf/0x22
 [<ffffffff8121a30d>] acpi_irq+0x11/0x2c
 [<ffffffff81086c87>] handle_IRQ_event+0x58/0x130
 [<ffffffff81088900>] handle_fasteoi_irq+0x8c/0xc7
 [<ffffffff81004f2e>] handle_irq+0x86/0x90

Comment 1 ykzhao 2010-09-08 00:26:28 UTC

Will you please attach the output of acpidump/dmesg on your box?

Thanks.

Comment 2 Bjorn Helgaas 2010-09-14 14:59:24 UTC

The stacktrace above is the relevant part of the dmesg log.  Here is
the relevant part of the FADT:

[FACP] 0x36EF1B38 0xF4 bytes
 PM1A_EVNT_BLK  : 0x0
 PM1B_EVNT_BLK  : 0x0
 PM1A_CNT_BLK   : 0x0
 PM1B_CNT_BLK   : 0x0
 PM2_CNT_BLK    : 0x0
 PM_TMR_BLK     : 0x0
 GPE0_BLK       : 0x0
 GPE1_BLK       : 0x0
 PM1_EVNT_LEN   : 0x4
 PM1_CNT_LEN    : 0x2
 PM2_CNT_LEN    : 0x0
 PM_TMR_LEN     : 0x4
 GPE0_BLK_LEN   : 0x20
 GPE1_BLK_LEN   : 0x20
 X_PM1a_EVT_BLK : 0xFE7E206000802002000
 X_PM1b_EVT_BLK : 0x1
 X_PM1a_CNT_BLK : 0xFE7E206000C01001000
 X_PM1b_CNT_BLK : 0x1
 X_PM2_CNT_BLK  : 0x1
 X_PM_TMR_BLK   : 0xFE7E206000403002000
 X_GPE0_BLK     : 0xFE7E206001001000000
 X_GPE1_BLK     : 0xFE7E206003001000000

Comment 3 Lin Ming 2010-09-30 06:04:49 UTC

We may need to rewrite acpi_os_read_memory with atomic mmio accessing method implemented in commit 15651291a(ACPI, IO memory pre-mapping and atomic accessing)

see http://marc.info/?l=git-commits-head&m=127508754831493&w=2

Comment 4 Bjorn Helgaas 2010-09-30 15:17:55 UTC

Yes, I think if we integrate some of the atomicio code into
acpi_os_map_memory() and acpi_os_read_memory(), we can have the
property that "acpi_os_read_memory() is safe in interrupt context
if acpi_os_map_memory() has been done previously."

That would fix this problem and also let us get rid of atomicio.

Myron has much of this already coded, and it looks feasible so far.

Comment 5 Myron Stowe 2010-10-21 22:25:02 UTC

I posted the initial patch series that addresses this issue.  Much like Bjorn hinted at in Comment #4 - The functionality of 'atomicio.c' has been re-factored into './drivers/acpi/osl.c' which now provides equivalent functionality but in a more generalized manner allowing usage in non-specific contexts.

 drivers/acpi/Makefile         |    1 
 drivers/acpi/apei/apei-base.c |   11 +
 drivers/acpi/apei/ghes.c      |    9 -
 drivers/acpi/atomicio.c       |  361 -----------------------------------------
 include/acpi/atomicio.h       |   10 -
 5 files changed, 9 insertions(+), 383 deletions(-)
 delete mode 100644 drivers/acpi/atomicio.c
 delete mode 100644 include/acpi/atomicio.h

see http://marc.info/?l=linux-acpi&m=128769263327206&w=2

Comment 6 Myron Stowe 2010-10-21 22:30:09 UTC

Darn - in Comment #5 I posted the diffstat for patch 7/7.  I meant to post it for the entire patch series - here is the diffstat for the entire series.

 drivers/acpi/Makefile         |    1 
 drivers/acpi/apei/apei-base.c |   11 +
 drivers/acpi/apei/ghes.c      |    9 -
 drivers/acpi/atomicio.c       |  361 -----------------------------------------
 drivers/acpi/osl.c            |  276 ++++++++++++++++++++++++++-----
 include/acpi/atomicio.h       |   10 -
 include/linux/acpi.h          |    3 
 7 files changed, 242 insertions(+), 429 deletions(-)
 delete mode 100644 drivers/acpi/atomicio.c
 delete mode 100644 include/acpi/atomicio.h

Comment 7 Zhang Rui 2010-12-27 01:28:46 UTC

Ming, any update on this one?

Comment 8 Len Brown 2011-01-18 05:03:33 UTC

I believe that this issue was addressed by Myron's series
that went into 2.6.37, including:

commit 620242ae8c3d9c0b1a77451744fb2d855d1e7342
Author: Myron Stowe <myron.stowe@hp.com>
Date:   Thu Oct 21 14:23:53 2010 -0600

    ACPI: Maintain a list of ACPI memory mapped I/O remappings

Please re-open if there is still a problem with
the upstream kernel.