Re: [Xen-devel] [PATCH v1 04/13] x86: implement data structure and CPU init flow for MBA

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

On 17-08-15 11:50:15, Wei Liu wrote:
> On Wed, Aug 09, 2017 at 03:41:43PM +0800, Yi Sun wrote:
> > -#define PSR_CAT        (1<<1)
> > -#define PSR_CDP        (1<<2)
> > +#define PSR_CMT        (1u << 0)
> > +#define PSR_CAT        (1u << 1)
> > +#define PSR_CDP        (1u << 2)
> > +#define PSR_MBA        (1u << 3)
> 
> I would split this part out to a separate patch so that it can be
> applied immediately.
> 
A fix patch has been sent out.

> >  struct feat_node {
> > -    /* cos_max and cbm_len are common values for all features so far. */
> > +    /* cos_max is common values for all features so far. */
> >      unsigned int cos_max;
> > -    unsigned int cbm_len;
> > +
> > +    /* Feature specific HW info. */
> > +    union {
> > +        struct {
> > +            /* The length of CBM got through CPUID. */
> > +            unsigned int cbm_len;
> > +        } cat_info;
> > +
> > +        struct {
> > +            /* The max throttling value got through CPUID. */
> > +            unsigned int thrtl_max;
> > +            unsigned int linear;
> > +        } mba_info;
> > +    };
> > +
> 
> I suggest you add a tag to specify which struct is in effect in the
> union and ASSERT accordingly in their respective type specific
> functions.

Before using struct in this union at all places, there is check to make sure the
correct feature node is used. So, is it necessary to add an additional flag to
check? Thanks!

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