Re: [PATCH v5 1/5] powerpc/code-patching: introduce patch_instructions()

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

 




Le 28/09/2023 à 21:48, Hari Bathini a écrit :
> patch_instruction() entails setting up pte, patching the instruction,
> clearing the pte and flushing the tlb. If multiple instructions need
> to be patched, every instruction would have to go through the above
> drill unnecessarily. Instead, introduce function patch_instructions()
> that sets up the pte, clears the pte and flushes the tlb only once per
> page range of instructions to be patched. This adds a slight overhead
> to patch_instruction() call while improving the patching time for
> scenarios where more than one instruction needs to be patched.

On my powerpc8xx, this patch leads to an increase of about 8% of the 
time needed to activate ftrace function tracer.

The problem is it complexifies patch_instruction().

Before your patch:

00000234 <patch_instruction>:
  234:	48 00 00 6c 	b       2a0 <patch_instruction+0x6c>
  238:	7c e0 00 a6 	mfmsr   r7
  23c:	7c 51 13 a6 	mtspr   81,r2
  240:	3d 40 00 00 	lis     r10,0
			242: R_PPC_ADDR16_HA	.data
  244:	39 4a 00 00 	addi    r10,r10,0
			246: R_PPC_ADDR16_LO	.data
  248:	7c 69 1b 78 	mr      r9,r3
  24c:	3d 29 40 00 	addis   r9,r9,16384
  250:	81 0a 00 08 	lwz     r8,8(r10)
  254:	55 29 00 26 	clrrwi  r9,r9,12
  258:	81 4a 00 04 	lwz     r10,4(r10)
  25c:	61 29 01 25 	ori     r9,r9,293
  260:	91 28 00 00 	stw     r9,0(r8)
  264:	55 49 00 26 	clrrwi  r9,r10,12
  268:	50 6a 05 3e 	rlwimi  r10,r3,0,20,31
  26c:	90 8a 00 00 	stw     r4,0(r10)
  270:	7c 00 50 6c 	dcbst   0,r10
  274:	7c 00 04 ac 	hwsync
  278:	7c 00 1f ac 	icbi    0,r3
  27c:	7c 00 04 ac 	hwsync
  280:	4c 00 01 2c 	isync
  284:	38 60 00 00 	li      r3,0
  288:	39 40 00 00 	li      r10,0
  28c:	91 48 00 00 	stw     r10,0(r8)
  290:	7c 00 4a 64 	tlbie   r9,r0
  294:	7c 00 04 ac 	hwsync
  298:	7c e0 01 24 	mtmsr   r7
  29c:	4e 80 00 20 	blr
  2a0:	90 83 00 00 	stw     r4,0(r3)
  2a4:	7c 00 18 6c 	dcbst   0,r3
  2a8:	7c 00 04 ac 	hwsync
  2ac:	7c 00 1f ac 	icbi    0,r3
  2b0:	7c 00 04 ac 	hwsync
  2b4:	4c 00 01 2c 	isync
  2b8:	38 60 00 00 	li      r3,0
  2bc:	4e 80 00 20 	blr
  2c0:	38 60 ff ff 	li      r3,-1
  2c4:	4b ff ff c4 	b       288 <patch_instruction+0x54>
  2c8:	38 60 ff ff 	li      r3,-1
  2cc:	4e 80 00 20 	blr


After you patch:

