Re: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel

From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Borislav Petkov <bp@alien8.de>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	"Zheng, Lv" <lv.zheng@intel.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
	"Moore, Robert" <robert.moore@intel.com>,
	"J?rg R?del" <joro@8bytes.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Linux ACPI <linux-acpi@vger.kernel.org>
Subject: Re: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel
Date: Tue, 10 Jan 2017 02:27:16 +0100	[thread overview]
Message-ID: <CAJZ5v0gRYcC5fJZF207iyehvPR9_2zqprSvWHA_Qt93W9njqAw@mail.gmail.com> (raw)
In-Reply-To: <20170109235211.yetponvbexvalkir@pd.tnic>

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

On Tue, Jan 10, 2017 at 12:52 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Jan 10, 2017 at 12:40:39AM +0100, Borislav Petkov wrote:
>> Lemme run it.
>
> Well, it boots but I get:
>
> [    0.291447] ------------[ cut here ]------------
> [    0.291702] WARNING: CPU: 0 PID: 1 at kernel/rcu/tree.c:3993 rcu_scheduler_starting+0x5c/0x70
> [    0.292107] Modules linked in:
> [    0.292277] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.10.0-rc3+ #21
> [    0.292540] Hardware name: HP HP EliteBook 745 G3/807E, BIOS N73 Ver. 01.08 01/28/2016
> [    0.292893] Call Trace:
> [    0.293072]  ? dump_stack+0x46/0x63
> [    0.293285]  ? __warn+0xec/0x110
> [    0.293487]  ? rcu_scheduler_starting+0x5c/0x70
> [    0.293735]  ? kernel_init_freeable+0x58/0x19a
> [    0.293976]  ? rest_init+0x80/0x80
> [    0.294153]  ? kernel_init+0xa/0x100
> [    0.294334]  ? ret_from_fork+0x22/0x30
> [    0.294525] ---[ end trace 4c0fe009ed4dc740 ]---
>
> TBH, I like Rafael's suggestion in the other mail to stick with fixing
> this in ACPI, especially this is an ACPI problem, not RCU. Well,
> more or less: RCU could be taught to *not* do schedule_work() if
> workqueue_init() hasn't happened yet but that's a tangential.
>
> So, I'm going to bed. When I wake up, I want to see working fixes!
>
> :-)))

Well, if the https://patchwork.kernel.org/patch/9504277/ patch from Lv
worked, the attached one should work too (please test), but it can be
justified in a slightly more convincing way.

Namely, the idea is that acpi_os_read/write_memory() should never be
used before invoking acpi_os_initialize() and since those are the only
places where the list of memory regions is walked under RCU without
extra locking, it is safe to skip the RCU synchronization until that
happens.

Thanks,
Rafael

[-- Attachment #2: acpi-mem-unmap.patch --]
[-- Type: text/x-patch, Size: 1040 bytes --]

---
 drivers/acpi/osl.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/acpi/osl.c
===================================================================

--- linux-pm.orig/drivers/acpi/osl.c
+++ linux-pm/drivers/acpi/osl.c
@@ -378,7 +378,9 @@ static void acpi_os_drop_map_ref(struct
 static void acpi_os_map_cleanup(struct acpi_ioremap *map)
 {
 	if (!map->refcount) {
-		synchronize_rcu_expedited();
+		if (acpi_os_initialized)
+			synchronize_rcu_expedited();
+
 		acpi_unmap(map->phys, map->virt);
 		kfree(map);
 	}
@@ -671,6 +673,8 @@ acpi_os_read_memory(acpi_physical_addres
 	bool unmap = false;
 	u64 dummy;
 
+	WARN_ON_ONCE(!acpi_os_initialized);
+
 	rcu_read_lock();
 	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
 	if (!virt_addr) {
@@ -716,6 +720,8 @@ acpi_os_write_memory(acpi_physical_addre
 	unsigned int size = width / 8;
 	bool unmap = false;
 
+	WARN_ON_ONCE(!acpi_os_initialized);
+
 	rcu_read_lock();
 	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
 	if (!virt_addr) {
