xen-ppc-devel
Re: [XenPPC] [Patch] Detect non hypervisor hardware
Jimi Xenidis wrote:
On Apr 14, 2006, at 11:12 AM, Maria Butrico wrote:
Hollis Blanchard wrote:
On Tue, 2006-04-11 at 16:53 -0400, Maria Butrico wrote:
Hi Maria, I don't think it makes sense to add these flags before there
is code that uses them.
I disagree. We are trying to stage this code in easy-to-understand
and isolated pieces, this being the first one, that is a way to
detect hardware where hyper/supervisor are not distinguished.
Hollis, I asked for this cuz I want to start seeing the bits of PMAC
go into the source an have it discussed and maybe even collaborated
on, plus we need to start mapping out capabilities.
I see now that I was a little short sighted when discussing this with
Maria.
IMHO we need a boot_machine_type() that uses the compat code to make
descisions on the platform.
It suits really to qucik and dirty purposes.
1. Do we know this platform is hypervisor capable?
2. Is this platform a powermac and therfore has a Zilog UART and its
location.
The Zilog UART is Maria's next task and I think could be quite
difficult, so I'd like to get her there ASAP. Later we can be a
little more robust in this code, but we are still debating that
of_boot.c will continue to exist as we know it.
In fact, after we find out it is a pmac we should probably:
ifdef CONFIG_PMAC
of_panic("Sorry, %s is not supported\n", pmac);
#else
of_write("WARNING, %s is experimental\n", pmac);
#endif
I'm happy to allow this code to trickle in.
more comments..
+ char xservers_string[] = "Power Macintosh";
"xservers" doesn't seem appropriate.
What would you use? I am open to reasonable suggestions.
how about 'static const char pmac[] = "Power Macintosh";'
that way it won't use any stack space.
I would prefer if there were a more targeted way to detect that
hypervisor mode has been disabled. If there is none (and AFAIK there
isn't), then I'm OK with checking the compatible property.
Not sure there is, at least at this point, but we can at least make
this distinction here.
+#if 0
+#endif
This looks like debugging code that we probably won't need to ever
enable again? In that case, I'd leave it out of the patch.
Only the ifdef'ed out part is debug code and I suspect we might need
again. (Otherwise I would not have sent it with the patch). If you
feel strongly about this I don't care, since it would be quite easy
to add it again.
Thats fine, then '#ifdef OF_DEBUG' it, or even '#ifdef DEBUG_COMPAT',
but no '#if 0's please.
+
+ boot_of_nohv();
hmm I guess here is where I'd like to see, vars and macros defined as
needed:
m = boot_machine_type();
switch (m) {
BOOT_MAPLE:
uart_type = UART_NS16550
uart_base = UART_NS16550_BASE;
HV_supported = 1;
What about when nohv is specified in the boot arguments? Should we turn
off this flag (that says that the hardware does not support the
distinction between superviror and hypervisor state) or should the value
of this HV_supported and the optional boot parameter should be wrapped
in a working flags (Mark's opt_nohv, for example)?
BTW, in my version of this all of uart type and base and the HV
supported flags are in a global struct (so far global only to boot_of).
break;
#ifdef CONFIG_PMAC
BOOT_PMAC:
uart_type = UART_ZILOG;
uart_base = UART_ZILOG_BASE;
HV_supported = 0;
of_printf("WARNING: PMAC platform is Experimental");
break;
#endif
default:
of_panic("unsupported platform");
break;
}
"Problem state" is a term that nobody outside PowerPC will understand,
so I would prefer a different name.
I leave this one for Mark to argue one way or another.
hmm, I like the idea of the arch specific code using arch specific
terminology.
Also, sched.h is shared among all architectures, and so the DOMF_*
flags
are not architecture-specific. I think it would be better to add a flag
to struct arch_domain.
Probably true, in which case we need to establish a few conventions,
with associated clear comments, that list the include files where the
definitions for the values of these bits are. Probably these are
already in place, but I don't know about them.
I agree with hollis, we should start an 32bit? bitmask of capabilities
in arch_domain{}, theres room att he end that is currently being padded.
-JX
_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
|
|
|