[MODERATED] Re: [PATCH v4 04/10] TAAv4 4

On Wed, Sep 25, 2019 at 05:58:31PM -0500, speck for Josh Poimboeuf wrote:
> On Mon, Sep 09, 2019 at 08:42:54AM -0700, speck for Dave Hansen wrote:
> > >>> +static void tsx_update_this_cpu(void *arg)
> > >>> +{
> > >>> +	struct cpuinfo_x86 *c = &cpu_data(smp_processor_id());
> > >>> +	unsigned long enable = (unsigned long)arg;
> > >>> +
> > >>> +	if (enable) {
> > >>> +		tsx_enable();
> > >>> +		set_cpu_cap(c, X86_FEATURE_RTM);
> > >>> +	} else {
> > >>> +		tsx_disable();
> > >>> +		clear_cpu_cap(c, X86_FEATURE_RTM);
> > >>> +	}
> > >>> +}
> > >>
> > >> This makes me really nervous.
> > >>
> > >> Do we have any other *runtime* (after early boot) manipulation of
> > >> X86_FEATURE_* flags?
> > > 
> > > I don't think we have an option.  TSX_CTRL MSR write would change the
> > > CPUID anyways.  It is probably better to have X86_FEATURE_RTM reflect
> > > the correct state.
> > 
> > We have a choice.
> > 
> > If we don't have the infrastructure to do something right, we don't do
> > it.  Poking at the bits and praying nothing breaks is not my preferred
> > approach.  Could you please take a look and try to answer my question:
> > 
> > 	Do we have any other *runtime* (after early boot) manipulation
> > 	of X86_FEATURE_* flags?
> > 
> > If the answer is no, then there's some homework that needs to be done to
> > see what the implications are here and to make sure that nothing will
> > break.  From the changelog and comments, it appears that research hasn't
> > been done.
> 
> Right.  This patch feels dangerous and fragile...
> 
> And what's the point anyway?  There's hardly any justification in the
> patch description.  Is it a theoretical problem or is there a real-world
> production use case?
> 
> Please drop this patch, or at least, put it at the very end of the set.

Updating the changelog and moving this to the end.

Thanks,
Pawan