ACPI: generic initramfs table override support #3

This patch set contains two patches:

1) Move populate_rootfs back to start_kernel, so it can be used by acpi_early_init
2) Add the ability to override ACPI tables from the initramfs

I'd like some review for the second patch. I expect objections to what is essentially open coding the top of acpi_load_table in a separate function, but Jan Beulich advised me that modifying code under acpica/ was not recommended. Ideally there would be a way to replace an existing table before it is loaded, or at the very least be able to request the table descriptor instead of just the table header.

I've tested this patch set successfully at home, but I don't have a particularly exotic setup.

Thanks!

-Jeff

--
Jeff Mahoney
SuSE Labs

Mainline commit 8d610dd5 introduced the rootfs_initcall and moved populate_rootfs out of start_kernel. This was because of issues with userspace helpers being executed way too early. Things like pipes weren't initialized yet and users were seeing Oopses or unpredictable behavior in certain circumstances.

The fix worked by causing the execve to fail because it couldn't lookup the helper in the file system since the file system wasn't populate yet. It turns out that's a really late place to fail since the entire usermodehelper infrastructure depends on a work queue that is already checked to see if it has been initialized. We can fail earlier without having to fork threads that will ultimately fail.

This patch moves populate_rootfs back to start_kernel and avoids the race against a very early userspace by moving the initialization of khelper_wq to rootfs_initcall.

This may seem like a small win, but it is essential for my next patch which adds the ability to override ACPI tables at boot-time.

---
 include/linux/init.h |    1 +
 init/initramfs.c     |    3 +--
 init/main.c          |   10 +++++++++-
 kernel/kmod.c        |    1 +
 4 files changed, 12 insertions(+), 3 deletions(-)

@@ -146,6 +146,7 @@ extern unsigned int reset_devices;
 /* used by init/main.c */
 void setup_arch(char **);
 void prepare_namespace(void);
+int populate_rootfs(void);
 
 extern void (*late_time_init)(void);
 

@@ -565,7 +565,7 @@ static void __init clean_rootfs(void)
 }
 #endif
 
-static int __init populate_rootfs(void)
+int __init populate_rootfs(void)
 {
 	char *err = unpack_to_rootfs(__initramfs_start,
 			 __initramfs_end - __initramfs_start);

@@ -605,4 +605,3 @@ static int __init populate_rootfs(void)
 	}
 	return 0;
 }
-rootfs_initcall(populate_rootfs);

@@ -747,6 +747,15 @@ asmlinkage void __init start_kernel(void
 
 	check_bugs();
 
+	/*
+	 * Do this before starting ACPI so we can read-in
+	 * override tables before the tables are actually
+	 * loaded. The usermode helper won't be initialized
+	 * until much later so we don't race against things
+	 * calling out to userspace.
+	 */
+	populate_rootfs();
+
 	acpi_early_init(); /* before LAPIC and SMP init */
 
 	ftrace_init();

@@ -844,7 +853,6 @@ static void __init do_basic_setup(void)
 	rcu_init_sched(); /* needed by module_init stage. */
 	init_workqueues();
 	cpuset_init_smp();
-	usermodehelper_init();
 	init_tmpfs();
 	driver_init();
 	init_irq_proc();

@@ -527,3 +527,4 @@ void __init usermodehelper_init(void)
 	khelper_wq = create_singlethread_workqueue("khelper");
 	BUG_ON(!khelper_wq);
 }
+rootfs_initcall(usermodehelper_init);

This patch allows the system administrator to override ACPI tables with versions provided in an initramfs.

This works by moving the initialization of populate_rootfs earlier in the initialization so that we can use the VFS file system routines. The system is initialized enough to support this by acpi_early_init(). My understanding is that an early version of original patch posted at
http://gaugusch.at/kernel.shtml may have done something similar.

This version provides the infrastructure to override any ACPI table, but only provides support for overriding DSDT. If other tables are desired, extending the support is trivial.

During early ACPI initialization, when the initramfs is still loaded, we go through a table of override entries which specify the name of the table to override, the file name that contains it, and a pointer to the data loaded from the file. The override tables and headers are kept in memory so that they available to the ACPI subsystem after the __init sections and the initramfs have been jettisoned.

