Re: [PATCH v6 10/18] x86/mm: Physical address comparisons in fill_p*d/pte

[Maciej Wieczór-Retman]

On 2025-11-10 at 17:24:38 +0100, Alexander Potapenko wrote:
>On Wed, Oct 29, 2025 at 9:07 PM Maciej Wieczor-Retman
><m.wieczorretman@xxxxx> wrote:
>>
>> From: Maciej Wieczor-Retman <maciej.wieczor-retman@xxxxxxxxx>
>>
>> Calculating page offset returns a pointer without a tag. When comparing
>> the calculated offset to a tagged page pointer an error is raised
>> because they are not equal.
>>
>> Change pointer comparisons to physical address comparisons as to avoid
>> issues with tagged pointers that pointer arithmetic would create. Open
>> code pte_offset_kernel(), pmd_offset(), pud_offset() and p4d_offset().
>> Because one parameter is always zero and the rest of the function
>> insides are enclosed inside __va(), removing that layer lowers the
>> complexity of final assembly.
>>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@xxxxxxxxx>
>> ---
>> Changelog v2:
>> - Open code *_offset() to avoid it's internal __va().
>>
>>  arch/x86/mm/init_64.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
>> index 0e4270e20fad..2d79fc0cf391 100644
>> --- a/arch/x86/mm/init_64.c
>> +++ b/arch/x86/mm/init_64.c
>> @@ -269,7 +269,10 @@ static p4d_t *fill_p4d(pgd_t *pgd, unsigned long vaddr)
>>         if (pgd_none(*pgd)) {
>>                 p4d_t *p4d = (p4d_t *)spp_getpage();
>>                 pgd_populate(&init_mm, pgd, p4d);
>> -               if (p4d != p4d_offset(pgd, 0))
>> +
>> +               if (__pa(p4d) != (pgtable_l5_enabled() ?
>> +                                 __pa(pgd) :
>> +                                 (unsigned long)pgd_val(*pgd) & PTE_PFN_MASK))
>
>Did you test with both 4- and 5-level paging?
>If I understand correctly, p4d and pgd are supposed to be the same
>under !pgtable_l5_enabled().

Yes, I do test on both paging modes. Looking at p4d_offset() I think I
got the cases reversed somehow. Weird that it didn't raise any issues
afterwards. Thanks for pointing it out :)