Re: [Xen-devel] [PATCH v2 5/5] tools/libxc: allow controlling the max C-state sub-state

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

On Wed, Jul 03, 2019 at 01:04:13PM +0000, Jan Beulich wrote:
> From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> 
> Make handling in do_pm_op() more homogeneous: Before interpreting
> op->cpuid as such, handle all operations not acting on a particular
> CPU. Also expose the setting via xenpm.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> v2: Adjust xenpm's usage message. Also adjust its output wording a
>      little.
> 
> --- a/tools/libxc/xc_pm.c
> +++ b/tools/libxc/xc_pm.c
> @@ -367,7 +367,7 @@ int xc_set_sched_opt_smt(xc_interface *x
>      return rc;
>   }
>   
> -int xc_get_cpuidle_max_cstate(xc_interface *xch, uint32_t *value)
> +static int get_max_cstate(xc_interface *xch, uint32_t *value, uint32_t type)
>   {
>       int rc;
>       DECLARE_SYSCTL;
> @@ -379,7 +379,7 @@ int xc_get_cpuidle_max_cstate(xc_interfa
>       }
>       sysctl.cmd = XEN_SYSCTL_pm_op;
>       sysctl.u.pm_op.cmd = XEN_SYSCTL_pm_op_get_max_cstate;
> -    sysctl.u.pm_op.cpuid = 0;
> +    sysctl.u.pm_op.cpuid = type;
>       sysctl.u.pm_op.u.get_max_cstate = 0;
>       rc = do_sysctl(xch, &sysctl);
>       *value = sysctl.u.pm_op.u.get_max_cstate;
> @@ -387,7 +387,17 @@ int xc_get_cpuidle_max_cstate(xc_interfa
>       return rc;
>   }
>   
> -int xc_set_cpuidle_max_cstate(xc_interface *xch, uint32_t value)
> +int xc_get_cpuidle_max_cstate(xc_interface *xch, uint32_t *value)
> +{
> +    return get_max_cstate(xch, value, 0);
> +}
> +
> +int xc_get_cpuidle_max_csubstate(xc_interface *xch, uint32_t *value)
> +{
> +    return get_max_cstate(xch, value, 1);
> +}
> +
> +static int set_max_cstate(xc_interface *xch, uint32_t value, uint32_t type)
>   {
>       DECLARE_SYSCTL;
>   
> @@ -398,12 +408,22 @@ int xc_set_cpuidle_max_cstate(xc_interfa
>       }
>       sysctl.cmd = XEN_SYSCTL_pm_op;
>       sysctl.u.pm_op.cmd = XEN_SYSCTL_pm_op_set_max_cstate;
> -    sysctl.u.pm_op.cpuid = 0;
> +    sysctl.u.pm_op.cpuid = type;
>       sysctl.u.pm_op.u.set_max_cstate = value;
>   
>       return do_sysctl(xch, &sysctl);
>   }
>   
> +int xc_set_cpuidle_max_cstate(xc_interface *xch, uint32_t value)
> +{
> +    return set_max_cstate(xch, value, 0);
> +}
> +
> +int xc_set_cpuidle_max_csubstate(xc_interface *xch, uint32_t value)
> +{
> +    return set_max_cstate(xch, value, 1);
> +}
> +
>   int xc_enable_turbo(xc_interface *xch, int cpuid)
>   {
>       DECLARE_SYSCTL;
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1906,6 +1906,9 @@ int xc_set_sched_opt_smt(xc_interface *x
>   int xc_get_cpuidle_max_cstate(xc_interface *xch, uint32_t *value);
>   int xc_set_cpuidle_max_cstate(xc_interface *xch, uint32_t value);
>   
> +int xc_get_cpuidle_max_csubstate(xc_interface *xch, uint32_t *value);
> +int xc_set_cpuidle_max_csubstate(xc_interface *xch, uint32_t value);
> +
>   int xc_enable_turbo(xc_interface *xch, int cpuid);
>   int xc_disable_turbo(xc_interface *xch, int cpuid);
>   
> --- a/tools/misc/xenpm.c
> +++ b/tools/misc/xenpm.c
> @@ -64,7 +64,9 @@ void show_help(void)
>               " set-sched-smt           enable|disable enable/disable 
> scheduler smt power saving\n"
>               " 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>|'unlimited' set the C-State 
> limitation (<num> >= 0)\n"
> +            " set-max-cstate        <num>|'unlimited'[,<num2>|'unlimited']\n"
> +            "                                     set the C-State limitation 
> (<num> >= 0) and\n"
> +            "                                     optionally the C-sub-state 
> limitation (<num2> >= 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"
> @@ -195,7 +197,15 @@ static int show_max_cstate(xc_interface
>           return ret;
>   
>       if ( value < XEN_SYSCTL_CX_UNLIMITED )
> -        printf("Max possible C-state: C%"PRIu32"\n\n", value);
> +    {
> +        printf("Max possible C-state: C%"PRIu32"\n", value);
> +        if ( (ret = xc_get_cpuidle_max_csubstate(xc_handle, &value)) )
> +            return ret;
> +        if ( value < XEN_SYSCTL_CX_UNLIMITED )
> +            printf("Max possible substate: %"PRIu32"\n\n", value);
> +        else
> +            puts("");
> +    }
>       else
>           printf("All C-states allowed\n\n");
>   
> @@ -1120,13 +1130,17 @@ void get_vcpu_migration_delay_func(int a
>   
>   void set_max_cstate_func(int argc, char *argv[])
>   {
> -    int value;
> +    int value, subval = XEN_SYSCTL_CX_UNLIMITED;
>       char buf[12];
>   
> -    if ( argc != 1 ||
> +    if ( argc < 1 || argc > 2 ||

I'm quite sure I'm missing something, but shouldn't argc still be 1
regardless of whether the max sub-state is set or not?

I would expect to scan for something like: "%d,%d" or some such, but
maybe there's a step I'm missing that splits the string using the ','
separator?

Thanks, Roger.

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