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:
> 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.
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.
> + // 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.
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.
--
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
|
Home