From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH 4/4] xenpm: Alow getting and setting the max
 sub C-State
Date: Wed, 18 Jun 2014 16:28:10 +0100
Message-ID: <53A1B00A.2000900@citrix.com>
References: <1403103855-23080-1-git-send-email-ross.lagerwall@citrix.com>
	<1403103855-23080-5-git-send-email-ross.lagerwall@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1403103855-23080-5-git-send-email-ross.lagerwall@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Keir Fraser <keir@xen.org>, Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Liu Jinsong <jinsong.liu@alibaba-inc.com>, Ian Jackson <ian.jackson@eu.citrix.com>, xen-devel@lists.xen.org, Jan Beulich <jbeulich@suse.com>
List-Id: xen-devel@lists.xenproject.org

On 18/06/14 16:04, Ross Lagerwall wrote:
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  tools/misc/xenpm.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
> index e43924c..25d8613 100644
> --- a/tools/misc/xenpm.c
> +++ b/tools/misc/xenpm.c
> @@ -64,6 +64,7 @@ void show_help(void)
>              " set-vcpu-migration-delay      <num> set scheduler vcpu migration delay in us\n"
>              " get-vcpu-migration-delay            get scheduler vcpu migration delay\n"
>              " set-max-cstate        <num>         set the C-State limitation (<num> >= 0)\n"
> +            " set-max-substate      <num>         set the sub C-State limitation (<num> >= 0)\n"
>              " start [seconds]                     start collect Cx/Px statistics,\n"
>              "                                     output after CTRL-C or SIGINT or several seconds.\n"
>              " enable-turbo-mode     [cpuid]       enable Turbo Mode for processors that support it.\n"
> @@ -188,7 +189,19 @@ static int show_max_cstate(xc_interface *xc_handle)
>      if ( (ret = xc_get_cpuidle_max_cstate(xc_handle, &value)) )
>          return ret;
>  
> -    printf("Max possible C-state: C%d\n\n", value);
> +    printf("Max possible C-state: C%d\n", value);
> +    return 0;
> +}
> +
> +static int show_max_substate(xc_interface *xc_handle)
> +{
> +    int ret = 0;
> +    uint32_t value;
> +
> +    if ( (ret = xc_get_cpuidle_max_substate(xc_handle, &value)) )
> +        return ret;
> +
> +    printf("Max possible sub C-state: %d\n\n", value);
>      return 0;
>  }
>  
> @@ -223,6 +236,7 @@ void cxstat_func(int argc, char *argv[])
>          parse_cpuid(argv[0], &cpuid);
>  
>      show_max_cstate(xc_handle);
> +    show_max_substate(xc_handle);
>  
>      if ( cpuid < 0 )
>      {
> @@ -1088,6 +1102,23 @@ void set_max_cstate_func(int argc, char *argv[])
>                  value, errno, strerror(errno));
>  }
>  
> +void set_max_substate_func(int argc, char *argv[])
> +{
> +    int value;
> +
> +    if ( argc != 1 || sscanf(argv[0], "%d", &value) != 1 || value < 0 )
> +    {
> +        fprintf(stderr, "Missing or invalid argument(s)\n");
> +        exit(EINVAL);
> +    }
> +
> +    if ( !xc_set_cpuidle_max_substate(xc_handle, (uint32_t)value) )
> +        printf("set max_substate to %d succeeded\n", value);
> +    else
> +        fprintf(stderr, "set max_substate to %d failed (%d - %s)\n",
> +                value, errno, strerror(errno));
> +}
> +

Please don't inherit the brokenness of set_max_cstate_func() for the
sake of copying it.

"value "should be unsigned all the way through, and you should use %u in
place of %d everywhere.

~Andrew

>  void enable_turbo_mode(int argc, char *argv[])
>  {
>      int cpuid = -1;
> @@ -1154,6 +1185,7 @@ struct {
>      { "get-vcpu-migration-delay", get_vcpu_migration_delay_func},
>      { "set-vcpu-migration-delay", set_vcpu_migration_delay_func},
>      { "set-max-cstate", set_max_cstate_func},
> +    { "set-max-substate", set_max_substate_func},
>      { "enable-turbo-mode", enable_turbo_mode },
>      { "disable-turbo-mode", disable_turbo_mode },
>  };