On 5/23/11 2:38 PM, Ingo Molnar wrote:
* Sasha Levin<[email protected]> wrote:@@ -511,7 +515,13 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix) kvm->nrcpus = nrcpus; memset(real_cmdline, 0, sizeof(real_cmdline)); - strcpy(real_cmdline, "notsc noapic noacpi pci=conf1 console=ttyS0 earlyprintk=serial"); + strcpy(real_cmdline, "notsc noapic noacpi pci=conf1"); + if (vnc) { + strcat(real_cmdline, " video=vesafb console=tty0"); + vidmode = 0x312; + } else { + strcat(real_cmdline, " console=ttyS0 earlyprintk=serial"); + }Hm, i think all the kernel parameter handling code wants to move into driver specific routines as well. Something like: serial_init(kvm, real_cmdline); where serial_init() would append to real_cmdline if needed. This removes a bit of serial-driver specific knowledge from kvm-run.c. Same goes for the VESA driver and the above video mode flag logic.@@ -597,6 +607,9 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix) kvm__init_ram(kvm); + if (vnc) + vesa__init(kvm);Shouldnt vesa__init() itself know about whether it's active (i.e. the 'vnc' flag is set) and return early if it's not set? That way this could become more encapsulated and self-sufficient: vesa__init(kvm); With no VESA driver specific state exposed to the generic kvm_cmd_run() function. Ideally kvm_cmd_run() hould just be a series of: serial_init(kvm, real_cmdline); vesa_init(kvm, real_cmdline); ... initialization routines. Later on even this could be removed: using section tricks we can put init functions into a section and drivers could register their init function like initcall(func) functions are registered within the kernel. kvm_cmd_run() could thus iterate over that (build time constructed) section like this: extern initcall_t __initcall_start[], __initcall_end[], __early_initcall_end[]; static void __init do_initcalls(void) { initcall_t *fn; for (fn = __early_initcall_end; fn< __initcall_end; fn++) do_one_initcall(*fn); } and would not actually have *any* knowledge about what drivers were built in. Currently it's fine to initialize everything explicitly - but this would be the long term model to work towards ...
Prasad, didn't you have patches to do exactly that? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
