On Wed, Sep 09, 2015 at 01:16:46PM +0800, He Chen wrote:
> CDP extends CAT and provides the capacity to control L3 code & data
> cache. With CDP, one COS correspond to two CMBs(code & data). cbm_type
> is added to support distinguish different CBM operation. Besides, new
> domctl cmds are introdunced to support set/get CDP CBM. Some CAT
> functions to operation CBMs are extended to support CDP.
>
> Signed-off-by: He Chen
> ---
> xen/arch/x86/domctl.c | 32 -
> xen/arch/x86/psr.c | 163
> ++--
> xen/include/asm-x86/psr.h | 12 +++-
> xen/include/public/domctl.h | 4 ++
> 4 files changed, 170 insertions(+), 41 deletions(-)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index bf62a88..734fddb 100644
> --- a/xen/arch/x86/domctl.c
>
> -int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
> +int psr_get_l3_cbm(struct domain *d, unsigned int socket,
> + uint64_t *cbm, enum cbm_type type)
> {
> struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>
> if ( IS_ERR(info) )
> return PTR_ERR(info);
>
> -*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
> +if ( type == PSR_CBM_TYPE_L3 )
> +{
> +if ( info->cdp_enabled )
> +return -EXDEV;
> +*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cbm;
> +}
> +else if ( type == PSR_CBM_TYPE_L3_CODE )
> +{
> +if ( !info->cdp_enabled )
> +return -EXDEV;
> +*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.code;
> +}
> +else
> +{
> +if ( !info->cdp_enabled )
> +return -EXDEV;
> +*cbm = info->cos_to_cbm[d->arch.psr_cos_ids[socket]].u.cdp.data;
> +}
Could I suggest to use switch? And the check can be done in advance.
>
> return 0;
> }
> @@ -371,65 +389,136 @@ static int write_l3_cbm(unsigned int socket, unsigned
> int cos,
> return 0;
> }
>
> -int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
> +static int find_cos(struct psr_cat_cbm *map, int cos_max,
> +uint64_t cbm_code, uint64_t cbm_data, bool_t cdp_enabled)
> {
> -unsigned int old_cos, cos;
> -struct psr_cat_cbm *map, *found = NULL;
> +unsigned int cos;
> +
> +if ( !cdp_enabled )
> +{
> +for ( cos = 0; cos <= cos_max; cos++ )
> +if ( map[cos].ref && map[cos].u.cbm == cbm_code )
> +return cos;
> +}
> +else
> +{
> +for ( cos = 0; cos <= cos_max; cos++ )
> +if ( map[cos].ref && map[cos].u.cdp.code == cbm_code &&
> + map[cos].u.cdp.data == cbm_data )
> +return cos;
> +}
I guess the '{' and '}' can be omitted if the body contains only one
sentence.
> +
> +return -ENOENT;
> +}
> +
> +static int pick_avail_cos(struct psr_cat_cbm *map, int cos_max, int old_cos)
> +{
> +int cos;
> +
> +/* If old cos is referred only by the domain, then use it. */
> +if ( map[old_cos].ref == 1 )
> +return old_cos;
> +
> +/* Then we pick an unused one, never pick 0 */
Find an unused one other than cos0.
> +for ( cos = 1; cos <= cos_max; cos++ )
> +if ( map[cos].ref == 0 )
> +return cos;
> +
> +return -ENOENT;
> +}
> +
> +int psr_set_l3_cbm(struct domain *d, unsigned int socket,
> + uint64_t cbm, enum cbm_type type)
> +{
> +unsigned int old_cos, cos_max;
> +int cos, ret;
> +uint64_t cbm_data, cbm_code;
> +bool_t need_write = 1;
> +struct psr_cat_cbm *map;
> struct psr_cat_socket_info *info = get_cat_socket_info(socket);
>
> if ( IS_ERR(info) )
> return PTR_ERR(info);
>
> +cbm_code = (1ull << info->cbm_len) - 1;
> +cbm_data = (1ull << info->cbm_len) - 1;
Why this is needed, as you will give them the value eventually.
> if ( !psr_check_cbm(info->cbm_len, cbm) )
> return -EINVAL;
>
> +cos_max = info->cos_max;
> old_cos = d->arch.psr_cos_ids[socket];
> map = info->cos_to_cbm;
>
> -spin_lock(>cbm_lock);
> -
> -for ( cos = 0; cos <= info->cos_max; cos++ )
> +switch( type )
Coding style.
Chao
> {
> -/* If still not found, then keep unused one. */
> -if ( !found && cos != 0 && map[cos].ref == 0 )
> -found = map + cos;
> -else if ( map[cos].u.cbm == cbm )
> -{
> -if ( unlikely(cos == old_cos) )
> -{
> -ASSERT(cos == 0 || map[cos].ref != 0);
> -spin_unlock(>cbm_lock);
> -return 0;
> -}
> -found = map + cos;
> +case PSR_CBM_TYPE_L3:
> +if ( info->cdp_enabled )
> +return -EINVAL;
> +cbm_code = cbm;
> +cbm_data = cbm;
> break;
> -}
> -}
>
> -/* If