Re: [PATCH 1/6] powerpc/code-patching: Implement generic text patching function

From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Benjamin Gray <bgray@linux.ibm.com>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Cc: "ajd@linux.ibm.com" <ajd@linux.ibm.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"ardb@kernel.org" <ardb@kernel.org>,
	"jbaron@akamai.com" <jbaron@akamai.com>,
	"rostedt@goodmis.org" <rostedt@goodmis.org>,
	"npiggin@gmail.com" <npiggin@gmail.com>,
	"jpoimboe@kernel.org" <jpoimboe@kernel.org>
Subject: Re: [PATCH 1/6] powerpc/code-patching: Implement generic text patching function
Date: Mon, 19 Sep 2022 06:38:05 +0000	[thread overview]
Message-ID: <05f79cf0-bdb3-6c17-ed32-4b4b381226ef@csgroup.eu> (raw)
In-Reply-To: <20220916062330.430468-2-bgray@linux.ibm.com>

Le 16/09/2022 à 08:23, Benjamin Gray a écrit :
>   
> -static int do_patch_instruction(u32 *addr, ppc_inst_t instr)
> +static int patch_text(void *dest, const void *src, size_t size, bool is_exec)
>   {
>   	int err;
>   	unsigned long flags;
>   
> -	/*
> -	 * During early early boot patch_instruction is called
> -	 * when text_poke_area is not ready, but we still need
> -	 * to allow patching. We just do the plain old patching
> -	 */
> +	/* Make sure we aren't patching a freed init section */
> +	if (static_branch_likely(&init_mem_is_free) && init_section_contains(dest, 4))
> +		return 0;
> +
>   	if (!static_branch_likely(&poking_init_done))
> -		return raw_patch_instruction(addr, instr);
> +		return __patch_text(dest, src, size, is_exec, dest);
>   

Test ordering looks odd. How can init_mem_is_free be true and 
poking_init_done be false ?

>   	local_irq_save(flags);
> -	err = __do_patch_instruction(addr, instr);
> +	err = do_patch_text(dest, src, size, is_exec);
>   	local_irq_restore(flags);
>   
>   	return err;
>   }