|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 02/15] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general
On Sat, Sep 23, 2017 at 09:48:11AM +0000, Yi Sun wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1439,60 +1439,60 @@ long arch_do_domctl(
> }
> break;
>
> - case XEN_DOMCTL_psr_cat_op:
> - switch ( domctl->u.psr_cat_op.cmd )
> + case XEN_DOMCTL_psr_alloc:
> + switch ( domctl->u.psr_alloc.cmd )
> {
> uint32_t val32;
>
> - case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
> - ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> - domctl->u.psr_cat_op.data,
> + case XEN_DOMCTL_PSR_SET_L3_CBM:
> + ret = psr_set_val(d, domctl->u.psr_alloc.target,
> + domctl->u.psr_alloc.data,
> PSR_CBM_TYPE_L3);
> break;
>
> - case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE:
> - ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> - domctl->u.psr_cat_op.data,
> + case XEN_DOMCTL_PSR_SET_L3_CODE:
> + ret = psr_set_val(d, domctl->u.psr_alloc.target,
> + domctl->u.psr_alloc.data,
> PSR_CBM_TYPE_L3_CODE);
> break;
>
> - case XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA:
> - ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> - domctl->u.psr_cat_op.data,
> + case XEN_DOMCTL_PSR_SET_L3_DATA:
> + ret = psr_set_val(d, domctl->u.psr_alloc.target,
> + domctl->u.psr_alloc.data,
> PSR_CBM_TYPE_L3_DATA);
> break;
>
> - case XEN_DOMCTL_PSR_CAT_OP_SET_L2_CBM:
> - ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> - domctl->u.psr_cat_op.data,
> + case XEN_DOMCTL_PSR_SET_L2_CBM:
> + ret = psr_set_val(d, domctl->u.psr_alloc.target,
> + domctl->u.psr_alloc.data,
> PSR_CBM_TYPE_L2);
> break;
>
> - case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
> - ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> + case XEN_DOMCTL_PSR_GET_L3_CBM:
> + ret = psr_get_val(d, domctl->u.psr_alloc.target,
> &val32, PSR_CBM_TYPE_L3);
> - domctl->u.psr_cat_op.data = val32;
> + domctl->u.psr_alloc.data = val32;
> copyback = true;
> break;
>
> - case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE:
> - ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> + case XEN_DOMCTL_PSR_GET_L3_CODE:
> + ret = psr_get_val(d, domctl->u.psr_alloc.target,
> &val32, PSR_CBM_TYPE_L3_CODE);
> - domctl->u.psr_cat_op.data = val32;
> + domctl->u.psr_alloc.data = val32;
> copyback = true;
> break;
>
> - case XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA:
> - ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> + case XEN_DOMCTL_PSR_GET_L3_DATA:
> + ret = psr_get_val(d, domctl->u.psr_alloc.target,
> &val32, PSR_CBM_TYPE_L3_DATA);
> - domctl->u.psr_cat_op.data = val32;
> + domctl->u.psr_alloc.data = val32;
> copyback = true;
> break;
>
> - case XEN_DOMCTL_PSR_CAT_OP_GET_L2_CBM:
> - ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> + case XEN_DOMCTL_PSR_GET_L2_CBM:
> + ret = psr_get_val(d, domctl->u.psr_alloc.target,
> &val32, PSR_CBM_TYPE_L2);
> - domctl->u.psr_cat_op.data = val32;
> + domctl->u.psr_alloc.data = val32;
> copyback = true;
This:
ret = psr_get_val(...);
domctl->u.psr...;
copyback = true;
Construct is quite repetitive, maybe you could consider adding a local
macro to hide it?
Maybe then you could also hide the val32 declaration inside of it.
> break;
>
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index daa2aeb..c851511 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -382,7 +382,7 @@ static bool l3_cdp_get_feat_info(const struct feat_node
> *feat,
> if ( !cat_get_feat_info(feat, data, array_len) )
> return false;
>
> - data[PSR_INFO_IDX_CAT_FLAG] |= XEN_SYSCTL_PSR_CAT_L3_CDP;
> + data[PSR_INFO_IDX_CAT_FLAG] |= XEN_SYSCTL_PSR_L3_CDP;
>
> return true;
> }
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index c3fdae8..e44d8ad 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -171,45 +171,45 @@ long arch_do_sysctl(
>
> break;
>
> - case XEN_SYSCTL_psr_cat_op:
> - switch ( sysctl->u.psr_cat_op.cmd )
> + case XEN_SYSCTL_psr_alloc:
> + switch ( sysctl->u.psr_alloc.cmd )
> {
> uint32_t data[PSR_INFO_ARRAY_SIZE];
>
> - case XEN_SYSCTL_PSR_CAT_get_l3_info:
> + case XEN_SYSCTL_PSR_get_l3_info:
> {
> - ret = psr_get_info(sysctl->u.psr_cat_op.target,
> + ret = psr_get_info(sysctl->u.psr_alloc.target,
> PSR_CBM_TYPE_L3, data, ARRAY_SIZE(data));
> if ( ret )
> break;
>
> - sysctl->u.psr_cat_op.u.cat_info.cos_max =
> + sysctl->u.psr_alloc.u.cat_info.cos_max =
> data[PSR_INFO_IDX_COS_MAX];
> - sysctl->u.psr_cat_op.u.cat_info.cbm_len =
> + sysctl->u.psr_alloc.u.cat_info.cbm_len =
> data[PSR_INFO_IDX_CAT_CBM_LEN];
> - sysctl->u.psr_cat_op.u.cat_info.flags =
> + sysctl->u.psr_alloc.u.cat_info.flags =
> data[PSR_INFO_IDX_CAT_FLAG];
>
> - if ( __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) )
> + if ( __copy_field_to_guest(u_sysctl, sysctl, u.psr_alloc) )
> ret = -EFAULT;
> break;
> }
>
> - case XEN_SYSCTL_PSR_CAT_get_l2_info:
> + case XEN_SYSCTL_PSR_get_l2_info:
> {
> - ret = psr_get_info(sysctl->u.psr_cat_op.target,
> + ret = psr_get_info(sysctl->u.psr_alloc.target,
> PSR_CBM_TYPE_L2, data, ARRAY_SIZE(data));
> if ( ret )
> break;
>
> - sysctl->u.psr_cat_op.u.cat_info.cos_max =
> + sysctl->u.psr_alloc.u.cat_info.cos_max =
> data[PSR_INFO_IDX_COS_MAX];
> - sysctl->u.psr_cat_op.u.cat_info.cbm_len =
> + sysctl->u.psr_alloc.u.cat_info.cbm_len =
> data[PSR_INFO_IDX_CAT_CBM_LEN];
> - sysctl->u.psr_cat_op.u.cat_info.flags =
> + sysctl->u.psr_alloc.u.cat_info.flags =
> data[PSR_INFO_IDX_CAT_FLAG];
The above repetition is also unneeded AFAICT. Why not simplify the
switch to:
uint32_t data[PSR_INFO_ARRAY_SIZE];
enum cbm_type type;
switch ( sysctl->u.psr_alloc.cmd )
{
case XEN_SYSCTL_PSR_get_l3_info:
type = PSR_CBM_TYPE_L3;
break;
case XEN_SYSCTL_PSR_get_l2_info:
type = PSR_CBM_TYPE_L2;
break;
default:
return -EOPNOTSUPP;
}
ret = psr_get_info(sysctl->u.psr_alloc.target, type, data, ARRAY_SIZE(data));
if ( ret )
break;
sysctl->u.psr_alloc.u.cat_info.cos_max = data[PSR_INFO_IDX_COS_MAX];
...
It would prevent some of this code repetition IMHO.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |