Re: [Xen-devel] [PATCH v8 18/24] x86: L2 CAT: implement get hw info flow.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.03.17 at 06:57, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> On 17-03-09 08:13:59, Jan Beulich wrote:
>> >>> On 15.02.17 at 09:49, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
>> > +static bool l2_cat_get_feat_info(const struct feat_node *feat,
>> > +                                 uint32_t data[], uint32_t array_len)
>> > +{
>> > +    if ( !data || 2 > array_len )
>> > +        return false;
>> > +
>> > +    data[CBM_LEN] = feat->info.l2_cat_info.cbm_len;
>> > +    data[COS_MAX] = feat->info.l2_cat_info.cos_max;
>> 
>> What about PSR_FLAG? Which puts under question the
>> array_len check at the start of this and its sibling functions: If
>> more array entries are provided, they'd all be left uninitialized
>> without the caller having a way to know. Coming back to the
>> data structures being the same for all features, there should
>> presumably be a structure instead of an array be used for
>> communication here, and ...
>> 
>> > @@ -754,6 +755,11 @@ struct xen_sysctl_psr_cat_op {
>> >  #define XEN_SYSCTL_PSR_CAT_L3_CDP       (1u << 0)
>> >              uint32_t flags;     /* OUT: CAT flags */
>> >          } l3_info;
>> > +
>> > +        struct {
>> > +            uint32_t cbm_len;   /* OUT: CBM length */
>> > +            uint32_t cos_max;   /* OUT: Maximum COS */
>> > +        } l2_info;
>> 
>> ... this should use the same structure as l3_info (flags being
>> set to zero until a use arises). This would then also allow for
>> more code to be shared instead of duplicated.
>> 
> Ok, will reuse l3_info for L2 CAT. May modify its name to cat_info.

Of course.

> But for MBA or future feature, its info is different with CAT. So, it may
> not be approriate to use a structure as parameter for 'psr_get_info'.
> So I would prefer to keep data[]. Of course, I will correct array_len check
> per Roger's suggestion, like "array_len != PSR_INFO_SIZE". Is that good to
> you?

Yes, if there is a need for it to be that way down the road, then I'm
fine as long as both the risk of array overrun and that of using
uninitialized data are minimized as much as possible.

Jan


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