[MODERATED] Re: [PATCH v4 03/10] TAAv4 3
Pawan Gupta <[email protected]>
| Newsgroups | org.kernel.lore.historical-speck |
|---|---|
| Message-ID | <[email protected]> |
> > > > +static int __init tsx_cmdline(char *str)
> > > > +{
> > > > + if (!str)
> > > > + return -EINVAL;
> > > > +
> > > > + if (!strcmp(str, "on"))
> > > > + tsx_ctrl_state = TSX_CTRL_ENABLE;
> > > > + else if (!strcmp(str, "off"))
> > > > + tsx_ctrl_state = TSX_CTRL_DISABLE;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +early_param("tsx", tsx_cmdline);
> > >
> > > Hmm... Let's say I have a non-TSX system. I specify tsx=on. This will
> > > set tsx_ctrl_state=TSX_CTRL_ENABLE, then tsx_ctrl_check_support() will
> > > set it to tsx_ctrl_state=TSX_CTRL_NOT_SUPPORTED.
> > >
> > > I *think* this all works, but I'd love a comment or two about it. Maybe
> > > even something that makes it clear that tsx_en/disable() are both only
> > > called when TSX *AND* TSX_CTRL_MSR are supported.
> > >
> > > This is all kinda weird and confusing because we're doing this all
> > > without the X86_FEATURE bits for TSX.
> >
> > If we add another X86_FEATURE bit it will have exact same state as
> > X86_FEATURE_RTM. Do we really need this?
>
> How about moving the tsx_init() call to early_init_intel()? Then you'd
> be able to check the feature bit above. And that might make the
> ordering a little easier to follow.
Sorry, I am failing to understand how it would make a difference.
early_init_intel() is also called from init_init().
Thanks,
Pawan