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

[Yi Sun <yi.y.sun@linux.intel.com>]

On 17-04-05 09:23:50, Jan Beulich wrote:
> >>> On 01.04.17 at 15:53, <yi.y.sun@linux.intel.com> wrote:
> > +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[].
> 
Per comment in previous patch, will add a 'flag' to indicate if any
feature has been initialized.

> > @@ -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?
> 
'cos_mask' is initialized in 'psr_assoc_init' during cpu starting. The
'psr_cos_ids' is allocated during domain init. Here is soft a protection
to handle abnormal case. Of course, we can use ASSERT to check it.

> 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).
> 
Ok, thanks!

> Jan

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