Re: [RFC] x86/alternative: Support relocations in alternatives

[Kirill A. Shutemov]

On Wed, Feb 01, 2023 at 03:10:33PM +0100, Peter Zijlstra wrote:
> +static void __init_or_module noinline
> +apply_relocation(u8 *instr, size_t len, u8 *dest, u8 *src, size_t src_len)
> +{
> +	struct insn insn;
> +	int i = 0;
> +
> +	for (;;) {
> +		if (insn_decode_kernel(&insn, &instr[i]))
> +			return;
> +
> +		switch (insn.opcode.bytes[0]) {
> +		case 0x0f:
> +			if (insn.opcode.bytes[1] < 0x80 ||
> +			    insn.opcode.bytes[1] > 0x8f)
> +				break;
> +
> +			fallthrough;	/* Jcc.d32 */
> +		case 0x70 ... 0x7f:	/* Jcc.d8 */
> +		case JMP8_INSN_OPCODE:
> +		case JMP32_INSN_OPCODE:
> +		case CALL_INSN_OPCODE:
> +			u8 *target = src + i + insn.length + insn.immediate.value;

LLVM doesn't like this declaration of 'target'. I've moved to the top of
the function.

I've tested the patch with both GCC and LLVM (after 'target' fix) with my
LAM case. See the patch below.

Works fine. Thanks.

Tested-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 6450a2723bcd..ec7613141db6 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -35,17 +35,12 @@ static inline unsigned long __untagged_addr(unsigned long addr)
 {
 	long sign;
 
-	/*
-	 * Refer tlbstate_untag_mask directly to avoid RIP-relative relocation
-	 * in alternative instructions. The relocation gets wrong when gets
-	 * copied to the target place.
-	 */
 	asm (ALTERNATIVE("",
 			 "sar $63, %[sign]\n\t" /* user_ptr ? 0 : -1UL */
-			 "or %%gs:tlbstate_untag_mask, %[sign]\n\t"
+			 "or " __percpu_arg([mask]) ", %[sign]\n\t"
 			 "and %[sign], %[addr]\n\t", X86_FEATURE_LAM)
 	     : [addr] "+r" (addr), [sign] "=r" (sign)
-	     : "m" (tlbstate_untag_mask), "[sign]" (addr));
+	     : [mask] "m" (tlbstate_untag_mask), "[sign]" (addr));
 
 	return addr;
 }
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 50b88aa3ebee..bb1437ac2f5a 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -212,6 +212,7 @@ static void __init_or_module noinline
 apply_relocation(u8 *instr, size_t len, u8 *dest, u8 *src, size_t src_len)
 {
 	struct insn insn;
+	u8 *target;
 	int i = 0;
 
 	for (;;) {
@@ -229,7 +230,7 @@ apply_relocation(u8 *instr, size_t len, u8 *dest, u8 *src, size_t src_len)
 		case JMP8_INSN_OPCODE:
 		case JMP32_INSN_OPCODE:
 		case CALL_INSN_OPCODE:
-			u8 *target = src + i + insn.length + insn.immediate.value;
+			target = src + i + insn.length + insn.immediate.value;
 			if (target < src || target > src + src_len) {
 				apply_reloc(insn.immediate.nbytes,
 					    instr + i + insn_offset_immediate(&insn),
-- 
  Kiryl Shutsemau / Kirill A. Shutemov