Message-ID: <ce0ae0fd-3a9e-4cc2-8a60-f4ff434f3872@os.amperecomputing.com>
Date: Fri, 30 May 2025 10:18:33 -0700
From: Yang Shi <yang@...amperecomputing.com>
To: Ryan Roberts <ryan.roberts@....com>, will@...nel.org,
 catalin.marinas@....com, Miko.Lenczewski@....com,
 scott@...amperecomputing.com, cl@...two.org
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Dev Jain <dev.jain@....com>
Subject: Re: [v3 PATCH 0/6] arm64: support FEAT_BBM level 2 and large block
 mapping when rodata=full



On 5/30/25 12:59 AM, Ryan Roberts wrote:
> On 29/05/2025 21:52, Yang Shi wrote:
>>>>>>>> The split_mapping() guarantees keep block mapping if it is fully
>>>>>>>> contained in
>>>>>>>> the range between start and end, this is my series's responsibility. I know
>>>>>>>> the
>>>>>>>> current code calls apply_to_page_range() to apply permission change and it
>>>>>>>> just
>>>>>>>> does it on PTE basis. So IIUC Dev's series will modify it or provide a new
>>>>>>>> API,
>>>>>>>> then __change_memory_common() will call it to change permission. There
>>>>>>>> should be
>>>>>>>> some overlap between mine and Dev's, but I don't see strong dependency.
>>>>>>> But if you have a block mapping in the region you are calling
>>>>>>> __change_memory_common() on, today that will fail because it can only handle
>>>>>>> page mappings.
>>>>>> IMHO letting __change_memory_common() manipulate on contiguous address
>>>>>> range is
>>>>>> another story and should be not a part of the split primitive.
>>>>> I 100% agree that it should not be part of the split primitive.
>>>>>
>>>>> But your series *depends* upon __change_memory_common() being able to change
>>>>> permissions on block mappings. Today it can only change permissions on page
>>>>> mappings.
>>>> I don't think split primitive depends on it. Changing permission on block
>>>> mappings is just the user of the new split primitive IMHO. We just have no real
>>>> user right now.
>>> But your series introduces a real user; after your series, the linear map is
>>> block mapped.
>> The users of the split primitive are the permission changers, for example,
>> module, bpf, secret mem, etc.
> Ahh, perhaps this is the crux of our misunderstanding... In my model, the split
> primitive is called from __change_memory_common() (or from other appropriate
> functions in pageattr.c). It's an implementation detail for arm64 and is not
> exposed to common code. arm64 knows that it can split live mappings in a
> transparent way so it uses huge pages eagerly and splits on demand.
>
> I personally wouldn't want to be relying on the memory user knowing it needs to
> split the mappings...

We are actually on the same page...

For example, when loading module, kernel currently does:

vmalloc() // Allocate memory for module
module_enable_text_rox() // change permission to ROX for text section
     set_memory_x
         change_memory_common
             for every page in the vmalloc area
                 __change_memory_common(addr, PAGE_SIZE, ...) // page basis
                     split_mapping(addr, addr + PAGE_SIZE)
                     apply_to_page_range() // apply the new permission

__change_memory_common() has to be called on page basis because we don't 
know whether the pages for the vmalloc area are contiguous or not. So 
the split primitive is called on page basis.


So we need do the below in order to keep large mapping:
check whether the vmalloc area is huge mapped (PMD/CONT PMD/CONT PTE) or not
if (it is huge mapped)
     __change_memory_common(addr, HUGE_SIZE, ...)
         split_mapping(addr, addr + HUGE_SIZE)
         change permission on (addr, addr + HUGE_SIZE)
else
     fallback to page basis


To have huge mapping for vmalloc, we need use vmalloc_huge() or the new 
implementation proposed by you to allocate memory for module in the 
first place. This is the "user" in my understanding.

Thanks,
Yang

Powered by blists - more mailing lists