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

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Aug 16, 2017 at 03:18:13PM +0800, Yi Sun wrote:
> On 17-08-15 11:50:15, Wei Liu wrote:
> > >  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!

OK, in that case I think it is fine to not add a tag.

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