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

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