Re: [PATCH V2 0/8] arm64/mm: Drop PXD_TABLE_BIT

[Ryan Roberts]

On 21/02/2025 04:42, Anshuman Khandual wrote:
> Remove the PXX_TABLE_BIT definitions and instead rely on PXX_TYPE_MASK,
> PXX_TYPE_SECT and PXX_TYPE_TABLE. The latter versions are more abstract
> and also include the PTE_VALID bit.
> 
> This abstraction is valuable for the impending D128 page table support,
> which doesn't have a single page table bit to determine table vs block.
> Instead it has the skip level (SKL) field, where it will consider 0 to
> mean table and any other value to mean a block entry. So PXX_TABLE_BIT
> therefore doesn't fit into the D128 model well, but the type fields do.

All the patches look logically correct to me and I agree with the intention of
removing PXX_TABLE_BIT. But personally I'd prefer to see a single patch that
just does everything that's required to remove PXX_TABLE_BIT. And then a second
patch for the pud_bad() fix/improvement (currently patch 6) which is orthogonal
to the removal of PXX_TABLE_BIT.

That would make it much easier to review IMHO, and would also allow for writing
a single commit log which provides the justification for the change. I find the
current set of 7 commit logs to not be hugely helpful.

But I wrote the original patches and wrote them as I'm suggesting, so I would
say that :)

I'm guessing I shouldn't provide a Reviewed-By here, given I wrote the code
originally...

Thanks,
Ryan


> 
> This series applies on v6.14-rc3.
> 
> Changes in V2:
> 
> - Changed pmd_mkhuge() and pud_mkhuge() implementation
> - Changed pud_bad() implementation with an additional patch
> 
> Changes in V1:
> 
> https://lore.kernel.org/all/20241005123824.1366397-1-anshuman.khandual@xxxxxxx/
> 
> Cc: Marc Zyngier <maz@xxxxxxxxxx>
> Cc: Oliver Upton <oliver.upton@xxxxxxxxx>
> Cc: James Morse <james.morse@xxxxxxx>
> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
> Cc: Ryan Roberts <ryan.roberts@xxxxxxx>
> Cc: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: kvmarm@xxxxxxxxxxxxxxx
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> 
> Anshuman Khandual (6):
>   KVM: arm64: ptdump: Test PMD_TYPE_MASK for block mapping
>   arm64/ptdump: Test PMD_TYPE_MASK for block mapping
>   arm64/mm: Clear PXX_TYPE_MASK in mk_[pmd|pud]_sect_prot()
>   arm64/mm: Clear PXX_TYPE_MASK and set PXD_TYPE_SECT in [pmd|pud]_mkhuge()
>   arm64/mm: Check PXD_TYPE_TABLE in [p4d|pgd]_bad()
>   arm64/mm: Drop PXD_TABLE_BIT
> 
> Ryan Roberts (2):
>   arm64/mm: Check PUD_TYPE_TABLE in pud_bad()
>   arm64/mm: Check pmd_table() in pmd_trans_huge()
> 
>  arch/arm64/include/asm/pgtable-hwdef.h |  5 --
>  arch/arm64/include/asm/pgtable.h       | 65 ++++++++++++++++++--------
>  arch/arm64/kvm/ptdump.c                |  4 +-
>  arch/arm64/mm/ptdump.c                 |  4 +-
>  4 files changed, 50 insertions(+), 28 deletions(-)
>