* Sasha Levin <levinsasha928@xxxxxxxxx> 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 ... Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html