* Thomas Gleixner <t...@linutronix.de> wrote:
> From: Tim Chen <tim.c.c...@linux.intel.com>
>
> CPUs can expose a MSR to control speculation. The initial function of this
> MSR is to control Indirect Branch Speculation, which is required to
> mitigate the Spectre_V2 attack on certain CPU generations.
s/a MSR
/an MSR
> If CPUID(7).RDX[26] is set then MSR_IA32_SPEC_CTRL (0x48) is available and
> bit 0 of that MSR controls whether Indirect Branch Speculation is
> restricted or not. The control bit is named IBRS (Indirect Branch
> Restricted Speculation). The IBSR bit can be unconditionally set to 1
> without clearing it before.
Argh for inverted logic: why was the control bit defined for a _negated_ value,
i.e. why does '0' mean "don't don't speculate"?
And yes, I know what's behind it: this way 'IBRS' can be called a 'mitigation
feature' that can be 'enabled', instead of calling it a 'broken CPU feature
feature' that has to be disabled ...
That's nonsense that causes confusion to no end:
> If IBRS is set, near returns and near indirect jumps/calls will not allow
> their predicted target address to be controlled by code that executed in a
> less privileged prediction mode before the IBRS mode was last written with
> a value of 1 or on another logical processor so long as all Return Stack
> Buffer (RSB) entries from the previous less privileged prediction mode are
> overwritten.
>
> Thus a near indirect jump/call/return may be affected by code in a less
> privileged prediction mode that executed AFTER IBRS mode was last written
> with a value of 1.
>
> Code executed by a sibling logical processor cannot control indirect
> jump/call/return predicted target when IBRS is set
>
> IBRS is not required in order to isolate branch predictions for SMM or SGX
> enclaves.
>
> Enabling IBRS can cause a measurable and depending on the workload
> significant CPU performance penalty.
This is bound to be really confusing due to the logic negation, in particular:
> +#define SPEC_CTRL_DISABLE_IBRS (0UL << 0)
> +#define SPEC_CTRL_ENABLE_IBRS (1UL << 0)
"SPEC_CTRL_ENABLE_IBRS" will _disable_ speculation!
Then that brokenness is propagated into higher code as well by the next patch,
i.e. "SPECTRE_V2_CMD_IBRS" et al.
This is totally brain-dead and should be inverted to follow natural logic
instead.
The lowest level hardware ABI will obviously stay broken,
Why not define a sane name instead? Something like:
CTRL_DISABLE_BR_SPECULATION
CTRL_ENABLE_BR_SPECULATION
and only explain the broken negated Intel naming and flag once in the comments
for
the definition and the lowest level MSR write.
Thanks,
Ingo
