|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 9/9] x86/mwait-idle: Add C-states validation
On 24.04.2026 21:15, Roger Pau Monné wrote:
> On Thu, Mar 12, 2026 at 05:58:21PM +0100, Jan Beulich wrote:
>> @@ -1589,6 +1594,41 @@ static char *__init get_cmdline_field(ch
>> }
>>
>> /**
>> + * validate_cmdline_cstate - Validate a C-state from cmdline.
>> + * @state: The C-state to validate.
>> + * @prev_state: The previous C-state in the table or NULL.
>> + *
>> + * Return: 0 if the C-state is valid or -EINVAL otherwise.
>
> Hm, I know we picked this up from upstream, but this function would
> better return a boolean, rather than 0 or -EINVAL.
I agree, but I didn't want to deviate from their code purely for cosmetic
reasons.
>> +static int __init validate_cmdline_cstate(struct cpuidle_state *state,
>> + struct cpuidle_state *prev_state)
>> +{
>> + if (state->exit_latency == 0)
>> + /* Exit latency 0 can only be used for the POLL state */
>> + return -EINVAL;
>> +
>> + if (state->exit_latency > MAX_CMDLINE_LATENCY_US)
>> + return -EINVAL;
>> +
>> + if (state->target_residency > MAX_CMDLINE_RESIDENCY_US)
>> + return -EINVAL;
>> +
>> + if (state->target_residency < state->exit_latency)
>> + return -EINVAL;
>> +
>> + if (!prev_state)
>> + return 0;
>> +
>> + if (state->exit_latency <= prev_state->exit_latency)
>> + return -EINVAL;
>> +
>> + if (state->target_residency <= prev_state->target_residency)
>> + return -EINVAL;
>
> I'm not an expert on C-states, but isn't this checking against the
> previous value kind of defeating part of the purpose of the command
> line?
I don't know. The question would need raising to the author.
> Also, it might help to also write down those limits in the command
> line documentation.
What do you mean there? Some of the values are universal, but some
checks are against model-specific values. I don't think you mean to
enumerate them all?
Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |