Re: [PATCH v4 1/4] arm64/mm: generalize PMD_PRESENT_INVALID for all levels

[Catalin Marinas]

On Tue, May 07, 2024 at 01:34:36PM +0100, Ryan Roberts wrote:
> On 07/05/2024 12:38, David Hildenbrand wrote:
> > On 03.05.24 16:45, Ryan Roberts wrote:
> >> --- a/arch/arm64/include/asm/pgtable-prot.h
> >> +++ b/arch/arm64/include/asm/pgtable-prot.h
> >> @@ -21,11 +21,11 @@
> >>   #define PTE_PROT_NONE        (_AT(pteval_t, 1) << 58) /* only when !PTE_VALID */
> >>     /*
> >> - * This bit indicates that the entry is present i.e. pmd_page()
> >> - * still points to a valid huge page in memory even if the pmd
> >> - * has been invalidated.
> >> + * PTE_PRESENT_INVALID=1 & PTE_VALID=0 indicates that the pte's fields should be
> >> + * interpreted according to the HW layout by SW but any attempted HW access to
> >> + * the address will result in a fault. pte_present() returns true.
> >>    */
> >> -#define PMD_PRESENT_INVALID    (_AT(pteval_t, 1) << 59) /* only when !PMD_SECT_VALID */
> >> +#define PTE_PRESENT_INVALID    (_AT(pteval_t, 1) << 59) /* only when !PTE_VALID */
> > 
> > Ah, so PTE_VALID == PMD_SECT_VALID. Would that also be a reasonable
> > generalization independent of this? (or do we keep it as is because it's a HW def?)
> 
> To be honest, I'm not sure of the history, but some things are implemented as
> wrappers around pte functions and others are implemented specifically for
> pmd/pud/etc.

There's also a bit of historical arm32 code moved over to arm64 when we
did the port. On classic arm32 page tables, the ptes and pmds were
pretty different.

> On arm64, block mappings (all levels except last level) have the same HW format
> as page mappings (last level) except that bit 1 must be 0 for block and 1 for
> page. And with this series, SW/non-present bits are all matching too. So my vote
> would be to harmonise toward a single implementation in future (modulus the bit
> 1 problem), which would include getting rid of things like PMD_SECT_VALID.

For PMD_SECT_VALID vs PTE_VALID, it's fine to only use the latter. The
PMD_TABLE_BIT, however, only makes sense for p*d levels. I think we can
get rid of all the PMD_SECT_* macros, just keeping PMD_TYPE_* and the
table bit.

For a bit of architecture history, the reason the pmd block entries have
bit 1 clear while the ptes have it set is to allow recursive mappings
where an entry in the pgd points to the pgd itself. The hardware page
table walk would end on the pmd entry when accessed at the specific VA,
giving quick access to the pte. The downside is wasting a bit of the VA
space.

-- 
Catalin