[PATCH v2] Warn user if the vga flag is passed but no vga device is created

[PATCH v2] Warn user if the vga flag is passed but no vga device is created

[Gautam Agrawal]

A global boolean variable "vga_interface_created"(declared in softmmu/globals.c)
has been used to track the creation of vga interface. If the vga flag is passed
in the command line "default_vga"(declared in softmmu/vl.c) variable is set to 0.
To warn user, the condition checks if vga_interface_created is false
and default_vga is equal to 0.If "-vga none" is passed, this patch will not warn the
user regarding the creation of VGA device.

The initialisation of Global variable "vga_interface_created" in softmmu/globals.c
has also been corrected.

The warning "A -vga option was passed but this
machine type does not use that option; no VGA device has been created"
is logged if vga flag is passed but no vga device is created.

"vga_interface_created" has also been included in "xen_machine_pv.c" and
"fuloong2e.c". This patch has been tested for x86_64, i386, sparc, sparc64 and arm boards.

Signed-off-by: Gautam Agrawal <gautamnagrawal@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/581
---
 hw/isa/isa-bus.c          | 1 +
 hw/mips/fuloong2e.c       | 1 +
 hw/pci/pci.c              | 1 +
 hw/sparc/sun4m.c          | 2 ++
 hw/sparc64/sun4u.c        | 1 +
 hw/xenpv/xen_machine_pv.c | 1 +
 include/sysemu/sysemu.h   | 1 +
 softmmu/globals.c         | 1 +
 softmmu/vl.c              | 6 ++++++
 9 files changed, 15 insertions(+)

diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 0ad1c5fd65..cd5ad3687d 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -166,6 +166,7 @@ bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp)
 
 ISADevice *isa_vga_init(ISABus *bus)
 {
+    vga_interface_created = true;
     switch (vga_interface_type) {
     case VGA_CIRRUS:
         return isa_create_simple(bus, "isa-cirrus-vga");
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index c9f14e70a0..538453b426 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -321,6 +321,7 @@ static void mips_fuloong2e_init(MachineState *machine)
 
     /* GPU */
     if (vga_interface_type != VGA_NONE) {
+        vga_interface_created = true;
         pci_dev = pci_new(-1, "ati-vga");
         dev = DEVICE(pci_dev);
         qdev_prop_set_uint32(dev, "vgamem_mb", 16);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index dae9119bfe..fab9c80f8d 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2038,6 +2038,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
 
 PCIDevice *pci_vga_init(PCIBus *bus)
 {
+    vga_interface_created = true;
     switch (vga_interface_type) {
     case VGA_CIRRUS:
         return pci_create_simple(bus, -1, "cirrus-vga");
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 7f3a7c0027..f45e29acc8 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -921,6 +921,7 @@ static void sun4m_hw_init(MachineState *machine)
             /* sbus irq 5 */
             cg3_init(hwdef->tcx_base, slavio_irq[11], 0x00100000,
                      graphic_width, graphic_height, graphic_depth);
+            vga_interface_created = true;
         } else {
             /* If no display specified, default to TCX */
             if (graphic_depth != 8 && graphic_depth != 24) {
@@ -936,6 +937,7 @@ static void sun4m_hw_init(MachineState *machine)
 
             tcx_init(hwdef->tcx_base, slavio_irq[11], 0x00100000,
                      graphic_width, graphic_height, graphic_depth);
+            vga_interface_created = true;
         }
     }
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index cda7df36e3..75334dba71 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -633,6 +633,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     switch (vga_interface_type) {
     case VGA_STD:
         pci_create_simple(pci_busA, PCI_DEVFN(2, 0), "VGA");
+        vga_interface_created = true;
         break;
     case VGA_NONE:
         break;
diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c
index 8df575a457..20c9611d71 100644
--- a/hw/xenpv/xen_machine_pv.c
+++ b/hw/xenpv/xen_machine_pv.c
@@ -63,6 +63,7 @@ static void xen_init_pv(MachineState *machine)
     if (vga_interface_type == VGA_XENFB) {
         xen_config_dev_vfb(0, "vnc");
         xen_config_dev_vkbd(0);
+        vga_interface_created = true;
     }
 
     /* configure disks */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b9421e03ff..a558b895e4 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -32,6 +32,7 @@ typedef enum {
 } VGAInterfaceType;
 
 extern int vga_interface_type;
+extern bool vga_interface_created;
 
 extern int graphic_width;
 extern int graphic_height;
diff --git a/softmmu/globals.c b/softmmu/globals.c
index 3ebd718e35..98b64e0492 100644
--- a/softmmu/globals.c
+++ b/softmmu/globals.c
@@ -40,6 +40,7 @@ int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int autostart = 1;
 int vga_interface_type = VGA_NONE;
+bool vga_interface_created;
 Chardev *parallel_hds[MAX_PARALLEL_PORTS];
 int win2k_install_hack;
 int singlestep;
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 6f646531a0..8c9f31fa35 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2734,6 +2734,12 @@ static void qemu_machine_creation_done(void)
     if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) {
         exit(1);
     }
+    if (!vga_interface_created && !default_vga &&
+        vga_interface_type != VGA_NONE) {
+        warn_report("A -vga option was passed but this machine "
+                    "type does not use that option; "
+                    "No VGA device has been created");
+    }
 }
 
 void qmp_x_exit_preconfig(Error **errp)
-- 
2.34.1

Re: [PATCH v2] Warn user if the vga flag is passed but no vga device is created

[Peter Maydell]

On Wed, 13 Apr 2022 at 18:23, Gautam Agrawal <gautamnagrawal@gmail.com> wrote:
>
> A global boolean variable "vga_interface_created"(declared in softmmu/globals.c)
> has been used to track the creation of vga interface. If the vga flag is passed
> in the command line "default_vga"(declared in softmmu/vl.c) variable is set to 0.
> To warn user, the condition checks if vga_interface_created is false
> and default_vga is equal to 0.If "-vga none" is passed, this patch will not warn the
> user regarding the creation of VGA device.
>
> The initialisation of Global variable "vga_interface_created" in softmmu/globals.c
> has also been corrected.
>
> The warning "A -vga option was passed but this
> machine type does not use that option; no VGA device has been created"
> is logged if vga flag is passed but no vga device is created.
>
> "vga_interface_created" has also been included in "xen_machine_pv.c" and
> "fuloong2e.c". This patch has been tested for x86_64, i386, sparc, sparc64 and arm boards.
>
> Signed-off-by: Gautam Agrawal <gautamnagrawal@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/581
> ---
>  hw/isa/isa-bus.c          | 1 +
>  hw/mips/fuloong2e.c       | 1 +
>  hw/pci/pci.c              | 1 +
>  hw/sparc/sun4m.c          | 2 ++
>  hw/sparc64/sun4u.c        | 1 +
>  hw/xenpv/xen_machine_pv.c | 1 +
>  include/sysemu/sysemu.h   | 1 +
>  softmmu/globals.c         | 1 +
>  softmmu/vl.c              | 6 ++++++
>  9 files changed, 15 insertions(+)

You've missed the uses of vga_interface_type in hw/hppa/machine.c
and hw/ppc/spapr.c. ("git grep vga_interface_type" will give you the
list of all the files which touch that variable.)

thanks
-- PMM

Re: [PATCH v2] Warn user if the vga flag is passed but no vga device is created

[Gautam Agrawal]

I will make changes in hw/ppc/spapr.c . In hw/hppa/machine.c ,
vga_interface_type is used inside if condition to check for graphics
device creation, but I am not able to find any relation with creation
of vga interface. Could you kindly provide some pointers about it?

thanks,

Regards,
Gautam Agrawal

On Thu, 21 Apr 2022 at 20:59, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 13 Apr 2022 at 18:23, Gautam Agrawal <gautamnagrawal@gmail.com> wrote:
> >
> > A global boolean variable "vga_interface_created"(declared in softmmu/globals.c)
> > has been used to track the creation of vga interface. If the vga flag is passed
> > in the command line "default_vga"(declared in softmmu/vl.c) variable is set to 0.
> > To warn user, the condition checks if vga_interface_created is false
> > and default_vga is equal to 0.If "-vga none" is passed, this patch will not warn the
> > user regarding the creation of VGA device.
> >
> > The initialisation of Global variable "vga_interface_created" in softmmu/globals.c
> > has also been corrected.
> >
> > The warning "A -vga option was passed but this
> > machine type does not use that option; no VGA device has been created"
> > is logged if vga flag is passed but no vga device is created.
> >
> > "vga_interface_created" has also been included in "xen_machine_pv.c" and
> > "fuloong2e.c". This patch has been tested for x86_64, i386, sparc, sparc64 and arm boards.
> >
> > Signed-off-by: Gautam Agrawal <gautamnagrawal@gmail.com>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/581
> > ---
> >  hw/isa/isa-bus.c          | 1 +
> >  hw/mips/fuloong2e.c       | 1 +
> >  hw/pci/pci.c              | 1 +
> >  hw/sparc/sun4m.c          | 2 ++
> >  hw/sparc64/sun4u.c        | 1 +
> >  hw/xenpv/xen_machine_pv.c | 1 +
> >  include/sysemu/sysemu.h   | 1 +
> >  softmmu/globals.c         | 1 +
> >  softmmu/vl.c              | 6 ++++++
> >  9 files changed, 15 insertions(+)
>
> You've missed the uses of vga_interface_type in hw/hppa/machine.c
> and hw/ppc/spapr.c. ("git grep vga_interface_type" will give you the
> list of all the files which touch that variable.)
>
> thanks
> -- PMM

Re: [PATCH v2] Warn user if the vga flag is passed but no vga device is created

[Peter Maydell]

On Sat, 23 Apr 2022 at 11:00, Gautam Agrawal <gautamnagrawal@gmail.com> wrote:
>
> I will make changes in hw/ppc/spapr.c . In hw/hppa/machine.c ,
> vga_interface_type is used inside if condition to check for graphics
> device creation, but I am not able to find any relation with creation
> of vga interface. Could you kindly provide some pointers about it?

It's exactly the same as all the other places that look
at vga_interface_type -- it looks at it and decides
whether to create a graphics device. (Arguably it should
not really be saying "anything except 'none' means create
the 'artist' graphics device", but that's a separate bug;
all you need to care about is "did we create a graphics
device because the user passed a -vga option".)

-- PMM

Re: [PATCH v2] Warn user if the vga flag is passed but no vga device is created

[Thomas Huth]

On 23/04/2022 15.27, Peter Maydell wrote:
> On Sat, 23 Apr 2022 at 11:00, Gautam Agrawal <gautamnagrawal@gmail.com> wrote:
>>
>> I will make changes in hw/ppc/spapr.c . In hw/hppa/machine.c ,
>> vga_interface_type is used inside if condition to check for graphics
>> device creation, but I am not able to find any relation with creation
>> of vga interface. Could you kindly provide some pointers about it?
> 
> It's exactly the same as all the other places that look
> at vga_interface_type -- it looks at it and decides
> whether to create a graphics device. (Arguably it should
> not really be saying "anything except 'none' means create
> the 'artist' graphics device", but that's a separate bug;
> all you need to care about is "did we create a graphics
> device because the user passed a -vga option".)

Agreed, that looks like a separate issue that we should fix one day... 
something for another BiteSizeTask ticket maybe? Same problem exists in the 
hw/mips/fuloong2e.c by the way.

  Thomas