On Tue, Jan 31, 2006 at 01:18:55PM -0500, Dave Anderson wrote:
Rachita, Badari,
I think I agree about the unnecessary duplication of code in
x86_64_cpu_pda_init() and x86_64_get_smp_cpus().
That's right Dave. It can be avoided.
And while abstracting some of it out to CPU_PDA_READ() may be
desirable, keeping them separate may make future maintenance
and/or understandability of the differences may make it worth
keeping them separate. But I don't have a preference either way.
However, upon further review, I don't see how this piece of the
patch can work for x86_64_display_cpu_data():
so not only will it display bogus data when sent to dump_struct(),
the increment at the bottom of the loop would be wrong for the
_cpu_data array. If you just run "mach -c", the x8664_pda display
is bogus, right?
It would just need get_symbol_data() to be run on the contents of
each entry in the _cpu_data[] array, and that value should be passed
to dump_struct(); and the increment at the bottom would just be the
size of a pointer.
Do you mean something like this:
@@ -2828,7 +2885,11 @@ x86_64_display_cpu_data(void)
boot_cpu = TRUE;
cpus = 1;
}
- cpu_pda = symbol_value("cpu_pda");
+ if (symbol_exists("_cpu_pda")) {
+ __cpu_pda = 1;
+ cpu_pda = symbol_value("_cpu_pda");
+ } else if (symbol_exists("cpu_pda"))
+ cpu_pda = symbol_value("cpu_pda");
for (cpu = 0; cpu < cpus; cpu++) {
if (boot_cpu)
@@ -2838,10 +2899,18 @@ x86_64_display_cpu_data(void)
dump_struct("cpuinfo_x86", cpu_data, 0);
fprintf(fp, "\n");
- dump_struct("x8664_pda", cpu_pda, 0);
-
+
+ if ( __cpu_pda == 1 ) {
+ cpu_pda_addr = 0;
+ readmem(cpu_pda, KVADDR, &cpu_pda_addr,
+ sizeof(unsigned long), "_cpu_pda addr",
FAULT_ON_ERROR);
+ dump_struct("x8664_pda", cpu_pda_addr, 0);
+ cpu_pda += sizeof(unsigned long);
+ } else {
+ dump_struct("x8664_pda", cpu_pda, 0);
+ cpu_pda += SIZE(x8664_pda);
+ }
cpu_data += SIZE(cpuinfo_x86);
- cpu_pda += SIZE(x8664_pda);
}
Thanks
Rachita