Re: [PATCH] acpi: Allow ignoring _OSC CPPC v2 bit via kernel parameter

Mario Limonciello
2024-06-18T19:28:03Z

On 6/18/2024 14:25, Aaron Rainbolt wrote:
> On Tue, Jun 18, 2024 at 01:58:07PM -0500, Mario Limonciello wrote:
>> On 6/18/2024 13:52, Aaron Rainbolt wrote:
>>> On Tue, Jun 18, 2024 at 01:35:57PM -0500, Mario Limonciello wrote:
>>>> On 6/18/2024 13:30, Aaron Rainbolt wrote:
>>>>> On Tue, Jun 18, 2024 at 12:09:19PM -0500, Mario Limonciello wrote:
>>>>>> On 6/17/2024 21:54, Aaron Rainbolt wrote:
>>>>>>> acpi: Allow ignoring _OSC CPPC v2 bit via kernel parameter
>>>>>>>
>>>>>>> The _OSC is supposed to contain a bit indicating whether the hardware
>>>>>>> supports CPPC v2 or not. This bit is not always set, causing CPPC v2 to
>>>>>>> be considered absent. This results in severe single-core performance
>>>>>>> issues with the EEVDF scheduler.
>>>>>>>
>>>>>>> To work around this, provide a new kernel parameter,
>>>>>>> "processor.ignore_osc_cppc_bit", which may be used to ignore the _OSC
>>>>>>> CPPC v2 bit and act as if the bit was enabled. This allows CPPC to be
>>>>>>> properly detected even if not "enabled" by _OSC, allowing users with
>>>>>>> problematic hardware to obtain decent single-core performance.
>>>>>>>
>>>>>>> Tested-by: Michael Mikowski <mmikowski@kfocus.org>
>>>>>>> Signed-off-by: Aaron Rainbolt <arainbolt@kfocus.org>
>>>>>>
>>>>>> This sounds like a platform bug and if we do accept a patch like this I
>>>>>> think we need a lot more documentation about the situation.
>>>>>
>>>>> It is a platform bug, yes. See my previous email,
>>>>> https://lore.kernel.org/linux-acpi/d01b0a1f-bd33-47fe-ab41-43843d8a374f@kfocus.org/T/#u
>>>>> (I meant to send this email as a reply to that one, but failed to do so.)
>>>>>
>>>>>> Can you please share more information about your hardware:
>>>>>> 1) Manufacturer?
>>>>>
>>>>> Carbon Systems, models Iridium 14 and Iridium 16.
>>>>>
>>>>>> 2) CPU?
>>>>>
>>>>> Intel Core i5-13500H.
>>>>>
>>>>>> 3) Manufacturer firmware version?
>>>>>
>>>>> The systems use an AMI BIOS with version N.1.10CAR01 according to
>>>>> dmidecode. This is the latest BIOS available from the manufacturer.
>>>>>
>>>>>> 4) If it's AMD what's the AGESA version?
>>>>>
>>>>> Both affected systems are Intel-based and use heterogenous cores, not AMD.
>>>>>
>>>>>> And most importantly do you have the latest system firmware version from
>>>>>> your manufacturer? If not; please upgrade that first.
>>>>>
>>>>> We are using the latest firmware. (We're trying to work with the ODM to
>>>>> potentially get a firmware update, but since this affects more than just
>>>>> us and a firmware update may not be possible for everyone, this would
>>>>> likely be worth providing a kernel-level workaround for.)
>>>>>
>>>>> I can easily provide more detailed information - would the full output of
>>>>> 'dmidecode' and 'acpidump' be useful?
>>>>
>>>> Does your BIOS offer any options for these?
>>>>
>>>> Intel(R) SpeedStep(TM)
>>>> Intel Speed Shift Technology(TM)
>>>>
>>>> I believe you need those enabled for this to work properly.
>>>
>>> Neither option is available in the BIOS settings UI, however our ODM
>>> confirmed that both Intel Speed Shift Technology and Intel Turbo Boost Max
>>> Technology 3.0 are enabled by default. They did not mention SpeedStep,
>>> but I assume SpeedStep is working since frequency scaling in general
>>> works and the kernel patch fixes the issue.
>>
>> Got it. If those are enabled I think it would be good to get comments from
>> Rafael and Srinivas about your specific situation then.
>>
>> But regarding the patch, if they are agreeable to this "kind" of knob for
>> debugging I personally think it's better to have cpc_supported_by_cpu() look
>> at the kernel command line than plumb arguments from the module down through
>> every function.
>
> Just to be clear since I'm not all too familiar with how kernel params work,
> should core_param be used here? Or is there a variable that allows
> accessing the entire command line to look through it? I don't think I can
> use module_param in 'arch/x86/kernel/acpi/cppc.c', core_param has a
> comment over it describing it as "historical" so I don't think I should
> use it, and early_param looks like something one is only supposed to use
> in code that runs very early at kernel startup. I can probably figure it
> out on my own, but a quick pointer would be helpful.

early_param is how I would do it.

You can save it to a static boolean global variable in that file. Make
sure that you update documentation about the new early_param though too.