Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <94df38ce-6bd7-a993-7d9f-0a1418a1c8df@redhat.com>
Date:   Sat, 19 Mar 2022 14:24:06 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Subject: Re: [PATCH -v1.2] kvm/emulate: Fix SETcc emulation function offsets
 with SLS
Content-Language: en-US
To:     Peter Zijlstra <peterz@infradead.org>,
        Borislav Petkov <bp@alien8.de>
Cc:     Jamie Heilman <jamie@audible.transient.net>,
        linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        Sean Christopherson <seanjc@google.com>, kvm@vger.kernel.org
References: <YjGzJwjrvxg5YZ0Z@audible.transient.net>
 <YjHYh3XRbHwrlLbR@zn.tnic> <YjIwRR5UsTd3W4Bj@audible.transient.net>
 <YjI69aUseN/IuzTj@zn.tnic> <YjJFb02Fc0jeoIW4@audible.transient.net>
 <YjJVWYzHQDbI6nZM@zn.tnic>
 <20220316220201.GM8939@worktop.programming.kicks-ass.net>
 <YjMBdMlhVMGLG5ws@zn.tnic> <YjMS8eTOhXBOPFOe@zn.tnic>
 <YjMVpfe/9ldmWX8W@hirez.programming.kicks-ass.net>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <YjMVpfe/9ldmWX8W@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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