Re: [PATCH v4 3/5] arm64: Add support for FEAT_HAFT

[Catalin Marinas <catalin.marinas@arm.com>]

On Tue, Nov 05, 2024 at 08:35:51AM +0000, Marc Zyngier wrote:
> On Mon, 04 Nov 2024 17:28:48 +0000,
> Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Sat, Nov 02, 2024 at 06:42:33PM +0800, Yicong Yang wrote:
> > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> > > index ccbae4525891..0bc88df7cb35 100644
> > > --- a/arch/arm64/mm/proc.S
> > > +++ b/arch/arm64/mm/proc.S
> > > @@ -498,6 +498,10 @@ alternative_else_nop_endif
> > >  	and	x9, x9, ID_AA64MMFR1_EL1_HAFDBS_MASK
> > >  	cbz	x9, 1f
> > >  	orr	tcr, tcr, #TCR_HA		// hardware Access flag update
> > > +
> > > +#ifdef CONFIG_ARM64_HAFT
> > > +	orr	tcr2, tcr2, TCR2_EL1x_HAFT
> > > +#endif /* CONFIG_ARM64_HAFT */
> > >  1:
> > >  #endif	/* CONFIG_ARM64_HW_AFDBM */
> > >  	msr	mair_el1, mair
> > 
> > If you still want #ifdefs, I'd have left it outside the HW_AFDBM. We
> > already have a dependency in the Kconfig. Anyway, I can fix this up.
> > 
> > I think as an additional patch we can also remove the ID checks for the
> > tcr bit in tge HW_AFDBM case. But that's unrelated to this series.
> 
> I think you want to be careful with this one. I know of at least one
> implementation that has a broken FEAT_HAFDBS implementation, that
> removes it from the ID registers, but where the control bit in TCR_ELx
> still takes effect.
> 
> Please see 6df696cd9bc1 ("arm64: errata: Mitigate Ampere1 erratum
> AC03_CPU_38 at stage-2") which indicates how we actually rely on the
> check for S1 translation.

Ah, thanks for this. So the hardware with the erratum above can still
update the pte after it has been marked invalid, hence we can't turn it
on in TCR_EL1 even if the rest of the kernel considers the feature
disabled. So yes, the HAFDBS code needs to stay as is.

Let's hope the hardware people learnt and we won't have similar errata
for FEAT_HAFT.

-- 
Catalin