Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets with SLS

[Paolo Bonzini <pbonzini@redhat.com>]

On 3/17/22 12:04, Peter Zijlstra wrote:
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
> Depending on what Paolo wants, it might make sense to merge this into
> tip/x86/urgent such that we can then resolve the merge conflict vs
> tip/x86/core with something like the below:

Sorry for responding late, I was sick the past few days.  Go ahead and 
apply it to tip/x86/core with the rest of the SLS and IBT patches.  If 
you place it in front of the actual insertion of the INT3 it will even 
be bisectable, but I'm not sure if your commit hashes are already frozen.

Just one thing:

> -#define SETCC_ALIGN	(4 * (1 + IS_ENABLED(CONFIG_SLS)))
> +/*
> + * Depending on .config the SETcc functions look like:
> + *
> + * setcc:
> + * +0	ENDBR		[CONFIG_X86_KERNEL_IBT]
> + * +4	SETcc	%al
> + * +7	RET
> + * +8	INT3		[CONFIG_SLS]
> + *
> + * Which gives possible sizes: 4, 5, 8, 9 which when rounded up to the
> + * next power-of-two alignment become: 4, 8, 16.
> + */
> +#define SETCC_ALIGN	(4 * (1 + IS_ENABLED(CONFIG_SLS)) * (1 + HAS_KERNEL_IBT))

This might be slightly nicer as (4 << IS_ENABLED(CONFIG_SLS) << 
HAS_KERNEL_IBT.  Or maybe not, depends on your taste.

It might also be worth doing:

#define SETCC_LENGTH (4 + IS_ENABLED(CONFIG_SLS) + 4 * HAS_KERNEL_IBT)
#define SETCC_ALIGN  (4 << IS_ENABLED(CONFIG_SLS) << HAS_KERNEL_IBT)
BUILD_BUG_ON(SETCC_LENGTH <= SETCC_ALIGN);

Thanks,

Paolo