Re: [Xen-devel] [PATCH v5 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general

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

On Sat, Sep 30, 2017 at 01:39:12AM +0000, Yi Sun wrote:
> This patch renames PSR sysctl/domctl interfaces and related xsm policy to
> make them be general for all resource allocation features but not only
> for CAT. Then, we can resuse the interfaces for all allocation features.
> 
> Basically, it changes 'psr_cat_op' to 'psr_alloc', and remove 'CAT_' from some
> macros. E.g.:
> 1. psr_cat_op -> psr_alloc
> 2. XEN_DOMCTL_psr_cat_op -> XEN_DOMCTL_psr_alloc
> 3. XEN_SYSCTL_psr_cat_op -> XEN_SYSCTL_psr_alloc
> 4. XEN_DOMCTL_PSR_CAT_SET_L3_CBM -> XEN_DOMCTL_PSR_SET_L3_CBM
> 5. XEN_SYSCTL_PSR_CAT_get_l3_info -> XEN_SYSCTL_PSR_get_l3_info
> 
> The sysctl version number is bumped.
> 
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Provided that the following comments are addressed.

> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -37,6 +37,16 @@
>  #include <asm/psr.h>
>  #include <asm/cpuid.h>
>  
> +#define domctl_psr_get_val(d, domctl, type, copyback) ({   \
> +    int r__;                                               \
> +    uint32_t v__;                                          \

No '__' in variable names, and missing newline.

Initializing 'r' can be done together with the variable declaration.

> +    r__ = psr_get_val(d, domctl->u.psr_alloc.target,       \

(domctl) here and below.

> +                      &v__, type);                         \
> +    domctl->u.psr_alloc.data = v__;                        \
> +    copyback = true;                                       \
> +    r__;                                                   \
> +})

Since the macro is only used in a single function, I would define it
inside of the function, and undef it afterwards.

> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 7830b98..24191f0 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -36,7 +36,7 @@
>  #include "physdev.h"
>  #include "tmem.h"
>  
> -#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000F
> +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000010

This has already been bumped for this release, not sure you need to
bump it again, if the same logic as the one used in domctl.h applies.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel