[PATCH v2 0/2] Avoid enabling LPI on non-ARM

[PATCH v2 0/2] Avoid enabling LPI on non-ARM

[Mario Limonciello]

On some AMD platforms the platform _OSC negotiation will lead to
`osc_pc_lpi_support_confirmed` being set and because the ACPI tables are
populated with `_LPI` entries the kernel will attempt to enable LPI.

On non-ARM kernels LPI is not supported and both
`acpi_processor_ffh_lpi_probe` and
`acpi_processor_ffh_lpi_enter` will return error codes.

When this happens there is no recourse though; the cpuidle code does
not switch to `_CST` mode, but rather it will continue to behave as though
LPI has been enabled and the CPU will remain in a high power state.

This patch series shifts the checks around while enabling LPI to detect
this situation and let the system continue to set up in _CST mode.

Link to v1: https://lore.kernel.org/linux-acpi/20220225031255.3647599-1-mario.limonciello@amd.com/T/#m90282a6e29c334d14b2854a27d7225fc57a22aa6

Mario Limonciello (2):
  PSCI: cpuidle: Move the `has_lpi` check to the beginning of the
    function
  ACPI: processor idle: Check for architectural support for LPI

 arch/arm64/kernel/cpuidle.c   |  6 +++---
 drivers/acpi/processor_idle.c | 15 ++++++++++-----
 2 files changed, 13 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH v2 0/2] Avoid enabling LPI on non-ARM

[Mario Limonciello]

On some AMD platforms the platform _OSC negotiation will lead to
`osc_pc_lpi_support_confirmed` being set and because the ACPI tables are
populated with `_LPI` entries the kernel will attempt to enable LPI.

On non-ARM kernels LPI is not supported and both
`acpi_processor_ffh_lpi_probe` and
`acpi_processor_ffh_lpi_enter` will return error codes.

When this happens there is no recourse though; the cpuidle code does
not switch to `_CST` mode, but rather it will continue to behave as though
LPI has been enabled and the CPU will remain in a high power state.

This patch series shifts the checks around while enabling LPI to detect
this situation and let the system continue to set up in _CST mode.

Link to v1: https://lore.kernel.org/linux-acpi/20220225031255.3647599-1-mario.limonciello@amd.com/T/#m90282a6e29c334d14b2854a27d7225fc57a22aa6

Mario Limonciello (2):
  PSCI: cpuidle: Move the `has_lpi` check to the beginning of the
    function
  ACPI: processor idle: Check for architectural support for LPI

 arch/arm64/kernel/cpuidle.c   |  6 +++---
 drivers/acpi/processor_idle.c | 15 ++++++++++-----
 2 files changed, 13 insertions(+), 8 deletions(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/2] PSCI: cpuidle: Move the `has_lpi` check to the beginning of the function

[Mario Limonciello]

Currently the first thing checked is whether the PCSI cpu_suspend function
has been initialized.

Another change will be overloading `acpi_processor_ffh_lpi_probe` and
calling it sooner.  So make the `has_lpi` check the first thing checked
to prepare for that change.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 arch/arm64/kernel/cpuidle.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index 03991eeff643..3006f4324808 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -54,6 +54,9 @@ static int psci_acpi_cpu_init_idle(unsigned int cpu)
 	struct acpi_lpi_state *lpi;
 	struct acpi_processor *pr = per_cpu(processors, cpu);
 
