Il 06/03/2014 15:39, Andreas Färber ha scritto: > Am 06.03.2014 15:33, schrieb Paolo Bonzini: >> Il 06/03/2014 14:37, Mark Wu ha scritto: >>> Thanks for your reply! I need confirm I am understanding your comments >>> correctly. I think you're suggesting to traverse the pci devices and >>> check if it owns the I/O port 0x3d4 to detect if the vga device >>> is initialized. But it seems not be able to resolve the bug. Because >>> the machine initialization code runs before the generic device >>> initialization, the I/O port 0x3d4 will not be registered at the time >>> machine initializes. So it can't change the return value of >>> pci_vga_init. The return value is checked in ppc code, which causes the >>> bug. >> >> Right. What about looking for any PCI device with VGA class? > > Since VGA doesn't need to be PCI (e.g., ISA, SysBus) maybe it would be a > good idea to add a QOM interface to those devices?
Actually, I now understand the problem better. All of this happens before the VGA device is even created, so it cannot be done with QOM. It's basically a command-line parsing problem. There are four cases: 1) "-vga none" on the command line 2) "-device VGA" on the command line 3) "-nodefaults" on the command line 4) "-nodefaults -device VGA" on the command line sPAPR wants to enable graphics in the second and fourth case. However, this is not exactly what the patch does, as I read it, because it actually enables graphics in the third and fourth cases, and not in the second case. Both the third and the fourth case have !has_defaults, and with has_defaults == 0 you always have default_vga == 0. The last three cases have "default_vga == 0" in vl.c. The problem is that -nodefaults sets default_vga to 0 too early. Perhaps something like this would work: diff --git a/vl.c b/vl.c index 685a7a9..d9afff4 100644 --- a/vl.c +++ b/vl.c @@ -213,6 +213,7 @@ ---> add a new variable enum xen_mode xen_mode = XEN_EMULATE; static int tcg_tb_size; +static int no_defaults = 0; static int default_serial = 1; static int default_parallel = 1; static int default_virtcon = 1; @@ -2041,7 +2042,7 @@ ---> select_vgahw is never called with VGA_DEVICE { const char *opts; - vga_interface_type = VGA_NONE; + assert(vga_interface_type == VGA_NONE); if (strstart(p, "std", &opts)) { if (vga_available()) { vga_interface_type = VGA_STD; @@ -2825,7 +2826,7 @@ ---> need to detect -vga const char *loadvm = NULL; QEMUMachine *machine; const char *cpu_model; - const char *vga_model = "none"; + const char *vga_model = NULL; const char *qtest_chrdev = NULL; const char *qtest_log = NULL; const char *pid_file = NULL; @@ -3682,16 +3683,7 @@ ---> just set a flag here runstate_set(RUN_STATE_INMIGRATE); break; case QEMU_OPTION_nodefaults: - default_serial = 0; - default_parallel = 0; - default_virtcon = 0; - default_sclp = 0; - default_monitor = 0; - default_net = 0; - default_floppy = 0; - default_cdrom = 0; - default_sdcard = 0; - default_vga = 0; + no_defaults = 1; break; case QEMU_OPTION_xen_domid: if (!(xen_available())) { @@ -3918,27 +3910,35 @@ ---> detect "-device VGA" here, also disable defaults for -nodefaults qemu_opts_foreach(qemu_find_opts("device"), default_driver_check, NULL, 0); qemu_opts_foreach(qemu_find_opts("global"), default_driver_check, NULL, 0); - if (machine->no_serial) { + if (!vga_model && !default_vga) { + vga_interface_type = VGA_DEVICE; + } + if (no_defaults || machine->no_serial) { default_serial = 0; } - if (machine->no_parallel) { + if (no_defaults || machine->no_parallel) { default_parallel = 0; } - if (!machine->use_virtcon) { + if (no_defaults || !machine->use_virtcon) { default_virtcon = 0; } - if (!machine->use_sclp) { + if (no_defaults || !machine->use_sclp) { default_sclp = 0; } - if (machine->no_floppy) { + if (no_defaults || machine->no_floppy) { default_floppy = 0; } - if (machine->no_cdrom) { + if (no_defaults || machine->no_cdrom) { default_cdrom = 0; } - if (machine->no_sdcard) { + if (no_defaults || machine->no_sdcard) { default_sdcard = 0; } + if (no_defaults) { + default_monitor = 0; + default_net = 0; + default_vga = 0; + } if (is_daemonized()) { /* According to documentation and historically, -nographic redirects @@ -4243,7 +4243,9 @@ ---> only call select_vgahw for "-vga" (or the default VGA, see the if ending here) vga_model = "std"; } } - select_vgahw(vga_model); + if (vga_model) { + select_vgahw(vga_model); + } if (watchdog) { i = select_watchdog(watchdog);