Re: [PATCH 01/12] arm64: Add missing APTable and TCR_ELx.HPD masks

[Marc Zyngier <maz@kernel.org>]

On Fri, 12 Jul 2024 09:32:12 +0100,
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
> 
> 
> On 6/25/24 19:05, Marc Zyngier wrote:
> > Although Linux doesn't make use of hierarchical permissions (TFFT!),
> > KVM needs to know where the various bits related to this feature
> > live in the TCR_ELx registers as well as in the page tables.
> > 
> > Add the missing bits.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/include/asm/kvm_arm.h       | 1 +
> >  arch/arm64/include/asm/pgtable-hwdef.h | 7 +++++++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > index b2adc2c6c82a5..c93ee1036cb09 100644
> > --- a/arch/arm64/include/asm/kvm_arm.h
> > +++ b/arch/arm64/include/asm/kvm_arm.h
> > @@ -108,6 +108,7 @@
> >  /* TCR_EL2 Registers bits */
> >  #define TCR_EL2_DS		(1UL << 32)
> >  #define TCR_EL2_RES1		((1U << 31) | (1 << 23))
> > +#define TCR_EL2_HPD		(1 << 24)
> >  #define TCR_EL2_TBI		(1 << 20)
> >  #define TCR_EL2_PS_SHIFT	16
> >  #define TCR_EL2_PS_MASK		(7 << TCR_EL2_PS_SHIFT)
> > diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
> > index 9943ff0af4c96..f75c9a7e6bd68 100644
> > --- a/arch/arm64/include/asm/pgtable-hwdef.h
> > +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> > @@ -146,6 +146,7 @@
> >  #define PMD_SECT_UXN		(_AT(pmdval_t, 1) << 54)
> >  #define PMD_TABLE_PXN		(_AT(pmdval_t, 1) << 59)
> >  #define PMD_TABLE_UXN		(_AT(pmdval_t, 1) << 60)
> > +#define PMD_TABLE_AP		(_AT(pmdval_t, 3) << 61)
> 
> APTable bits are also present in all table descriptors at each non-L3
> level. Should not corresponding corresponding macros i.e PUD_TABLE_AP,
> P4D_TABLE_AP, and PGD_TABLE_AP be added as well ?

My problem with that is that it doesn't make much sense from an
architecture perspective. It doesn't define any of these, because
these names make no sense.

Maybe I should just drop the PMD prefix and write it as S1_TABLE_AP,
so that it can be reused if we ever need the P*D names.

> 
> >  
> >  /*
> >   * AttrIndx[2:0] encoding (mapping attributes defined in the MAIR* registers).
> > @@ -307,6 +308,12 @@
> >  #define TCR_TCMA1		(UL(1) << 58)
> >  #define TCR_DS			(UL(1) << 59)
> >  
> > +#define TCR_HPD0_SHIFT		41
> > +#define TCR_HPD0		BIT(TCR_HPD0_SHIFT)
> > +
> > +#define TCR_HPD1_SHIFT		42
> > +#define TCR_HPD1		BIT(TCR_HPD1_SHIFT)
> 
> Should not these new register fields follow the current ascending bit
> order in the listing i.e get added after TCR_HD (bit 40).

Yup, I'll move them up.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.