+	if (unlikely(!pr || !pr->flags.has_lpi))
+		return -EINVAL;
+
 	/*
 	 * If the PSCI cpu_suspend function hook has not been initialized
 	 * idle states must not be enabled, so bail out
@@ -61,9 +64,6 @@ static int psci_acpi_cpu_init_idle(unsigned int cpu)
 	if (!psci_ops.cpu_suspend)
 		return -EOPNOTSUPP;
 
-	if (unlikely(!pr || !pr->flags.has_lpi))
-		return -EINVAL;
-
 	count = pr->power.count - 1;
 	if (count <= 0)
 		return -ENODEV;
-- 
2.34.1

[PATCH v2 1/2] PSCI: cpuidle: Move the `has_lpi` check to the beginning of the function

[Mario Limonciello]

Currently the first thing checked is whether the PCSI cpu_suspend function
has been initialized.

Another change will be overloading `acpi_processor_ffh_lpi_probe` and
calling it sooner.  So make the `has_lpi` check the first thing checked
to prepare for that change.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 arch/arm64/kernel/cpuidle.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/cpuidle.c b/arch/arm64/kernel/cpuidle.c
index 03991eeff643..3006f4324808 100644
--- a/arch/arm64/kernel/cpuidle.c
+++ b/arch/arm64/kernel/cpuidle.c
@@ -54,6 +54,9 @@ static int psci_acpi_cpu_init_idle(unsigned int cpu)
 	struct acpi_lpi_state *lpi;
 	struct acpi_processor *pr = per_cpu(processors, cpu);
 
+	if (unlikely(!pr || !pr->flags.has_lpi))
+		return -EINVAL;
+
 	/*
 	 * If the PSCI cpu_suspend function hook has not been initialized
 	 * idle states must not be enabled, so bail out
@@ -61,9 +64,6 @@ static int psci_acpi_cpu_init_idle(unsigned int cpu)
 	if (!psci_ops.cpu_suspend)
 		return -EOPNOTSUPP;
 
-	if (unlikely(!pr || !pr->flags.has_lpi))
-		return -EINVAL;
-
 	count = pr->power.count - 1;
 	if (count <= 0)
 		return -ENODEV;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/2] ACPI: processor idle: Check for architectural support for LPI

[Mario Limonciello]

When `osc_pc_lpi_support_confirmed` is set through `_OSC` and `_LPI` is
populated then the cpuidle driver assumes that LPI is fully functional.

However currently the kernel only provides architectural support for LPI
on ARM.  This leads to high power consumption on X86 platforms that
otherwise try to enable LPI.

So probe whether or not LPI support is implemented before enabling LPI in
the kernel.  This is done by overloading `acpi_processor_ffh_lpi_probe` to
check whether it returns `-EOPNOTSUPP`. It also means that all future
implementations of `acpi_processor_ffh_lpi_probe` will need to follow
these semantics as well.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/processor_idle.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index f8e9fa82cb9b..32b20efff5f8 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1080,6 +1080,11 @@ static int flatten_lpi_states(struct acpi_processor *pr,
 	return 0;
 }
 
+int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
+{
+	return -EOPNOTSUPP;
+}
+
 static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
 {
 	int ret, i;
@@ -1088,6 +1093,11 @@ static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
 	struct acpi_device *d = NULL;
 	struct acpi_lpi_states_array info[2], *tmp, *prev, *curr;
 
+	/* make sure our architecture has support */
+	ret = acpi_processor_ffh_lpi_probe(pr->id);
+	if (ret == -EOPNOTSUPP)
+		return ret;
+
 	if (!osc_pc_lpi_support_confirmed)
 		return -EOPNOTSUPP;
 
@@ -1139,11 +1149,6 @@ static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
 	return 0;
 }
 
