Re: [PATCH v2 5/9] LoongArch: Simplify the invtlb wrappers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Good evening! (I really have to get some sleep real soon.)

On 6/25/23 02:55, Xi Ruoyao wrote:
On Sun, 2023-06-25 at 02:40 +0800, WANG Xuerui wrote:
From: WANG Xuerui <git@xxxxxxxxxx>

The invtlb instruction has been supported by upstream LoongArch
toolchains from day one, so ditch the raw opcode trickery and just use
plain inline asm for it.

While at it, also make the invtlb asm statements barriers, for proper
modeling of the side effects.

The signature of the other more specific invtlb wrappers contain unused
arguments right now, but these are not removed right away in order for
the patch to be focused. In the meantime, assertions are added to ensure
no accidental misuse happens before the refactor. (The more specific
wrappers cannot re-use the generic invtlb wrapper, because the ISA
manual says $zero shall be used in case a particular op does not take
the respective argument: re-using the generic wrapper would mean losing
control over the register usage.)

Signed-off-by: WANG Xuerui <git@xxxxxxxxxx>
---
  arch/loongarch/include/asm/tlb.h | 39 ++++++++++++++++----------------
  1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/arch/loongarch/include/asm/tlb.h b/arch/loongarch/include/asm/tlb.h
index 0dc9ee2b05d2..15750900540c 100644
--- a/arch/loongarch/include/asm/tlb.h
+++ b/arch/loongarch/include/asm/tlb.h
@@ -88,52 +88,51 @@ enum invtlb_ops {
         INVTLB_GID_ADDR = 0x16,
  };
-/*
- * invtlb op info addr
- * (0x1 << 26) | (0x24 << 20) | (0x13 << 15) |
- * (addr << 10) | (info << 5) | op
- */
  static inline void invtlb(u32 op, u32 info, u64 addr)
Oh, technically these wrappers should be __always_inline, not only
inline because they don't work at all if not inlined.  Should we change
them to __always_inline in this patch by the way?
Makes sense... let me send a quick v3 tomorrow (and maybe after Huacai has taken a look).

  {
+       BUILD_BUG_ON(!__builtin_constant_p(op));
Hmm, I guess it's redundant.  If op is not a compile-time constant, it
won't satisfy the "i" constraint then the compiler will complain anyway.
You're right (in v1 this wasn't done, yet it compiled just fine with GCC and Clang). I'll remove the op assertions in v3. Thanks for the reviews.

         __asm__ __volatile__(
-               "parse_r addr,%0\n\t"
-               "parse_r info,%1\n\t"
-               ".word ((0x6498000) | (addr << 10) | (info << 5) | %2)\n\t"
-               :
-               : "r"(addr), "r"(info), "i"(op)
+               "invtlb %0, %1, %2\n\t"
                 :
+               : "i"(op), "r"(info), "r"(addr)
+               : "memory"
                 );
  }
Likewise for other wrappers.

/* snip */

--
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/




[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux