Re: [Xen-devel] [PATCH v2 3/5] x86/AMD: make C-state handling independent of Dom0

[Jan Beulich <JBeulich@xxxxxxxx>]

On 17.07.2019 12:26, Roger Pau Monné  wrote:
> On Wed, Jul 17, 2019 at 09:04:55AM +0000, Jan Beulich wrote:
>> On 16.07.2019 16:26, Roger Pau Monné  wrote:
>>> On Wed, Jul 03, 2019 at 01:01:48PM +0000, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/acpi/cpu_idle.c
>>>> +++ b/xen/arch/x86/acpi/cpu_idle.c
>>>> @@ -110,6 +110,8 @@ boolean_param("lapic_timer_c2_ok", local
>>>>     
>>>>     struct acpi_processor_power *__read_mostly processor_powers[NR_CPUS];
>>>>     
>>>> +static int8_t __read_mostly vendor_override;
>>>
>>> AFAICT from the code below this is a tri-state (-1, 0, 1). Could it
>>> maybe be turned into an enum with definitions of the different
>>> states?
>>>
>>> I think it would make the usage clearer.
>>
>> Well, personally I prefer doing it this way for tristates. I'll
>> wait to see what others think.
> 
> Ack, I think the code is correct hence:
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

> But I personally would prefer an enum or at least a description of
> the meaning of the values vendor_override can take. IMO it would help
> understanding the code, since it's not obvious to me at first sight.

I've added

/*
  * This field starts out as zero, and can be set to -1 just to signal it has
  * been set (and that vendor specific logic has failed, and shouldn't be
  * tried again), or to +1 to ignore Dom0 side uploads of C-state ACPI data.
  */

ahead of the definition.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel