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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jul 17, 2019 at 12:49:49PM +0000, Jan Beulich wrote:
> 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

Thanks! I'm happy with this.

> /*
>   * 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.
                               ^ signal vendor specific setup has
                                 succeed and dom0 side uploads of
                                 C-state ACPI data should be ignored.

I think is even clearer, but might be to verbose? Anyway, I don't
have a strong opinion, and your text is certainly fine.

Roger.

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