Re: [PATCH 01/10] mm: Move common parts of pagetable_*_[cd]tor to helpers

[Kevin Brodsky <kevin.brodsky@arm.com>]

On 20/12/2024 12:46, Qi Zheng wrote:
> Hi Kevin,
>
> On 2024/12/20 18:49, Kevin Brodsky wrote:
>> [...]
>>
>> Qi, shall we collaborate to make our series complementary? I believe my
>> series covers patch 2 and 4 of your series, but it goes further by
>> covering all levels and all architectures, and patches introducing
>> ctor/dtor are already split as Alexander suggested on your series. So my
>> suggestion would be:
>>
>> * Remove patch 1 in my series - I'd just introduce
>> pagetable_{p4d,pgd}_[cd]tor with the same implementation as
>> pagetable_pud_[cd]tor.
>> * Remove patch 2 and 4 from your series and rebase it on mine.
>
> I quickly went through your patch series. It looks like my patch 2 and
> your patch 6 are duplicated, so you want me to remove my patch 2.
>
> But I think you may not be able to simple let arm64, riscv and x86 to
> use generic p4d_{alloc_one,free}(). Because even if
> CONFIG_PGTABLE_LEVELS > 4, the pgtable_l5_enabled() may not be true.
>
> For example, in arm64:
>
> #if CONFIG_PGTABLE_LEVELS > 4
>
> static __always_inline bool pgtable_l5_enabled(void)
> {
>     if (!alternative_has_cap_likely(ARM64_ALWAYS_BOOT))
>         return vabits_actual == VA_BITS;
>     return alternative_has_cap_unlikely(ARM64_HAS_VA52);
> }

Correct. That's why the implementation of p4d_free() I introduce in
patch 6 checks mm_p4d_folded(), which is implemented as
!pgtable_l5_enabled() on those architectures (see last paragraph in
commit message). In fact it turns out Alexander suggested exactly this
approach [2].

>
> Did I miss something?
>
> My patch series is not only for cleanup, but also for fixes of
> UAF issue [1], so is it possible to rebase your patch series onto
> mine? I can post v3 ASAP.

I see, yours should be merged first then. The issue is that yours would
depend on some of the patches in mine, not the other way round.

My suggestion would then be for you to take patch 5, 6 and 7 from my
series, as they match Alexander's suggestions (and patch 5 is I think a
useful simplification), and replace patch 2 in your series with those. I
would then rebase my series on top and adapt it accordingly. Does that
sound reasonable?

- Kevin

[2]
https://lore.kernel.org/all/Z2RKpdv7pL34MIEt@tuxmaker.boeblingen.de.ibm.com/