Changes:
v2 - Moved code from acpi/acpica/tbxface.c to acpi/osl.c
v3 - Moved init code into a separate patch which handles the race initially
fixed by mainline commit 8d610dd5.

This patch is derived from the work by Éric Piel <[email protected]>.

---

 Documentation/acpi/dsdt-override.txt     |    8 +
 Documentation/acpi/initramfs-add-dsdt.sh |   43 +++++++
 Documentation/acpi/table-override.txt    |   21 +++
 Documentation/kernel-parameters.txt      |    4 
 drivers/acpi/Kconfig                     |   13 ++
 drivers/acpi/bus.c                       |    7 +
 drivers/acpi/osl.c                       |  169 +++++++++++++++++++++++++++++++
 include/acpi/acpiosxf.h                  |    7 +
 8 files changed, 271 insertions(+), 1 deletion(-)

@@ -1,7 +1,13 @@
-Linux supports a method of overriding the BIOS DSDT:
+Linux supports two methods of overriding the BIOS DSDT:
 
 CONFIG_ACPI_CUSTOM_DSDT builds the image into the kernel.
 
+CONFIG_ACPI_CUSTOM_OVERRIDE_INITRAMFS loads the image from
+the initramfs at boot-time. It is more flexible in that it
+does not need to be built into the kernel and tables other
+than DSDT can potentially be overridden. Please see
+Documentation/acpi/table-override.txt for more information.
+
 When to use this method is described in detail on the
 Linux/ACPI home page:
 http://www.lesswatts.org/projects/acpi/overridingDSDT.php

@@ -0,0 +1,21 @@
+CONFIG_ACPI_CUSTOM_OVERRIDE_INITRAMFS provides a mechanism for
+the user to add table images to the initramfs for loading at
+runtime. Tables used before expansion of the initramfs may not
+be replaced. Fortunately this list is small and the one most
+typically used, DSDT, is not one of them.
+
+In order to override a table, the image must be placed in the root
+of the initramfs with a filename of <table-name>.aml (e.g. DSDT.aml).
+
+As the ACPI subsystem initializes, it will load the tables into memory
+and override them as the tables are needed.
+
+This option takes precedence over the in-kernel method provided by
+the ACPI_CUSTOM_DSDT config option.
+
+When to use these methods is described in detail on the
+Linux/ACPI home page:
+http://www.lesswatts.org/projects/acpi/overridingDSDT.php
+
+Documentation/initramfs-add-dsdt.sh is provided for convenience
+for use with the CONFIG_ACPI_CUSTOM_OVERRIDE_INITRAMFS method.

@@ -216,6 +216,10 @@ and is between 256 and 4096 characters.
 
 	acpi_no_auto_ssdt	[HW,ACPI] Disable automatic loading of SSDT
 
+	acpi_no_initrd_override		[KNL,ACPI]
+	acpi_no_initramfs_override	[KNL,ACPI]
+			Disable loading custom ACPI tables from the initramfs
+
 	acpi_os_name=	[HW,ACPI] Tell ACPI BIOS the name of the OS
 			Format: To spoof as Windows 98: ="Microsoft Windows"
 

@@ -233,6 +233,19 @@ config ACPI_CUSTOM_DSDT
 	bool
 	default ACPI_CUSTOM_DSDT_FILE != ""
 
+config ACPI_CUSTOM_OVERRIDE_INITRAMFS
+	bool "Load ACPI override tables from initramfs"
+	depends on BLK_DEV_INITRD
+	default n
+	help
+	  This option supports loading custom replacement tables by optionally
+	  loading them from the initramfs.
+
+	  See Documentation/acpi/table-override.txt
+
+	  If you are not using this feature now, but may use it later,
+	  it is safe to say Y here.
+
 config ACPI_BLACKLIST_YEAR
 	int "Disable ACPI for systems before Jan 1st this year" if X86_32
 	default 0

@@ -669,6 +669,13 @@ void __init acpi_early_init(void)
 		goto error0;
 	}
 