-int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
-{
-	return -ENODEV;
-}
-
 int __weak acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
 {
 	return -ENODEV;
-- 
2.34.1

[PATCH v2 2/2] ACPI: processor idle: Check for architectural support for LPI

[Mario Limonciello]

When `osc_pc_lpi_support_confirmed` is set through `_OSC` and `_LPI` is
populated then the cpuidle driver assumes that LPI is fully functional.

However currently the kernel only provides architectural support for LPI
on ARM.  This leads to high power consumption on X86 platforms that
otherwise try to enable LPI.

So probe whether or not LPI support is implemented before enabling LPI in
the kernel.  This is done by overloading `acpi_processor_ffh_lpi_probe` to
check whether it returns `-EOPNOTSUPP`. It also means that all future
implementations of `acpi_processor_ffh_lpi_probe` will need to follow
these semantics as well.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/processor_idle.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index f8e9fa82cb9b..32b20efff5f8 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1080,6 +1080,11 @@ static int flatten_lpi_states(struct acpi_processor *pr,
 	return 0;
 }
 
+int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
+{
+	return -EOPNOTSUPP;
+}
+
 static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
 {
 	int ret, i;
@@ -1088,6 +1093,11 @@ static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
 	struct acpi_device *d = NULL;
 	struct acpi_lpi_states_array info[2], *tmp, *prev, *curr;
 
+	/* make sure our architecture has support */
+	ret = acpi_processor_ffh_lpi_probe(pr->id);
+	if (ret == -EOPNOTSUPP)
+		return ret;
+
 	if (!osc_pc_lpi_support_confirmed)
 		return -EOPNOTSUPP;
 
@@ -1139,11 +1149,6 @@ static int acpi_processor_get_lpi_info(struct acpi_processor *pr)
 	return 0;
 }
 
-int __weak acpi_processor_ffh_lpi_probe(unsigned int cpu)
-{
-	return -ENODEV;
-}
-
 int __weak acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi)
 {
 	return -ENODEV;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/2] Avoid enabling LPI on non-ARM

[Rafael J. Wysocki]

On Fri, Feb 25, 2022 at 8:11 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> On some AMD platforms the platform _OSC negotiation will lead to
> `osc_pc_lpi_support_confirmed` being set and because the ACPI tables are
> populated with `_LPI` entries the kernel will attempt to enable LPI.
>
> On non-ARM kernels LPI is not supported and both
> `acpi_processor_ffh_lpi_probe` and
> `acpi_processor_ffh_lpi_enter` will return error codes.
>
> When this happens there is no recourse though; the cpuidle code does
> not switch to `_CST` mode, but rather it will continue to behave as though
> LPI has been enabled and the CPU will remain in a high power state.
>
> This patch series shifts the checks around while enabling LPI to detect
> this situation and let the system continue to set up in _CST mode.
>
> Link to v1: https://lore.kernel.org/linux-acpi/20220225031255.3647599-1-mario.limonciello@amd.com/T/#m90282a6e29c334d14b2854a27d7225fc57a22aa6
>
> Mario Limonciello (2):
>   PSCI: cpuidle: Move the `has_lpi` check to the beginning of the
>     function
>   ACPI: processor idle: Check for architectural support for LPI
>
>  arch/arm64/kernel/cpuidle.c   |  6 +++---
>  drivers/acpi/processor_idle.c | 15 ++++++++++-----
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> --

Both patches in the series applied as 5.18 material, thanks!

Re: [PATCH v2 0/2] Avoid enabling LPI on non-ARM

[Rafael J. Wysocki]

On Fri, Feb 25, 2022 at 8:11 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> On some AMD platforms the platform _OSC negotiation will lead to
> `osc_pc_lpi_support_confirmed` being set and because the ACPI tables are
> populated with `_LPI` entries the kernel will attempt to enable LPI.
>
> On non-ARM kernels LPI is not supported and both
> `acpi_processor_ffh_lpi_probe` and
> `acpi_processor_ffh_lpi_enter` will return error codes.
>
> When this happens there is no recourse though; the cpuidle code does
> not switch to `_CST` mode, but rather it will continue to behave as though
> LPI has been enabled and the CPU will remain in a high power state.
>
> This patch series shifts the checks around while enabling LPI to detect
> this situation and let the system continue to set up in _CST mode.
>
> Link to v1: https://lore.kernel.org/linux-acpi/20220225031255.3647599-1-mario.limonciello@amd.com/T/#m90282a6e29c334d14b2854a27d7225fc57a22aa6
>
> Mario Limonciello (2):
>   PSCI: cpuidle: Move the `has_lpi` check to the beginning of the
>     function
>   ACPI: processor idle: Check for architectural support for LPI
>
>  arch/arm64/kernel/cpuidle.c   |  6 +++---
>  drivers/acpi/processor_idle.c | 15 ++++++++++-----
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> --

Both patches in the series applied as 5.18 material, thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel