| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <56EB18B9.7070009@hpe.com>
Date: Thu, 17 Mar 2016 16:51:05 -0400
From: Linda Knippers <linda.knippers@....com>
To: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
rjw@...ysocki.net
Cc: tony.luck@...el.com, bp@...en8.de, tglx@...utronix.de,
mingo@...hat.com, hpa@...or.com, lenb@...nel.org,
linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] acpi: Issue _OSC call for native thermal interrupt
handling
On 3/17/2016 4:36 PM, Srinivas Pandruvada wrote:
> On Thu, 2016-03-17 at 16:03 -0400, Linda Knippers wrote:
>> On 3/17/2016 2:24 PM, Srinivas Pandruvada wrote:
>>>
>>> There are several reports of freeze on enabling HWP (Hardware
>>> PStates)
>>> feature on Skylake based systems by Intel P states driver. The root
>>> cause is identified as the HWP interrupts causing BIOS code to
>>> freeze.
>>> HWP interrupts uses thermal LVT.
>>> Linux natively handles thermal interrupts, but in Skylake based
>>> systems
>>> SMM will take control of thermal interrupts. This is a problem for
>>> several
>>> reasons:
>>> - It is freezing in BIOS when tries to handle thermal interrupt,
>>> which
>>> will require BIOS upgrade
>>> - With SMM handling thermal we loose all the reporting features of
>>> Linux arch/x86/kernel/cpu/mcheck/therm_throt driver
>>> - Some thermal drivers like x86-package-temp driver depends on the
>>> thermal
>>> threshold interrupts
>>> - The HWP interrupts are useful for debugging and tuning
>>> performance
>>>
>>> So we need native handling of thermal interrupts.
>> Is this working around a firmware bug?
> Yes. But Linux always had capability to handle thermal interrupts
> natively, so we should continue to have this in OS control, when
> possible.
>
>>
>>>
>>> To inform SMM that
>>> OS will handle thermal interrupts, we need to use _OSC under
>>> processor
>>> scope very early in ACPI initialization flow.
>> Does _OSC say that OS "will" handle thermal interrupts or that it
>> "can"
>> handle thermal interrupts?
>
> Can handle.
>
>> Couldn't a platform decide it wants to handle
>> them, regardless of what the OS may be capable of?
>>
> Yes. In that case platform can change the delivery mode bits to SMM in
> LVT.
And would still exhibit the BIOS bug?
>>>
>>> This needs to be done
>>> before SMM code path looks for _OSC capabilities. The bit 12 of
>>> _OSC in processor scope defines whether OS will handle thermal
>>> interrupts.
>>> When bit 12 is set to 1, OS will handle thermal interrupts.
>>> Refer to this document for details on _OSC
>>> http://www.intel.com/content/www/us/en/standards/processor-vendor-
>>> specific-acpi-specification.html
>> Where is bit 12 documented?
>>
> In the above document.
When I look at that document, I see bit 12 described as
"If set, OSPM supports native interrupt handling for Collaborative Processor
Performance Control notifications." Is that the same thing or am
I looking at the wrong table?
-- ljk
>
>>>
>>> This change introduces a new function
>>> acpi_early_processor_set_osc(),
>>> which walks acpi name space and finds acpi processor object and
>>> set capability via _OSC method to take over thermal LVT.
>> Does this change just affect Skylake platforms or all platforms?
> Any platform which has Intel ® Speed Shift Technology (aka HWP) feature
> present and enabled.
>
> Thanks,
> Srinivas
>
>>
>> -- ljk
>>>
>>>
>>> Also this change writes HWP status bits to 0 to clear any HWP
>>> status
>>> bits in intel_thermal_interrupt().
>>>
>>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel
>>> .com>
>>> ---
>>> v4:
>>> Suggestion by Boris for removing use of intermediate variable for
>>> clearing HWP status and using boot_cpu_has instead of
>>> static_cpu_has
>>>
>>> v3:
>>> - Added CONFIG_X86 around static_cpu_has() to fix compile error on
>>> ARCH=ia64, reported by kbuild test robot
>>> - return AE_CTRL_TERMINATE to terminate acpi name walk space, when
>>> _OSC
>>> is set already once.
>>> v2:
>>> Unnecessary newline was introduced, removed that in
>>> acpi_processor.c
>>>
>>> arch/x86/kernel/cpu/mcheck/therm_throt.c | 3 ++
>>> drivers/acpi/acpi_processor.c | 47
>>> ++++++++++++++++++++++++++++++++
>>> drivers/acpi/bus.c | 3 ++
>>> drivers/acpi/internal.h | 2 ++
>>> 4 files changed, 55 insertions(+)
>>>
>>> diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c
>>> b/arch/x86/kernel/cpu/mcheck/therm_throt.c
>>> index 2c5aaf8..0553858 100644
>>> --- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
>>> +++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
>>> @@ -385,6 +385,9 @@ static void intel_thermal_interrupt(void)
>>> {
>>> __u64 msr_val;
>>>
>>> + if (static_cpu_has(X86_FEATURE_HWP))
>>> + wrmsrl_safe(MSR_HWP_STATUS, 0);
>>> +
>>> rdmsrl(MSR_IA32_THERM_STATUS, msr_val);
>>>
>>> /* Check for violation of core thermal thresholds*/
>>> diff --git a/drivers/acpi/acpi_processor.c
>>> b/drivers/acpi/acpi_processor.c
>>> index 6979186..18da84f 100644
>>> --- a/drivers/acpi/acpi_processor.c
>>> +++ b/drivers/acpi/acpi_processor.c
>>> @@ -491,6 +491,53 @@ static void acpi_processor_remove(struct
>>> acpi_device *device)
>>> }
>>> #endif /* CONFIG_ACPI_HOTPLUG_CPU */
>>>
>>> +#ifdef CONFIG_X86
>>> +static bool acpi_hwp_native_thermal_lvt_set;
>>> +static acpi_status acpi_set_hwp_native_thermal_lvt_osc(acpi_handle
>>> handle,
>>> + u32 lvl,
>>> void *context,
>>> + void **rv)
>>> +{
>>> + u8 sb_uuid_str[] = "4077A616-290C-47BE-9EBD-D87058713953";
>>> + 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))) {
>>> + acpi_hwp_native_thermal_lvt_set = true;
>>> + kfree(osc_context.ret.pointer);
>>> + }
>>> +
>>> + return AE_OK;
>>> +}
>>> +
>>> +void acpi_early_processor_set_osc(void)
>>> +{
>>> + if (boot_cpu_has(X86_FEATURE_HWP)) {
>>> + acpi_walk_namespace(ACPI_TYPE_PROCESSOR,
>>> ACPI_ROOT_OBJECT,
>>> + ACPI_UINT32_MAX,
>>> + acpi_set_hwp_native_thermal_lv
>>> t_osc,
>>> + NULL, NULL, NULL);
>>> + acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID,
>>> + acpi_set_hwp_native_thermal_lvt_o
>>> sc,
>>> + NULL, NULL);
>>> + }
>>> +}
>>> +#else
>>> +
>>> +void acpi_early_processor_set_osc(void) {}
>>> +
>>> +#endif
>>> +
>>> /*
>>> * The following ACPI IDs are known to be suitable for
>>> representing as
>>> * processor devices.
>>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>>> index 891c42d..7e73aea 100644
>>> --- a/drivers/acpi/bus.c
>>> +++ b/drivers/acpi/bus.c
>>> @@ -1005,6 +1005,9 @@ static int __init acpi_bus_init(void)
>>> goto error1;
>>> }
>>>
>>> + /* Set capability bits for _OSC under processor scope */
>>> + acpi_early_processor_set_osc();
>>> +
>>> /*
>>> * _OSC method may exist in module level code,
>>> * so it must be run after ACPI_FULL_INITIALIZATION
>>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>>> index 1e6833a..5c787ac 100644
>>> --- a/drivers/acpi/internal.h
>>> +++ b/drivers/acpi/internal.h
>>> @@ -138,6 +138,8 @@ void acpi_early_processor_set_pdc(void);
>>> static inline void acpi_early_processor_set_pdc(void) {}
>>> #endif
>>>
>>> +void acpi_early_processor_set_osc(void);
>>> +
>>> /* -------------------------------------------------------------
>>> -------------
>>> Embedded Controller
>>> ---------------------------------------------------------------
>>> ----------- */
>>>
Powered by blists - more mailing lists