Re: [PATCH resend] powerpc/code-patching: Avoid r/w mapping of the zero page

[Ard Biesheuvel]

On Wed, 20 May 2026, at 11:59, Christophe Leroy (CS GROUP) wrote:
> Le 20/05/2026 à 11:40, Ard Biesheuvel a écrit :
>> 
>> On Wed, 20 May 2026, at 11:36, Christophe Leroy (CS GROUP) wrote:
>>> Le 20/05/2026 à 10:54, Ard Biesheuvel a écrit :
>>>> The only remaining use of map_patch_area() is mapping the zero page, and
>>>> immediately unmapping it again so that the intermediate page table
>>>> levels are all guaranteed to be populated.
>>>>
>>>> The use of the zero page here is completely arbitrary, and not harmful
>>>> per se, but currently, it creates a writable mapping, and does so in a
>>>> manner that requires that the empty_zero_page[] symbol is not
>>>> const-qualified.
>>>>
>>>> Given that this is about to change, and that map_patch_area() now never
>>>> maps anything other than the zero page, let's simplify the code and
>>>> - take the PA of empty_zero_page directly
>>>> - create a read-only temporary mapping.
>>>>
>>>> This allows empty_zero_page[] to be repainted as const u8[] in a
>>>> subsequent patch, without making substantial changes to this code
>>>> patching logic.
>>>>
>>>> Cc: Madhavan Srinivasan <maddy@xxxxxxxxxxxxx>
>>>> Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
>>>> Cc: Nicholas Piggin <npiggin@xxxxxxxxx>
>>>> Cc: "Christophe Leroy (CS GROUP)" <chleroy@xxxxxxxxxx>
>>>> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
>>>> ---
>>>> Build tested only (Clang)
>>>>
>>>> Resending from my non-Google email. Apologies if you are receiving this
>>>> twice.
>>>>
>>>>    arch/powerpc/lib/code-patching.c | 11 +++++------
>>>>    1 file changed, 5 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
>>>> index f84e0337cc02..13a8acf851f1 100644
>>>> --- a/arch/powerpc/lib/code-patching.c
>>>> +++ b/arch/powerpc/lib/code-patching.c
>>>> @@ -60,7 +60,7 @@ struct patch_context {
>>>>    
>>>>    static DEFINE_PER_CPU(struct patch_context, cpu_patching_context);
>>>>    
>>>> -static int map_patch_area(void *addr, unsigned long text_poke_addr);
>>>> +static int map_patch_area(unsigned long text_poke_addr);
>>>>    static void unmap_patch_area(unsigned long addr);
>>>>    
>>>>    static bool mm_patch_enabled(void)
>>>> @@ -117,7 +117,7 @@ static int text_area_cpu_up(unsigned int cpu)
>>>>    
>>>>    	// Map/unmap the area to ensure all page tables are pre-allocated
>>>>    	addr = (unsigned long)area->addr;
>>>> -	err = map_patch_area(empty_zero_page, addr);
>>>> +	err = map_patch_area(addr);
>>>
>>> I would get rid of map_patch_area() completely and just do:
>>>
>>> 	err = map_kernel_page(addr, __pa_symbol(empty_zero_page), PAGE_KERNEL_RO);
>>>
>> 
>> I think retaining the symmetry of map_patch_area() and unmap_patch_area()
>> makes sense too.
>
> Could also drop unmap_patch_area() and use unmap_kernel_page() instead.
>

Good point. That way, we'll end up with

 arch/powerpc/lib/code-patching.c | 52 ++--------------------------------------
 1 file changed, 2 insertions(+), 50 deletions(-

I'll spin a v2 with those changes once everyone on cc has had the opportunity
to chime in.