Re: [Xen-devel] [PATCH v10 06/25] x86: refactor psr: L3 CAT: implement Domain init/free and schedule flows.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.04.17 at 15:53, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> @@ -376,11 +378,39 @@ void psr_free_rmid(struct domain *d)
>      d->arch.psr_rmid = 0;
>  }
>  
> -static inline void psr_assoc_init(void)
> +static unsigned int get_max_cos_max(const struct psr_socket_info *info)
> +{
> +    const struct feat_node *feat;
> +    unsigned int cos_max = 0, i;
> +
> +    for ( i = 0; i < PSR_SOCKET_MAX_FEAT; i++ )
> +    {
> +        feat = info->features[i];
> +        if ( !feat )
> +            continue;
> +
> +        cos_max = max(feat->props->cos_max, cos_max);
> +    }
> +
> +    return cos_max;
> +}
> +
> +static void psr_assoc_init(void)
>  {
>      struct psr_assoc *psra = &this_cpu(psr_assoc);
>  
> -    if ( psr_cmt_enabled() )
> +    if ( psr_alloc_feat_enabled() )
> +    {
> +        unsigned int socket = cpu_to_socket(smp_processor_id());
> +        const struct psr_socket_info *info = socket_info + socket;
> +        unsigned int cos_max = get_max_cos_max(info);
> +
> +        if ( feat_init_done(info) )

I think the use here is different from the one in the earlier patch:
While there looking at props[] appears to be desirable, I think here
you indeed only want to look at features[]. And btw, I wouldn't
mind a simple flag or counter in info telling whether any (or how
many) features have been enabled, to avoid such iterations. It's
just that the original feature mask was fully redundant with
features[].

> @@ -397,6 +434,11 @@ void psr_ctxt_switch_to(struct domain *d)
>      if ( psr_cmt_enabled() )
>          psr_assoc_rmid(&reg, d->arch.psr_rmid);
>  
> +    if ( psra->cos_mask )
> +        psr_assoc_cos(&reg, d->arch.psr_cos_ids ?
> +                      d->arch.psr_cos_ids[cpu_to_socket(smp_processor_id())] 
> :
> +                      0, psra->cos_mask);

I may have asked this question before, but if so you can see that
the code above continues puzzling me: Under what conditions
would psra->cos_mask be non-zero, but d->arch.psr_cos_ids be
NULL? And why is zero the right value in that case?

Also you need to deal with alignment issues here: Part of an
expression at equal rank should align with one another. This
implies that you want to move the 2nd argument on a new line
(and the 3rd one would then better be moved to its own line
too).

Jan


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