Re: [PATCH v3 3/6] x86/bugs: Fix RSB clearing in indirect_branch_prediction_barrier()

[Nikolay Borisov <nik.borisov@suse.com>]

On 5.04.25 г. 3:56 ч., Josh Poimboeuf wrote:
> On Sat, Apr 05, 2025 at 01:56:59AM +0300, Nikolay Borisov wrote:
>> On 4.04.25 г. 18:17 ч., Josh Poimboeuf wrote:
>>> On Fri, Apr 04, 2025 at 05:45:37PM +0300, Nikolay Borisov wrote:
>>>>
>>>>
>>>> On 2.04.25 г. 21:19 ч., Josh Poimboeuf wrote:
>>>>> IBPB is expected to clear the RSB.  However, if X86_BUG_IBPB_NO_RET is
>>>>> set, that doesn't happen.  Make indirect_branch_prediction_barrier()
>>>>> take that into account by calling __write_ibpb() which already does the
>>>>> right thing.
>>>>
>>>> I find this changelog somewhat dubious. So zen < 4 basically have
>>>> IBPB_NO_RET, your patch 2 in this series makes using SBPB for cores which
>>>> have SRSO_NO or if the mitigation is disabled. So if you have a core which
>>>> is zen <4 and doesn't use SBPB then what happens?
>>>
>>> I'm afraid I don't understand the question.  In that case write_ibpb()
>>> uses IBPB and manually clears the RSB.
>>>
>>
>> Actually isn't this patch a noop. The old code simply wrote the value of
>> x86_pred_cmd to the IA32-PRED_CMD register iff FEATURE_IBPB was set. So
>> x86_pred_cmd might contain either PRED_CMD_IBPB or PRED_CMD_SBPB, meaning
>> the correct value was written.
>>
>> With your change you now call __write_ibpb() which does effectively the same
>> thing.
> 
> Hm, are you getting SBPB and IBPB_NO_RET mixed up?  They're completely
> separate and distinct:
> 
>    - SBPB is an AMD feature which is just like IBPB, except it doesn't
>      flush branch type predictions.  It can be used when the SRSO
>      mitigation isn't needed.  That was fixed by the previous patch.
> 
>    - AMD has a bug on older CPUs where IBPB doesn't flush the RSB.  Such
>      CPUs have X86_BUG_IBPB_NO_RET set.  That's fixed with this patch due
>      to the fact that write_ibpb() has this:
> 
> 	/* Make sure IBPB clears return stack preductions too. */
> 	FILL_RETURN_BUFFER %rax, RSB_CLEAR_LOOPS, X86_BUG_IBPB_NO_RET
> 
> So you're right in that this patch doesn't change SBPB behavior.  But
> that's not what it intends to do :-)

Fair enough, it just wasn't obvious from the diff context that there is 
the FILL_RETURN_BUFFER , my bad for not looking at __write_ibpb.

Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>