Re: [PATCH v4 2/4] x86: add domctl cmd to set/get CDP code/data CBM

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 17/09/15 10:35, He Chen wrote:
> @@ -375,10 +401,48 @@ 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;
> +
> +    for ( cos = 0; cos <= cos_max; cos++ )
> +    {
> +        if( map[cos].ref &&
> +            ((!cdp_enabled && map[cos].u.cbm == cbm_code) ||
> +            (cdp_enabled && map[cos].u.code == cbm_code &&

Correct alignment here would have one extra space, as the ( should match
the inner ( of the line above.

> @@ -387,53 +451,80 @@ int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
>      if ( !psr_check_cbm(info->cbm_len, cbm) )
>          return -EINVAL;
>  
> +    if ( !cdp_enabled && (type == PSR_CBM_TYPE_L3_CODE ||
> +                          type == PSR_CBM_TYPE_L3_DATA) )
> +        return -EINVAL;
> +
> +    cos_max = info->cos_max;
>      old_cos = d->arch.psr_cos_ids[socket];
>      map = info->cos_to_cbm;
>  
> -    spin_lock(&info->cbm_lock);
> -
> -    for ( cos = 0; cos <= info->cos_max; cos++ )
> +    switch( type )
>      {
> -        /* 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(&info->cbm_lock);
> -                return 0;
> -            }
> -            found = map + cos;
> -            break;
> -        }
> -    }
> +    case PSR_CBM_TYPE_L3:
> +        cbm_code = cbm;
> +        cbm_data = cbm;
> +        break;
>  
> -    /* If old cos is referred only by the domain, then use it. */
> -    if ( !found && map[old_cos].ref == 1 )
> -        found = map + old_cos;
> +    case PSR_CBM_TYPE_L3_CODE:
> +        cbm_code = cbm;
> +        cbm_data = map[old_cos].u.data;
> +        break;
>  
> -    if ( !found )
> -    {
> -        spin_unlock(&info->cbm_lock);
> -        return -EOVERFLOW;
> +    case PSR_CBM_TYPE_L3_DATA:
> +        cbm_code = map[old_cos].u.code;
> +        cbm_data = cbm;
> +        break;
> +
> +    default:
> +            ASSERT_UNREACHABLE();

Alignment here as well.

With these two alignment issues fixed, and the knock-on effect of
dropping .u from the previous patch, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>