| Message ID | 1447268956-27500-10-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11/11/2015 20:09, Eduardo Habkost wrote: > All DisplayType values are just UI options that don't affect any > hardware emulation code, except for DT_NOGRAPHIC. Replace > DT_NOGRAPHIC with DT_NONE plus a new MachineState.nographic > field, so hardware emulation code don't need to use the > display_type variable. > > Cc: Michael Walle <michael@walle.cc> > Cc: Blue Swirl <blauwirbel@gmail.com> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Can you add a QOM property too, so that "-machine graphics=yes|no" can be used? Paolo > --- > hw/lm32/milkymist.c | 2 +- > hw/nvram/fw_cfg.c | 6 ++++-- > hw/sparc/sun4m.c | 2 +- > include/hw/boards.h | 1 + > include/sysemu/sysemu.h | 1 - > vl.c | 12 ++++++------ > 6 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c > index e46283a..947c7db 100644 > --- a/hw/lm32/milkymist.c > +++ b/hw/lm32/milkymist.c > @@ -163,7 +163,7 @@ milkymist_init(MachineState *machine) > milkymist_memcard_create(0x60004000); > milkymist_ac97_create(0x60005000, irq[4], irq[5], irq[6], irq[7]); > milkymist_pfpu_create(0x60006000, irq[8]); > - if (display_type != DT_NOGRAPHIC) { > + if (!machine->nographic) { > milkymist_tmu2_create(0x60007000, irq[9]); > } > milkymist_minimac2_create(0x60008000, 0x30000000, irq[10], irq[11]); > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 73b0a81..e42b198 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -24,6 +24,7 @@ > #include "hw/hw.h" > #include "sysemu/sysemu.h" > #include "sysemu/dma.h" > +#include "hw/boards.h" > #include "hw/isa/isa.h" > #include "hw/nvram/fw_cfg.h" > #include "hw/sysbus.h" > @@ -755,16 +756,17 @@ static void fw_cfg_machine_ready(struct Notifier *n, void *data) > static void fw_cfg_init1(DeviceState *dev) > { > FWCfgState *s = FW_CFG(dev); > + MachineState *machine = MACHINE(qdev_get_machine()); > > assert(!object_resolve_path(FW_CFG_PATH, NULL)); > > - object_property_add_child(qdev_get_machine(), FW_CFG_NAME, OBJECT(s), NULL); > + object_property_add_child(OBJECT(machine), FW_CFG_NAME, OBJECT(s), NULL); > > qdev_init_nofail(dev); > > fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4); > fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16); > - fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC)); > + fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)machine->nographic); > fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus); > fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu); > fw_cfg_bootsplash(s); > diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c > index 230dac9..d47f06a 100644 > --- a/hw/sparc/sun4m.c > +++ b/hw/sparc/sun4m.c > @@ -1017,7 +1017,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, > slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus); > > slavio_serial_ms_kbd_init(hwdef->ms_kb_base, slavio_irq[14], > - display_type == DT_NOGRAPHIC, ESCC_CLOCK, 1); > + machine->nographic, ESCC_CLOCK, 1); > /* Slavio TTYA (base+4, Linux ttyS0) is the first QEMU serial device > Slavio TTYB (base+0, Linux ttyS1) is the second QEMU serial device */ > escc_init(hwdef->serial_base, slavio_irq[15], slavio_irq[15], > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 3e9a92c..1353f8a 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -120,6 +120,7 @@ struct MachineState { > char *firmware; > bool iommu; > bool suppress_vmdesc; > + bool nographic; > > ram_addr_t ram_size; > ram_addr_t maxram_size; > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 0f4e520..f92a53c 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -139,7 +139,6 @@ typedef enum DisplayType > DT_SDL, > DT_COCOA, > DT_GTK, > - DT_NOGRAPHIC, > DT_NONE, > } DisplayType; > > diff --git a/vl.c b/vl.c > index 57064ea..5d0228b 100644 > --- a/vl.c > +++ b/vl.c > @@ -2980,6 +2980,7 @@ int main(int argc, char **argv, char **envp) > int show_vnc_port = 0; > bool defconfig = true; > bool userconfig = true; > + bool nographic = false; > const char *log_mask = NULL; > const char *log_file = NULL; > const char *trace_events = NULL; > @@ -3226,7 +3227,8 @@ int main(int argc, char **argv, char **envp) > display_type = select_display(optarg); > break; > case QEMU_OPTION_nographic: > - display_type = DT_NOGRAPHIC; > + nographic = true; > + display_type = DT_NONE; > break; > case QEMU_OPTION_curses: > #ifdef CONFIG_CURSES > @@ -4177,7 +4179,7 @@ int main(int argc, char **argv, char **envp) > * -nographic _and_ redirects all ports explicitly - this is valid > * usage, -nographic is just a no-op in this case. > */ > - if (display_type == DT_NOGRAPHIC > + if (nographic > && (default_parallel || default_serial > || default_monitor || default_virtcon)) { > error_report("-nographic cannot be used with -daemonize"); > @@ -4191,7 +4193,7 @@ int main(int argc, char **argv, char **envp) > #endif > } > > - if (display_type == DT_NOGRAPHIC) { > + if (nographic) { > if (default_parallel) > add_device_config(DEV_PARALLEL, "null"); > if (default_serial && default_monitor) { > @@ -4510,6 +4512,7 @@ int main(int argc, char **argv, char **envp) > current_machine->ram_slots = ram_slots; > current_machine->boot_order = boot_order; > current_machine->cpu_model = cpu_model; > + current_machine->nographic = nographic; > > machine_class->init(current_machine); > > @@ -4560,9 +4563,6 @@ int main(int argc, char **argv, char **envp) > > /* init local displays */ > switch (display_type) { > - case DT_NOGRAPHIC: > - (void)ds; /* avoid warning if no display is configured */ > - break; > case DT_CURSES: > curses_display_init(ds, full_screen); > break; >
On Thu, Nov 12, 2015 at 10:48:12AM +0100, Paolo Bonzini wrote: > > > On 11/11/2015 20:09, Eduardo Habkost wrote: > > All DisplayType values are just UI options that don't affect any > > hardware emulation code, except for DT_NOGRAPHIC. Replace > > DT_NOGRAPHIC with DT_NONE plus a new MachineState.nographic > > field, so hardware emulation code don't need to use the > > display_type variable. > > > > Cc: Michael Walle <michael@walle.cc> > > Cc: Blue Swirl <blauwirbel@gmail.com> > > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > Can you add a QOM property too, so that "-machine graphics=yes|no" can > be used? I can, but I would like to clarify the expected semantics. With the -machine option, we would have: * -display, which affects only the display UI. * -nographic, which affects: * The display UI; * Hardware emulation; * serial/paralllel/virtioconsole output redirection. * -machine graphics=no, which would affect only hardware emulation. Is that correct?
> > Can you add a QOM property too, so that "-machine graphics=yes|no" can > > be used? > > I can, but I would like to clarify the expected semantics. With > the -machine option, we would have: > > * -display, which affects only the display UI. > * -nographic, which affects: > * The display UI; > * Hardware emulation; > * serial/paralllel/virtioconsole output redirection. > * -machine graphics=no, which would affect only hardware > emulation. > > Is that correct? Yes. In the case of PC, it might also add "-device sga" unless -nodefaults (or "-device sga") is specified. But that's left for later. Paolo
On 12 November 2015 at 09:48, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 11/11/2015 20:09, Eduardo Habkost wrote: >> All DisplayType values are just UI options that don't affect any >> hardware emulation code, except for DT_NOGRAPHIC. Replace >> DT_NOGRAPHIC with DT_NONE plus a new MachineState.nographic >> field, so hardware emulation code don't need to use the >> display_type variable. >> >> Cc: Michael Walle <michael@walle.cc> >> Cc: Blue Swirl <blauwirbel@gmail.com> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > Can you add a QOM property too, so that "-machine graphics=yes|no" can > be used? We already have both '-nographic' and '-display none'. I think adding yet another way to turn off graphics which isn't the same as either of our existing command line options would worsen this confusion... thanks -- PMM
Paolo Bonzini <pbonzini@redhat.com> writes: >> > Can you add a QOM property too, so that "-machine graphics=yes|no" can >> > be used? >> >> I can, but I would like to clarify the expected semantics. With >> the -machine option, we would have: >> >> * -display, which affects only the display UI. >> * -nographic, which affects: >> * The display UI; >> * Hardware emulation; >> * serial/paralllel/virtioconsole output redirection. >> * -machine graphics=no, which would affect only hardware >> emulation. Like usb=, this is a request to board code to enable/disable an optional component. >> Is that correct? > > Yes. In the case of PC, it might also add "-device sga" unless -nodefaults > (or "-device sga") is specified. But that's left for later. I figure -nographic should then set -machine graphic=no to do its hardware emulation part.
On 13/11/2015 12:49, Peter Maydell wrote: > On 12 November 2015 at 09:48, Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> >> On 11/11/2015 20:09, Eduardo Habkost wrote: >>> All DisplayType values are just UI options that don't affect any >>> hardware emulation code, except for DT_NOGRAPHIC. Replace >>> DT_NOGRAPHIC with DT_NONE plus a new MachineState.nographic >>> field, so hardware emulation code don't need to use the >>> display_type variable. >>> >>> Cc: Michael Walle <michael@walle.cc> >>> Cc: Blue Swirl <blauwirbel@gmail.com> >>> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> >> >> Can you add a QOM property too, so that "-machine graphics=yes|no" can >> be used? > > We already have both '-nographic' and '-display none'. > I think adding yet another way to turn off graphics which isn't > the same as either of our existing command line options would > worsen this confusion... I proposed the property exactly so that -nographic becomes the same as "-display none -machine graphics=no -serial mon:stdio". Eduardo's patches achieve that at thecode level, but not at the command line level. Paolo
On Fri, Nov 13, 2015 at 11:49:33AM +0000, Peter Maydell wrote: > On 12 November 2015 at 09:48, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > > > On 11/11/2015 20:09, Eduardo Habkost wrote: > >> All DisplayType values are just UI options that don't affect any > >> hardware emulation code, except for DT_NOGRAPHIC. Replace > >> DT_NOGRAPHIC with DT_NONE plus a new MachineState.nographic > >> field, so hardware emulation code don't need to use the > >> display_type variable. > >> > >> Cc: Michael Walle <michael@walle.cc> > >> Cc: Blue Swirl <blauwirbel@gmail.com> > >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > > Can you add a QOM property too, so that "-machine graphics=yes|no" can > > be used? > > We already have both '-nographic' and '-display none'. -display none is not a way to turn off graphics hardware emulation, it is just a way to control QEMU's GUI. > I think adding yet another way to turn off graphics which isn't > the same as either of our existing command line options would > worsen this confusion... I blame the confusion on the fact that "-nographic" does too much (it affects hardware emulation, QEMU's GUI, and device output redirection at the same time).
diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c index e46283a..947c7db 100644 --- a/hw/lm32/milkymist.c +++ b/hw/lm32/milkymist.c @@ -163,7 +163,7 @@ milkymist_init(MachineState *machine) milkymist_memcard_create(0x60004000); milkymist_ac97_create(0x60005000, irq[4], irq[5], irq[6], irq[7]); milkymist_pfpu_create(0x60006000, irq[8]); - if (display_type != DT_NOGRAPHIC) { + if (!machine->nographic) { milkymist_tmu2_create(0x60007000, irq[9]); } milkymist_minimac2_create(0x60008000, 0x30000000, irq[10], irq[11]); diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 73b0a81..e42b198 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -24,6 +24,7 @@ #include "hw/hw.h" #include "sysemu/sysemu.h" #include "sysemu/dma.h" +#include "hw/boards.h" #include "hw/isa/isa.h" #include "hw/nvram/fw_cfg.h" #include "hw/sysbus.h" @@ -755,16 +756,17 @@ static void fw_cfg_machine_ready(struct Notifier *n, void *data) static void fw_cfg_init1(DeviceState *dev) { FWCfgState *s = FW_CFG(dev); + MachineState *machine = MACHINE(qdev_get_machine()); assert(!object_resolve_path(FW_CFG_PATH, NULL)); - object_property_add_child(qdev_get_machine(), FW_CFG_NAME, OBJECT(s), NULL); + object_property_add_child(OBJECT(machine), FW_CFG_NAME, OBJECT(s), NULL); qdev_init_nofail(dev); fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4); fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16); - fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC)); + fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)machine->nographic); fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus); fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu); fw_cfg_bootsplash(s); diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c index 230dac9..d47f06a 100644 --- a/hw/sparc/sun4m.c +++ b/hw/sparc/sun4m.c @@ -1017,7 +1017,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus); slavio_serial_ms_kbd_init(hwdef->ms_kb_base, slavio_irq[14], - display_type == DT_NOGRAPHIC, ESCC_CLOCK, 1); + machine->nographic, ESCC_CLOCK, 1); /* Slavio TTYA (base+4, Linux ttyS0) is the first QEMU serial device Slavio TTYB (base+0, Linux ttyS1) is the second QEMU serial device */ escc_init(hwdef->serial_base, slavio_irq[15], slavio_irq[15], diff --git a/include/hw/boards.h b/include/hw/boards.h index 3e9a92c..1353f8a 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -120,6 +120,7 @@ struct MachineState { char *firmware; bool iommu; bool suppress_vmdesc; + bool nographic; ram_addr_t ram_size; ram_addr_t maxram_size; diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 0f4e520..f92a53c 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -139,7 +139,6 @@ typedef enum DisplayType DT_SDL, DT_COCOA, DT_GTK, - DT_NOGRAPHIC, DT_NONE, } DisplayType; diff --git a/vl.c b/vl.c index 57064ea..5d0228b 100644 --- a/vl.c +++ b/vl.c @@ -2980,6 +2980,7 @@ int main(int argc, char **argv, char **envp) int show_vnc_port = 0; bool defconfig = true; bool userconfig = true; + bool nographic = false; const char *log_mask = NULL; const char *log_file = NULL; const char *trace_events = NULL; @@ -3226,7 +3227,8 @@ int main(int argc, char **argv, char **envp) display_type = select_display(optarg); break; case QEMU_OPTION_nographic: - display_type = DT_NOGRAPHIC; + nographic = true; + display_type = DT_NONE; break; case QEMU_OPTION_curses: #ifdef CONFIG_CURSES @@ -4177,7 +4179,7 @@ int main(int argc, char **argv, char **envp) * -nographic _and_ redirects all ports explicitly - this is valid * usage, -nographic is just a no-op in this case. */ - if (display_type == DT_NOGRAPHIC + if (nographic && (default_parallel || default_serial || default_monitor || default_virtcon)) { error_report("-nographic cannot be used with -daemonize"); @@ -4191,7 +4193,7 @@ int main(int argc, char **argv, char **envp) #endif } - if (display_type == DT_NOGRAPHIC) { + if (nographic) { if (default_parallel) add_device_config(DEV_PARALLEL, "null"); if (default_serial && default_monitor) { @@ -4510,6 +4512,7 @@ int main(int argc, char **argv, char **envp) current_machine->ram_slots = ram_slots; current_machine->boot_order = boot_order; current_machine->cpu_model = cpu_model; + current_machine->nographic = nographic; machine_class->init(current_machine); @@ -4560,9 +4563,6 @@ int main(int argc, char **argv, char **envp) /* init local displays */ switch (display_type) { - case DT_NOGRAPHIC: - (void)ds; /* avoid warning if no display is configured */ - break; case DT_CURSES: curses_display_init(ds, full_screen); break;
All DisplayType values are just UI options that don't affect any hardware emulation code, except for DT_NOGRAPHIC. Replace DT_NOGRAPHIC with DT_NONE plus a new MachineState.nographic field, so hardware emulation code don't need to use the display_type variable. Cc: Michael Walle <michael@walle.cc> Cc: Blue Swirl <blauwirbel@gmail.com> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/lm32/milkymist.c | 2 +- hw/nvram/fw_cfg.c | 6 ++++-- hw/sparc/sun4m.c | 2 +- include/hw/boards.h | 1 + include/sysemu/sysemu.h | 1 - vl.c | 12 ++++++------ 6 files changed, 13 insertions(+), 11 deletions(-)