From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from russell.cc (russell.cc [IPv6:2404:9400:2:0:216:3eff:fee0:3370]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3vPsDy1Tz0zDq5x for ; Fri, 17 Feb 2017 23:06:10 +1100 (AEDT) Message-ID: <1487333152.11368.1.camel@russell.cc> Subject: Re: [PATCH v2] powerpc: Add POWER9 architected mode to cputable From: Russell Currey To: Michael Ellerman , linuxppc-dev@lists.ozlabs.org Cc: mikey@neuling.org Date: Fri, 17 Feb 2017 23:05:52 +1100 In-Reply-To: <87y3x5i04k.fsf@concordia.ellerman.id.au> References: <20170217020135.6361-1-ruscur@russell.cc> <87y3x5i04k.fsf@concordia.ellerman.id.au> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, 2017-02-17 at 21:26 +1100, Michael Ellerman wrote: > Russell Currey writes: > > > diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c > > index 6a82ef039c50..d23a54b09436 100644 > > --- a/arch/powerpc/kernel/cputable.c > > +++ b/arch/powerpc/kernel/cputable.c > > @@ -386,6 +386,25 @@ static struct cpu_spec __initdata cpu_specs[] = { > >   .machine_check_early = > > __machine_check_early_realmode_p8, > >   .platform = "power8", > >   }, > > + { /* 3.00-compliant processor, i.e. Power9 "architected" > > mode */ > > + .pvr_mask = 0xffffffff, > > + .pvr_value = 0x0f000005, > > + .cpu_name = "POWER9 (architected)", > > + .cpu_features = CPU_FTRS_POWER9, > > + .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, > > It's important *not* to set num_pmcs for the architected PVRs. > > See the comment in setup_cpu_spec(): > > /* > * If we are overriding a previous value derived from the real > * PVR with a new value obtained using a logical PVR value, > * don't modify the performance monitor fields. > */ > if (old.num_pmcs && !s->num_pmcs) { > t->num_pmcs = old.num_pmcs; > t->pmc_type = old.pmc_type; > > I realise that having that requirement in the code is serious foot gun > material on our part, but c'est la vie. > > The reason we do that is there's no "compat mode" for the PMU. So if you > boot on a Power9, and then the logical PVR says "actually pretend you're > on a Power8", we flip most of the cpu_spec to have the Power8 values, > but *not* the PMU fields. That way the Power9 PMU code will still detect > that it's on a Power9 and work correctly. > > Possibly now that oprofile is more or less dead we can rip all that crap > out, and have perf just look at the PVR directly. Thanks a lot for explaining, that's interesting. I thought it might just have been an accidental omission in the architected entries but I should've dug deeper. > > cheers