ACPI 3.0 introduced a new Operating System Capabilities _OSC control
method. This method is similar to _PDC, which was marked as deprecated
in ACPI 3.0.
Prefer using _OSC method over deprecated _PDC in the acpi_bus_init(). In
case of the failure of the _OSC, try using _PDC as a fallback.
Testing done:
Tested on physical server with BIOS implementing _OSC methods. In this
case acpi_processor_osc() was executed for each CPU core. acpi_run_osc()
returned success indicating that _OSC method succeeded.
Tested on qemu VM to check whether the code would work on a SeaBios (the
default for qemu, doesn't support _OSC methods, or _PDC). This way I was
able to see how code behaves in case BIOS doesn't implement _OSC. In
that case the function
acpi_run_osc() returned failure, which propagated all the way up to
acpi_early_processor_osc(). The logic responsible for triggering _PDC
execution was triggered correctly.
Tested this using debug messages with printk.
v3:
- split into more commits to make review easier
- changed "_UID" to METHOD_NAME_UID
- changed hard-coded constant to ACPI_PDC_COLLAB_PROC_PERF
- added Suggested-by tags
- fixed style issues
- fixed whitespaces
- refactored code
v2:
- fixed compilation issues on ia64 and arm
Michal Wilczynski (5):
acpi: Move logic responsible for conveying processor OSPM capabilities
acpi: Refactor arch_acpi_set_pdc_bits()
acpi: Introduce new function callback for _OSC
acpi: Use _OSC method to convey processor OSPM capabilities
acpi: Remove acpi_hwp_native_thermal_lvt_osc()
arch/ia64/include/asm/acpi.h | 4 +-
arch/x86/include/asm/acpi.h | 13 +--
drivers/acpi/acpi_processor.c | 151 +++++++++++++++++++++++++++-------
drivers/acpi/bus.c | 13 ++-
drivers/acpi/internal.h | 10 +--
drivers/acpi/processor_pdc.c | 82 +-----------------
include/acpi/pdc_intel.h | 1 +
include/acpi/processor.h | 2 +-
8 files changed, 148 insertions(+), 128 deletions(-)
--
2.41.0
Workaround for buggy skylake BIOS is implemented in acpi_processor_osc()
and acpi_hwp_native_thermal_lvt_osc() function is not called anywhere.
Remove it.
Suggested-by: Rafael J. Wysocki <[email protected]>
Signed-off-by: Michal Wilczynski <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/acpi_processor.c | 35 -----------------------------------
1 file changed, 35 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 8965e01406e0..1e305ad9540e 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -634,41 +634,6 @@ static acpi_status __init acpi_processor_osc(acpi_handle handle, u32 lvl,
return AE_OK;
}
-static acpi_status __init acpi_hwp_native_thermal_lvt_osc(acpi_handle handle,
- u32 lvl,
- void *context,
- void **rv)
-{
- u32 capbuf[2];
- struct acpi_osc_context osc_context = {
- .uuid_str = sb_uuid_str,
- .rev = 1,
- .cap.length = 8,
- .cap.pointer = capbuf,
- };
-
- if (acpi_hwp_native_thermal_lvt_set)
- return AE_CTRL_TERMINATE;
-
- capbuf[0] = 0x0000;
- capbuf[1] = 0x1000; /* set bit 12 */
-
- if (ACPI_SUCCESS(acpi_run_osc(handle, &osc_context))) {
- if (osc_context.ret.pointer && osc_context.ret.length > 1) {
- u32 *capbuf_ret = osc_context.ret.pointer;
-
- if (capbuf_ret[1] & 0x1000) {
- acpi_handle_info(handle,
- "_OSC native thermal LVT Acked\n");
- acpi_hwp_native_thermal_lvt_set = true;
- }
- }
- kfree(osc_context.ret.pointer);
- }
-
- return AE_OK;
-}
-
acpi_status __init acpi_early_processor_osc(void)
{
acpi_status status;
--
2.41.0
Change acpi_early_processor_osc() to return value in case of the failure.
Make it more generic - previously it served only to execute workaround
for buggy BIOS in Skylake systems. Now it will walk through ACPI
namespace looking for processor objects and will convey OSPM processor
capabilities using _OSC method.
Prefer using _OSC method over deprecated _PDC in the acpi_bus_init(). In
case of the failure of the _OSC, try using _PDC as a fallback.
Suggested-by: Rafael J. Wysocki <[email protected]>
Signed-off-by: Michal Wilczynski <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/acpi_processor.c | 23 +++++++++++++----------
drivers/acpi/bus.c | 13 +++++++++----
drivers/acpi/internal.h | 9 +--------
3 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 0de0b05b6f53..8965e01406e0 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -669,17 +669,20 @@ static acpi_status __init acpi_hwp_native_thermal_lvt_osc(acpi_handle handle,
return AE_OK;
}
-void __init acpi_early_processor_osc(void)
+acpi_status __init acpi_early_processor_osc(void)
{
- if (boot_cpu_has(X86_FEATURE_HWP)) {
- acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
- ACPI_UINT32_MAX,
- acpi_hwp_native_thermal_lvt_osc,
- NULL, NULL, NULL);
- acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID,
- acpi_hwp_native_thermal_lvt_osc,
- NULL, NULL);
- }
+ acpi_status status;
+
+ processor_dmi_check();
+
+ status = acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_processor_osc, NULL,
+ NULL, NULL);
+ if (ACPI_FAILURE(status))
+ return status;
+
+ return acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID, acpi_processor_osc,
+ NULL, NULL);
}
#endif
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index d161ff707de4..e8d1f645224f 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1317,9 +1317,6 @@ static int __init acpi_bus_init(void)
goto error1;
}
- /* Set capability bits for _OSC under processor scope */
- acpi_early_processor_osc();
-
/*
* _OSC method may exist in module level code,
* so it must be run after ACPI_FULL_INITIALIZATION
@@ -1335,7 +1332,15 @@ static int __init acpi_bus_init(void)
acpi_sysfs_init();
- acpi_early_processor_set_pdc();
+#ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
+ status = acpi_early_processor_osc();
+ if (ACPI_FAILURE(status)) {
+ pr_err("_OSC methods failed, trying _PDC\n");
+ acpi_early_processor_set_pdc();
+ } else {
+ pr_info("_OSC methods ran successfully\n");
+ }
+#endif
/*
* Maybe EC region is required at bus_scan/acpi_get_devices. So it
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index f979a2f7077c..e7cc41313997 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -151,17 +151,10 @@ int acpi_wakeup_device_init(void);
-------------------------------------------------------------------------- */
#ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
void acpi_early_processor_set_pdc(void);
+acpi_status acpi_early_processor_osc(void);
void processor_dmi_check(void);
bool processor_physically_present(acpi_handle handle);
-#else
-static inline void acpi_early_processor_set_pdc(void) {}
-#endif
-
-#ifdef CONFIG_X86
-void acpi_early_processor_osc(void);
-#else
-static inline void acpi_early_processor_osc(void) {}
#endif
/* --------------------------------------------------------------------------
--
2.41.0
On Tue, Jun 13, 2023 at 6:12 PM Michal Wilczynski
<[email protected]> wrote:
>
> Change acpi_early_processor_osc() to return value in case of the failure.
> Make it more generic - previously it served only to execute workaround
> for buggy BIOS in Skylake systems. Now it will walk through ACPI
> namespace looking for processor objects and will convey OSPM processor
> capabilities using _OSC method.
>
> Prefer using _OSC method over deprecated _PDC in the acpi_bus_init(). In
> case of the failure of the _OSC, try using _PDC as a fallback.
>
> Suggested-by: Rafael J. Wysocki <[email protected]>
> Signed-off-by: Michal Wilczynski <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> drivers/acpi/acpi_processor.c | 23 +++++++++++++----------
> drivers/acpi/bus.c | 13 +++++++++----
> drivers/acpi/internal.h | 9 +--------
> 3 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 0de0b05b6f53..8965e01406e0 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -669,17 +669,20 @@ static acpi_status __init acpi_hwp_native_thermal_lvt_osc(acpi_handle handle,
> return AE_OK;
> }
>
> -void __init acpi_early_processor_osc(void)
I would rename this to something like
acpi_early_processor_control_setup() and would make it attempt to call
_PDC if _OSC doesn't work.
Then it could remain void and it could be put under a
CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC #ifdef.
> +acpi_status __init acpi_early_processor_osc(void)
> {
> - if (boot_cpu_has(X86_FEATURE_HWP)) {
> - acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
> - ACPI_UINT32_MAX,
> - acpi_hwp_native_thermal_lvt_osc,
> - NULL, NULL, NULL);
> - acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID,
> - acpi_hwp_native_thermal_lvt_osc,
> - NULL, NULL);
> - }
> + acpi_status status;
> +
> + processor_dmi_check();
> +
> + status = acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
> + ACPI_UINT32_MAX, acpi_processor_osc, NULL,
> + NULL, NULL);
> + if (ACPI_FAILURE(status))
> + return status;
> +
> + return acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID, acpi_processor_osc,
> + NULL, NULL);
> }
> #endif
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index d161ff707de4..e8d1f645224f 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -1317,9 +1317,6 @@ static int __init acpi_bus_init(void)
> goto error1;
> }
>
> - /* Set capability bits for _OSC under processor scope */
> - acpi_early_processor_osc();
> -
> /*
> * _OSC method may exist in module level code,
> * so it must be run after ACPI_FULL_INITIALIZATION
> @@ -1335,7 +1332,15 @@ static int __init acpi_bus_init(void)
>
> acpi_sysfs_init();
>
> - acpi_early_processor_set_pdc();
> +#ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
> + status = acpi_early_processor_osc();
> + if (ACPI_FAILURE(status)) {
> + pr_err("_OSC methods failed, trying _PDC\n");
> + acpi_early_processor_set_pdc();
> + } else {
> + pr_info("_OSC methods ran successfully\n");
> + }
> +#endif
>
> /*
> * Maybe EC region is required at bus_scan/acpi_get_devices. So it
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index f979a2f7077c..e7cc41313997 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -151,17 +151,10 @@ int acpi_wakeup_device_init(void);
> -------------------------------------------------------------------------- */
> #ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
> void acpi_early_processor_set_pdc(void);
> +acpi_status acpi_early_processor_osc(void);
>
> void processor_dmi_check(void);
> bool processor_physically_present(acpi_handle handle);
> -#else
> -static inline void acpi_early_processor_set_pdc(void) {}
> -#endif
> -
> -#ifdef CONFIG_X86
> -void acpi_early_processor_osc(void);
> -#else
> -static inline void acpi_early_processor_osc(void) {}
> #endif
>
> /* --------------------------------------------------------------------------
> --
On 6/29/2023 4:23 PM, Rafael J. Wysocki wrote:
> On Tue, Jun 13, 2023 at 6:12 PM Michal Wilczynski
> <[email protected]> wrote:
>> Change acpi_early_processor_osc() to return value in case of the failure.
>> Make it more generic - previously it served only to execute workaround
>> for buggy BIOS in Skylake systems. Now it will walk through ACPI
>> namespace looking for processor objects and will convey OSPM processor
>> capabilities using _OSC method.
>>
>> Prefer using _OSC method over deprecated _PDC in the acpi_bus_init(). In
>> case of the failure of the _OSC, try using _PDC as a fallback.
>>
>> Suggested-by: Rafael J. Wysocki <[email protected]>
>> Signed-off-by: Michal Wilczynski <[email protected]>
>> Reviewed-by: Andy Shevchenko <[email protected]>
>> ---
>> drivers/acpi/acpi_processor.c | 23 +++++++++++++----------
>> drivers/acpi/bus.c | 13 +++++++++----
>> drivers/acpi/internal.h | 9 +--------
>> 3 files changed, 23 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
>> index 0de0b05b6f53..8965e01406e0 100644
>> --- a/drivers/acpi/acpi_processor.c
>> +++ b/drivers/acpi/acpi_processor.c
>> @@ -669,17 +669,20 @@ static acpi_status __init acpi_hwp_native_thermal_lvt_osc(acpi_handle handle,
>> return AE_OK;
>> }
>>
>> -void __init acpi_early_processor_osc(void)
> I would rename this to something like
> acpi_early_processor_control_setup() and would make it attempt to call
> _PDC if _OSC doesn't work.
>
> Then it could remain void and it could be put under a
> CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC #ifdef.
Sure, makes sense to me
>
>> +acpi_status __init acpi_early_processor_osc(void)
>> {
>> - if (boot_cpu_has(X86_FEATURE_HWP)) {
>> - acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
>> - ACPI_UINT32_MAX,
>> - acpi_hwp_native_thermal_lvt_osc,
>> - NULL, NULL, NULL);
>> - acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID,
>> - acpi_hwp_native_thermal_lvt_osc,
>> - NULL, NULL);
>> - }
>> + acpi_status status;
>> +
>> + processor_dmi_check();
>> +
>> + status = acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
>> + ACPI_UINT32_MAX, acpi_processor_osc, NULL,
>> + NULL, NULL);
>> + if (ACPI_FAILURE(status))
>> + return status;
>> +
>> + return acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID, acpi_processor_osc,
>> + NULL, NULL);
>> }
>> #endif
>>
>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>> index d161ff707de4..e8d1f645224f 100644
>> --- a/drivers/acpi/bus.c
>> +++ b/drivers/acpi/bus.c
>> @@ -1317,9 +1317,6 @@ static int __init acpi_bus_init(void)
>> goto error1;
>> }
>>
>> - /* Set capability bits for _OSC under processor scope */
>> - acpi_early_processor_osc();
>> -
>> /*
>> * _OSC method may exist in module level code,
>> * so it must be run after ACPI_FULL_INITIALIZATION
>> @@ -1335,7 +1332,15 @@ static int __init acpi_bus_init(void)
>>
>> acpi_sysfs_init();
>>
>> - acpi_early_processor_set_pdc();
>> +#ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
>> + status = acpi_early_processor_osc();
>> + if (ACPI_FAILURE(status)) {
>> + pr_err("_OSC methods failed, trying _PDC\n");
>> + acpi_early_processor_set_pdc();
>> + } else {
>> + pr_info("_OSC methods ran successfully\n");
>> + }
>> +#endif
>>
>> /*
>> * Maybe EC region is required at bus_scan/acpi_get_devices. So it
>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>> index f979a2f7077c..e7cc41313997 100644
>> --- a/drivers/acpi/internal.h
>> +++ b/drivers/acpi/internal.h
>> @@ -151,17 +151,10 @@ int acpi_wakeup_device_init(void);
>> -------------------------------------------------------------------------- */
>> #ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
>> void acpi_early_processor_set_pdc(void);
>> +acpi_status acpi_early_processor_osc(void);
>>
>> void processor_dmi_check(void);
>> bool processor_physically_present(acpi_handle handle);
>> -#else
>> -static inline void acpi_early_processor_set_pdc(void) {}
>> -#endif
>> -
>> -#ifdef CONFIG_X86
>> -void acpi_early_processor_osc(void);
>> -#else
>> -static inline void acpi_early_processor_osc(void) {}
>> #endif
>>
>> /* --------------------------------------------------------------------------
>> --