Re: [PATCH v1 1/3] powerpc/code-patching: Test patch_instructions() during boot

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 15/03/2024 à 03:57, Benjamin Gray a écrit :
> patch_instructions() introduces new behaviour with a couple of
> variations. Test each case of
> 
>    * a repeated 32-bit instruction,
>    * a repeated 64-bit instruction (ppc64), and
>    * a copied sequence of instructions
> 
> for both on a single page and when it crosses a page boundary.
> 
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> ---
>   arch/powerpc/lib/test-code-patching.c | 92 +++++++++++++++++++++++++++
>   1 file changed, 92 insertions(+)
> 
> diff --git a/arch/powerpc/lib/test-code-patching.c b/arch/powerpc/lib/test-code-patching.c
> index c44823292f73..35a3756272df 100644
> --- a/arch/powerpc/lib/test-code-patching.c
> +++ b/arch/powerpc/lib/test-code-patching.c
> @@ -347,6 +347,97 @@ static void __init test_prefixed_patching(void)
>   	check(!memcmp(iptr, expected, sizeof(expected)));
>   }
>   
> +static void __init test_multi_instruction_patching(void)
> +{
> +	u32 code[256];

Build failure:

   CC      arch/powerpc/lib/test-code-patching.o
arch/powerpc/lib/test-code-patching.c: In function 
'test_multi_instruction_patching':
arch/powerpc/lib/test-code-patching.c:439:1: error: the frame size of 
1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
   439 | }
       | ^
cc1: all warnings being treated as errors
make[4]: *** [scripts/Makefile.build:243: 
arch/powerpc/lib/test-code-patching.o] Error 1


I have to avoid big arrays on the stack.


> +	void *buf;
> +	u32 *addr32;
> +	u64 *addr64;
> +	ppc_inst_t inst64 = ppc_inst_prefix(OP_PREFIX << 26 | 3UL << 24, PPC_RAW_TRAP());
> +	u32 inst32 = PPC_RAW_NOP();
> +
> +	buf = vzalloc(PAGE_SIZE * 8);
> +	check(buf);
> +	if (!buf)
> +		return;
> +
> +	/* Test single page 32-bit repeated instruction */
> +	addr32 = buf + PAGE_SIZE;
> +	check(!patch_instructions(addr32 + 1, &inst32, 12, true));
> +
> +	check(addr32[0] == 0);
> +	check(addr32[1] == inst32);
> +	check(addr32[2] == inst32);
> +	check(addr32[3] == inst32);
> +	check(addr32[4] == 0);
> +
> +	/* Test single page 64-bit repeated instruction */
> +	if (IS_ENABLED(CONFIG_PPC64)) {
> +		check(ppc_inst_prefixed(inst64));
> +
> +		addr64 = buf + PAGE_SIZE * 2;
> +		ppc_inst_write(code, inst64);
> +		check(!patch_instructions((u32 *)(addr64 + 1), code, 24, true));
> +
> +		check(addr64[0] == 0);
> +		check(ppc_inst_equal(ppc_inst_read((u32 *)&addr64[1]), inst64));
> +		check(ppc_inst_equal(ppc_inst_read((u32 *)&addr64[2]), inst64));
> +		check(ppc_inst_equal(ppc_inst_read((u32 *)&addr64[3]), inst64));
> +		check(addr64[4] == 0);
> +	}
> +
> +	/* Test single page memcpy */
> +	addr32 = buf + PAGE_SIZE * 3;
> +
> +	for (int i = 0; i < ARRAY_SIZE(code); i++)
> +		code[i] = i + 1;
> +
> +	check(!patch_instructions(addr32 + 1, code, sizeof(code), false));
> +
> +	check(addr32[0] == 0);
> +	check(!memcmp(&addr32[1], code, sizeof(code)));
> +	check(addr32[ARRAY_SIZE(code) + 1] == 0);
> +
> +	/* Test multipage 32-bit repeated instruction */
> +	addr32 = buf + PAGE_SIZE * 4 - 8;
> +	check(!patch_instructions(addr32 + 1, &inst32, 12, true));
> +
> +	check(addr32[0] == 0);
> +	check(addr32[1] == inst32);
> +	check(addr32[2] == inst32);
> +	check(addr32[3] == inst32);
> +	check(addr32[4] == 0);
> +
> +	/* Test multipage 64-bit repeated instruction */
> +	if (IS_ENABLED(CONFIG_PPC64)) {
> +		check(ppc_inst_prefixed(inst64));
> +
> +		addr64 = buf + PAGE_SIZE * 5 - 8;
> +		ppc_inst_write(code, inst64);
> +		check(!patch_instructions((u32 *)(addr64 + 1), code, 24, true));
> +
> +		check(addr64[0] == 0);
> +		check(ppc_inst_equal(ppc_inst_read((u32 *)&addr64[1]), inst64));
> +		check(ppc_inst_equal(ppc_inst_read((u32 *)&addr64[2]), inst64));
> +		check(ppc_inst_equal(ppc_inst_read((u32 *)&addr64[3]), inst64));
> +		check(addr64[4] == 0);
> +	}
> +
> +	/* Test multipage memcpy */
> +	addr32 = buf + PAGE_SIZE * 6 - 12;
> +
> +	for (int i = 0; i < ARRAY_SIZE(code); i++)
> +		code[i] = i + 1;
> +
> +	check(!patch_instructions(addr32 + 1, code, sizeof(code), false));
> +
> +	check(addr32[0] == 0);
> +	check(!memcmp(&addr32[1], code, sizeof(code)));
> +	check(addr32[ARRAY_SIZE(code) + 1] == 0);
> +
> +	vfree(buf);
> +}
> +
>   static int __init test_code_patching(void)
>   {
>   	pr_info("Running code patching self-tests ...\n");
> @@ -356,6 +447,7 @@ static int __init test_code_patching(void)
>   	test_create_function_call();
>   	test_translate_branch();
>   	test_prefixed_patching();
> +	test_multi_instruction_patching();
>   
>   	return 0;
>   }