Re: [Xen-devel] [PATCH v1 07/13] x86: implement set value flow for MBA

[Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>]

On 17-08-18 11:32:08, Chao Peng wrote:
> 
> > +    if ( feat->mba_info.linear )
> > +    {
> > +        unsigned int mod;
> > +
> > +        if ( feat->mba_info.thrtl_max >= 100 )
> > +            return false;
> 
> Can we do this check earlier, e.g. when it gets enumerated from CPUID?
> 
Ok, sound reasonable.

> > +
> > +        mod = *thrtl % (100 - feat->mba_info.thrtl_max);
> > +        *thrtl -= mod;
> > +    }
> > +    else
> > +    {
> > +        /* Not power of 2. */
> > +        if ( *thrtl & (*thrtl - 1) )
> > +            *thrtl = *thrtl & (1 << (flsl(*thrtl) - 1));
> > +    }
> > +
> 
> Is it possible for *thrtl to be zero? Otherwise we need check that at
> the beginning.
> 
Yes, it could be 0. I missed this. Code should be:
    if ( *thrtl && (*thrtl & (*thrtl - 1)) )

> >  
> > +/*
> > + * Because multiple features may co-exist, we need handle all
> > features to write
> > + * values of them into a COS register with new COS ID. E.g:
> > + * 1. L3 CAT and MBA co-exist.
> > + * 2. Dom1 and Dom2 share a same COS ID (2). The L3 CAT CBM of Dom1
> > is 0x1ff,
> > + *    the MBA Thrtle of Dom1 is 0xa.
> > + * 3. User wants to change MBA Thrtl of Dom1 to be 0x14. Because COS
> > ID 2 is
> > + *    used by Dom2 too, we have to pick a new COS ID 3. The original
> > values of
> > + *    Dom1 on COS ID 3 may be below:
> > + *            ---------
> > + *            | COS 3 |
> > + *            ---------
> > + *    L3 CAT  | 0x7ff |
> > + *            ---------
> > + *    MBA     | 0x0   |
> > + *            ---------
> > + * 4. After setting, the L3 CAT CBM value of Dom1 should be kept and
> > the new MBA
> > + *    Thrtl is set. So, the values on COS ID 3 should be below.
> > + *            ---------
> > + *            | COS 3 |
> > + *            ---------
> > + *    L3 CAT  | 0x1ff |
> > + *            ---------
> > + *    MBA     | 0x14  |
> > + *            ---------
> > + *
> > + * So, we should write all features values into their MSRs. That
> > requires the
> > + * feature array, feature properties array and value array are input.
> > + */
> 
> Although I understand them, I still have a feeling of the necessity to
> reword these comments.
> 
How about move comments into commit message?

> Chao

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