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
