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 will be coming later 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.
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 (6): 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: ARM: exclude DMI calls ACPI: do not reserve memory regions for some FADT entries in HW reduced mode ACPI: in HW reduced mode, using FADT PM information is not allowed.
drivers/acpi/Kconfig | 8 ++++++++ drivers/acpi/bus.c | 3 ++- drivers/acpi/osl.c | 24 ++++++++++++------------ drivers/acpi/pci_link.c | 2 ++ drivers/acpi/processor_idle.c | 16 ++++++++++++++-- include/acpi/platform/aclinux.h | 4 ++++ include/linux/dmi.h | 2 +- 7 files changed, 43 insertions(+), 16 deletions(-)
From: Al Stone al.stone@linaro.org
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.
This can be done more resonably 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
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 | 4 ++++ 2 files changed, 12 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..ae93a91 100644 --- a/include/acpi/platform/aclinux.h +++ b/include/acpi/platform/aclinux.h @@ -67,6 +67,10 @@
/* Host-dependent types and defines for in-kernel ACPICA */
+#ifdef CONFIG_ACPI_REDUCED_HARDWARE +#define ACPI_REDUCED_HARDWARE TRUE +#endif + #define ACPI_MACHINE_WIDTH BITS_PER_LONG #define ACPI_EXPORT_SYMBOL(symbol) EXPORT_SYMBOL(symbol); #define strtoul simple_strtoul
From: Al Stone al.stone@linaro.org
Remove the saving and restoring of 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 | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 597cdab..b18e7ab 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -202,7 +202,7 @@ static void lapic_timer_state_broadcast(struct acpi_processor *pr,
#endif
-#ifdef CONFIG_PM_SLEEP +#if (IS_ENABLED(CONFIG_PM_SLEEP) && !IS_ENABLED(CONFIG_ACPI_REDUCED_HARDWARE)) static u32 saved_bm_rld;
static int acpi_processor_suspend(void) @@ -236,7 +236,11 @@ void acpi_processor_syscore_exit(void) { unregister_syscore_ops(&acpi_processor_syscore_ops); } -#endif /* CONFIG_PM_SLEEP */ +#else +/* Bus master reload is not supported in reduced HW mode. */ +static int acpi_processor_suspend(void) { return 0; } +static void acpi_processor_resume(void) { return; } +#endif
#if defined(CONFIG_X86) static void tsc_check_state(int state)
On 2013-11-22 8:41, al.stone@linaro.org wrote:
From: Al Stone al.stone@linaro.org
Remove the saving and restoring of 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 | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 597cdab..b18e7ab 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -202,7 +202,7 @@ static void lapic_timer_state_broadcast(struct acpi_processor *pr, #endif -#ifdef CONFIG_PM_SLEEP +#if (IS_ENABLED(CONFIG_PM_SLEEP) && !IS_ENABLED(CONFIG_ACPI_REDUCED_HARDWARE)) static u32 saved_bm_rld; static int acpi_processor_suspend(void) @@ -236,7 +236,11 @@ void acpi_processor_syscore_exit(void) { unregister_syscore_ops(&acpi_processor_syscore_ops); } -#endif /* CONFIG_PM_SLEEP */
I think the code below is not needed, because acpi_processor_suspend/resume() are not used by anywhere else.
+#else +/* Bus master reload is not supported in reduced HW mode. */ +static int acpi_processor_suspend(void) { return 0; } +static void acpi_processor_resume(void) { return; } +#endif
Thanks Hanjun
On 11/21/2013 08:06 PM, Hanjun Guo wrote:
On 2013-11-22 8:41, al.stone@linaro.org wrote:
From: Al Stone al.stone@linaro.org
Remove the saving and restoring of 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 | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 597cdab..b18e7ab 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -202,7 +202,7 @@ static void lapic_timer_state_broadcast(struct acpi_processor *pr,
#endif
-#ifdef CONFIG_PM_SLEEP +#if (IS_ENABLED(CONFIG_PM_SLEEP) && !IS_ENABLED(CONFIG_ACPI_REDUCED_HARDWARE)) static u32 saved_bm_rld;
static int acpi_processor_suspend(void) @@ -236,7 +236,11 @@ void acpi_processor_syscore_exit(void) { unregister_syscore_ops(&acpi_processor_syscore_ops); } -#endif /* CONFIG_PM_SLEEP */
I think the code below is not needed, because acpi_processor_suspend/resume() are not used by anywhere else.
+#else +/* Bus master reload is not supported in reduced HW mode. */ +static int acpi_processor_suspend(void) { return 0; } +static void acpi_processor_resume(void) { return; } +#endif
Thanks Hanjun
Agreed. Removed in next version.
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, remove that usage 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 | 3 ++- drivers/acpi/osl.c | 16 ++++------------ drivers/acpi/pci_link.c | 2 ++ 3 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index bba9b72..de3a259 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -541,7 +541,8 @@ void __init acpi_early_init(void) goto error0; }
-#ifdef CONFIG_X86 +#ifndef CONFIG_ACPI_REDUCED_HARDWARE + /* NOTE: in HW reduced mode, FADT sci_interrupt has no meaning */ if (!acpi_ioapic) { /* compatible (0) means level (3) */ if (!(acpi_sci_flags & ACPI_MADT_TRIGGER_MASK)) { diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 54a20ff..34cd2c5 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; @@ -795,13 +796,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 +812,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 +1798,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 064cd5b..9ae105b 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; }
From: Al Stone al.stone@linaro.org
Modified #ifdef so that DMI is not used on ARM platforms which are currently implementing ACPI reduced HW mode.
Signed-off-by: Al Stone al.stone@linaro.org --- include/linux/dmi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dmi.h b/include/linux/dmi.h index f820f0a..a03deb8 100644 --- a/include/linux/dmi.h +++ b/include/linux/dmi.h @@ -83,7 +83,7 @@ struct dmi_device { void *device_data; /* Type specific data */ };
-#ifdef CONFIG_DMI +#if IS_ENABLED(CONFIG_DMI) && !IS_ENABLED(CONFIG_ACPI_REDUCED_HARDWARE)
struct dmi_dev_onboard { struct dmi_device dev;
On Thu, Nov 21, 2013 at 6:41 PM, al.stone@linaro.org wrote:
From: Al Stone al.stone@linaro.org
Modified #ifdef so that DMI is not used on ARM platforms which are currently implementing ACPI reduced HW mode.
It is really not allowed or is optional? There are various people that want DMI tables on ARM.
Rob
Signed-off-by: Al Stone al.stone@linaro.org
include/linux/dmi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dmi.h b/include/linux/dmi.h index f820f0a..a03deb8 100644 --- a/include/linux/dmi.h +++ b/include/linux/dmi.h @@ -83,7 +83,7 @@ struct dmi_device { void *device_data; /* Type specific data */ };
-#ifdef CONFIG_DMI +#if IS_ENABLED(CONFIG_DMI) && !IS_ENABLED(CONFIG_ACPI_REDUCED_HARDWARE)
struct dmi_dev_onboard { struct dmi_device dev; -- 1.8.3.1
linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel
On 11/22/2013 06:25 AM, Rob Herring wrote:
On Thu, Nov 21, 2013 at 6:41 PM, al.stone@linaro.org wrote:
From: Al Stone al.stone@linaro.org
Modified #ifdef so that DMI is not used on ARM platforms which are currently implementing ACPI reduced HW mode.
It is really not allowed or is optional? There are various people that want DMI tables on ARM.
Rob
True. DMI is optional. I see it as orthogonal to reduced HW mode; I have to hope that when DMI patches are forthcoming they'll do the right thing here.
Is there a better way to do this in the #if ?
Signed-off-by: Al Stone al.stone@linaro.org
include/linux/dmi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dmi.h b/include/linux/dmi.h index f820f0a..a03deb8 100644 --- a/include/linux/dmi.h +++ b/include/linux/dmi.h @@ -83,7 +83,7 @@ struct dmi_device { void *device_data; /* Type specific data */ };
-#ifdef CONFIG_DMI +#if IS_ENABLED(CONFIG_DMI) && !IS_ENABLED(CONFIG_ACPI_REDUCED_HARDWARE)
struct dmi_dev_onboard { struct dmi_device dev; -- 1.8.3.1
linaro-kernel mailing list linaro-kernel@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-kernel
On Fri, Nov 22, 2013 at 10:03 AM, Al Stone al.stone@linaro.org wrote:
On 11/22/2013 06:25 AM, Rob Herring wrote:
On Thu, Nov 21, 2013 at 6:41 PM, al.stone@linaro.org wrote:
From: Al Stone al.stone@linaro.org
Modified #ifdef so that DMI is not used on ARM platforms which are currently implementing ACPI reduced HW mode.
It is really not allowed or is optional? There are various people that want DMI tables on ARM.
Rob
True. DMI is optional. I see it as orthogonal to reduced HW mode; I have to hope that when DMI patches are forthcoming they'll do the right thing here.
Is there a better way to do this in the #if ?
Doing all of these things at compile time seems odd, shouldn't it be handled at runtime? What happens when someone wants to build a kernel that boots both on the reduced hw mode platforms and regular ones?
-Olof
On Friday, November 22, 2013 10:53:09 AM Olof Johansson wrote:
On Fri, Nov 22, 2013 at 10:03 AM, Al Stone al.stone@linaro.org wrote:
On 11/22/2013 06:25 AM, Rob Herring wrote:
On Thu, Nov 21, 2013 at 6:41 PM, al.stone@linaro.org wrote:
From: Al Stone al.stone@linaro.org
Modified #ifdef so that DMI is not used on ARM platforms which are currently implementing ACPI reduced HW mode.
It is really not allowed or is optional? There are various people that want DMI tables on ARM.
Rob
True. DMI is optional. I see it as orthogonal to reduced HW mode; I have to hope that when DMI patches are forthcoming they'll do the right thing here.
Is there a better way to do this in the #if ?
Doing all of these things at compile time seems odd, shouldn't it be handled at runtime? What happens when someone wants to build a kernel that boots both on the reduced hw mode platforms and regular ones?
I agree.
My suggestion would be to harden dmi_check_system() so that it works if DMI is not present (if it doesn't already).
Thanks!
On 11/22/2013 04:15 PM, Rafael J. Wysocki wrote:
On Friday, November 22, 2013 10:53:09 AM Olof Johansson wrote:
On Fri, Nov 22, 2013 at 10:03 AM, Al Stone al.stone@linaro.org wrote:
On 11/22/2013 06:25 AM, Rob Herring wrote:
On Thu, Nov 21, 2013 at 6:41 PM, al.stone@linaro.org wrote:
From: Al Stone al.stone@linaro.org
Modified #ifdef so that DMI is not used on ARM platforms which are currently implementing ACPI reduced HW mode.
It is really not allowed or is optional? There are various people that want DMI tables on ARM.
Rob
True. DMI is optional. I see it as orthogonal to reduced HW mode; I have to hope that when DMI patches are forthcoming they'll do the right thing here.
Is there a better way to do this in the #if ?
Doing all of these things at compile time seems odd, shouldn't it be handled at runtime? What happens when someone wants to build a kernel that boots both on the reduced hw mode platforms and regular ones?
I agree.
My suggestion would be to harden dmi_check_system() so that it works if DMI is not present (if it doesn't already).
Thanks!
I agree -- runtime would be better. For dmi_check_system(), that can be done and I'll look into that. I'll also go double check the rest of my #ifdefs and see if I can remove any more of them from the Linux side of the ACPI code.
For reduced hardware mode, however, I have to rely on the underlying ACPICA reference implementation to behave properly. Right now, ACPICA relies on compile time changes to implement either reduced HW mode or legacy mode so I have to follow suit. When I looked at making ACPICA change behavior at runtime, the changes became more and more invasive. Since x86/ia64 depend on ACPICA to behave also, that seemed a far more dangerous approach to me.
On Fri, Nov 22, 2013 at 05:05:48PM -0700, Al Stone wrote:
For reduced hardware mode, however, I have to rely on the underlying ACPICA reference implementation to behave properly. Right now, ACPICA relies on compile time changes to implement either reduced HW mode or legacy mode so I have to follow suit. When I looked at making ACPICA change behavior at runtime, the changes became more and more invasive. Since x86/ia64 depend on ACPICA to behave also, that seemed a far more dangerous approach to me.
Ugh. Really? People have been fairly careful about making sure that the x86 SoC code is selected correctly at runtime, and losing that because ACPICA is broken would be a shame. I think this is something that needs to support runtime switching even if there's also support for building kernels that only implement the reduced hardware profile.
From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Matthew Garrett Sent: Sunday, November 24, 2013 12:39 AM
On Fri, Nov 22, 2013 at 05:05:48PM -0700, Al Stone wrote:
For reduced hardware mode, however, I have to rely on the underlying ACPICA reference implementation to behave properly. Right now, ACPICA relies on compile time changes to implement either reduced HW mode or legacy mode so I have to follow suit. When I looked at making ACPICA change behavior at runtime, the changes became more and more invasive. Since x86/ia64 depend on ACPICA to behave also, that seemed a far more dangerous approach to me.
Ugh. Really? People have been fairly careful about making sure that the x86 SoC code is selected correctly at runtime, and losing that because ACPICA is broken would be a shame. I think this is something that needs to support runtime switching even if there's also support for building kernels that only implement the reduced hardware profile.
If my reading is correct, do you mean x86 SoCs should have already tested the code.
So if ARM need ACPI_REDUCED_HARDWARE to be defined, the <include/acpi/platform/aclinux.h> should have lines like: #ifdef CONFIG_ARCH_IS_ACPI_REDUCED_HARDWARE #define ACPI_REDUCED_HARDWARE TRUE #endif And ARCH_IS_ACPI_REDUCED_HARDWARE should only be selected by CONFIG_ARM.
Well, I don't have knowledge about "x86 ACPI_REDUCED_HARDWARE tests", so I don't have any idea on whether the above code or original one can reflect the real world.
Thanks and best regards -Lv
-- Matthew Garrett | mjg59@srcf.ucam.org -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 25, 2013 at 05:10:46AM +0000, Zheng, Lv wrote:
From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Matthew Garrett Ugh. Really? People have been fairly careful about making sure that the x86 SoC code is selected correctly at runtime, and losing that because ACPICA is broken would be a shame. I think this is something that needs to support runtime switching even if there's also support for building kernels that only implement the reduced hardware profile.
If my reading is correct, do you mean x86 SoCs should have already tested the code.
I don't know if anyone has deployed x86 SoCs with reduced hardware yet, but it seems like something that might happen.
So if ARM need ACPI_REDUCED_HARDWARE to be defined, the <include/acpi/platform/aclinux.h> should have lines like: #ifdef CONFIG_ARCH_IS_ACPI_REDUCED_HARDWARE #define ACPI_REDUCED_HARDWARE TRUE #endif And ARCH_IS_ACPI_REDUCED_HARDWARE should only be selected by CONFIG_ARM.
Is ACPI_REDUCED_HARDWARE supposed to indicate support for the reduced hardware profile, or that the platform *only* implements the reduced hardware profile?
On 11/25/2013 08:30 AM, Matthew Garrett wrote:
On Mon, Nov 25, 2013 at 05:10:46AM +0000, Zheng, Lv wrote:
From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Matthew Garrett Ugh. Really? People have been fairly careful about making sure that the x86 SoC code is selected correctly at runtime, and losing that because ACPICA is broken would be a shame. I think this is something that needs to support runtime switching even if there's also support for building kernels that only implement the reduced hardware profile.
If my reading is correct, do you mean x86 SoCs should have already tested the code.
I don't know if anyone has deployed x86 SoCs with reduced hardware yet, but it seems like something that might happen.
So if ARM need ACPI_REDUCED_HARDWARE to be defined, the <include/acpi/platform/aclinux.h> should have lines like: #ifdef CONFIG_ARCH_IS_ACPI_REDUCED_HARDWARE #define ACPI_REDUCED_HARDWARE TRUE #endif And ARCH_IS_ACPI_REDUCED_HARDWARE should only be selected by CONFIG_ARM.
Is ACPI_REDUCED_HARDWARE supposed to indicate support for the reduced hardware profile, or that the platform *only* implements the reduced hardware profile?
From what I can see in ACPICA, ACPI_REDUCED_HARDWARE indicates the platform *only* implements the reduced hardware profile. This *seems* to be consistent with the specification -- see 3.11.1, second bullet, for example:
Boot in ACPI mode only (ACPI Enable, ACPI Disable, SMI_CMD and Legacy mode are not supported)
...if by "not supported" one takes that to mean "does not exist when compiled." I can look at the ACPICA code again, just the same; perhaps there is some reasonable way to at least select one or the other at boot as the first step, and then allow switching between modes as a later step.
To Lv's point, since hardware reduced mode was added in ACPI 5.0, I don't think there has been a lot of exposure to it, especially on working platforms on the Linux side, so I doubt there has been any significant Linux testing of it until now.
On Mon, Nov 25, 2013 at 10:43:27AM -0700, Al Stone wrote:
On 11/25/2013 08:30 AM, Matthew Garrett wrote:
Is ACPI_REDUCED_HARDWARE supposed to indicate support for the reduced hardware profile, or that the platform *only* implements the reduced hardware profile?
From what I can see in ACPICA, ACPI_REDUCED_HARDWARE indicates the platform *only* implements the reduced hardware profile. This *seems* to be consistent with the specification -- see 3.11.1, second bullet, for example:
Ok, so a kernel built without ACPI_REDUCED_HARDWARE would still support the reduced hardware profile?
...if by "not supported" one takes that to mean "does not exist when compiled." I can look at the ACPICA code again, just the same; perhaps there is some reasonable way to at least select one or the other at boot as the first step, and then allow switching between modes as a later step.
I don't think you'd ever want to switch after init time. There's a flag in the FADT that indicates whether a system is implementing the reduced hardware profile or not.
On 11/25/2013 10:45 AM, Matthew Garrett wrote:
On Mon, Nov 25, 2013 at 10:43:27AM -0700, Al Stone wrote:
On 11/25/2013 08:30 AM, Matthew Garrett wrote:
Is ACPI_REDUCED_HARDWARE supposed to indicate support for the reduced hardware profile, or that the platform *only* implements the reduced hardware profile?
From what I can see in ACPICA, ACPI_REDUCED_HARDWARE indicates the platform *only* implements the reduced hardware profile. This *seems* to be consistent with the specification -- see 3.11.1, second bullet, for example:
Ok, so a kernel built without ACPI_REDUCED_HARDWARE would still support the reduced hardware profile?
Let me check on that. The reduced hardware profile is a pretty strict subset, and I think I can see a way where I could force selecting the reduced HW profile on boot if the kernel has been built *without* ACPI_REDUCED_HARDWARE. What I am not convinced of is that all of the proper guards are in place in ACPICA -- I trust that they have been, but I would like to double check.
If I can get that to work properly, I'll add it to this patch set.
...if by "not supported" one takes that to mean "does not exist when compiled." I can look at the ACPICA code again, just the same; perhaps there is some reasonable way to at least select one or the other at boot as the first step, and then allow switching between modes as a later step.
I don't think you'd ever want to switch after init time. There's a flag in the FADT that indicates whether a system is implementing the reduced hardware profile or not.
Agreed. I could see it as something to use when experimenting perhaps, but I think just allowing the switch at boot would cover the majority of the use cases.
On 11/25/2013 11:01 AM, Al Stone wrote:
On 11/25/2013 10:45 AM, Matthew Garrett wrote:
On Mon, Nov 25, 2013 at 10:43:27AM -0700, Al Stone wrote:
On 11/25/2013 08:30 AM, Matthew Garrett wrote:
Is ACPI_REDUCED_HARDWARE supposed to indicate support for the reduced hardware profile, or that the platform *only* implements the reduced hardware profile?
From what I can see in ACPICA, ACPI_REDUCED_HARDWARE indicates the platform *only* implements the reduced hardware profile. This *seems* to be consistent with the specification -- see 3.11.1, second bullet, for example:
Ok, so a kernel built without ACPI_REDUCED_HARDWARE would still support the reduced hardware profile?
Let me check on that. The reduced hardware profile is a pretty strict subset, and I think I can see a way where I could force selecting the reduced HW profile on boot if the kernel has been built *without* ACPI_REDUCED_HARDWARE. What I am not convinced of is that all of the proper guards are in place in ACPICA -- I trust that they have been, but I would like to double check.
If I can get that to work properly, I'll add it to this patch set.
I thought I could get this to work, but I am going to have to defer to the ACPICA upstream folks. For the time being, I think that all of the architectures that want to use ACPI in either legacy mode or in stripped down reduced HW mode will have to use different kernels, one for each mode. I thought there might be enough safety checks to allow the kernel to boot in legacy mode and switch into reduced HW at boot, but there are not, in my opinion. I don't see a way to make the switch *and* maintain conformance with the spec without significant change to ACPICA itself.
For example, enforcing that various functions are not allowed while in reduced HW mode could be done by a check of the reduced HW flag on entry. If it is set, return the value the function would have returned had it been stubbed out for reduced HW mode. This is simple enough but to do so would require modifying at least 29 functions in ACPICA, by my count, not something upstream is particularly keen on -- nor am I. I'd rather step back and work with ACPICA over the longer term and see if there's some way to get this functionality implemented properly instead of trying to bolt it on somehow.
...if by "not supported" one takes that to mean "does not exist when compiled." I can look at the ACPICA code again, just the same; perhaps there is some reasonable way to at least select one or the other at boot as the first step, and then allow switching between modes as a later step.
I don't think you'd ever want to switch after init time. There's a flag in the FADT that indicates whether a system is implementing the reduced hardware profile or not.
Agreed. I could see it as something to use when experimenting perhaps, but I think just allowing the switch at boot would cover the majority of the use cases.
On Tue, Dec 03, 2013 at 06:30:48PM -0700, Al Stone wrote:
I thought I could get this to work, but I am going to have to defer to the ACPICA upstream folks. For the time being, I think that all of the architectures that want to use ACPI in either legacy mode or in stripped down reduced HW mode will have to use different kernels, one for each mode. I thought there might be enough safety checks to allow the kernel to boot in legacy mode and switch into reduced HW at boot, but there are not, in my opinion. I don't see a way to make the switch *and* maintain conformance with the spec without significant change to ACPICA itself.
Ugh. That needs fixing. There's been a huge amount of work done to ensure that x86 only needs a single kernel image for 64-bit, it really needs to be runtime. We shouldn't have merged it in this state.
For example, enforcing that various functions are not allowed while in reduced HW mode could be done by a check of the reduced HW flag on entry. If it is set, return the value the function would have returned had it been stubbed out for reduced HW mode. This is simple enough but to do so would require modifying at least 29 functions in ACPICA, by my count, not something upstream is particularly keen on -- nor am I. I'd rather step back and work with ACPICA over the longer term and see if there's some way to get this functionality implemented properly instead of trying to bolt it on somehow.
How many of those are calls that we'll actually execute in the HW reduced case?
On Sat, 23 Nov 2013 16:38:54 +0000, Matthew Garrett mjg59@srcf.ucam.org wrote:
On Fri, Nov 22, 2013 at 05:05:48PM -0700, Al Stone wrote:
For reduced hardware mode, however, I have to rely on the underlying ACPICA reference implementation to behave properly. Right now, ACPICA relies on compile time changes to implement either reduced HW mode or legacy mode so I have to follow suit. When I looked at making ACPICA change behavior at runtime, the changes became more and more invasive. Since x86/ia64 depend on ACPICA to behave also, that seemed a far more dangerous approach to me.
Ugh. Really? People have been fairly careful about making sure that the x86 SoC code is selected correctly at runtime, and losing that because ACPICA is broken would be a shame. I think this is something that needs to support runtime switching even if there's also support for building kernels that only implement the reduced hardware profile.
Yeah, that is a really big problem. At the very least push the hacks back into ACPICA and make that project sort it out (add stub functions if needed). I don't like seeing the kernel having #ifdef blocks to stub out normal ACPI paths.
g.
From: Al Stone al.stone@linaro.org
Since some of the FADT fields reserved are not to be used by the OSPM in the HW reduced mode, do not map in the memory areas that the FADT fields reference.
Signed-off-by: Al Stone al.stone@linaro.org --- drivers/acpi/osl.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 34cd2c5..3ac5076 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -160,6 +160,9 @@ static u32 acpi_osi_handler(acpi_string interface, u32 supported) return supported; }
+#ifdef CONFIG_ACPI_REDUCED_HARDWARE +static int __init acpi_reserve_resources(void) { return 0; } +#else static void __init acpi_request_region (struct acpi_generic_address *gas, unsigned int length, char *desc) { @@ -210,6 +213,7 @@ static int __init acpi_reserve_resources(void) return 0; } device_initcall(acpi_reserve_resources); +#endif
void acpi_os_printf(const char *fmt, ...) { @@ -1772,6 +1776,9 @@ static int __init acpi_no_auto_ssdt_setup(char *s)
__setup("acpi_no_auto_ssdt", acpi_no_auto_ssdt_setup);
+#ifdef CONFIG_ACPI_REDUCED_HARDWARE +acpi_status __init acpi_os_initialize(void) { return AE_OK; } +#else acpi_status __init acpi_os_initialize(void) { acpi_os_map_generic_address(&acpi_gbl_FADT.xpm1a_event_block); @@ -1781,6 +1788,7 @@ acpi_status __init acpi_os_initialize(void)
return AE_OK; } +#endif
acpi_status __init acpi_os_initialize1(void) {
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.og --- 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 b18e7ab..52859da 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -272,6 +272,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;
On Thursday, November 21, 2013 05:41:49 PM al.stone@linaro.org wrote:
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 will be coming later 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.
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 (6): 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: ARM: exclude DMI calls ACPI: do not reserve memory regions for some FADT entries in HW reduced mode ACPI: in HW reduced mode, using FADT PM information is not allowed.
As Olof noted in another thread, all ACPI patches related to ARM should be CCed to linux-arm-kernel.
Can you please do that in the future?
Rafael
On 11/22/2013 04:18 PM, Rafael J. Wysocki wrote:
On Thursday, November 21, 2013 05:41:49 PM al.stone@linaro.org wrote:
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 will be coming later 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.
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 (6): 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: ARM: exclude DMI calls ACPI: do not reserve memory regions for some FADT entries in HW reduced mode ACPI: in HW reduced mode, using FADT PM information is not allowed.
As Olof noted in another thread, all ACPI patches related to ARM should be CCed to linux-arm-kernel.
Can you please do that in the future?
Rafael
Of course.
linaro-kernel@lists.linaro.org
-
Al Stone -
al.stone＠linaro.org
-
Grant Likely -
Hanjun Guo -
Matthew Garrett -
Olof Johansson -
Rafael J. Wysocki -
Rob Herring -
Zheng, Lv