Nicholas Piggin <npig...@gmail.com> writes:
> DD2.3 missed out on getting its feature flag bits.
>
> This meant when booting with dt-cpu-ftrs, CPU_FTR_P9_TM_HV_ASSIST is
> missing (unless the firmware contains it, which mine does not seem to).
> And when booting without, CPU_FTR_P9_TM_XER_SO_BUG is set.
>
> In practice this doesn't make any difference to pseries guests, only
> powernv.
>
> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> ---
>  arch/powerpc/include/asm/cputable.h |  2 ++
>  arch/powerpc/kernel/cputable.c      | 22 ++++++++++++++++++++--
>  arch/powerpc/kernel/dt_cpu_ftrs.c   | 14 +++++---------
>  3 files changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputable.h 
> b/arch/powerpc/include/asm/cputable.h
> index e85c849214a2..46bae9624784 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -440,6 +440,8 @@ static inline void cpu_feature_keys_init(void) { }
>  #define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
>                              CPU_FTR_P9_TM_HV_ASSIST | \
>                              CPU_FTR_P9_TM_XER_SO_BUG)
> +#define CPU_FTRS_POWER9_DD2_3 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
> +                            CPU_FTR_P9_TM_HV_ASSIST)
>  #define CPU_FTRS_POWER10 (CPU_FTR_LWSYNC | \
>           CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
>           CPU_FTR_MMCRA | CPU_FTR_SMT | \
> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> index ae0fdef0ac11..9ab97d1fd5a2 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -487,11 +487,29 @@ static struct cpu_spec __initdata cpu_specs[] = {
>               .machine_check_early    = __machine_check_early_realmode_p9,
>               .platform               = "power9",
>       },
> -     {       /* Power9 DD2.2 or later */
> +     {       /* Power9 DD 2.2 */
> +             .pvr_mask               = 0xffffefff,
> +             .pvr_value              = 0x004e0202,
> +             .cpu_name               = "POWER9 (raw)",
> +             .cpu_features           = CPU_FTRS_POWER9_DD2_2,
> +             .cpu_user_features      = COMMON_USER_POWER9,
> +             .cpu_user_features2     = COMMON_USER2_POWER9,
> +             .mmu_features           = MMU_FTRS_POWER9,
> +             .icache_bsize           = 128,
> +             .dcache_bsize           = 128,
> +             .num_pmcs               = 6,
> +             .pmc_type               = PPC_PMC_IBM,
> +             .oprofile_cpu_type      = "ppc64/power9",
> +             .cpu_setup              = __setup_cpu_power9,
> +             .cpu_restore            = __restore_cpu_power9,
> +             .machine_check_early    = __machine_check_early_realmode_p9,
> +             .platform               = "power9",
> +     },
> +     {       /* Power9 DD 2.3 or later */
>               .pvr_mask               = 0xffff0000,
>               .pvr_value              = 0x004e0000,
>               .cpu_name               = "POWER9 (raw)",
> -             .cpu_features           = CPU_FTRS_POWER9_DD2_2,
> +             .cpu_features           = CPU_FTRS_POWER9_DD2_3,
>               .cpu_user_features      = COMMON_USER_POWER9,
>               .cpu_user_features2     = COMMON_USER2_POWER9,
>               .mmu_features           = MMU_FTRS_POWER9,
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c 
> b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 358aee7c2d79..af95f337e54b 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -764,18 +764,14 @@ static __init void cpufeatures_cpu_quirks(void)
>        * Not all quirks can be derived from the cpufeatures device tree.
>        */
>       if ((version & 0xffffefff) == 0x004e0200) {
> -             /* DD2.0 has no feature flag */
> -             cur_cpu_spec->cpu_features |= CPU_FTR_P9_RADIX_PREFETCH_BUG;
> +             cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_0;
>       } else if ((version & 0xffffefff) == 0x004e0201) {
> -             cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
> -             cur_cpu_spec->cpu_features |= CPU_FTR_P9_RADIX_PREFETCH_BUG;
> +             cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_1;
>       } else if ((version & 0xffffefff) == 0x004e0202) {
> -             cur_cpu_spec->cpu_features |= CPU_FTR_P9_TM_HV_ASSIST;
> -             cur_cpu_spec->cpu_features |= CPU_FTR_P9_TM_XER_SO_BUG;
> -             cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
> +             cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_2;
>       } else if ((version & 0xffff0000) == 0x004e0000) {
> -             /* DD2.1 and up have DD2_1 */
> -             cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
> +             /* DD2.3 and up */
> +             cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_3;
>       }

As we discussed on slack, this part is wrong.

Or'ing in the full CPU_FTRS_POWER9_DDx mask sets bits that may have been
turned off, or never enabled, by the device tree CPU features.

In particular it causes CPU_FTR_TM to be turned on (from
CPU_FTRS_POWER9), even though the machine may not have TM enabled.

We can probably just drop the dt_cpu_ftrs.c change, but if you don't
mind I'll get you to test that and resubmit the series.

cheers

Reply via email to