0000020c <__do_patch_instructions>:
  20c:	94 21 ff e0 	stwu    r1,-32(r1)
  210:	3d 40 00 00 	lis     r10,0
			212: R_PPC_ADDR16_HA	.data
  214:	93 81 00 10 	stw     r28,16(r1)
  218:	93 c1 00 18 	stw     r30,24(r1)
  21c:	93 a1 00 14 	stw     r29,20(r1)
  220:	93 e1 00 1c 	stw     r31,28(r1)
  224:	39 4a 00 00 	addi    r10,r10,0
			226: R_PPC_ADDR16_LO	.data
  228:	7c 69 1b 78 	mr      r9,r3
  22c:	7c be 2b 79 	mr.     r30,r5
  230:	3d 29 40 00 	addis   r9,r9,16384
  234:	83 ea 00 04 	lwz     r31,4(r10)
  238:	83 aa 00 08 	lwz     r29,8(r10)
  23c:	55 29 00 26 	clrrwi  r9,r9,12
  240:	61 29 01 25 	ori     r9,r9,293
  244:	57 fc 00 26 	clrrwi  r28,r31,12
  248:	91 3d 00 00 	stw     r9,0(r29)
  24c:	50 7f 05 3e 	rlwimi  r31,r3,0,20,31
  250:	40 82 00 4c 	bne     29c <__do_patch_instructions+0x90>
  254:	81 24 00 00 	lwz     r9,0(r4)
  258:	91 3f 00 00 	stw     r9,0(r31)
  25c:	7c 00 f8 6c 	dcbst   0,r31
  260:	7c 00 04 ac 	hwsync
  264:	7c 00 1f ac 	icbi    0,r3
  268:	7c 00 04 ac 	hwsync
  26c:	4c 00 01 2c 	isync
  270:	38 60 00 00 	li      r3,0
  274:	39 20 00 00 	li      r9,0
  278:	91 3d 00 00 	stw     r9,0(r29)
  27c:	7c 00 e2 64 	tlbie   r28,r0
  280:	7c 00 04 ac 	hwsync
  284:	83 81 00 10 	lwz     r28,16(r1)
  288:	83 a1 00 14 	lwz     r29,20(r1)
  28c:	83 c1 00 18 	lwz     r30,24(r1)
  290:	83 e1 00 1c 	lwz     r31,28(r1)
  294:	38 21 00 20 	addi    r1,r1,32
  298:	4e 80 00 20 	blr
  29c:	2c 06 00 00 	cmpwi   r6,0
  2a0:	7c 08 02 a6 	mflr    r0
  2a4:	90 01 00 24 	stw     r0,36(r1)
  2a8:	40 82 00 24 	bne     2cc <__do_patch_instructions+0xc0>
  2ac:	7f e3 fb 78 	mr      r3,r31
  2b0:	48 00 00 01 	bl      2b0 <__do_patch_instructions+0xa4>
			2b0: R_PPC_REL24	memcpy
  2b4:	7c 9f f2 14 	add     r4,r31,r30
  2b8:	7f e3 fb 78 	mr      r3,r31
  2bc:	48 00 00 01 	bl      2bc <__do_patch_instructions+0xb0>
			2bc: R_PPC_REL24	flush_icache_range
  2c0:	80 01 00 24 	lwz     r0,36(r1)
  2c4:	7c 08 03 a6 	mtlr    r0
  2c8:	4b ff ff a8 	b       270 <__do_patch_instructions+0x64>
  2cc:	80 84 00 00 	lwz     r4,0(r4)
  2d0:	57 c5 f0 be 	srwi    r5,r30,2
  2d4:	7f e3 fb 78 	mr      r3,r31
  2d8:	48 00 00 01 	bl      2d8 <__do_patch_instructions+0xcc>
			2d8: R_PPC_REL24	memset32
  2dc:	4b ff ff d8 	b       2b4 <__do_patch_instructions+0xa8>
  2e0:	38 60 ff ff 	li      r3,-1
  2e4:	4b ff ff 90 	b       274 <__do_patch_instructions+0x68>

...

