| Message ID | 1503537289-56036-11-git-send-email-yi.y.sun@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 24, 2017 at 09:14:44AM +0800, Yi Sun wrote: > This patch implements the new libxl get hw info interface, > 'libxl_psr_get_hw_info', which is suitable to all psr allocation > features. It also implements corresponding list free function, > 'libxl_psr_hw_info_list_free' and make 'libxl_psr_cat_get_info' to call > 'libxl_psr_get_hw_info' to avoid redundant codes in libxl_psr.c. > > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > --- > v2: > - split this patch out from a big patch in v1. > (suggested by Wei Liu) > - change 'CAT_INFO'/'MBA_INFO' to 'CAT' and 'MBA. Also the libxl structure > name 'cat_info'/'mba_info' is changed to 'cat'/'mba'. > (suggested by Chao Peng) > - call 'libxl_psr_hw_info_list_free' in 'libxl_psr_cat_get_info' to free > allocated resources. > (suggested by Chao Peng) > --- > tools/libxl/libxl_psr.c | 145 +++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 112 insertions(+), 33 deletions(-) > > diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c > index b183305..d7da7d7 100644 > --- a/tools/libxl/libxl_psr.c > +++ b/tools/libxl/libxl_psr.c > @@ -382,56 +382,51 @@ static inline xc_psr_feat_type libxl__psr_feat_type_to_libxc_psr_feat_type( > return xc_type; > } > > +static inline int libxl__psr_hw_info_to_libxl_psr_cat_info( No inline. Maybe you could try to shorter the name? > + libxl_psr_feat_type type, libxl_psr_hw_info *hw_info, > + libxl_psr_cat_info *cat_info) > +{ > + if (type != LIBXL_PSR_FEAT_TYPE_CAT) > + return ERROR_INVAL; > + > + cat_info->id = hw_info->id; > + cat_info->cos_max = hw_info->u.cat.cos_max; > + cat_info->cbm_len = hw_info->u.cat.cbm_len; > + cat_info->cdp_enabled = hw_info->u.cat.cdp_enabled; > + > + return 0; > +} > + > int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info, > int *nr, unsigned int lvl) > { > GC_INIT(ctx); > int rc; > - int i = 0, socketid, nr_sockets; > - libxl_bitmap socketmap; > + unsigned int i; > + libxl_psr_hw_info *hw_info; > libxl_psr_cat_info *ptr; > - xc_psr_hw_info hw_info; > - xc_psr_feat_type xc_type; > - > - libxl_bitmap_init(&socketmap); > - > - rc = libxl__count_physical_sockets(gc, &nr_sockets); > - if (rc) { > - LOGE(ERROR, "failed to get system socket count"); > - goto out; > - } > > - libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets); > - rc = libxl_get_online_socketmap(ctx, &socketmap); > - if (rc < 0) { > - LOGE(ERROR, "failed to get available sockets"); > + rc = libxl_psr_get_hw_info(ctx, &hw_info, (unsigned int *)nr, Is there any reason nr is int instead of unsigned int? I would rather avoid casting things. Since this interface has not been present in a release yet, could you please send a separate patch to fix this if nr has no reason to be signed? > + LIBXL_PSR_FEAT_TYPE_CAT, lvl); > + if (rc) > goto out; > - } > > - xc_type = libxl__psr_feat_type_to_libxc_psr_feat_type( > - LIBXL_PSR_FEAT_TYPE_CAT, lvl); > + ptr = libxl__malloc(NOGC, *nr * sizeof(libxl_psr_cat_info)); > > - ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_cat_info)); > - > - libxl_for_each_set_bit(socketid, socketmap) { > - ptr[i].id = socketid; > - if (xc_psr_get_hw_info(ctx->xch, socketid, xc_type, &hw_info)) { > + for (i = 0; i < *nr; i++) { > + if (libxl__psr_hw_info_to_libxl_psr_cat_info( > + LIBXL_PSR_FEAT_TYPE_CAT, > + &hw_info[i], &ptr[i])) { > + libxl_psr_hw_info_list_free(hw_info, (unsigned int)*nr); > rc = ERROR_FAIL; > free(ptr); > goto out; > } > - > - ptr[i].cos_max = hw_info.u.xc_cat_info.cos_max; > - ptr[i].cbm_len = hw_info.u.xc_cat_info.cbm_len; > - ptr[i].cdp_enabled = hw_info.u.xc_cat_info.cdp_enabled; > - > - i++; > } > > *info = ptr; > - *nr = i; > + libxl_psr_hw_info_list_free(hw_info, (unsigned int)*nr); > out: > - libxl_bitmap_dispose(&socketmap); > GC_FREE; > return rc; > } > @@ -469,15 +464,99 @@ int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid, > return ERROR_FAIL; > } > > +static inline int libxc__psr_hw_info_to_libxl_psr_hw_info( No inline. Again shorter names would be better (although I understand this might not be possible). Also, why are you adding a libxc__ prefixed function to libxl? > + libxl_psr_feat_type type, xc_psr_hw_info *xc_hw_info, > + libxl_psr_hw_info *xl_hw_info) you could drop the '_hw' in the parameter names, so all the assignments below would fit on a single line. > +{ > + switch (type) { > + case LIBXL_PSR_FEAT_TYPE_CAT: > + xl_hw_info->u.cat.cos_max = xc_hw_info->u.xc_cat_info.cos_max; > + xl_hw_info->u.cat.cbm_len = xc_hw_info->u.xc_cat_info.cbm_len; > + xl_hw_info->u.cat.cdp_enabled = > + xc_hw_info->u.xc_cat_info.cdp_enabled; > + break; > + case LIBXL_PSR_FEAT_TYPE_MBA: > + xl_hw_info->u.mba.cos_max = xc_hw_info->u.xc_mba_info.cos_max; > + xl_hw_info->u.mba.thrtl_max = xc_hw_info->u.xc_mba_info.thrtl_max; > + xl_hw_info->u.mba.linear = xc_hw_info->u.xc_mba_info.linear; > + break; > + default: > + return ERROR_INVAL; > + } > + > + return 0; > +} > + > int libxl_psr_get_hw_info(libxl_ctx *ctx, libxl_psr_hw_info **info, > unsigned int *nr, libxl_psr_feat_type type, > unsigned int lvl) > { > - return ERROR_FAIL; > + GC_INIT(ctx); > + int rc, nr_sockets; > + unsigned int i = 0, socketid; > + libxl_bitmap socketmap; > + libxl_psr_hw_info *ptr; > + xc_psr_feat_type xc_type; > + xc_psr_hw_info hw_info; > + > + libxl_bitmap_init(&socketmap); > + > + if (type == LIBXL_PSR_FEAT_TYPE_CAT && lvl != 3 && lvl != 2) { > + LOGE(ERROR, "input lvl %d is wrong!\n", lvl); LOGE is used when errno is set, which I don't think it's the case here. Please use plain LOG. > + rc = ERROR_FAIL; > + goto out; > + } > + > + xc_type = libxl__psr_feat_type_to_libxc_psr_feat_type(type, lvl); Isn't the above function going to return and invalid type if the feature is CAT and the level is not correct? In which case you could remove the above if and just check that the returned type here is not invalid? > + > + rc = libxl__count_physical_sockets(gc, &nr_sockets); > + if (rc) { > + LOGE(ERROR, "failed to get system socket count"); Again, libxl__ functions don't set errno. In this case this might be fine, because libxl__count_physical_sockets calls into libxc which sets errno, but you should only use LOGE when reporting errors from system calls or libxc functions. Roger.
On 17-08-30 10:15:22, Roger Pau Monn� wrote: > On Thu, Aug 24, 2017 at 09:14:44AM +0800, Yi Sun wrote: > > diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c > > index b183305..d7da7d7 100644 > > --- a/tools/libxl/libxl_psr.c > > +++ b/tools/libxl/libxl_psr.c > > @@ -382,56 +382,51 @@ static inline xc_psr_feat_type libxl__psr_feat_type_to_libxc_psr_feat_type( > > return xc_type; > > } > > > > +static inline int libxl__psr_hw_info_to_libxl_psr_cat_info( > > No inline. Maybe you could try to shorter the name? > Got it. Will remove the last '_psr'. [...] > > int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info, > > int *nr, unsigned int lvl) > > { > > GC_INIT(ctx); > > int rc; > > - int i = 0, socketid, nr_sockets; > > - libxl_bitmap socketmap; > > + unsigned int i; > > + libxl_psr_hw_info *hw_info; > > libxl_psr_cat_info *ptr; > > - xc_psr_hw_info hw_info; > > - xc_psr_feat_type xc_type; > > - > > - libxl_bitmap_init(&socketmap); > > - > > - rc = libxl__count_physical_sockets(gc, &nr_sockets); > > - if (rc) { > > - LOGE(ERROR, "failed to get system socket count"); > > - goto out; > > - } > > > > - libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets); > > - rc = libxl_get_online_socketmap(ctx, &socketmap); > > - if (rc < 0) { > > - LOGE(ERROR, "failed to get available sockets"); > > + rc = libxl_psr_get_hw_info(ctx, &hw_info, (unsigned int *)nr, > > Is there any reason nr is int instead of unsigned int? > > I would rather avoid casting things. Since this interface has not been > present in a release yet, could you please send a separate patch to > fix this if nr has no reason to be signed? > This is a historical issue. The first version of PSR introduced 'libxl_psr_cat_get_l3_info'. The input parameter is 'int *nr'. I think we cannot change the interface which has been merged and used by others. Right? > > + LIBXL_PSR_FEAT_TYPE_CAT, lvl); [...] > > > > +static inline int libxc__psr_hw_info_to_libxl_psr_hw_info( > > No inline. Again shorter names would be better (although I understand > this might not be possible). > > Also, why are you adding a libxc__ prefixed function to libxl? > Because this function is to convert 'xc_psr_hw_info' to 'libxl_psr_hw_info'. I think I may change the name to 'libxl__xc_psr_info_to_libxl_psr_info'. > > + libxl_psr_feat_type type, xc_psr_hw_info *xc_hw_info, > > + libxl_psr_hw_info *xl_hw_info) > > you could drop the '_hw' in the parameter names, so all the > assignments below would fit on a single line. > Ok, thanks! > > +{ > > + switch (type) { > > + case LIBXL_PSR_FEAT_TYPE_CAT: > > + xl_hw_info->u.cat.cos_max = xc_hw_info->u.xc_cat_info.cos_max; > > + xl_hw_info->u.cat.cbm_len = xc_hw_info->u.xc_cat_info.cbm_len; > > + xl_hw_info->u.cat.cdp_enabled = > > + xc_hw_info->u.xc_cat_info.cdp_enabled; > > + break; > > + case LIBXL_PSR_FEAT_TYPE_MBA: > > + xl_hw_info->u.mba.cos_max = xc_hw_info->u.xc_mba_info.cos_max; > > + xl_hw_info->u.mba.thrtl_max = xc_hw_info->u.xc_mba_info.thrtl_max; > > + xl_hw_info->u.mba.linear = xc_hw_info->u.xc_mba_info.linear; > > + break; > > + default: > > + return ERROR_INVAL; > > + } > > + > > + return 0; > > +} > > + > > int libxl_psr_get_hw_info(libxl_ctx *ctx, libxl_psr_hw_info **info, > > unsigned int *nr, libxl_psr_feat_type type, > > unsigned int lvl) > > { > > - return ERROR_FAIL; > > + GC_INIT(ctx); > > + int rc, nr_sockets; > > + unsigned int i = 0, socketid; > > + libxl_bitmap socketmap; > > + libxl_psr_hw_info *ptr; > > + xc_psr_feat_type xc_type; > > + xc_psr_hw_info hw_info; > > + > > + libxl_bitmap_init(&socketmap); > > + > > + if (type == LIBXL_PSR_FEAT_TYPE_CAT && lvl != 3 && lvl != 2) { > > + LOGE(ERROR, "input lvl %d is wrong!\n", lvl); > > LOGE is used when errno is set, which I don't think it's the case > here. Please use plain LOG. > > > + rc = ERROR_FAIL; > > + goto out; > > + } > > + > > + xc_type = libxl__psr_feat_type_to_libxc_psr_feat_type(type, lvl); > > Isn't the above function going to return and invalid type if the > feature is CAT and the level is not correct? In which case you could > remove the above if and just check that the returned type here is not > invalid? > Ok, will check remove the first check and check return value here. > > + > > + rc = libxl__count_physical_sockets(gc, &nr_sockets); > > + if (rc) { > > + LOGE(ERROR, "failed to get system socket count"); > > Again, libxl__ functions don't set errno. In this case this might be > fine, because libxl__count_physical_sockets calls into libxc which > sets errno, but you should only use LOGE when reporting errors from > system calls or libxc functions. > Ok will change it to LOG. > Roger.
On Thu, Aug 31, 2017 at 11:16:34AM +0800, Yi Sun wrote: > On 17-08-30 10:15:22, Roger Pau Monn� wrote: > > On Thu, Aug 24, 2017 at 09:14:44AM +0800, Yi Sun wrote: > > > diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c > > > index b183305..d7da7d7 100644 > > > --- a/tools/libxl/libxl_psr.c > > > +++ b/tools/libxl/libxl_psr.c > > > @@ -382,56 +382,51 @@ static inline xc_psr_feat_type libxl__psr_feat_type_to_libxc_psr_feat_type( > > > return xc_type; > > > } > > > > > > +static inline int libxl__psr_hw_info_to_libxl_psr_cat_info( > > > > No inline. Maybe you could try to shorter the name? > > > Got it. Will remove the last '_psr'. > > [...] > > > > int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info, > > > int *nr, unsigned int lvl) > > > { > > > GC_INIT(ctx); > > > int rc; > > > - int i = 0, socketid, nr_sockets; > > > - libxl_bitmap socketmap; > > > + unsigned int i; > > > + libxl_psr_hw_info *hw_info; > > > libxl_psr_cat_info *ptr; > > > - xc_psr_hw_info hw_info; > > > - xc_psr_feat_type xc_type; > > > - > > > - libxl_bitmap_init(&socketmap); > > > - > > > - rc = libxl__count_physical_sockets(gc, &nr_sockets); > > > - if (rc) { > > > - LOGE(ERROR, "failed to get system socket count"); > > > - goto out; > > > - } > > > > > > - libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets); > > > - rc = libxl_get_online_socketmap(ctx, &socketmap); > > > - if (rc < 0) { > > > - LOGE(ERROR, "failed to get available sockets"); > > > + rc = libxl_psr_get_hw_info(ctx, &hw_info, (unsigned int *)nr, > > > > Is there any reason nr is int instead of unsigned int? > > > > I would rather avoid casting things. Since this interface has not been > > present in a release yet, could you please send a separate patch to > > fix this if nr has no reason to be signed? > > > This is a historical issue. > > The first version of PSR introduced 'libxl_psr_cat_get_l3_info'. The input > parameter is 'int *nr'. > > I think we cannot change the interface which has been merged and used by > others. Right? Has libxl_psr_cat_get_info been present in any Xen release? (I don't think so) If it hasn't then you can change the interface without issues. Thanks, Roger.
On 17-08-31 09:40:37, Roger Pau Monn� wrote: > On Thu, Aug 31, 2017 at 11:16:34AM +0800, Yi Sun wrote: > > On 17-08-30 10:15:22, Roger Pau Monn� wrote: > > > On Thu, Aug 24, 2017 at 09:14:44AM +0800, Yi Sun wrote: > > > > int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info, > > > > int *nr, unsigned int lvl) > > > > { > > > > GC_INIT(ctx); > > > > int rc; > > > > - int i = 0, socketid, nr_sockets; > > > > - libxl_bitmap socketmap; > > > > + unsigned int i; > > > > + libxl_psr_hw_info *hw_info; > > > > libxl_psr_cat_info *ptr; > > > > - xc_psr_hw_info hw_info; > > > > - xc_psr_feat_type xc_type; > > > > - > > > > - libxl_bitmap_init(&socketmap); > > > > - > > > > - rc = libxl__count_physical_sockets(gc, &nr_sockets); > > > > - if (rc) { > > > > - LOGE(ERROR, "failed to get system socket count"); > > > > - goto out; > > > > - } > > > > > > > > - libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets); > > > > - rc = libxl_get_online_socketmap(ctx, &socketmap); > > > > - if (rc < 0) { > > > > - LOGE(ERROR, "failed to get available sockets"); > > > > + rc = libxl_psr_get_hw_info(ctx, &hw_info, (unsigned int *)nr, > > > > > > Is there any reason nr is int instead of unsigned int? > > > > > > I would rather avoid casting things. Since this interface has not been > > > present in a release yet, could you please send a separate patch to > > > fix this if nr has no reason to be signed? > > > > > This is a historical issue. > > > > The first version of PSR introduced 'libxl_psr_cat_get_l3_info'. The input > > parameter is 'int *nr'. > > > > I think we cannot change the interface which has been merged and used by > > others. Right? > > Has libxl_psr_cat_get_info been present in any Xen release? (I don't > think so) If it hasn't then you can change the interface without > issues. > libxl_psr_cat_get_info is just introduced and not released yet. But libxl_psr_cat_get_l3_info has been released for long time. It declares 'int *nr' and it calls 'libxl_psr_cat_get_info' to do the work. So, there must be a casting anyway. What do you suggest? Shall I submit a patch to fix 'libxl_psr_cat_get_info'? Or, a patch to fix both 'libxl_psr_cat_get_info' and 'libxl_psr_cat_get_l3_info'? > Thanks, Roger.
On Thu, Aug 31, 2017 at 05:19:56PM +0800, Yi Sun wrote: > On 17-08-31 09:40:37, Roger Pau Monn� wrote: > > On Thu, Aug 31, 2017 at 11:16:34AM +0800, Yi Sun wrote: > > > On 17-08-30 10:15:22, Roger Pau Monn� wrote: > > > > On Thu, Aug 24, 2017 at 09:14:44AM +0800, Yi Sun wrote: > > Has libxl_psr_cat_get_info been present in any Xen release? (I don't > > think so) If it hasn't then you can change the interface without > > issues. > > > libxl_psr_cat_get_info is just introduced and not released yet. But > libxl_psr_cat_get_l3_info has been released for long time. It declares > 'int *nr' and it calls 'libxl_psr_cat_get_info' to do the work. So, there > must be a casting anyway. > > What do you suggest? Shall I submit a patch to fix 'libxl_psr_cat_get_info'? > Or, a patch to fix both 'libxl_psr_cat_get_info' and 'libxl_psr_cat_get_l3_info'? Just fixing libxl_psr_cat_get_info would be fine IMHO, let's try to limit the int usage as much as possible. Thanks, Roger.
On 17-08-31 10:32:09, Roger Pau Monn� wrote: > On Thu, Aug 31, 2017 at 05:19:56PM +0800, Yi Sun wrote: > > On 17-08-31 09:40:37, Roger Pau Monn� wrote: > > > On Thu, Aug 31, 2017 at 11:16:34AM +0800, Yi Sun wrote: > > > > On 17-08-30 10:15:22, Roger Pau Monn� wrote: > > > > > On Thu, Aug 24, 2017 at 09:14:44AM +0800, Yi Sun wrote: > > > Has libxl_psr_cat_get_info been present in any Xen release? (I don't > > > think so) If it hasn't then you can change the interface without > > > issues. > > > > > libxl_psr_cat_get_info is just introduced and not released yet. But > > libxl_psr_cat_get_l3_info has been released for long time. It declares > > 'int *nr' and it calls 'libxl_psr_cat_get_info' to do the work. So, there > > must be a casting anyway. > > > > What do you suggest? Shall I submit a patch to fix 'libxl_psr_cat_get_info'? > > Or, a patch to fix both 'libxl_psr_cat_get_info' and 'libxl_psr_cat_get_l3_info'? > > Just fixing libxl_psr_cat_get_info would be fine IMHO, let's try to > limit the int usage as much as possible. > Ok, then, I think another separate fix patch is needed. > Thanks, Roger.
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c index b183305..d7da7d7 100644 --- a/tools/libxl/libxl_psr.c +++ b/tools/libxl/libxl_psr.c @@ -382,56 +382,51 @@ static inline xc_psr_feat_type libxl__psr_feat_type_to_libxc_psr_feat_type( return xc_type; } +static inline int libxl__psr_hw_info_to_libxl_psr_cat_info( + libxl_psr_feat_type type, libxl_psr_hw_info *hw_info, + libxl_psr_cat_info *cat_info) +{ + if (type != LIBXL_PSR_FEAT_TYPE_CAT) + return ERROR_INVAL; + + cat_info->id = hw_info->id; + cat_info->cos_max = hw_info->u.cat.cos_max; + cat_info->cbm_len = hw_info->u.cat.cbm_len; + cat_info->cdp_enabled = hw_info->u.cat.cdp_enabled; + + return 0; +} + int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info, int *nr, unsigned int lvl) { GC_INIT(ctx); int rc; - int i = 0, socketid, nr_sockets; - libxl_bitmap socketmap; + unsigned int i; + libxl_psr_hw_info *hw_info; libxl_psr_cat_info *ptr; - xc_psr_hw_info hw_info; - xc_psr_feat_type xc_type; - - libxl_bitmap_init(&socketmap); - - rc = libxl__count_physical_sockets(gc, &nr_sockets); - if (rc) { - LOGE(ERROR, "failed to get system socket count"); - goto out; - } - libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets); - rc = libxl_get_online_socketmap(ctx, &socketmap); - if (rc < 0) { - LOGE(ERROR, "failed to get available sockets"); + rc = libxl_psr_get_hw_info(ctx, &hw_info, (unsigned int *)nr, + LIBXL_PSR_FEAT_TYPE_CAT, lvl); + if (rc) goto out; - } - xc_type = libxl__psr_feat_type_to_libxc_psr_feat_type( - LIBXL_PSR_FEAT_TYPE_CAT, lvl); + ptr = libxl__malloc(NOGC, *nr * sizeof(libxl_psr_cat_info)); - ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_cat_info)); - - libxl_for_each_set_bit(socketid, socketmap) { - ptr[i].id = socketid; - if (xc_psr_get_hw_info(ctx->xch, socketid, xc_type, &hw_info)) { + for (i = 0; i < *nr; i++) { + if (libxl__psr_hw_info_to_libxl_psr_cat_info( + LIBXL_PSR_FEAT_TYPE_CAT, + &hw_info[i], &ptr[i])) { + libxl_psr_hw_info_list_free(hw_info, (unsigned int)*nr); rc = ERROR_FAIL; free(ptr); goto out; } - - ptr[i].cos_max = hw_info.u.xc_cat_info.cos_max; - ptr[i].cbm_len = hw_info.u.xc_cat_info.cbm_len; - ptr[i].cdp_enabled = hw_info.u.xc_cat_info.cdp_enabled; - - i++; } *info = ptr; - *nr = i; + libxl_psr_hw_info_list_free(hw_info, (unsigned int)*nr); out: - libxl_bitmap_dispose(&socketmap); GC_FREE; return rc; } @@ -469,15 +464,99 @@ int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid, return ERROR_FAIL; } +static inline int libxc__psr_hw_info_to_libxl_psr_hw_info( + libxl_psr_feat_type type, xc_psr_hw_info *xc_hw_info, + libxl_psr_hw_info *xl_hw_info) +{ + switch (type) { + case LIBXL_PSR_FEAT_TYPE_CAT: + xl_hw_info->u.cat.cos_max = xc_hw_info->u.xc_cat_info.cos_max; + xl_hw_info->u.cat.cbm_len = xc_hw_info->u.xc_cat_info.cbm_len; + xl_hw_info->u.cat.cdp_enabled = + xc_hw_info->u.xc_cat_info.cdp_enabled; + break; + case LIBXL_PSR_FEAT_TYPE_MBA: + xl_hw_info->u.mba.cos_max = xc_hw_info->u.xc_mba_info.cos_max; + xl_hw_info->u.mba.thrtl_max = xc_hw_info->u.xc_mba_info.thrtl_max; + xl_hw_info->u.mba.linear = xc_hw_info->u.xc_mba_info.linear; + break; + default: + return ERROR_INVAL; + } + + return 0; +} + int libxl_psr_get_hw_info(libxl_ctx *ctx, libxl_psr_hw_info **info, unsigned int *nr, libxl_psr_feat_type type, unsigned int lvl) { - return ERROR_FAIL; + GC_INIT(ctx); + int rc, nr_sockets; + unsigned int i = 0, socketid; + libxl_bitmap socketmap; + libxl_psr_hw_info *ptr; + xc_psr_feat_type xc_type; + xc_psr_hw_info hw_info; + + libxl_bitmap_init(&socketmap); + + if (type == LIBXL_PSR_FEAT_TYPE_CAT && lvl != 3 && lvl != 2) { + LOGE(ERROR, "input lvl %d is wrong!\n", lvl); + rc = ERROR_FAIL; + goto out; + } + + xc_type = libxl__psr_feat_type_to_libxc_psr_feat_type(type, lvl); + + rc = libxl__count_physical_sockets(gc, &nr_sockets); + if (rc) { + LOGE(ERROR, "failed to get system socket count"); + goto out; + } + + libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets); + rc = libxl_get_online_socketmap(ctx, &socketmap); + if (rc < 0) { + LOGE(ERROR, "failed to get available sockets"); + goto out; + } + + ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_hw_info)); + + libxl_for_each_set_bit(socketid, socketmap) { + ptr[i].id = socketid; + if (xc_psr_get_hw_info(ctx->xch, socketid, xc_type, &hw_info)) { + rc = ERROR_FAIL; + free(ptr); + goto out; + } + + if (libxc__psr_hw_info_to_libxl_psr_hw_info(type, &hw_info, &ptr[i])) { + LOGE(ERROR, "Input type %d is wrong!\n", type); + rc = ERROR_FAIL; + free(ptr); + goto out; + } + + i++; + } + + *info = ptr; + *nr = i; +out: + libxl_bitmap_dispose(&socketmap); + GC_FREE; + return rc; } void libxl_psr_hw_info_list_free(libxl_psr_hw_info *list, unsigned int nr) { + unsigned int i; + + for (i = 0; i < nr; i++) + libxl_psr_hw_info_dispose(&list[i]); + free(list); } /*
This patch implements the new libxl get hw info interface, 'libxl_psr_get_hw_info', which is suitable to all psr allocation features. It also implements corresponding list free function, 'libxl_psr_hw_info_list_free' and make 'libxl_psr_cat_get_info' to call 'libxl_psr_get_hw_info' to avoid redundant codes in libxl_psr.c. Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> --- v2: - split this patch out from a big patch in v1. (suggested by Wei Liu) - change 'CAT_INFO'/'MBA_INFO' to 'CAT' and 'MBA. Also the libxl structure name 'cat_info'/'mba_info' is changed to 'cat'/'mba'. (suggested by Chao Peng) - call 'libxl_psr_hw_info_list_free' in 'libxl_psr_cat_get_info' to free allocated resources. (suggested by Chao Peng) --- tools/libxl/libxl_psr.c | 145 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 112 insertions(+), 33 deletions(-)