Re: [PATCH 4/4] xenpm: Alow getting and setting the max sub C-State

[Andrew Cooper <andrew.cooper3@citrix.com>]

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 },
>  };