When converting indirect to direct calls, there is no need to execute endbr64
instructions.  Detect and optimise this case, leaving a warning in the case
that no endbr64 was found, as it likely indicates a build error.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/alternative.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index ec24692e9595..5ae4c80d5119 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -18,6 +18,7 @@
 #include <xen/delay.h>
 #include <xen/types.h>
 #include <asm/apic.h>
+#include <asm/endbr.h>
 #include <asm/processor.h>
 #include <asm/alternative.h>
 #include <xen/init.h>
@@ -279,6 +280,27 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start,
 
                 if ( dest )
                 {
+                    /*
+                     * When building for CET-IBT, all function pointer targets
+                     * should have an endbr64 instruction.
+                     *
+                     * If this is not the case, leave a warning because
+                     * something is wrong with the build.
+                     *
+                     * Otherwise, skip the endbr64 instruction.  This is a
+                     * marginal perf improvement which saves on instruction
+                     * decode bandwidth.
+                     */
+                    if ( IS_ENABLED(CONFIG_HAS_CC_CET_IBT) )
+                    {
+                        if ( is_endbr64(dest) )
+                            dest += 4;
+                        else
+                            printk(XENLOG_WARNING
+                                   "altcall %ps dest %ps has no endbr64\n",
+                                   orig, dest);
+                    }
+
                     disp = dest - (orig + 5);
                     ASSERT(disp == (int32_t)disp);
                     *(int32_t *)(buf + 1) = disp;
-- 
2.11.0

On 26.11.2021 22:22, Andrew Cooper wrote:
> @@ -279,6 +280,27 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start,
>  
>                  if ( dest )
>                  {
> +                    /*
> +                     * When building for CET-IBT, all function pointer targets
> +                     * should have an endbr64 instruction.
> +                     *
> +                     * If this is not the case, leave a warning because
> +                     * something is wrong with the build.
> +                     *
> +                     * Otherwise, skip the endbr64 instruction.  This is a
> +                     * marginal perf improvement which saves on instruction
> +                     * decode bandwidth.
> +                     */
> +                    if ( IS_ENABLED(CONFIG_HAS_CC_CET_IBT) )
> +                    {
> +                        if ( is_endbr64(dest) )

I would have given my R-b, but I don't see where is_endbr64() is coming
from, and you don't list any prereqs here or in the cover letter. I'm
afraid I don't fancy going hunt for it in the many other pending patches.
Hence only on the assumption that the helper has got introduced before:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

On 01/12/2021 08:10, Jan Beulich wrote:
> On 26.11.2021 22:22, Andrew Cooper wrote:
>> @@ -279,6 +280,27 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start,
>>  
>>                  if ( dest )
>>                  {
>> +                    /*
>> +                     * When building for CET-IBT, all function pointer targets
>> +                     * should have an endbr64 instruction.
>> +                     *
>> +                     * If this is not the case, leave a warning because
>> +                     * something is wrong with the build.
>> +                     *
>> +                     * Otherwise, skip the endbr64 instruction.  This is a
>> +                     * marginal perf improvement which saves on instruction
>> +                     * decode bandwidth.
>> +                     */
>> +                    if ( IS_ENABLED(CONFIG_HAS_CC_CET_IBT) )
>> +                    {
>> +                        if ( is_endbr64(dest) )
> I would have given my R-b, but I don't see where is_endbr64() is coming
> from, and you don't list any prereqs here or in the cover letter. I'm
> afraid I don't fancy going hunt for it in the many other pending patches.
> Hence only on the assumption that the helper has got introduced before:
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Oh sorry - this series is based on the CET-IBT series, which adds
CONFIG_HAS_CC_CET_IBT and is_endbr64().

~Andrew