[MODERATED] Re: [PATCH v7 10/10] TAAv7 10

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Mon, Oct 21, 2019 at 01:32:03PM -0700, speck for Pawan Gupta wrote:
> From: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Subject: [PATCH v7 10/10] x86/tsx: Add sysfs interface to control TSX
> 
> Transactional Synchronization Extensions (TSX) is an extension to the
> x86 instruction set architecture (ISA) that adds Hardware Transactional
> Memory (HTM) support. Changing TSX state currently requires a reboot.
> This may not be desirable when rebooting imposes a huge penalty. Add
> support to control TSX feature via a new sysfs file:
> /sys/devices/system/cpu/hw_tx_mem
> 
> - Writing 0|off|N|n to this file disables TSX feature on all the CPUs.
>   This is equivalent to boot parameter tsx=off.
> - Writing 1|on|Y|y to this file enables TSX feature on all the CPUs.
>   This is equivalent to boot parameter tsx=on.
> - Reading from this returns the status of TSX feature.
> - When TSX control is not supported this interface is not visible in
>   sysfs.
> 
> Changing the TSX state from this interface also updates CPUID.RTM
> feature bit.  From the kernel side, this feature bit doesn't result in
> any ALTERNATIVE code patching.  No memory allocations are done to
> save/restore user state. No code paths in outside of the tests for
> vulnerability to TAA are dependent on the value of the feature bit.  In
> general the kernel doesn't care whether RTM is present or not.

Shouldn't the patch change X86_FEATURE_RTM?  I don't see where that
happens, though changing such bits at runtime is dangerous anyway...

Regardless, this patch adds complexity and still seems very fragile.  It
will add maintenance issues and probably introduce bugs.  And, most
importantly there's *still* not a real world justification for it.
Please drop it from the series.

Nacked-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh