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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.09.17 at 04:39, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> On 17-09-26 10:39:31, Roger Pau Monn wrote:
>> On Sat, Sep 23, 2017 at 09:48:16AM +0000, Yi Sun wrote:
>> > @@ -274,29 +277,6 @@ static enum psr_feat_type psr_type_to_feat_type(enum 
>> > psr_type type)
>> >      return feat_type;
>> >  }
>> >  
>> > -static bool psr_check_cbm(unsigned int cbm_len, unsigned long cbm)
>> > -{
>> > -    unsigned int first_bit, zero_bit;
>> > -
>> > -    /* Set bits should only in the range of [0, cbm_len]. */
>> > -    if ( cbm & (~0ul << cbm_len) )
>> > -        return false;
>> > -
>> > -    /* At least one bit need to be set. */
>> > -    if ( cbm == 0 )
>> > -        return false;
>> > -
>> > -    first_bit = find_first_bit(&cbm, cbm_len);
>> > -    zero_bit = find_next_zero_bit(&cbm, cbm_len, first_bit);
>> > -
>> > -    /* Set bits should be contiguous. */
>> > -    if ( zero_bit < cbm_len &&
>> > -         find_next_bit(&cbm, cbm_len, zero_bit) < cbm_len )
>> > -        return false;
>> > -
>> > -    return true;
>> > -}
>> > -
>> >  /* Implementation of allocation features' functions. */
>> >  static bool cat_init_feature(const struct cpuid_leaf *regs,
>> >                              struct feat_node *feat,
>> > @@ -426,11 +406,36 @@ static bool cat_get_feat_info(const struct feat_node 
>> > *feat,
>> >      return true;
>> >  }
>> >  
>> > +static bool cat_check_cbm(const struct feat_node *feat, unsigned long cbm)
>> > +{
>> > +    unsigned int first_bit, zero_bit;
>> > +    unsigned int cbm_len = feat->cat.cbm_len;
>> > +
>> > +    /*
>> > +     * Set bits should only in the range of [0, cbm_len].
>> > +     * And, at least one bit need to be set.
>> > +     */
>> > +    if ( cbm & (~0ul << cbm_len) || cbm == 0 )
>> > +        return false;
>> > +
>> > +    first_bit = find_first_bit(&cbm, cbm_len);
>> > +    zero_bit = find_next_zero_bit(&cbm, cbm_len, first_bit);
>> > +
>> > +    /* Set bits should be contiguous. */
>> > +    if ( zero_bit < cbm_len &&
>> > +         find_next_bit(&cbm, cbm_len, zero_bit) < cbm_len )
>> > +        return false;
>> > +
>> > +    return true;
>> > +}
>> 
>> Why do you need to move the code apart from renaming it?
>> 
> Because it is CAT specific function now. I moved it into CAT section.
>     '/* Implementation of allocation features' functions. */'

The easier thing then would have been to move the comment
line up. That way actual changes to the function also become
obvious. Besides that - isn't the A in MBA also "Allocation"?

Jan


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