tlbilxlpid -> error: invalid instruction

[nickdesaulniers]

via https://lore.kernel.org/llvm/[email protected]/

it looks like:

void foo(void) {
    asm volatile("tlbilxlpid");
}

produces:

<source>:2:18: error: invalid instruction
    asm volatile("tlbilxlpid");
                 ^
<inline asm>:1:2: note: instantiated into assembly here
        tlbilxlpid
        ^~~~~~~~~~

for --target=powerpc-linux-gnu. I wonder if clang is missing support for this instruction (or perhaps it's an alias to something that clang does understand).

Reported upstream: llvm/llvm-project#64080

[nickdesaulniers]

https://reviews.llvm.org/D156204

[nathanchance]

llvm/llvm-project@53648ac

[nickdesaulniers]

We should use .inst in the kernel for clang < 18 (or 17 if we get a backport). There are 2 different values necessary dependent on the endianness.

[nickdesaulniers]

.inst might be aarch64-only :(

[nickdesaulniers]

ah, .long is used, include/asm/dcr-native.h for example

[nickdesaulniers]

diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index d58df71ace58..f3d32e250b08 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -92,7 +92,12 @@ void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500 *vcpu_e500)
 
        local_irq_save(flags);
        mtspr(SPRN_MAS5, MAS5_SGS | get_lpid(&vcpu_e500->vcpu));
+// https://github.com/ClangBuiltLinux/linux/issues/1891
+#if defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 180000
+       asm (".long 0x7c000024");
+#else
        asm volatile("tlbilxlpid");
+#endif
        mtspr(SPRN_MAS5, 0);
        local_irq_restore(flags);
 }

is the tentative patch. volatile is not necessary since there are no outputs. We don't need to flip the .long value based on endianness. Will wait to see about backports for clang 17 to get the version right.

[nickdesaulniers]

@mpe Looking at include/asm/dcr-native.h, do you have a preference between the above vs just changing the existing asm statement to use .long and a comment (no preprocessor directives)?

[mpe]

The usual solution is to define the value in ppc-opcode.h. Actually we already have a PPC_TLBILX macro in there, so maybe you can use that, or add a variant of it. I don't think it's worth an #ifdef CLANG, just always use the .long version, though I guess a comment saying it can be removed when Clang version X is unsupported would be good.

[nickdesaulniers]

@mpe thoughts on:

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index ef6972aa33b9..d84473db7c02 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -616,6 +616,7 @@
 #define PPC_TLBILX(t, a, b)    stringify_in_c(.long PPC_RAW_TLBILX(t, a, b))
 #define PPC_TLBILX_ALL(a, b)   PPC_TLBILX(0, a, b)
 #define PPC_TLBILX_PID(a, b)   PPC_TLBILX(1, a, b)
+#define PPC_TLBILX_LPID()      PPC_TLBILX(0, 0, 0)
 #define PPC_TLBILX_VA(a, b)    PPC_TLBILX(3, a, b)
 #define PPC_WAIT_v203          stringify_in_c(.long PPC_RAW_WAIT_v203)
 #define PPC_WAIT(w, p)         stringify_in_c(.long PPC_RAW_WAIT(w, p))
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index d58df71ace58..96ea8507d8a3 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -20,6 +20,7 @@
 #include <asm/cputable.h>
 #include <asm/kvm_ppc.h>
 #include <asm/dbell.h>
+#include <asm/opcode.h>
 
 #include "booke.h"
 #include "e500.h"
@@ -92,7 +93,10 @@ void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500 *vcpu_e500)
 
        local_irq_save(flags);
        mtspr(SPRN_MAS5, MAS5_SGS | get_lpid(&vcpu_e500->vcpu));
-       asm volatile("tlbilxlpid");
+       /* clang-17 and older could not assemble tlbilxlpid.
+        * https://github.com/ClangBuiltLinux/linux/issues/1891
+        */
+       PPC_TLBILX_LPID();
        mtspr(SPRN_MAS5, 0);
        local_irq_restore(flags);
 }

[nickdesaulniers]

or does that need to be asm volatile (PPC_TLBILX_LPID());? (looks like yes)

[mpe]

The brackets on the macro aren't needed because it takes no params, see eg. PPC_ISA_3_0_INVALIDATE_ERAT. And then yes the call site is asm volatile (PPC_TLBILX_LPID). The volatile is not needed there but that's an orthogonal change I guess.

[nickdesaulniers]

Submitted: https://lore.kernel.org/llvm/[email protected]/

[nickdesaulniers]

accepted: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ae7936d232d862e5b8311180036281ffe93735b8

[nathanchance]

https://git.kernel.org/linus/ae7936d232d862e5b8311180036281ffe93735b8