WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ppc-devel

Re: [XenPPC] [Patch] Detect non hypervisor hardware

Subject: Re: [XenPPC] [Patch] Detect non hypervisor hardware
From: Maria Butrico <butrico@xxxxxxxxxxxxx>
Date: Fri, 14 Apr 2006 11:12:22 -0400
Cc: xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 14 Apr 2006 08:13:01 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <1145026480.6003.9.camel@xxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-ppc-devel-request@lists.xensource.com?subject=help>
List-id: Xen PPC development <xen-ppc-devel.lists.xensource.com>
List-post: <mailto:xen-ppc-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ppc-devel>, <mailto:xen-ppc-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ppc-devel>, <mailto:xen-ppc-devel-request@lists.xensource.com?subject=unsubscribe>
References: <E1FTPri-00024F-Jn@xxxxxxxxxxxxxxxxxxxxx> <1145026480.6003.9.camel@xxxxxxxxxxxxxxxxxxxxx>
Reply-to: butrico@xxxxxxxxxxxxxx
Sender: xen-ppc-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 1.5 (Windows/20051201)
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