Re: [Xen-devel] [PATCH v4 09/15] tools: implement the new libxc get hw info interface

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

On Sat, Sep 23, 2017 at 09:48:18AM +0000, Yi Sun wrote:
> +enum xc_psr_feat_type {
> +    XC_PSR_CAT_L3,
> +    XC_PSR_CAT_L2,
> +    XC_PSR_MBA,
> +};
> +typedef enum xc_psr_feat_type xc_psr_feat_type;
> +
> +struct xc_psr_hw_info {
> +    union {
> +        struct {
> +            uint32_t cos_max;
> +            uint32_t cbm_len;
> +            bool     cdp_enabled;
> +        } cat;
> +
> +        struct {
> +            uint32_t cos_max;
> +            uint32_t thrtl_max;
> +            bool     linear;
> +        } mba;
> +    } u;
> +};

Why not:

union xc_psr_hw_info {
    struct {
        uint32_t cos_max;
        uint32_t cbm_len;
        bool     cdp_enabled;
    } cat;

    struct {
        uint32_t cos_max;
        uint32_t thrtl_max;
        bool     linear;
    } mba;
};

> +typedef struct xc_psr_hw_info xc_psr_hw_info;
> +
>  int xc_psr_cmt_attach(xc_interface *xch, uint32_t domid);
>  int xc_psr_cmt_detach(xc_interface *xch, uint32_t domid);
>  int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
> @@ -2480,9 +2504,8 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, 
> uint32_t domid,
>  int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
>                                 xc_psr_cat_type type, uint32_t target,
>                                 uint64_t *data);
> -int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, unsigned int lvl,
> -                        uint32_t *cos_max, uint32_t *cbm_len,
> -                        bool *cdp_enabled);
> +int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
> +                       xc_psr_feat_type type, xc_psr_hw_info *hw_info);
>  
>  int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps);
>  int xc_get_cpu_featureset(xc_interface *xch, uint32_t index,
> diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
> index 623e26b..f2b5ee6 100644
> --- a/tools/libxc/xc_psr.c
> +++ b/tools/libxc/xc_psr.c
> @@ -323,37 +323,57 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, 
> uint32_t domid,
>      return rc;
>  }
>  
> -int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, unsigned int lvl,
> -                        uint32_t *cos_max, uint32_t *cbm_len, bool 
> *cdp_enabled)
> +int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
> +                       xc_psr_feat_type type, xc_psr_hw_info *hw_info)
>  {
>      int rc = -1;
>      DECLARE_SYSCTL;
>  
> +    if ( !hw_info )
> +    {
> +        errno = EINVAL;
> +        return rc;
> +    }
> +
>      sysctl.cmd = XEN_SYSCTL_psr_alloc;
>      sysctl.u.psr_alloc.target = socket;
>  
> -    switch ( lvl )
> +    switch ( type )
>      {
> -    case 2:
> +    case XC_PSR_CAT_L2:
>          sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_get_l2_info;
>          rc = xc_sysctl(xch, &sysctl);
> -        if ( !rc )
> -        {
> -            *cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
> -            *cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
> -            *cdp_enabled = false;
> -        }
> +        if ( rc )
> +            break;
> +
> +        hw_info->u.cat.cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
> +        hw_info->u.cat.cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
> +        hw_info->u.cat.cdp_enabled = false;
> +
>          break;
> -    case 3:
> +    case XC_PSR_CAT_L3:
>          sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_get_l3_info;
>          rc = xc_sysctl(xch, &sysctl);
> -        if ( !rc )
> -        {
> -            *cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
> -            *cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
> -            *cdp_enabled = sysctl.u.psr_alloc.u.cat_info.flags &
> -                           XEN_SYSCTL_PSR_L3_CDP;
> -        }
> +        if ( rc )
> +            break;
> +
> +        hw_info->u.cat.cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
> +        hw_info->u.cat.cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
> +        hw_info->u.cat.cdp_enabled = sysctl.u.psr_alloc.u.cat_info.flags &
> +                                     XEN_SYSCTL_PSR_L3_CDP;

The two cases (XC_PSR_CAT_L2 and XC_PSR_CAT_L3) can be easily joined
into a single one IMHO.

Thanks, Roger.

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