Message ID | 1234557928.10378.8.camel@norville.austin.ibm.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
On Fri, 2009-02-13 at 14:45 -0600, Dave Kleikamp wrote: > When identify_cpu() is called a second time with a logical PVR, it only > copies a subset of the cpu_spec structure to avoid overwriting the > performance monitor fields that were initialized based on the real PVR. > > If the real PVR is not recognized, the structure is initialized with > default data in which the pvr_mask field is zero. Overriding the default > definition with the one based on the logical PVR does not update the > pvr_mask field. Since the pvr_mask field remains zero, show_cpuinfo() > reports the cpu as "unknown". > > identify_cpu() should update the pvr_mask, so that show_cpuinfo() reports > the logical cpu name, as it would if the kernel recognized the real PVR. > > For completeness, this patch also updates pvr_value, although its only > use appears to be in booke_wdt_init() setting ident.firmware_version. I'm > not sure which value (the real or logical) is the most useful here, if it > matters at all. > > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com> > > diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c > index 923f87a..a216fad 100644 > --- a/arch/powerpc/kernel/cputable.c > +++ b/arch/powerpc/kernel/cputable.c > @@ -1780,6 +1780,8 @@ struct cpu_spec * __init identify_cpu(unsigned long offset, unsigned int pvr) > * performance monitor fields. > */ > if (t->num_pmcs && !s->num_pmcs) { > + t->pvr_mask = s->pvr_mask; > + t->pvr_value = s->pvr_value; > t->cpu_name = s->cpu_name; > t->cpu_features = s->cpu_features; > t->cpu_user_features = s->cpu_user_features; Seems like we'd be better off copying the whole structure, and then putting back the performance monitor fields. So we don't end up with another bug if something's added to struct cpu_spec. cheers
On Sun, 2009-02-15 at 01:00 +1100, Michael Ellerman wrote: > On Fri, 2009-02-13 at 14:45 -0600, Dave Kleikamp wrote: > > When identify_cpu() is called a second time with a logical PVR, it only > > copies a subset of the cpu_spec structure to avoid overwriting the > > performance monitor fields that were initialized based on the real PVR. > > > > If the real PVR is not recognized, the structure is initialized with > > default data in which the pvr_mask field is zero. Overriding the default > > definition with the one based on the logical PVR does not update the > > pvr_mask field. Since the pvr_mask field remains zero, show_cpuinfo() > > reports the cpu as "unknown". > > > > identify_cpu() should update the pvr_mask, so that show_cpuinfo() reports > > the logical cpu name, as it would if the kernel recognized the real PVR. > > > > For completeness, this patch also updates pvr_value, although its only > > use appears to be in booke_wdt_init() setting ident.firmware_version. I'm > > not sure which value (the real or logical) is the most useful here, if it > > matters at all. > > > > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com> > > > > diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c > > index 923f87a..a216fad 100644 > > --- a/arch/powerpc/kernel/cputable.c > > +++ b/arch/powerpc/kernel/cputable.c > > @@ -1780,6 +1780,8 @@ struct cpu_spec * __init identify_cpu(unsigned long offset, unsigned int pvr) > > * performance monitor fields. > > */ > > if (t->num_pmcs && !s->num_pmcs) { > > + t->pvr_mask = s->pvr_mask; > > + t->pvr_value = s->pvr_value; > > t->cpu_name = s->cpu_name; > > t->cpu_features = s->cpu_features; > > t->cpu_user_features = s->cpu_user_features; > > Seems like we'd be better off copying the whole structure, and then > putting back the performance monitor fields. So we don't end up with > another bug if something's added to struct cpu_spec. I feel that may be a bit of overkill, but I don't feel strongly either way. I'd like the patch to be picked up in one fashion or another. Thanks, Shaggy
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c index 923f87a..a216fad 100644 --- a/arch/powerpc/kernel/cputable.c +++ b/arch/powerpc/kernel/cputable.c @@ -1780,6 +1780,8 @@ struct cpu_spec * __init identify_cpu(unsigned long offset, unsigned int pvr) * performance monitor fields. */ if (t->num_pmcs && !s->num_pmcs) { + t->pvr_mask = s->pvr_mask; + t->pvr_value = s->pvr_value; t->cpu_name = s->cpu_name; t->cpu_features = s->cpu_features; t->cpu_user_features = s->cpu_user_features;
When identify_cpu() is called a second time with a logical PVR, it only copies a subset of the cpu_spec structure to avoid overwriting the performance monitor fields that were initialized based on the real PVR. If the real PVR is not recognized, the structure is initialized with default data in which the pvr_mask field is zero. Overriding the default definition with the one based on the logical PVR does not update the pvr_mask field. Since the pvr_mask field remains zero, show_cpuinfo() reports the cpu as "unknown". identify_cpu() should update the pvr_mask, so that show_cpuinfo() reports the logical cpu name, as it would if the kernel recognized the real PVR. For completeness, this patch also updates pvr_value, although its only use appears to be in booke_wdt_init() setting ident.firmware_version. I'm not sure which value (the real or logical) is the most useful here, if it matters at all. Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>