xen-ppc-devel
Re: [XenPPC] [Patch] Detect non hypervisor hardware
Hollis Blanchard wrote:
On Tue, 2006-04-11 at 16:53 -0400, Maria Butrico wrote:
Signed-off-by: Maria Butrico <butrico@xxxxxxxxxxxxxx> and Mark Mergen
<mergen@xxxxxxxxxx>
summary: Detect non hypervisor hardware
Added a flag, opt_nohv. Ths flag is set if
1) the operator specifies the nohv boot argument; or
2) the hardware on which we are running has the open firmware
property '/compatible' such that it contains the string
"Power Macintosh". Both the rack mounted G5 and the tower
G5 do have such string in this property.
Added a domain flag option, _DOMF_prob, that indicates that the
domain is running in problem state. Code that uses this flag is
not part of this patch.
Hi Maria, I don't think it makes sense to add these flags before there
is code that uses them.
A few additional comments below:
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.
diff -r 47697f5b6e13 xen/arch/ppc/boot_of.c
--- a/xen/arch/ppc/boot_of.c Tue Apr 4 18:21:00 2006 -0400
+++ b/xen/arch/ppc/boot_of.c Tue Apr 11 12:13:49 2006 -0400
@@ -24,6 +24,7 @@
#include <xen/compile.h>
#include <public/of-devtree.h>
#include <asm/page.h>
+#include <xen/string.h>
static ulong of_vec;
static ulong of_msr;
@@ -33,6 +34,7 @@ static char dom0args[256];
static char dom0args[256];
extern unsigned int timebase_freq;
+extern int opt_nohv;
#undef OF_DEBUG
@@ -825,6 +827,56 @@ int __init boot_of_rtas(void)
return 1;
}
+/*
+ * return 0 if all ok
+ * OF_FAILURE if error
+ */
+int __init boot_of_nohv(void)
+{
+ char xservers_string[] = "Power Macintosh";
"xservers" doesn't seem appropriate.
What would you use? I am open to reasonable suggestions.
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.
+ int of_root;
+ char compatible_string[256];
+ int compatible_length;
+
+ of_root = of_finddevice("/");
+ if (OF_FAILURE == of_root)
+ return OF_FAILURE;
+
+ compatible_length = of_getprop(of_root, "compatible", compatible_string,
+ sizeof (compatible_string));
+ if (compatible_length != OF_FAILURE) {
+ // null separated string yuck
+ char *cmpstr;
+ int used_length;
+#if 0
+ of_printf("%s :\n", __func__);
+ for (used_length = 0; used_length < compatible_length;
+ used_length++) {
+ if (compatible_string[used_length] == '\0' ) {
+ of_printf(" NULL\n");
+ } else {
+ of_printf("%c", compatible_string[used_length]);
+ }
+ }
+#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.
+ // loop over each null terminated piece of the compatible property
+ for (cmpstr = compatible_string, used_length = 0;
+ used_length < compatible_length;
+ cmpstr = &compatible_string[used_length]) {
+ if (strstr(cmpstr, xservers_string)) {
+ of_printf("Non hypervisor hardware found. Call Mark
Mergen!\n");
+ opt_nohv = 1;
+ return 0;
+ }
+ used_length += strlen(cmpstr) + 1;
+ }
+ } else {
+ return OF_FAILURE;
+ }
+
+ return 0;
+}
+
multiboot_info_t __init *boot_of_init(
ulong r3, ulong r4, ulong vec, ulong r6, ulong r7, ulong orig_msr)
{
@@ -861,6 +913,8 @@ multiboot_info_t __init *boot_of_init(
boot_of_serial();
boot_of_cpus();
boot_of_rtas();
+
+ boot_of_nohv();
/* end of OF */
of_printf("closing OF stdout...\n");
diff -r 47697f5b6e13 xen/arch/ppc/setup.c
--- a/xen/arch/ppc/setup.c Tue Apr 4 18:21:00 2006 -0400
+++ b/xen/arch/ppc/setup.c Tue Apr 11 12:13:49 2006 -0400
@@ -43,6 +43,12 @@ int opt_noht = 0;
int opt_noht = 0;
boolean_param("noht", opt_noht);
+/* opt_nohv: If true, hardware has no hypervisor (HV) mode or nohv boot
+ * parameter was specified.
+ */
+int opt_nohv = 0;
+boolean_param("nohv", opt_nohv);
+
int opt_earlygdb = 0;
boolean_param("earlygdb", opt_earlygdb);
@@ -282,6 +288,8 @@ static void __init __start_xen(multiboot
if (dom0 == NULL)
panic("Error creating domain 0\n");
set_bit(_DOMF_privileged, &dom0->domain_flags);
+ if (opt_nohv)
+ set_bit(_DOMF_prob, &dom0->domain_flags);
/* Grab the DOM0 command line. Skip past the image name. */
cmdline = (char *)(mod[0].string ? __va((ulong)mod[0].string) : NULL);
diff -r 47697f5b6e13 xen/include/xen/sched.h
--- a/xen/include/xen/sched.h Tue Apr 4 18:21:00 2006 -0400
+++ b/xen/include/xen/sched.h Tue Apr 11 12:13:49 2006 -0400
@@ -388,6 +388,10 @@ extern struct domain *domain_list;
/* Domain is being debugged by controller software. */
#define _DOMF_debugging 4
#define DOMF_debugging (1UL<<_DOMF_debugging)
+ /* run domain in prob mode */
+#define _DOMF_prob 7
+#define DOMF_prob (1UL<<_DOMF_prob)
"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.
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.
_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
|
|
|