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

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

On 17-03-09 08:13:59, Jan Beulich wrote:
> >>> On 15.02.17 at 09:49, <yi.y.sun@linux.intel.com> 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.

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?

> Jan

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