[PATCH v4 0/5] Hardware Reduced Mode Cleanup for ACPI

[PATCH v4 0/5] Hardware Reduced Mode Cleanup for ACPI

[al.stone at linaro.org]

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:_ACPI_Software_Programming_Model
[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(-)

-- 
1.8.3.1

[PATCH v4 1/5] ACPI: introduce CONFIG_ACPI_REDUCED_HARDWARE to enable this ACPI mode

[al.stone at linaro.org]

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>
-- 
1.8.3.1

[PATCH v4 2/5] ACPI: bus master reload not supported in reduced HW mode

[al.stone at linaro.org]

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 */
 
-- 
1.8.3.1

[PATCH v4 3/5] ACPI: HW reduced mode does not allow use of the FADT sci_interrupt field

[al.stone at linaro.org]

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;
 }
-- 
1.8.3.1

[PATCH v4 4/5] ACPI: in HW reduced mode, using FADT PM information is not allowed.

[al.stone at linaro.org]

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;
-- 
1.8.3.1

[PATCH v4 5/5] ACPI: do not map/unmap memory regions for FADT entries in reduced HW mode

[al.stone at linaro.org]

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);
-- 
1.8.3.1

[PATCH v4 3/5] ACPI: HW reduced mode does not allow use of the FADT sci_interrupt field

[Matthew Garrett]

On Mon, Dec 16, 2013 at 05:16:39PM -0700, al.stone at 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?

-- 
Matthew Garrett | mjg59 at srcf.ucam.org

[PATCH v4 1/5] ACPI: introduce CONFIG_ACPI_REDUCED_HARDWARE to enable this ACPI mode

[Matthew Garrett]

On Mon, Dec 16, 2013 at 05:16:37PM -0700, al.stone at 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 ?

-- 
Matthew Garrett | mjg59 at srcf.ucam.org

[PATCH v4 1/5] ACPI: introduce CONFIG_ACPI_REDUCED_HARDWARE to enable this ACPI mode

[Al Stone]

On 12/26/2013 09:58 PM, Matthew Garrett wrote:
> On Mon, Dec 16, 2013 at 05:16:37PM -0700, al.stone at 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.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone at linaro.org
-----------------------------------

[PATCH v4 3/5] ACPI: HW reduced mode does not allow use of the FADT sci_interrupt field

[Al Stone]

On 12/26/2013 09:56 PM, Matthew Garrett wrote:
> On Mon, Dec 16, 2013 at 05:16:39PM -0700, al.stone at 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.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone at linaro.org
-----------------------------------