Re: [PATCH] powerpc/dt_cpu_ftrs: Set CPU_FTR_P11_PVR for Power11 and later processors

[Amit Machhiwal <amachhiw@linux.ibm.com>]

Hi Christophe,

<snip>

> > > diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> > > index 3af6c06af02f..e5853daa6a48 100644
> > > --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> > > +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> > > @@ -704,6 +704,15 @@ static void __init cpufeatures_setup_start(u32 isa)
> > >   	if (isa >= ISA_V3_1) {
> > >   		cur_cpu_spec->cpu_features |= CPU_FTR_ARCH_31;
> > >   		cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_ARCH_3_1;
> > > +
> > > +		/*
> > > +		 * CPU_FTR_P11_PVR is a kernel-internal flag to identify
> > > +		 * Power11 and later processors. While ISA v3.1 is supported
> > > +		 * by Power10+, this flag specifically indicates Power11+
> > > +		 * for code that needs to distinguish between P10 and P11.
> > > +		 */
> > > +		if (PVR_VER(mfspr(SPRN_PVR)) >= PVR_POWER11)
> > 
> > Are we sure this test will always be correct ?
> > 
> > For instance PVR_PA6T is higher than PVR_POWER11 allthough it is not ISA 3.1
> > 
> > Wouldn't is be cleaner and safer to just do:
> > 
> > 	PVR_VER(mfspr(SPRN_PVR)) == PVR_POWER11
> 
> You're absolutely right to point out the PVR ordering concern. But PA6T
> cannot actually reach this path because we're already gated by:
> 
>   if (isa >= ISA_V3_1)
> 
> and PA6T does not implement ISA v3.1.
> 
> My rationale for using `>= PVR_POWER11` is that `CPU_FTR_P11_PVR` is
> intended to be included for Power11 and later processors, not just
> Power11 itself, as it identifies a CPU feature. Using `== PVR_POWER11`
> would mean we'd need to revisit this code for every future generation.
> 
> This approach is consistent with existing kernel code. For example, in
> arch/powerpc/perf/hv-gpci.c:
> 
>   /* sysinfo interface files are only available for power10 and above platforms */
>   if (PVR_VER(mfspr(SPRN_PVR)) >= PVR_POWER10)
>       add_sysinfo_interface_files();
> 
> Also, I couldn't find any current users of `PVR_PA6T` or `PVR_BE` in the
> kernel tree, so there doesn't appear to be a present-day ISA v3.1+
> example where the comparison would misidentify a processor.
> 
> Please let me know your further thoughts on this.

Just checking in — did my previous response address your concern, or do
you have further comments?

Thanks,
Amit