00000310 <patch_instruction>:
  310:	94 21 ff e0 	stwu    r1,-32(r1)
  314:	90 81 00 08 	stw     r4,8(r1)
  318:	48 00 00 40 	b       358 <patch_instruction+0x48>
  31c:	7c 08 02 a6 	mflr    r0
  320:	90 01 00 24 	stw     r0,36(r1)
  324:	93 e1 00 1c 	stw     r31,28(r1)
  328:	7f e0 00 a6 	mfmsr   r31
  32c:	7c 51 13 a6 	mtspr   81,r2
  330:	38 c0 00 00 	li      r6,0
  334:	38 81 00 08 	addi    r4,r1,8
  338:	38 a0 00 00 	li      r5,0
  33c:	4b ff fe d1 	bl      20c <__do_patch_instructions>
  340:	7f e0 01 24 	mtmsr   r31
  344:	80 01 00 24 	lwz     r0,36(r1)
  348:	83 e1 00 1c 	lwz     r31,28(r1)
  34c:	7c 08 03 a6 	mtlr    r0
  350:	38 21 00 20 	addi    r1,r1,32
  354:	4e 80 00 20 	blr
  358:	81 21 00 08 	lwz     r9,8(r1)
  35c:	91 23 00 00 	stw     r9,0(r3)
  360:	7c 00 18 6c 	dcbst   0,r3
  364:	7c 00 04 ac 	hwsync
  368:	7c 00 1f ac 	icbi    0,r3
  36c:	7c 00 04 ac 	hwsync
  370:	4c 00 01 2c 	isync
  374:	38 60 00 00 	li      r3,0
  378:	4b ff ff d8 	b       350 <patch_instruction+0x40>
  37c:	38 60 ff ff 	li      r3,-1
  380:	4b ff ff d0 	b       350 <patch_instruction+0x40>


Christophe

