[PATCH v2 0/6] Hardware Reduced Mode cleanup for ACPI

[PATCH v2 0/6] Hardware Reduced Mode cleanup for ACPI

[al.stone]

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

-- 
1.8.3.1

[PATCH v2 1/6] ACPI: introduce CONFIG_ACPI_REDUCED_HARDWARE to enable this ACPI mode

[al.stone]

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

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

[al.stone]

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

Re: [PATCH v2 2/6] ACPI: bus master reload not supported in reduced HW mode

[Hanjun Guo]

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

Re: [PATCH v2 2/6] ACPI: bus master reload not supported in reduced HW mode

[Al Stone]

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.

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

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

[al.stone]

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

[PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[al.stone]

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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Rob Herring]

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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Al Stone]

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


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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Olof Johansson]

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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Rafael J. Wysocki]

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 speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Al Stone]

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.

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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Matthew Garrett]

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.

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

RE: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Zheng, Lv]

> 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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Matthew Garrett]

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?

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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Al Stone]

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.

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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Matthew Garrett]

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.

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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Al Stone]

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.

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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Al Stone]

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


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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Matthew Garrett]

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?

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

Re: [PATCH v2 4/6] ACPI: ARM: exclude DMI calls

[Grant Likely]

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.

[PATCH v2 5/6] ACPI: do not reserve memory regions for some FADT entries in HW reduced mode

[al.stone]

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

[PATCH v2 6/6] ACPI: in HW reduced mode, using FADT PM information is not allowed.

[al.stone]

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

Re: [PATCH v2 0/6] Hardware Reduced Mode cleanup for ACPI

[Rafael J. Wysocki]

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

Re: [PATCH v2 0/6] Hardware Reduced Mode cleanup for ACPI

[Al Stone]

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:_ACPI_Software_Programming_Model
>> [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.

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