From: Al Stone al.stone@linaro.org
This series of patches starts with Hanjun's patch to create a kernel config item for CONFIG_ACPI_REDUCED_HARDWARE [0]. Building on that, I then reviewed all of the code that touched any of several fields in the FADT that the OSPM is supposed to ignore when ACPI is in Hardware Reduced mode [1]. Any time there was a use of one of the fields to be ignored, I evaluated whether or not the code was implementing Hardware Reduced mode correctly. Similarly, for each the flags in the FADT flags field that are to be ignored in Hardware Reduced mode, the kernel code was again scanned for proper usage. The remainder of the patches are to fix all of the situations I could find where the kernel would not behave correctly in this ACPI mode.
These seem to work just fine on the RTSM model for ARMv7, both with and without ACPI enabled, and with and without ACPI_REDUCED_HARDWARE enabled; similarly for the FVP model for ARMv8. The patches for ACPI on ARM hardware have been submitted elsewhere but they presume that reduced HW mode is functioning correctly. In the meantime, there's no way I can think of to test all possible scenarios so feedback would be greatly appreciated.
[0] List at https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI/AcpiReducedHw#Section_5:... [1] Please see the ACPI Specification v5.0 for details on Hardware Reduced mode (sections 3.11.1, 4.1, 5.2.9, at a minimum).
Changes for v4: -- Given the current state of ACPICA, disable CONFIG_ACPI_REDUCED_HARDWARE for use on anything other than ARM. -- Replaced #ifdefs with run-time checking for hardware reduced mode, whenever possible
Changes for v3: -- Modified enabling ACPI_REDUCED_HARDWARE in ACPICA when using kernel config item CONFIG_ACPI_REDUCED_HARDWARE; now consistent with ACPICA code base where needed -- Enable X86 for CONFIG_ACPI_REDUCED_HARDWARE -- Minimize bus master reload patching -- Remove unneeded patch for dmi_check_system() (was 4/6) -- Correct the patch for removing unneeded map/unmap of FADT fields
Changes for v2: -- Remove patch that was outside of reduced HW mode changes -- Simplify CONFIG_ACPI_REDUCED_HARDWARE in Kconfig -- Simplify use of CONFIG_ACPI_REDUCED_HARDWARE in #ifdefs -- Ensure changelogs are present -- Combine and simplify previous patches 8 & 10
Al Stone (5): ACPI: introduce CONFIG_ACPI_REDUCED_HARDWARE to enable this ACPI mode ACPI: bus master reload not supported in reduced HW mode ACPI: HW reduced mode does not allow use of the FADT sci_interrupt field ACPI: in HW reduced mode, using FADT PM information is not allowed. ACPI: do not map/unmap memory regions for FADT entries in reduced HW mode
drivers/acpi/Kconfig | 8 ++++++++ drivers/acpi/bus.c | 30 ++++++++++++++++-------------- drivers/acpi/osl.c | 38 +++++++++++++++++++------------------- drivers/acpi/pci_link.c | 2 ++ drivers/acpi/processor_idle.c | 14 ++++++++++++-- include/acpi/platform/aclinux.h | 6 ++++++ 6 files changed, 63 insertions(+), 35 deletions(-)
From: Al Stone al.stone@linaro.org
Hardware reduced mode, despite the name, exists primarily to allow newer platforms to use a much simpler form of ACPI that does not require supporting the legacy of previous versions of the specification. This mode was first introduced in the ACPI 5.0 specification, but because it is so much simpler and reduces the size of the object code needed to support ACPI, it is likely to be used more often in the near future.
To enable the hardware reduced mode of ACPI on some platforms (such as ARM), we need to modify the kernel code and set ACPI_REDUCED_HARDWARE to TRUE in the ACPICA source. For ARM/ARM64, hardware reduced ACPI should be the only mode used; legacy mode would require modifications to SoCs in order to provide several x86-specific hardware features (e.g., an NMI and SMI support).
We set ACPI_REDUCED_HARDWARE to TRUE in the ACPICA source by introducing a kernel config item to enable/disable ACPI_REDUCED_HARDWARE. We can then change the kernel config instead of having to modify the kernel source directly to enable the reduced hardware mode of ACPI.
Lv Zheng suggested that this configuration item does not belong in ACPICA, the upstream source for much of the ACPI internals, but rather to the Linux kernel itself. Hence, we introduce this flag so that we can make ACPI_REDUCED_HARDWARE configurable. For the details of the discussion, please refer to: http://www.spinics.net/lists/linux-acpi/msg46369.html
Even though support for X86 in hardware reduced mode is possible, it is NOT enabled. Extensive effort has gone into the Linux kernel so that there is a single kernel image than can run on all x86 hardware; the kernel changes run-time behavior to adapt to the hardware being used. This is not currently possible with the existing ACPICA infrastructure but only presents a problem on achitectures supporting both hardware-reduced and legacy modes of ACPI -- i.e., on x86 only.
The problem with the current ACPICA code base is that if one builds legacy ACPI (a proper superset of hardware-reduced), the kernel can run in hardware- reduced with the proper ACPI tables, but there is still ACPICA code that could be executed even though it is not allowed by the specification. If one builds a hardware-reduced only ACPI, the kernel cannot run with ACPI tables that are for legacy mode. To ensure compliance with ACPI, one must therefore build two separate kernels. Once this problem has been properly fixed, we can then enable x86 hardware-reduced mode and use a single kernel.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Al Stone al.stone@linaro.org --- drivers/acpi/Kconfig | 8 ++++++++ include/acpi/platform/aclinux.h | 6 ++++++ 2 files changed, 14 insertions(+)
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 5d92485..53f0f16 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -343,6 +343,14 @@ config ACPI_BGRT data from the firmware boot splash. It will appear under /sys/firmware/acpi/bgrt/ .
+config ACPI_REDUCED_HARDWARE + bool "Hardware-reduced ACPI support" + depends on ARM || ARM64 + help + This config adds support for Hardware-reduced ACPI. When this option + is selected, will generate a specialized version of ACPICA that ONLY + supports the ACPI "reduced hardware". + source "drivers/acpi/apei/Kconfig"
config ACPI_EXTLOG diff --git a/include/acpi/platform/aclinux.h b/include/acpi/platform/aclinux.h index 28f4f4d..a33f502 100644 --- a/include/acpi/platform/aclinux.h +++ b/include/acpi/platform/aclinux.h @@ -52,6 +52,12 @@
#ifdef __KERNEL__
+/* Compile for reduced hardware mode if requested for this kernel config */ + +#ifdef CONFIG_ACPI_REDUCED_HARDWARE +#define ACPI_REDUCED_HARDWARE 1 +#endif + #include <linux/string.h> #include <linux/kernel.h> #include <linux/ctype.h>
On Mon, Dec 16, 2013 at 05:16:37PM -0700, al.stone@linaro.org wrote:
+config ACPI_REDUCED_HARDWARE
- bool "Hardware-reduced ACPI support"
- depends on ARM || ARM64
- help
- This config adds support for Hardware-reduced ACPI.
Well, that's not really true - most of the hardware-reduced ACPI support seems to be built even if this isn't turned on. Maybe ACPI_REDUCED_HARDWARE_ONLY ?
On 12/26/2013 09:58 PM, Matthew Garrett wrote:
On Mon, Dec 16, 2013 at 05:16:37PM -0700, al.stone@linaro.org wrote:
+config ACPI_REDUCED_HARDWARE
- bool "Hardware-reduced ACPI support"
- depends on ARM || ARM64
- help
- This config adds support for Hardware-reduced ACPI.
Well, that's not really true - most of the hardware-reduced ACPI support seems to be built even if this isn't turned on. Maybe ACPI_REDUCED_HARDWARE_ONLY ?
Aha. Good point; semantically, it is unclear as is. I'll clarify and re-submit.
From: Al Stone al.stone@linaro.org
Do not save and restore bus master reload registers in suspend/resume when in reduced HW mode; according to the spec, no such registers should exist
Signed-off-by: Al Stone al.stone@linaro.org --- drivers/acpi/processor_idle.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 86d11a6..b91dcaa 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -229,12 +229,14 @@ static struct syscore_ops acpi_processor_syscore_ops = {
void acpi_processor_syscore_init(void) { - register_syscore_ops(&acpi_processor_syscore_ops); + if (!acpi_gbl_reduced_hardware) + register_syscore_ops(&acpi_processor_syscore_ops); }
void acpi_processor_syscore_exit(void) { - unregister_syscore_ops(&acpi_processor_syscore_ops); + if (!acpi_gbl_reduced_hardware) + unregister_syscore_ops(&acpi_processor_syscore_ops); } #endif /* CONFIG_PM_SLEEP */
From: Al Stone al.stone@linaro.org
In HW reduced mode, the use of the SCI interrupt is not allowed. In all those places that use the FADT sci_interrupt field, make sure we do not execute that path when in HW reduced mode.
In the case of acpi_os_install_interrupt_handler() in osl.c, this allows us to open up the routine to installing interrupt handlers other than acpi_gbl_FADT.sci_interrupt regardless of whether we are in ACPI legacy mode or reduced HW mode; acpi_os_remove_interrupt_handler() changes to maintain symmetry.
Signed-off-by: Al Stone al.stone@linaro.org --- drivers/acpi/bus.c | 30 ++++++++++++++++-------------- drivers/acpi/osl.c | 18 +++++++----------- drivers/acpi/pci_link.c | 2 ++ 3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index bba9b72..33ff87f 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -542,21 +542,23 @@ void __init acpi_early_init(void) }
#ifdef CONFIG_X86 - if (!acpi_ioapic) { - /* compatible (0) means level (3) */ - if (!(acpi_sci_flags & ACPI_MADT_TRIGGER_MASK)) { - acpi_sci_flags &= ~ACPI_MADT_TRIGGER_MASK; - acpi_sci_flags |= ACPI_MADT_TRIGGER_LEVEL; + if (!acpi_gbl_reduced_hardware) { + if (!acpi_ioapic) { + /* compatible (0) means level (3) */ + if (!(acpi_sci_flags & ACPI_MADT_TRIGGER_MASK)) { + acpi_sci_flags &= ~ACPI_MADT_TRIGGER_MASK; + acpi_sci_flags |= ACPI_MADT_TRIGGER_LEVEL; + } + /* Set PIC-mode SCI trigger type */ + acpi_pic_sci_set_trigger(acpi_gbl_FADT.sci_interrupt, + (acpi_sci_flags & ACPI_MADT_TRIGGER_MASK) >> 2); + } else { + /* + * now that acpi_gbl_FADT is initialized, + * update it with result from INT_SRC_OVR parsing + */ + acpi_gbl_FADT.sci_interrupt = acpi_sci_override_gsi; } - /* Set PIC-mode SCI trigger type */ - acpi_pic_sci_set_trigger(acpi_gbl_FADT.sci_interrupt, - (acpi_sci_flags & ACPI_MADT_TRIGGER_MASK) >> 2); - } else { - /* - * now that acpi_gbl_FADT is initialized, - * update it with result from INT_SRC_OVR parsing - */ - acpi_gbl_FADT.sci_interrupt = acpi_sci_override_gsi; } #endif
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 44c07eb..c946a3a 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -84,6 +84,7 @@ static int (*__acpi_os_prepare_extended_sleep)(u8 sleep_state, u32 val_a,
static acpi_osd_handler acpi_irq_handler; static void *acpi_irq_context; +static u32 acpi_irq_number; static struct workqueue_struct *kacpid_wq; static struct workqueue_struct *kacpi_notify_wq; static struct workqueue_struct *kacpi_hotplug_wq; @@ -178,6 +179,10 @@ static void __init acpi_request_region (struct acpi_generic_address *gas,
static int __init acpi_reserve_resources(void) { + /* Handle hardware reduced mode: i.e., do nothing. */ + if (acpi_gbl_reduced_hardware) + return 0; + acpi_request_region(&acpi_gbl_FADT.xpm1a_event_block, acpi_gbl_FADT.pm1_event_length, "ACPI PM1a_EVT_BLK");
@@ -795,13 +800,6 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler handler,
acpi_irq_stats_init();
- /* - * ACPI interrupts different from the SCI in our copy of the FADT are - * not supported. - */ - if (gsi != acpi_gbl_FADT.sci_interrupt) - return AE_BAD_PARAMETER; - if (acpi_irq_handler) return AE_ALREADY_ACQUIRED;
@@ -818,15 +816,13 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler handler, acpi_irq_handler = NULL; return AE_NOT_ACQUIRED; } + acpi_irq_number = irq;
return AE_OK; }
acpi_status acpi_os_remove_interrupt_handler(u32 irq, acpi_osd_handler handler) { - if (irq != acpi_gbl_FADT.sci_interrupt) - return AE_BAD_PARAMETER; - free_irq(irq, acpi_irq); acpi_irq_handler = NULL;
@@ -1806,7 +1802,7 @@ acpi_status __init acpi_os_initialize1(void) acpi_status acpi_os_terminate(void) { if (acpi_irq_handler) { - acpi_os_remove_interrupt_handler(acpi_gbl_FADT.sci_interrupt, + acpi_os_remove_interrupt_handler(acpi_irq_number, acpi_irq_handler); }
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c index 2652a61..d5c155e 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -505,6 +505,8 @@ int __init acpi_irq_penalty_init(void) } } /* Add a penalty for the SCI */ + if (acpi_gbl_reduced_hardware) + return 0; acpi_irq_penalty[acpi_gbl_FADT.sci_interrupt] += PIRQ_PENALTY_PCI_USING; return 0; }
On Mon, Dec 16, 2013 at 05:16:39PM -0700, al.stone@linaro.org wrote:
- if (!acpi_gbl_reduced_hardware) {
I don't really understand this. You've explained that runtime hardware reduced mode switching isn't expected to work, but you're making this conditional on a runtime flag?
On 12/26/2013 09:56 PM, Matthew Garrett wrote:
On Mon, Dec 16, 2013 at 05:16:39PM -0700, al.stone@linaro.org wrote:
- if (!acpi_gbl_reduced_hardware) {
I don't really understand this. You've explained that runtime hardware reduced mode switching isn't expected to work, but you're making this conditional on a runtime flag?
Correct. My apologies if I've made this unclear.
Suppose I build a kernel with full ACPI support. The kernel would then include legacy and hardware-reduced support. If I pass that kernel ACPI tables specifying legacy mode, things work fine. If I pass that same kernel ACPI tables with the hardware-reduced flag set in the FADT, that should also work -- iff the ACPI driver checks the acpi_gbl_reduced_hardware flag at run-time. This patch is adding some of those checks that were missing so that at least the files in drivers/acpi/*.[ch] are correct; I contend that there are still places in the drivers/acpi/acpica/*.[ch] files where a Linux driver can call code it should not be allowed to when in hardware-reduced (e.g., when using the ACPI global lock [0]). Patching the ACPICA code I'm viewing as a separate issue yet to be done.
If I build a kernel with only hardware-reduced ACPI support, using ACPI tables without the hardware-reduced flag set in the FADT will clearly not work -- nor do I expect it to in this case since most of the legacy code gets removed at compile time. If the ACPI tables do have hardware-reduced mode set in the FADT, I expect the kernel to behave correctly. It may turn out to be exactly the same patch for ACPICA as mentioned above, but I contend that ACPICA does not completely handle this case properly at runtime, either.
Does that make sense?
[0] No driver should be doing this but it cannot currently be precluded by the kernel. You can call me paranoid but I think if it can be abused by a driver, it will be.
From: Al Stone al.stone@linaro.org
Per the ACPI 5.0 specification, section 5.2.9, none of the power management fields in the FADT are to be used. Short-circuit using any of those fields in acpi_processor_get_power_info_fadt().
Signed-off-by: Al Stone al.stone@linaro.org --- drivers/acpi/processor_idle.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index b91dcaa..4917a9e 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -270,6 +270,14 @@ static int acpi_processor_get_power_info_fadt(struct acpi_processor *pr) if (!pr->pblk) return -ENODEV;
+ /* + * Using power management information from the FADT is not + * allowed when in HW reduced mode. See ACPI 5.0, section + * 5.2.9. + */ + if (acpi_gbl_reduced_hardware) + return -ENODEV; + /* if info is obtained from pblk/fadt, type equals state */ pr->power.states[ACPI_STATE_C2].type = ACPI_STATE_C2; pr->power.states[ACPI_STATE_C3].type = ACPI_STATE_C3;
From: Al Stone al.stone@linaro.org
Several of the FADT fields are normally kept in specific memory regions. Since these fields are to be ignored in hardware reduced ACPI mode, do not map those addresses when in that mode, and of course do not release the mappings that have not been made.
The function acpi_os_initialize() could become a stub in the header file but is left here in case it can be of further use.
Signed-off-by: Al Stone al.stone@linaro.org --- drivers/acpi/osl.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index c946a3a..7822821 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -1778,10 +1778,12 @@ __setup("acpi_no_auto_ssdt", acpi_no_auto_ssdt_setup);
acpi_status __init acpi_os_initialize(void) { - acpi_os_map_generic_address(&acpi_gbl_FADT.xpm1a_event_block); - acpi_os_map_generic_address(&acpi_gbl_FADT.xpm1b_event_block); - acpi_os_map_generic_address(&acpi_gbl_FADT.xgpe0_block); - acpi_os_map_generic_address(&acpi_gbl_FADT.xgpe1_block); + if (!acpi_gbl_reduced_hardware) { + acpi_os_map_generic_address(&acpi_gbl_FADT.xpm1a_event_block); + acpi_os_map_generic_address(&acpi_gbl_FADT.xpm1b_event_block); + acpi_os_map_generic_address(&acpi_gbl_FADT.xgpe0_block); + acpi_os_map_generic_address(&acpi_gbl_FADT.xgpe1_block); + }
return AE_OK; } @@ -1806,10 +1808,12 @@ acpi_status acpi_os_terminate(void) acpi_irq_handler); }
- acpi_os_unmap_generic_address(&acpi_gbl_FADT.xgpe1_block); - acpi_os_unmap_generic_address(&acpi_gbl_FADT.xgpe0_block); - acpi_os_unmap_generic_address(&acpi_gbl_FADT.xpm1b_event_block); - acpi_os_unmap_generic_address(&acpi_gbl_FADT.xpm1a_event_block); + if (!acpi_gbl_reduced_hardware) { + acpi_os_unmap_generic_address(&acpi_gbl_FADT.xgpe1_block); + acpi_os_unmap_generic_address(&acpi_gbl_FADT.xgpe0_block); + acpi_os_unmap_generic_address(&acpi_gbl_FADT.xpm1b_event_block); + acpi_os_unmap_generic_address(&acpi_gbl_FADT.xpm1a_event_block); + }
destroy_workqueue(kacpid_wq); destroy_workqueue(kacpi_notify_wq);
linaro-kernel@lists.linaro.org
-
Al Stone -
al.stone＠linaro.org
-
Matthew Garrett