[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


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <JBeulich@xxxxxxxx>
  • Date: Wed, 17 Jul 2019 12:49:49 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1;spf=pass smtp.mailfrom=suse.com;dmarc=pass action=none header.from=suse.com;dkim=pass header.d=suse.com;arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=azA4yRd274fnGczU0bOqmU6PWpW/dur0908CSKnSlYc=; b=h873QasfawuL2fca0e/7yYFKOPUhOYVMdLjLAyOUeFVunMpt00LceEQ7lusBs2kdF2UOakMM0TLUmlxk3/auMN+TDQHL7sx4QtvtrnRg/szWBtr0+0d60vjQed2tFeQ+m6kakGiRLMkiBEuu8onuCU/pg2Yzj/FYqexHJJyNYSZGqpn7aZSZaP+H2xSnQpUGLShRzXPZEUXQly3MHNFu4rJASHyi0vW8N6AZiL5bjdHx6XPHguQS2MtOrkYAfgr6fXq7VDNFeXA/fJaGhyPcxYRGSd2mZERSHq4audZs1tRP0fGzZ9XJggAtxcmmQqouQpHDWSEyYCmK8G9urE0mDw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dmDms1uy3G3TEvMBbDw/d83a99FGJe4Nfyca36g1pihtttV3I+ttdIhdn9ZXXgaDHwxHKTle+4T17OlOFz+flgcAjyIk4QxRqgs1CdBi1KPYXH1eh3LOVzftp39DFo69TThMdYaGjPKoC0HUASM2t3tpb7eo5zUxBPsRK60spqmqr9/CVm2TAAzuNxMuXaxqOOLfazTWyEdqXJSaciTga09LrtmJ5e2bziCBtB1HpI/mdOch/pxxTW/l+B6hSUSbGoAljhWWFrM9aoZGLHcpFaU2jLD9qL5nZis7Mc7bDUwJdGoT10N1JtVnn30iHZ21BmAoX5+xcs3WIU5A5sCEtg==
  • Authentication-results: spf=none (sender IP is ) smtp.mailfrom=JBeulich@xxxxxxxx;
  • Cc: AndrewCooper <andrew.cooper3@xxxxxxxxxx>, Brian Woods <brian.woods@xxxxxxx>, Wei Liu <wl@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 17 Jul 2019 12:52:47 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHVMZ95Pg5KTi9KKk6xsZ3jj2BciKbNYdpqgAE4SYCAABcgWoAAJ7cA
  • Thread-topic: [PATCH v2 3/5] x86/AMD: make C-state handling independent of Dom0

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.