+	status = acpi_load_override_tables();
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_ERR PREFIX
+			"Unable to load Override Tables\n");
+		goto error0;
+	}
+
 	status = acpi_load_tables();
 	if (ACPI_FAILURE(status)) {
 		printk(KERN_ERR PREFIX

@@ -91,6 +91,13 @@ acpi_status
 acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
 			    acpi_string * new_val);
 
+#ifdef CONFIG_ACPI_CUSTOM_OVERRIDE_INITRAMFS
+acpi_status
+acpi_load_override_tables(void);
+#else
+static inline acpi_status acpi_load_override_tables(void) {}
+#endif
+
 acpi_status
 acpi_os_table_override(struct acpi_table_header *existing_table,
 		       struct acpi_table_header **new_table);

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

I'm really unenthusiastic about this. The majority of use cases appear to be people working around Linux bugs and never reporting the bugs to us, leaving everybody worse off.

Matthew Garrett | [email protected]

I can't judge the technical quality of the patch, but I don't think the social side of it is a problem (assuming I correctly understood the change in question).

Case in point: I reported a problem[0] with the thermal/fan subsystems which was left unanswered and forced me to recompile kernels with overridden DSDTs for every new release expecting a fix and eventually replace the laptop in question. I'm obviously not demanding that my problem be solved, but the point is that not all problems can be solved in a timely fashion even when they are reported (for whatever reason:
lack of manpower, lack of test machines, etc), so avoiding this change which could ease the testing of new DSDTs seems a bit sadistic. :)

Another point of view to the same argument is that the current situation doesn't guarantee feedback either. The added hassle of needing a recompile doesn't IMHO necessarily mean users report these sort of problems.

Just my technically inapt 0,2 €. :)

Cheers

[0] http://thread.gmane.org/gmane.linux.acpi.devel/37612

Leo "costela" Antunes
[insert a witty retort here]

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

I agree. In an ideal world this would be unnecessary. It's true that ACPI bugs have become must less common than they used to be but I think users would prefer their machines be usable while the ACPI developers are diagnosing why they need a workaround in the first place.

In some cases it's just that the MS AML compiler is just so much more lax than the Intel one. I had a notebook where I had to extract, fix, and rebuild my DSDT and then it worked fine. Without the ability to override it, I would've been stuck.

BTW, a similar version of this patch was committed to mainline in Feb 2008 though it was reverted shortly after that. Judging from the commit message, it's because it reintroduced the early userspace bugs. I believe this patch addresses that by not allowing the usermode helper to run until rootfs_initcall has been completed.

-Jeff

Jeff Mahoney
SUSE Labs

If that were the tradeoff then I'd be happy, but what tends to happen is that the section of users who are able to help us debug just work around it instead and the ones who have no idea how to report a bug end up with broken machines.

For a user who can do these fixups themselves, it's easy enough to rebuild a kernel with the DSDT override. However, pretty much every recent ACPI bug I've seen has had nothing to do with the MS compiler's less strict error checking - we run the in-kernel interpreter in a lax mode that's intended to be bug for bug compatible with the Microsoft one.

Matthew Garrett | [email protected]

Well then maybe the answer is to make it complain loudly that it's a bug that is being worked around. More than just the taint message. Perhaps even an autogenerated report on the DSDT if it's overridden to be sent at the user's discretion. There has to be a middle ground here. Most distros don't backport minor ACPI fixes for their maintenance kernels. So it's great that it would get fixed in the next release, but it still leaves the user out in the cold in the meantime. Sometimes it's the temperature or battery usage getting misreported. Other times it means the user can't suspend or a docking station won't work.

Sure, it was a few years ago and things have changed. It was just an example of how it fixed my system. I've seen several openSUSE bug reports that get fixed by replacing the DSDT. And yes, I do assign those bugs to our ACPI developers for actual fixing.

The thing is that there are sites out there that provide fixed DSDTs so users don't actually have to go through the effort of extracting, fixing, and rebuilding themselves. Even if they did have to do that, building your own kernel to fix your DSDT kicks you right out of the maintenance stream of your distro. So then they're stuck between rebuilding the kernel at every update or losing security fixes. Neither sound particularly fun.

-Jeff

Jeff Mahoney
SUSE Labs