[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 3/5] x86/AMD: make C-state handling independent of Dom0
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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |