[Qemu-devel] [PATCH 0/2] vga: add secondary stdvga variant

[Qemu-devel] [PATCH 0/2] vga: add secondary stdvga variant

[Gerd Hoffmann]

  Hi,

This little series adds a legacy-free variant of the qemu standard vga,
which can easily be used as secondary vga.  No BIOS, thus no vesa
support, thus you need guest drivers.

If you wanna play with this walk over here:
  http://www.kraxel.org/cgit/linux/log/?h=qemu-drm

The bochs drm driver handles both this new device and the qemu stdvga.

Gerd Hoffmann (2):
  vga: allow non-global vmstate
  vga: add secondary stdvga variant

 docs/specs/standard-vga.txt | 13 ++++++---
 hw/display/cirrus_vga.c     |  4 +--
 hw/display/qxl.c            |  2 +-
 hw/display/vga-isa-mm.c     |  2 +-
 hw/display/vga-isa.c        |  2 +-
 hw/display/vga-pci.c        | 64 ++++++++++++++++++++++++++++++++++++++++++++-
 hw/display/vga.c            |  8 ++++--
 hw/display/vga_int.h        |  2 +-
 hw/display/vmware_vga.c     |  2 +-
 9 files changed, 86 insertions(+), 13 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] vga: allow non-global vmstate

[Gerd Hoffmann]

Need a way to opt-out from vga.vram being global vmstate, for
secondary vga cards.  Add a bool parameter to vga_common_init
to support this.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/cirrus_vga.c | 4 ++--
 hw/display/qxl.c        | 2 +-
 hw/display/vga-isa-mm.c | 2 +-
 hw/display/vga-isa.c    | 2 +-
 hw/display/vga-pci.c    | 2 +-
 hw/display/vga.c        | 4 ++--
 hw/display/vga_int.h    | 2 +-
 hw/display/vmware_vga.c | 2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index dbd1f4a..3514761 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2914,7 +2914,7 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev, Error **errp)
     ISACirrusVGAState *d = ISA_CIRRUS_VGA(dev);
     VGACommonState *s = &d->cirrus_vga.vga;
 
-    vga_common_init(s, OBJECT(dev));
+    vga_common_init(s, OBJECT(dev), true);
     cirrus_init_common(&d->cirrus_vga, OBJECT(dev), CIRRUS_ID_CLGD5430, 0,
                        isa_address_space(isadev),
                        isa_address_space_io(isadev));
@@ -2961,7 +2961,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
      int16_t device_id = pc->device_id;
 
      /* setup VGA */
-     vga_common_init(&s->vga, OBJECT(dev));
+     vga_common_init(&s->vga, OBJECT(dev), true);
      cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev),
                         pci_address_space_io(dev));
      s->vga.con = graphic_console_init(DEVICE(dev), s->vga.hw_ops, &s->vga);
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index ee2db0d..6ebf471 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -2069,7 +2069,7 @@ static int qxl_init_primary(PCIDevice *dev)
     qxl->id = 0;
     qxl_init_ramsize(qxl);
     vga->vram_size_mb = qxl->vga.vram_size >> 20;
-    vga_common_init(vga, OBJECT(dev));
+    vga_common_init(vga, OBJECT(dev), true);
     vga_init(vga, OBJECT(dev),
              pci_address_space(dev), pci_address_space_io(dev), false);
     portio_list_init(qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list,
diff --git a/hw/display/vga-isa-mm.c b/hw/display/vga-isa-mm.c
index 8b514cc..cc5c582 100644
--- a/hw/display/vga-isa-mm.c
+++ b/hw/display/vga-isa-mm.c
@@ -132,7 +132,7 @@ int isa_vga_mm_init(hwaddr vram_base,
     s = g_malloc0(sizeof(*s));
 
     s->vga.vram_size_mb = VGA_RAM_SIZE >> 20;
-    vga_common_init(&s->vga, NULL);
+    vga_common_init(&s->vga, NULL, true);
     vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space);
 
     s->vga.con = graphic_console_init(NULL, s->vga.hw_ops, s);
diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
index c2a19ad..f5742ad 100644
--- a/hw/display/vga-isa.c
+++ b/hw/display/vga-isa.c
@@ -56,7 +56,7 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
     MemoryRegion *vga_io_memory;
     const MemoryRegionPortio *vga_ports, *vbe_ports;
 
-    vga_common_init(s, OBJECT(dev));
+    vga_common_init(s, OBJECT(dev), true);
     s->legacy_address_space = isa_address_space(isadev);
     vga_io_memory = vga_init_io(s, OBJECT(dev), &vga_ports, &vbe_ports);
     isa_register_portio_list(isadev, 0x3b0, vga_ports, s, "vga");
diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index b3a45c8..dee180f 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -147,7 +147,7 @@ static int pci_std_vga_initfn(PCIDevice *dev)
     VGACommonState *s = &d->vga;
 
     /* vga + console init */
-    vga_common_init(s, OBJECT(dev));
+    vga_common_init(s, OBJECT(dev), true);
     vga_init(s, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev),
              true);
 
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 7b91d9c..fea30e5 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2257,7 +2257,7 @@ static const GraphicHwOps vga_ops = {
     .text_update = vga_update_text,
 };
 
-void vga_common_init(VGACommonState *s, Object *obj)
+void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate)
 {
     int i, j, v, b;
 
@@ -2294,7 +2294,7 @@ void vga_common_init(VGACommonState *s, Object *obj)
 
     s->is_vbe_vmstate = 1;
     memory_region_init_ram(&s->vram, obj, "vga.vram", s->vram_size);
-    vmstate_register_ram_global(&s->vram);
+    vmstate_register_ram(&s->vram, global_vmstate ? NULL : DEVICE(obj));
     xen_register_framebuffer(&s->vram);
     s->vram_ptr = memory_region_get_ram_ptr(&s->vram);
     s->get_bpp = vga_get_bpp;
diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index e641890..d42ac92 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -177,7 +177,7 @@ static inline int c6_to_8(int v)
     return (v << 2) | (b << 1) | b;
 }
 
-void vga_common_init(VGACommonState *s, Object *obj);
+void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate);
 void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
               MemoryRegion *address_space_io, bool init_vga_ports);
 MemoryRegion *vga_init_io(VGACommonState *s, Object *obj,
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index a6a8cdc..f4f8b80 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -1205,7 +1205,7 @@ static void vmsvga_init(DeviceState *dev, struct vmsvga_state_s *s,
     vmstate_register_ram_global(&s->fifo_ram);
     s->fifo_ptr = memory_region_get_ram_ptr(&s->fifo_ram);
 
-    vga_common_init(&s->vga, OBJECT(dev));
+    vga_common_init(&s->vga, OBJECT(dev), true);
     vga_init(&s->vga, OBJECT(dev), address_space, io, true);
     vmstate_register(NULL, 0, &vmstate_vga_common, &s->vga);
     s->new_depth = 32;
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] vga: add secondary stdvga variant

[Gerd Hoffmann]

Add a standard vga variant which doesn't occupy any legacy
ressources and thus can easily be used as secondary (or legacy-free)
graphics adapter.  Programming must be done using the MMIO bar.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/specs/standard-vga.txt | 13 +++++++---
 hw/display/vga-pci.c        | 62 +++++++++++++++++++++++++++++++++++++++++++++
 hw/display/vga.c            |  4 +++
 3 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/docs/specs/standard-vga.txt b/docs/specs/standard-vga.txt
index 8a4c1e9..f82773e 100644
--- a/docs/specs/standard-vga.txt
+++ b/docs/specs/standard-vga.txt
@@ -5,9 +5,10 @@ QEMU Standard VGA
 Exists in two variants, for isa and pci.
 
 command line switches:
-    -vga std            [ picks isa for -M isapc, otherwise pci ]
-    -device VGA         [ pci variant ]
-    -device isa-vga     [ isa variant ]
+    -vga std               [ picks isa for -M isapc, otherwise pci ]
+    -device VGA            [ pci variant ]
+    -device isa-vga        [ isa variant ]
+    -device secondary-vga  [ legacy-free pci variant ]
 
 
 PCI spec
@@ -31,9 +32,15 @@ PCI ROM Region:
    Holds the vgabios (qemu 0.14+).
 
 
+The legacy-free variant has no ROM and has PCI_CLASS_DISPLAY_OTHER
+instead of PCI_CLASS_DISPLAY_VGA.
+
+
 IO ports used
 -------------
 
+Doesn't apply to the legacy-free pci variant, use the MMIO bar instead.
+
 03c0 - 03df : standard vga ports
 01ce        : bochs vbe interface index port
 01cf        : bochs vbe interface data port (x86 only)
diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index dee180f..38a0fca 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -179,12 +179,52 @@ static int pci_std_vga_initfn(PCIDevice *dev)
     return 0;
 }
 
+static int pci_secondary_vga_initfn(PCIDevice *dev)
+{
+    PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, dev);
+    VGACommonState *s = &d->vga;
+
+    /* vga + console init */
+    vga_common_init(s, OBJECT(dev), false);
+    s->con = graphic_console_init(DEVICE(dev), s->hw_ops, s);
+
+    /* mmio bar */
+    memory_region_init(&d->mmio, OBJECT(dev), "vga.mmio", 4096);
+    memory_region_init_io(&d->ioport, OBJECT(dev), &pci_vga_ioport_ops, d,
+                          "vga ioports remapped", PCI_VGA_IOPORT_SIZE);
+    memory_region_init_io(&d->bochs, OBJECT(dev), &pci_vga_bochs_ops, d,
+                          "bochs dispi interface", PCI_VGA_BOCHS_SIZE);
+
+    memory_region_add_subregion(&d->mmio, PCI_VGA_IOPORT_OFFSET,
+                                &d->ioport);
+    memory_region_add_subregion(&d->mmio, PCI_VGA_BOCHS_OFFSET,
+                                &d->bochs);
+
+    pci_register_bar(&d->dev, 0, (PCI_BASE_ADDRESS_MEM_PREFETCH |
+                                  PCI_BASE_ADDRESS_MEM_TYPE_64), &s->vram);
+    pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
+
+    return 0;
+}
+
+static void pci_secondary_vga_reset(DeviceState *dev)
+{
+    PCIVGAState *d = DO_UPCAST(PCIVGAState, dev.qdev, dev);
+
+    vga_common_reset(&d->vga);
+}
+
 static Property vga_pci_properties[] = {
     DEFINE_PROP_UINT32("vgamem_mb", PCIVGAState, vga.vram_size_mb, 16),
     DEFINE_PROP_BIT("mmio", PCIVGAState, flags, PCI_VGA_FLAG_ENABLE_MMIO, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static Property secondary_pci_properties[] = {
+    DEFINE_PROP_UINT32("vgamem_mb", PCIVGAState, vga.vram_size_mb, 16),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void vga_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -201,6 +241,20 @@ static void vga_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
 }
 
+static void secondary_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->init = pci_secondary_vga_initfn;
+    k->vendor_id = PCI_VENDOR_ID_QEMU;
+    k->device_id = PCI_DEVICE_ID_QEMU_VGA;
+    k->class_id = PCI_CLASS_DISPLAY_OTHER;
+    dc->vmsd = &vmstate_vga_pci;
+    dc->props = secondary_pci_properties;
+    dc->reset = pci_secondary_vga_reset;
+}
+
 static const TypeInfo vga_info = {
     .name          = "VGA",
     .parent        = TYPE_PCI_DEVICE,
@@ -208,9 +262,17 @@ static const TypeInfo vga_info = {
     .class_init    = vga_class_init,
 };
 
+static const TypeInfo secondary_info = {
+    .name          = "secondary-vga",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIVGAState),
+    .class_init    = secondary_class_init,
+};
+
 static void vga_register_types(void)
 {
     type_register_static(&vga_info);
+    type_register_static(&secondary_info);
 }
 
 type_init(vga_register_types)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index fea30e5..8eb4dc4 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -171,6 +171,10 @@ static void vga_update_memory_access(VGACommonState *s)
     MemoryRegion *region, *old_region = s->chain4_alias;
     hwaddr base, offset, size;
 
+    if (s->legacy_address_space == NULL) {
+        return;
+    }
+
     s->chain4_alias = NULL;
 
     if ((s->sr[VGA_SEQ_PLANE_WRITE] & VGA_SR02_ALL_PLANES) ==
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/2] vga: allow non-global vmstate

[Hervé Poussineau]

Gerd Hoffmann a écrit :
> Need a way to opt-out from vga.vram being global vmstate, for
> secondary vga cards.  Add a bool parameter to vga_common_init
> to support this.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/cirrus_vga.c | 4 ++--
>  hw/display/qxl.c        | 2 +-
>  hw/display/vga-isa-mm.c | 2 +-
>  hw/display/vga-isa.c    | 2 +-
>  hw/display/vga-pci.c    | 2 +-
>  hw/display/vga.c        | 4 ++--
>  hw/display/vga_int.h    | 2 +-
>  hw/display/vmware_vga.c | 2 +-
>  8 files changed, 10 insertions(+), 10 deletions(-)
>
>   
[...]

> diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
> index b3a45c8..dee180f 100644
> --- a/hw/display/vga-pci.c
> +++ b/hw/display/vga-pci.c
> @@ -147,7 +147,7 @@ static int pci_std_vga_initfn(PCIDevice *dev)
>      VGACommonState *s = &d->vga;
>  
>      /* vga + console init */
> -    vga_common_init(s, OBJECT(dev));
> +    vga_common_init(s, OBJECT(dev), true);
>      vga_init(s, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev),
>               true);
>  
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 7b91d9c..fea30e5 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -2257,7 +2257,7 @@ static const GraphicHwOps vga_ops = {
>      .text_update = vga_update_text,
>  };
>  
> -void vga_common_init(VGACommonState *s, Object *obj)
> +void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate)
>  {
>      int i, j, v, b;
>  
> @@ -2294,7 +2294,7 @@ void vga_common_init(VGACommonState *s, Object *obj)
>  
>      s->is_vbe_vmstate = 1;
>      memory_region_init_ram(&s->vram, obj, "vga.vram", s->vram_size);
> -    vmstate_register_ram_global(&s->vram);
> +    vmstate_register_ram(&s->vram, global_vmstate ? NULL : DEVICE(obj));
>   

Is it possible to do it depending of the QEMU compatibility version, or 
of some property, so we don't have to keep this global vmstate forever?
With this, I think we can also expect to be able to specify "-device 
VGA" twice, and see the BIOS messages on one card or another.

Hervé

Re: [Qemu-devel] [PATCH 1/2] vga: allow non-global vmstate

[Gerd Hoffmann]

> > diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
> > index b3a45c8..dee180f 100644
> > --- a/hw/display/vga-pci.c
> > +++ b/hw/display/vga-pci.c
> > @@ -147,7 +147,7 @@ static int pci_std_vga_initfn(PCIDevice *dev)
> >      VGACommonState *s = &d->vga;
> >  
> >      /* vga + console init */
> > -    vga_common_init(s, OBJECT(dev));
> > +    vga_common_init(s, OBJECT(dev), true);

> > -void vga_common_init(VGACommonState *s, Object *obj)
> > +void vga_common_init(VGACommonState *s, Object *obj, bool global_vmstate)

> > -    vmstate_register_ram_global(&s->vram);
> > +    vmstate_register_ram(&s->vram, global_vmstate ? NULL : DEVICE(obj));
> >   
> 
> Is it possible to do it depending of the QEMU compatibility version, or 
> of some property, so we don't have to keep this global vmstate forever?

Sure.  We can easily add a property to stdvga, then make the new
vga_common_init paramater depend on the property.

> With this, I think we can also expect to be able to specify "-device 
> VGA" twice, and see the BIOS messages on one card or another.

It would get us a step closer, but the two vga cards would still compete
for the legacy vga io ports at 0x3c0.

Thats why there is patch #2 of this series, which adds a stdvga variant
not allocating legacy ports, so it can be added without port conflicts.

cheers,
  Gerd