Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
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 > Cc: Blue Swirl > Cc: Mark Cave-Ayland > Signed-off-by: Eduardo Habkost 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, 0x3000, 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_
Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
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 > > Cc: Blue Swirl > > Cc: Mark Cave-Ayland > > Signed-off-by: Eduardo Habkost > > 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? -- Eduardo
Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
> > 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
Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
On 12 November 2015 at 09:48, 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 >> Cc: Blue Swirl >> Cc: Mark Cave-Ayland >> Signed-off-by: Eduardo Habkost > > 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
Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
Paolo Bonzini 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.
Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
On 13/11/2015 12:49, Peter Maydell wrote: > On 12 November 2015 at 09:48, 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 >>> Cc: Blue Swirl >>> Cc: Mark Cave-Ayland >>> Signed-off-by: Eduardo Habkost >> >> 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
Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
On Fri, Nov 13, 2015 at 11:49:33AM +, Peter Maydell wrote: > On 12 November 2015 at 09:48, 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 > >> Cc: Blue Swirl > >> Cc: Mark Cave-Ayland > >> Signed-off-by: Eduardo Habkost > > > > 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). -- Eduardo