Re: [PATCH] arm64: Support ARM64_VA_BITS=52 when setting ARCH_MMAP_RND_BITS_MAX

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 4/17/25 13:38, Kornel Dulęba wrote:
> On Thu, Apr 17, 2025 at 7:15 AM Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>>
>>
>> On 4/4/25 22:16, Kornel Dulęba wrote:
>>> On Thu, Apr 3, 2025 at 11:46 PM Anshuman Khandual
>>> <anshuman.khandual@arm.com> wrote:
>>>>
>>>>
>>>>
>>>> On 4/4/25 00:06, Kornel Dulęba wrote:
>>>>> When the 52-bit virtual addressing was enabled the select like
>>>>> ARCH_MMAP_RND_BITS_MAX logic was never updated to account for it.
>>>>> Because of that the rnd max bits would be set to the default value of
>>>>> 18 when ARM64_VA_BITS=52.
>>>>> Fix this by setting ARCH_MMAP_RND_BITS_MAX to the same value that would
>>>>> be used if 48-bit addressing was used. That's because the 52-bit
>>>>> addressing is used only if the caller provides a hint to mmap, with a
>>>>> fallback to 48-bit addressing.
>>>>
>>>> Why should ARCH_MMAP_RND_BITS_MAX value be same for both 48 bits and 52
>>>> bits VA in case the user does request for 52 bit VA via mmap() hint and
>>>> the HW supports it ?
>>>
>>> Two reasons really.
>>> 1. The whole behavior is controlled through a global knob -
>>> /proc/sys/vm/mmap_rnd_bits. ARCH_MMAP_RND_BITS_MAX is used as an upper
>>> bound for the value that can be set to that knob.
>>> So we have a single setting for all processes. Some might want 52 bit
>>> addressing, others will stick with 48.
>>> 2. Quoting the documentation for this knob:
>>>
>>> """
>>> mmap_rnd_bits
>>> This value can be used to select the number of bits to use to
>>> determine the random offset to the base address of vma regions
>>> resulting from mmap allocations on architectures which support tuning
>>> address space randomization. This value will be bounded by the
>>> architecture’s minimum and maximum supported values.
>>> """
>>>
>>> I suppose that it's legal for some calls to mmap from the same process
>>> to request a 52 bit VA, while other calls will want only 48 bits.
>>> Because of that the random offset can't be larger than what would work
>>> for the 48 bit case.
>>
>> Agreed but should not this rationale also be added in the commit
>> message as well ?
> 
> Sure, I will update this in v2.
>>
>>
>>>
>>>>
>>>>>
>>>>> Fixes: b6d00d47e81a ("arm64: mm: Introduce 52-bit Kernel VAs")
>>
>> Correct commit to be attributed for this fix.
> 
> What commit would you like me to point at? I selected b6d00d47e81a,
> because it introduced "ARM64_VA_BITS=52".
> Looking at git blame, ARCH_MMAP_RND_BITS_MAX logic was introduced back
> in 2016 in 8f0d3aa9de57 ("arm64: mm: support ARCH_MMAP_RND_BITS"),
> which is before "ARM64_VA_BITS=52" was a thing.
> I suppose that there's also 3cb7e662a930 ("arm64: Kconfig: Fix
> indentation and add comments"), but that's just some whitespace
> adjustments.

It was just a statement confirming that the commit ID chosen here in
this patch is the right one indeed. Apologies for the confusion.

> 
>>
>>
>>>>> Signed-off-by: Kornel Dulęba <korneld@google.com>
>>>>> ---
>>>>>  arch/arm64/Kconfig | 6 +++---
>>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>>> index 748c34dc953c..38e0bac567f5 100644
>>>>> --- a/arch/arm64/Kconfig
>>>>> +++ b/arch/arm64/Kconfig
>>>>> @@ -332,9 +332,9 @@ config ARCH_MMAP_RND_BITS_MAX
>>>>>       default 24 if ARM64_VA_BITS=39
>>>>>       default 27 if ARM64_VA_BITS=42
>>>>>       default 30 if ARM64_VA_BITS=47
>>>>> -     default 29 if ARM64_VA_BITS=48 && ARM64_64K_PAGES
>>>>> -     default 31 if ARM64_VA_BITS=48 && ARM64_16K_PAGES
>>>>> -     default 33 if ARM64_VA_BITS=48
>>>>> +     default 29 if (ARM64_VA_BITS=48 || ARM64_VA_BITS=52) && ARM64_64K_PAGES
>>>>> +     default 31 if (ARM64_VA_BITS=48 || ARM64_VA_BITS=52) && ARM64_16K_PAGES
>>>>> +     default 33 if (ARM64_VA_BITS=48 || ARM64_VA_BITS=52)
>>>>>       default 14 if ARM64_64K_PAGES
>>>>>       default 16 if ARM64_16K_PAGES
>>>>>       default 18
>>
>> Otherwise LGTM.
>>
>> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>