> 
> Signed-off-by: Hari Bathini <hbathini@xxxxxxxxxxxxx>
> ---
>   arch/powerpc/include/asm/code-patching.h |  1 +
>   arch/powerpc/lib/code-patching.c         | 93 +++++++++++++++++++++---
>   2 files changed, 85 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
> index 3f881548fb61..43a4aedfa703 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -74,6 +74,7 @@ int create_cond_branch(ppc_inst_t *instr, const u32 *addr,
>   int patch_branch(u32 *addr, unsigned long target, int flags);
>   int patch_instruction(u32 *addr, ppc_inst_t instr);
>   int raw_patch_instruction(u32 *addr, ppc_inst_t instr);
> +int patch_instructions(void *addr, void *code, size_t len, bool repeat_instr);
>   
>   static inline unsigned long patch_site_addr(s32 *site)
>   {
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
> index b00112d7ad46..4ff002bc41f6 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -278,7 +278,36 @@ static void unmap_patch_area(unsigned long addr)
>   	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
>   }
>   
> -static int __do_patch_instruction_mm(u32 *addr, ppc_inst_t instr)
> +static int __patch_instructions(u32 *patch_addr, void *code, size_t len, bool repeat_instr)
> +{
> +	unsigned long start = (unsigned long)patch_addr;
> +
> +	/* Repeat instruction */
> +	if (repeat_instr) {
> +		ppc_inst_t instr = ppc_inst_read(code);
> +
> +		if (ppc_inst_prefixed(instr)) {
> +			u64 val = ppc_inst_as_ulong(instr);
> +
> +			memset64((uint64_t *)patch_addr, val, len / 8);
> +		} else {
> +			u32 val = ppc_inst_val(instr);
> +
> +			memset32(patch_addr, val, len / 4);
> +		}
> +	} else
> +		memcpy(patch_addr, code, len);
> +
> +	smp_wmb();	/* smp write barrier */
> +	flush_icache_range(start, start + len);
> +	return 0;
> +}
> +
> +/*
> + * A page is mapped and instructions that fit the page are patched.
> + * Assumes 'len' to be (PAGE_SIZE - offset_in_page(addr)) or below.
> + */
> +static int __do_patch_instructions_mm(u32 *addr, void *code, size_t len, bool repeat_instr)
>   {
>   	int err;
>   	u32 *patch_addr;
> @@ -307,11 +336,15 @@ static int __do_patch_instruction_mm(u32 *addr, ppc_inst_t instr)
>   
>   	orig_mm = start_using_temp_mm(patching_mm);
>   
> -	err = __patch_instruction(addr, instr, patch_addr);
> +	/* Single instruction case. */
> +	if (len == 0) {
> +		err = __patch_instruction(addr, *(ppc_inst_t *)code, patch_addr);
>   
> -	/* hwsync performed by __patch_instruction (sync) if successful */
> -	if (err)
> -		mb();  /* sync */
> +		/* hwsync performed by __patch_instruction (sync) if successful */
> +		if (err)
> +			mb();  /* sync */
> +	} else
> +		err = __patch_instructions(patch_addr, code, len, repeat_instr);
>   
>   	/* context synchronisation performed by __patch_instruction (isync or exception) */
>   	stop_using_temp_mm(patching_mm, orig_mm);
> @@ -328,7 +361,11 @@ static int __do_patch_instruction_mm(u32 *addr, ppc_inst_t instr)
>   	return err;
>   }
>   
> -static int __do_patch_instruction(u32 *addr, ppc_inst_t instr)
> +/*
> + * A page is mapped and instructions that fit the page are patched.
> + * Assumes 'len' to be (PAGE_SIZE - offset_in_page(addr)) or below.
> + */
> +static int __do_patch_instructions(u32 *addr, void *code, size_t len, bool repeat_instr)
>   {
>   	int err;
>   	u32 *patch_addr;
> @@ -345,7 +382,11 @@ static int __do_patch_instruction(u32 *addr, ppc_inst_t instr)
>   	if (radix_enabled())
>   		asm volatile("ptesync": : :"memory");
>   
> -	err = __patch_instruction(addr, instr, patch_addr);
> +	/* Single instruction case. */
> +	if (len == 0)
> +		err = __patch_instruction(addr, *(ppc_inst_t *)code, patch_addr);
> +	else
> +		err = __patch_instructions(patch_addr, code, len, repeat_instr);
>   
>   	pte_clear(&init_mm, text_poke_addr, pte);
>   	flush_tlb_kernel_range(text_poke_addr, text_poke_addr + PAGE_SIZE);
> @@ -369,15 +410,49 @@ int patch_instruction(u32 *addr, ppc_inst_t instr)
>   
>   	local_irq_save(flags);
>   	if (mm_patch_enabled())
> -		err = __do_patch_instruction_mm(addr, instr);
> +		err = __do_patch_instructions_mm(addr, &instr, 0, false);
>   	else
> -		err = __do_patch_instruction(addr, instr);
> +		err = __do_patch_instructions(addr, &instr, 0, false);
>   	local_irq_restore(flags);
>   
>   	return err;
>   }
>   NOKPROBE_SYMBOL(patch_instruction);
>   
> +/*
> + * Patch 'addr' with 'len' bytes of instructions from 'code'.
> + *
> + * If repeat_instr is true, the same instruction is filled for
> + * 'len' bytes.
> + */
> +int patch_instructions(void *addr, void *code, size_t len, bool repeat_instr)
> +{
> +	unsigned long flags;
> +	size_t plen;
> +	int err;
> +
> +	while (len > 0) {
> +		plen = min_t(size_t, PAGE_SIZE - offset_in_page(addr), len);
> +
> +		local_irq_save(flags);
> +		if (mm_patch_enabled())
> +			err = __do_patch_instructions_mm(addr, code, plen, repeat_instr);
> +		else
> +			err = __do_patch_instructions(addr, code, plen, repeat_instr);
> +		local_irq_restore(flags);
> +		if (err)
> +			break;
> +
> +		len -= plen;
> +		addr = addr + plen;
> +		if (!repeat_instr)
> +			code = code + plen;
> +	}
> +
> +	return err;
> +}
> +NOKPROBE_SYMBOL(patch_instructions);
> +
>   int patch_branch(u32 *addr, unsigned long target, int flags)
>   {
>   	ppc_inst_t instr;




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux