On 7/17/20 11:14 AM, Jordan Niethe wrote:
On Fri, Jul 17, 2020 at 2:10 PM Ravi Bangoria
<ravi.bango...@linux.ibm.com> wrote:

Add new device-tree feature for 2nd DAWR. If this feature is present,
2nd DAWR is supported, otherwise not.

Signed-off-by: Ravi Bangoria <ravi.bango...@linux.ibm.com>
---
  arch/powerpc/include/asm/cputable.h | 7 +++++--
  arch/powerpc/kernel/dt_cpu_ftrs.c   | 7 +++++++
  2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/cputable.h 
b/arch/powerpc/include/asm/cputable.h
index e506d429b1af..3445c86e1f6f 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -214,6 +214,7 @@ static inline void cpu_feature_keys_init(void) { }
  #define CPU_FTR_P9_TLBIE_ERAT_BUG      LONG_ASM_CONST(0x0001000000000000)
  #define CPU_FTR_P9_RADIX_PREFETCH_BUG  LONG_ASM_CONST(0x0002000000000000)
  #define CPU_FTR_ARCH_31                        
LONG_ASM_CONST(0x0004000000000000)
+#define CPU_FTR_DAWR1                  LONG_ASM_CONST(0x0008000000000000)

  #ifndef __ASSEMBLY__

@@ -497,14 +498,16 @@ static inline void cpu_feature_keys_init(void) { }
  #define CPU_FTRS_POSSIBLE      \
             (CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | CPU_FTRS_POWER8 | \
              CPU_FTR_ALTIVEC_COMP | CPU_FTR_VSX_COMP | CPU_FTRS_POWER9 | \
-            CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+            CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | 
\
+            CPU_FTR_DAWR1)
  #else
  #define CPU_FTRS_POSSIBLE      \
             (CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
              CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
              CPU_FTRS_POWER8 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
              CPU_FTR_VSX_COMP | CPU_FTR_ALTIVEC_COMP | CPU_FTRS_POWER9 | \
-            CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+            CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 | 
\
+            CPU_FTR_DAWR1)
Instead of putting CPU_FTR_DAWR1 into CPU_FTRS_POSSIBLE should it go
into CPU_FTRS_POWER10?
Then it will be picked up by CPU_FTRS_POSSIBLE.

I remember a discussion about this with Mikey and we decided to do it
this way. Obviously, the purpose is to make CPU_FTR_DAWR1 independent of
CPU_FTRS_POWER10 because DAWR1 is an optional feature in p10. I fear
including CPU_FTR_DAWR1 in CPU_FTRS_POWER10 can make it forcefully enabled
even when device-tree property is not present or pa-feature bit it not set,
because we do:

      {       /* 3.1-compliant processor, i.e. Power10 "architected" mode */
              .pvr_mask               = 0xffffffff,
              .pvr_value              = 0x0f000006,
              .cpu_name               = "POWER10 (architected)",
              .cpu_features           = CPU_FTRS_POWER10,

  #endif /* CONFIG_CPU_LITTLE_ENDIAN */
  #endif
  #else
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c 
b/arch/powerpc/kernel/dt_cpu_ftrs.c
index ac650c233cd9..c78cd3596ec4 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -574,6 +574,12 @@ static int __init feat_enable_mma(struct dt_cpu_feature *f)
         return 1;
  }

+static int __init feat_enable_debug_facilities_v31(struct dt_cpu_feature *f)
+{
+       cur_cpu_spec->cpu_features |= CPU_FTR_DAWR1;
+       return 1;
+}
+
  struct dt_cpu_feature_match {
         const char *name;
         int (*enable)(struct dt_cpu_feature *f);
@@ -649,6 +655,7 @@ static struct dt_cpu_feature_match __initdata
         {"wait-v3", feat_enable, 0},
         {"prefix-instructions", feat_enable, 0},
         {"matrix-multiply-assist", feat_enable_mma, 0},
+       {"debug-facilities-v31", feat_enable_debug_facilities_v31, 0},
Since all feat_enable_debug_facilities_v31() does is set
CPU_FTR_DAWR1, if you just have:
{"debug-facilities-v31", feat_enable, CPU_FTR_DAWR1},
I think cpufeatures_process_feature() should set it in for you at this point:
             if (m->enable(f)) {
                 cur_cpu_spec->cpu_features |= m->cpu_ftr_bit_mask;
                 break;
             }

Yes, that seems a better option.

Thanks,
Ravi

Reply via email to