Message ID | 1460182444-2468-69-git-send-email-aneesh.kumar@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
On Sat, 2016-09-04 at 06:14:04 UTC, "Aneesh Kumar K.V" wrote: > We can depend on ibm,pa-features to enable/disable radix. This gives us > a nice way to test p9 hash config, by changing device tree property. I think we might want to be more careful here. You set MMU_FTR_RADIX in the cputable entry. So it's on by default on P9 cpus. Then if there is an ibm,pa-features property *and* it is >= 41 bytes long, the below feature entry will hit. In that case the firmware controls whether it's on or off. I think it would be clearer if we removed RADIX from the cputable, and the below became the only way to turn it on. Would that break anything? > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 7030b035905d..a4d1f44364b8 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -165,6 +165,7 @@ static struct ibm_pa_feature { > * which is 0 if the kernel doesn't support TM. > */ > {CPU_FTR_TM_COMP, 0, 0, 22, 0, 0}, > + {0, MMU_FTR_RADIX, 0, 40, 0, 0}, So that says bit 0 of byte 40 enables MMU_FTR_RADIX. Where is that documented? cheers
Michael Ellerman <mpe@ellerman.id.au> writes: > On Sat, 2016-09-04 at 06:14:04 UTC, "Aneesh Kumar K.V" wrote: >> We can depend on ibm,pa-features to enable/disable radix. This gives us >> a nice way to test p9 hash config, by changing device tree property. > > I think we might want to be more careful here. > > You set MMU_FTR_RADIX in the cputable entry. So it's on by default on P9 cpus. > > Then if there is an ibm,pa-features property *and* it is >= 41 bytes long, the > below feature entry will hit. In that case the firmware controls whether it's on > or off. > > I think it would be clearer if we removed RADIX from the cputable, and the below > became the only way to turn it on. Would that break anything? I was thinking we want to use RADIX by default on Power9. Hence the idea of clearing feature bit. Now we will be using this to switch between Radix and hash segment table. We are not really going to use this to switch between the long tested SLB hash to the newly added Power9 Radix. So it is not really there to make sure we run safely with old code by default. > >> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c >> index 7030b035905d..a4d1f44364b8 100644 >> --- a/arch/powerpc/kernel/prom.c >> +++ b/arch/powerpc/kernel/prom.c >> @@ -165,6 +165,7 @@ static struct ibm_pa_feature { >> * which is 0 if the kernel doesn't support TM. >> */ >> {CPU_FTR_TM_COMP, 0, 0, 22, 0, 0}, >> + {0, MMU_FTR_RADIX, 0, 40, 0, 0}, > > So that says bit 0 of byte 40 enables MMU_FTR_RADIX. Where is that documented? > In PAPR document. -aneehs
On Wed, 2016-04-20 at 12:59 +1000, Michael Ellerman wrote: > On Sat, 2016-09-04 at 06:14:04 UTC, "Aneesh Kumar K.V" wrote: > > We can depend on ibm,pa-features to enable/disable radix. This gives us > > a nice way to test p9 hash config, by changing device tree property. > > I think we might want to be more careful here. > > You set MMU_FTR_RADIX in the cputable entry. So it's on by default on P9 cpus. > > Then if there is an ibm,pa-features property *and* it is >= 41 bytes long, the > below feature entry will hit. In that case the firmware controls whether it's on > or off. > > I think it would be clearer if we removed RADIX from the cputable, and the below > became the only way to turn it on. Would that break anything? I don't think it'll break anything. FWIW I'm good with this approach. We'll need to teach firmware about this feature, but I think we should do that anyway. skiboot is a bit of a mess WRT to this property with HDAT vs device tree, but we can clean that up there. Mikey > > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > > index 7030b035905d..a4d1f44364b8 100644 > > --- a/arch/powerpc/kernel/prom.c > > +++ b/arch/powerpc/kernel/prom.c > > @@ -165,6 +165,7 @@ static struct ibm_pa_feature { > > * which is 0 if the kernel doesn't support TM. > > */ > > {CPU_FTR_TM_COMP, 0, 0, 22, 0, 0}, > > + {0, MMU_FTR_RADIX, 0, 40, 0, 0}, > > So that says bit 0 of byte 40 enables MMU_FTR_RADIX. Where is that documented? > > cheers > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 7030b035905d..a4d1f44364b8 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -165,6 +165,7 @@ static struct ibm_pa_feature { * which is 0 if the kernel doesn't support TM. */ {CPU_FTR_TM_COMP, 0, 0, 22, 0, 0}, + {0, MMU_FTR_RADIX, 0, 40, 0, 0}, }; static void __init scan_features(unsigned long node, const unsigned char *ftrs,
We can depend on ibm,pa-features to enable/disable radix. This gives us a nice way to test p9 hash config, by changing device tree property. Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> --- arch/powerpc/kernel/prom.c | 1 + 1 file changed, 1 insertion(+)