Re: [Qemu-devel] [PATCH 7/8] virtio-vga: v1
On Mi, 2014-01-08 at 09:35 +1000, Dave Airlie wrote: On Fri, Dec 6, 2013 at 6:58 PM, Dave Airlie airl...@gmail.com wrote: On Fri, Dec 6, 2013 at 6:24 PM, Gerd Hoffmann kra...@redhat.com wrote: Hi, Now the advice given was to have virtio-vga wrap virtio-gpu-base but from what I can see it really can't. Since it needs to act and look like a PCI device Oops, yes. VirtIOPCIProxy wasn't on my radar. That makes things a bit more messy. Can you just push what you have now somewhere? I'll take a closer look next week. http://cgit.freedesktop.org/~airlied/qemu/log/?h=virtio-gpu-inherit Well I didn't really get anything working, but the top commit in that branch was where I was on my last random fail. I think another object is probably required, or making the base one not abstract. Hi Gerd, just repinging on this, not sure if you can see a solution that works with VirtIOPCIProxy that avoids wrapping. Havn't found time yet. Was sick over xmas/newyear, back online now. So I have a big email backlog to wade through now, and there are also the input/sdl2 bits which need some love to get it merged. Also not checked yet bugzilla for urgent stuff. So I expect this has to wait at least one more week ... happy new years cheers, Gerd
Re: [Qemu-devel] [PATCH 7/8] virtio-vga: v1
On Fri, Dec 6, 2013 at 6:58 PM, Dave Airlie airl...@gmail.com wrote: On Fri, Dec 6, 2013 at 6:24 PM, Gerd Hoffmann kra...@redhat.com wrote: Hi, Now the advice given was to have virtio-vga wrap virtio-gpu-base but from what I can see it really can't. Since it needs to act and look like a PCI device Oops, yes. VirtIOPCIProxy wasn't on my radar. That makes things a bit more messy. Can you just push what you have now somewhere? I'll take a closer look next week. http://cgit.freedesktop.org/~airlied/qemu/log/?h=virtio-gpu-inherit Well I didn't really get anything working, but the top commit in that branch was where I was on my last random fail. I think another object is probably required, or making the base one not abstract. Hi Gerd, just repinging on this, not sure if you can see a solution that works with VirtIOPCIProxy that avoids wrapping. Dave.
[Qemu-devel] [PATCH 7/8] virtio-vga: v1
From: Dave Airlie airl...@redhat.com
This is a virtio-vga device built on top of the virtio-gpu device.
Signed-off-by: Dave Airlie airl...@redhat.com
---
Makefile | 2 +-
default-configs/x86_64-softmmu.mak | 1 +
hw/display/Makefile.objs | 1 +
hw/display/virtio-vga.c| 156 +
hw/pci/pci.c | 2 +
include/sysemu/sysemu.h| 2 +-
pc-bios/vgabios-virtio.bin | Bin 0 - 40448 bytes
roms/Makefile | 2 +-
roms/config.vga.virtio | 6 ++
vl.c | 13
10 files changed, 182 insertions(+), 3 deletions(-)
create mode 100644 hw/display/virtio-vga.c
create mode 100644 pc-bios/vgabios-virtio.bin
create mode 100644 roms/config.vga.virtio
diff --git a/Makefile b/Makefile
index bdff4e4..a7dabe4 100644
--- a/Makefile
+++ b/Makefile
@@ -291,7 +291,7 @@ bepocz
ifdef INSTALL_BLOBS
BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
-vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
+vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin vgabios-virtio.bin \
acpi-dsdt.aml q35-acpi-dsdt.aml \
ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin \
pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
diff --git a/default-configs/x86_64-softmmu.mak
b/default-configs/x86_64-softmmu.mak
index 1a00b78..22d8587 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -10,6 +10,7 @@ CONFIG_VGA_ISA=y
CONFIG_VGA_CIRRUS=y
CONFIG_VMWARE_VGA=y
CONFIG_VIRTIO_GPU=y
+CONFIG_VIRTIO_VGA=y
CONFIG_VMMOUSE=y
CONFIG_SERIAL=y
CONFIG_PARALLEL=y
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 10e4066..63427e9 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -34,3 +34,4 @@ obj-$(CONFIG_VGA) += vga.o
common-obj-$(CONFIG_QXL) += qxl.o qxl-logger.o qxl-render.o
obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu.o
+obj-$(CONFIG_VIRTIO_VGA) += virtio-vga.o
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
new file mode 100644
index 000..9e8eff9
--- /dev/null
+++ b/hw/display/virtio-vga.c
@@ -0,0 +1,156 @@
+#include hw/hw.h
+#include hw/pci/pci.h
+#include ui/console.h
+#include vga_int.h
+#include hw/virtio/virtio-pci.h
+
+/*
+ * virtio-vga: This extends VirtIOGPUPCI
+ */
+#define TYPE_VIRTIO_VGA virtio-vga
+#define VIRTIO_VGA(obj) \
+OBJECT_CHECK(VirtIOVGA, (obj), TYPE_VIRTIO_VGA)
+
+typedef struct VirtIOVGA VirtIOVGA;
+struct VirtIOVGA {
+struct VirtIOGPUPCI gpu;
+VGACommonState vga;
+const struct GraphicHwOps *wrapped_ops;
+void *wrapped_opaque;
+};
+
+static void virtio_vga_invalidate_display(void *opaque)
+{
+VirtIOVGA *vvga = opaque;
+
+if (!vvga-gpu.vdev.enable) {
+ vvga-vga.hw_ops-invalidate(vvga-vga);
+ return;
+}
+
+vvga-wrapped_ops-invalidate(vvga-wrapped_opaque);
+}
+
+static void virtio_vga_update_display(void *opaque)
+{
+VirtIOVGA *vvga = opaque;
+
+if (!vvga-gpu.vdev.enable) {
+ vvga-vga.hw_ops-gfx_update(vvga-vga);
+}
+vvga-wrapped_ops-gfx_update(vvga-wrapped_opaque);
+}
+
+static void virtio_vga_text_update(void *opaque, console_ch_t *chardata)
+{
+VirtIOVGA *vvga = opaque;
+
+if (!vvga-gpu.vdev.enable) {
+ if (vvga-vga.hw_ops-text_update)
+ vvga-vga.hw_ops-text_update(vvga-vga, chardata);
+}
+vvga-wrapped_ops-text_update(vvga-wrapped_opaque, chardata);
+}
+
+static void virtio_vga_notify_state(void *opaque, int idx, int x, int y,
uint32_t width, uint32_t height)
+{
+VirtIOVGA *vvga = opaque;
+
+if (!vvga-gpu.vdev.enable) {
+if (vvga-vga.hw_ops-notify_state)
+ vvga-vga.hw_ops-notify_state(vvga-vga, idx, x, y, width,
height);
+}
+vvga-wrapped_ops-notify_state(vvga-wrapped_opaque, idx, x, y, width,
height);
+}
+
+static const GraphicHwOps virtio_vga_ops = {
+.invalidate = virtio_vga_invalidate_display,
+.gfx_update = virtio_vga_update_display,
+.text_update = virtio_vga_text_update,
+.notify_state = virtio_vga_notify_state,
+};
+
+/* VGA device wrapper around PCI device around virtio GPU */
+static int virtio_vga_init(VirtIOPCIProxy *vpci_dev)
+{
+VirtIOVGA *vvga = VIRTIO_VGA(vpci_dev);
+DeviceState *vdev = DEVICE(vvga-gpu.vdev);
+VGACommonState *vga = vvga-vga;
+
+qdev_set_parent_bus(vdev, BUS(vpci_dev-bus));
+if (qdev_init(vdev) 0) {
+ return -1;
+}
+
+graphic_console_wrap(vvga-gpu.vdev.con[0], DEVICE(vpci_dev),
virtio_vga_ops, vvga, vvga-wrapped_ops, vvga-wrapped_opaque);
+vga-con = vvga-gpu.vdev.con[0];
+
+vga_common_init(vga, OBJECT(vpci_dev));
+vga_init(vga, OBJECT(vpci_dev), pci_address_space(vpci_dev-pci_dev),
pci_address_space_io(vpci_dev-pci_dev), true);
+
+pci_register_bar(vpci_dev-pci_dev, 2, PCI_BASE_ADDRESS_MEM_PREFETCH,
vga-vram);
+
+if (!vpci_dev-pci_dev.rom_bar) {
+
Re: [Qemu-devel] [PATCH 7/8] virtio-vga: v1
Hi, Now the advice given was to have virtio-vga wrap virtio-gpu-base but from what I can see it really can't. Since it needs to act and look like a PCI device Oops, yes. VirtIOPCIProxy wasn't on my radar. That makes things a bit more messy. Can you just push what you have now somewhere? I'll take a closer look next week. cheers, Gerd
Re: [Qemu-devel] [PATCH 7/8] virtio-vga: v1
On Fri, Dec 6, 2013 at 6:24 PM, Gerd Hoffmann kra...@redhat.com wrote: Hi, Now the advice given was to have virtio-vga wrap virtio-gpu-base but from what I can see it really can't. Since it needs to act and look like a PCI device Oops, yes. VirtIOPCIProxy wasn't on my radar. That makes things a bit more messy. Can you just push what you have now somewhere? I'll take a closer look next week. http://cgit.freedesktop.org/~airlied/qemu/log/?h=virtio-gpu-inherit Well I didn't really get anything working, but the top commit in that branch was where I was on my last random fail. I think another object is probably required, or making the base one not abstract. Dave.
Re: [Qemu-devel] [PATCH 7/8] virtio-vga: v1
On Thu, Nov 21, 2013 at 9:06 PM, Gerd Hoffmann kra...@redhat.com wrote: On Do, 2013-11-21 at 13:12 +1000, Dave Airlie wrote: On Wed, Nov 20, 2013 at 10:02 PM, Gerd Hoffmann kra...@redhat.com wrote: On Mi, 2013-11-20 at 15:52 +1000, Dave Airlie wrote: From: Dave Airlie airl...@redhat.com This is a virtio-vga device built on top of the virtio-gpu device. Ah, I see what you use the wrapping for. Hmm. I think you should use a common base class instead, i.e. something like virtio-gpu-base which holds all the common stuff. Both virtio-gpu and virtio-vga can use that as TypeInfo-parent then. This way virtio-vga doesn't have to muck with virtio-gpu internals. virtio-gpu-base can be tagged as abstract class (using .abstract = true) so it will not be instantiated directly. I'm not sure what that buys me here, I need the virtio-vga to attach the vga ops the first console that the virtio-gpu registers, it can't be a separate console, and since virtio-gpu initialises before virtio-vga I can't tell it to not register the console. virtio-gpu-core registers no consoles. It just export the hw_ops functions. virtio-gpu-core inly initializes the stuff which is identical for both virtio-gpu and virtio-vga, everything else is left to the init functions of the subclasses. virtio-gpu uses virtio-gpu-core as parent. Registers the the consoles, using the hw_ops functions exported by virtio-gpu-core. Also sets the pci class to DISPLAY_OTHER. virtio-vga uses virtio-gpu-core as parent too. Registers the consoles, using functions basically doing if vgamode then call vga hw_ops else call virtio-gpu-core hw_ops. Simliar to what you have today but without the funky wrapping. Sets pci class to DISPLAY_VGA and initializes vga stuff. cheers, Gerd Okay I'm really missing something here and I think I've confused myself completely. My plan was virtio-gpu-base - VirtIOGPUBase object - VirtIODevice parent_obj - abstract class - contains vqs + exposes ops virtio-gpu - virtio-gpu-base wrapper with init sequence for non-VGA virtio gpus (mmio + pci) virtio-gpu-pci - VirtIOPCIProxy parent_obj - contains a VirtIOGPU vdev that it instantiates in its instance init like all the PCI wrappers Now the advice given was to have virtio-vga wrap virtio-gpu-base but from what I can see it really can't. Since it needs to act and look like a PCI device virtio-vga: Also has a VirtIOPCIProxy parent_obj, however as virtio-gpu-base is abstract I can't directly instantiate it, and I can't instantiate virtio-gpu as its the wrong thing, so do I really need to add another class? rename virtio-vga to virtio-pci-vga and add a new virtio-vga that just wraps virtio-gpu-base? This is getting a lot messier than the code I had, and the benefits are escaping me, which must mean I'm misinterpreting the instructions given. This then led to another question how do I call the virtio-gpu-base init functions? directly? as I can't use qdev as they are abstract from what I can see. Dave.
Re: [Qemu-devel] [PATCH 7/8] virtio-vga: v1
On Do, 2013-11-21 at 13:12 +1000, Dave Airlie wrote: On Wed, Nov 20, 2013 at 10:02 PM, Gerd Hoffmann kra...@redhat.com wrote: On Mi, 2013-11-20 at 15:52 +1000, Dave Airlie wrote: From: Dave Airlie airl...@redhat.com This is a virtio-vga device built on top of the virtio-gpu device. Ah, I see what you use the wrapping for. Hmm. I think you should use a common base class instead, i.e. something like virtio-gpu-base which holds all the common stuff. Both virtio-gpu and virtio-vga can use that as TypeInfo-parent then. This way virtio-vga doesn't have to muck with virtio-gpu internals. virtio-gpu-base can be tagged as abstract class (using .abstract = true) so it will not be instantiated directly. I'm not sure what that buys me here, I need the virtio-vga to attach the vga ops the first console that the virtio-gpu registers, it can't be a separate console, and since virtio-gpu initialises before virtio-vga I can't tell it to not register the console. virtio-gpu-core registers no consoles. It just export the hw_ops functions. virtio-gpu-core inly initializes the stuff which is identical for both virtio-gpu and virtio-vga, everything else is left to the init functions of the subclasses. virtio-gpu uses virtio-gpu-core as parent. Registers the the consoles, using the hw_ops functions exported by virtio-gpu-core. Also sets the pci class to DISPLAY_OTHER. virtio-vga uses virtio-gpu-core as parent too. Registers the consoles, using functions basically doing if vgamode then call vga hw_ops else call virtio-gpu-core hw_ops. Simliar to what you have today but without the funky wrapping. Sets pci class to DISPLAY_VGA and initializes vga stuff. cheers, Gerd
Re: [Qemu-devel] [PATCH 7/8] virtio-vga: v1
On Mi, 2013-11-20 at 15:52 +1000, Dave Airlie wrote: From: Dave Airlie airl...@redhat.com This is a virtio-vga device built on top of the virtio-gpu device. Ah, I see what you use the wrapping for. Hmm. I think you should use a common base class instead, i.e. something like virtio-gpu-base which holds all the common stuff. Both virtio-gpu and virtio-vga can use that as TypeInfo-parent then. This way virtio-vga doesn't have to muck with virtio-gpu internals. virtio-gpu-base can be tagged as abstract class (using .abstract = true) so it will not be instantiated directly. cheers, Gerd
Re: [Qemu-devel] [PATCH 7/8] virtio-vga: v1
On Wed, Nov 20, 2013 at 10:02 PM, Gerd Hoffmann kra...@redhat.com wrote: On Mi, 2013-11-20 at 15:52 +1000, Dave Airlie wrote: From: Dave Airlie airl...@redhat.com This is a virtio-vga device built on top of the virtio-gpu device. Ah, I see what you use the wrapping for. Hmm. I think you should use a common base class instead, i.e. something like virtio-gpu-base which holds all the common stuff. Both virtio-gpu and virtio-vga can use that as TypeInfo-parent then. This way virtio-vga doesn't have to muck with virtio-gpu internals. virtio-gpu-base can be tagged as abstract class (using .abstract = true) so it will not be instantiated directly. I'm not sure what that buys me here, I need the virtio-vga to attach the vga ops the first console that the virtio-gpu registers, it can't be a separate console, and since virtio-gpu initialises before virtio-vga I can't tell it to not register the console. Its no use attaching just the vga or just the gpu ops to the console I need a wrapper and I can't see how having a common base class would help. I already have a base class, that pci subclasses then vga subclasses that. Dave.
Re: [Qemu-devel] [PATCH 7/8] virtio-vga: v1
Il 21/11/2013 04:12, Dave Airlie ha scritto: I'm not sure what that buys me here, I need the virtio-vga to attach the vga ops the first console that the virtio-gpu registers, it can't be a separate console, and since virtio-gpu initialises before virtio-vga I can't tell it to not register the console. If you make a base class, the ops can be registered in the base class and they just call a method in the base class. The default implementation of the method can be virtio-gpu's, but virtio-vga can override them and make them go to either the virtio-gpu implementation or the VGA implementation. Paolo
[Qemu-devel] [PATCH 7/8] virtio-vga: v1
From: Dave Airlie airl...@redhat.com
This is a virtio-vga device built on top of the virtio-gpu device.
Signed-off-by: Dave Airlie airl...@redhat.com
---
Makefile | 2 +-
default-configs/x86_64-softmmu.mak | 1 +
hw/display/Makefile.objs | 1 +
hw/display/virtio-vga.c| 156 +
hw/pci/pci.c | 2 +
include/sysemu/sysemu.h| 2 +-
pc-bios/vgabios-virtio.bin | Bin 0 - 40448 bytes
roms/Makefile | 2 +-
roms/config.vga.virtio | 6 ++
vl.c | 13
10 files changed, 182 insertions(+), 3 deletions(-)
create mode 100644 hw/display/virtio-vga.c
create mode 100644 pc-bios/vgabios-virtio.bin
create mode 100644 roms/config.vga.virtio
diff --git a/Makefile b/Makefile
index b15003f..093ff9a 100644
--- a/Makefile
+++ b/Makefile
@@ -284,7 +284,7 @@ bepo
ifdef INSTALL_BLOBS
BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
-vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
+vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin vgabios-virtio.bin \
acpi-dsdt.aml q35-acpi-dsdt.aml \
ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \
pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
diff --git a/default-configs/x86_64-softmmu.mak
b/default-configs/x86_64-softmmu.mak
index 1a00b78..22d8587 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -10,6 +10,7 @@ CONFIG_VGA_ISA=y
CONFIG_VGA_CIRRUS=y
CONFIG_VMWARE_VGA=y
CONFIG_VIRTIO_GPU=y
+CONFIG_VIRTIO_VGA=y
CONFIG_VMMOUSE=y
CONFIG_SERIAL=y
CONFIG_PARALLEL=y
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 10e4066..63427e9 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -34,3 +34,4 @@ obj-$(CONFIG_VGA) += vga.o
common-obj-$(CONFIG_QXL) += qxl.o qxl-logger.o qxl-render.o
obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu.o
+obj-$(CONFIG_VIRTIO_VGA) += virtio-vga.o
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
new file mode 100644
index 000..a6c9438
--- /dev/null
+++ b/hw/display/virtio-vga.c
@@ -0,0 +1,156 @@
+#include hw/hw.h
+#include hw/pci/pci.h
+#include ui/console.h
+#include vga_int.h
+#include hw/virtio/virtio-pci.h
+
+/*
+ * virtio-vga: This extends VirtIOGPUPCI
+ */
+#define TYPE_VIRTIO_VGA virtio-vga
+#define VIRTIO_VGA(obj) \
+OBJECT_CHECK(VirtIOVGA, (obj), TYPE_VIRTIO_VGA)
+
+typedef struct VirtIOVGA VirtIOVGA;
+struct VirtIOVGA {
+struct VirtIOGPUPCI gpu;
+VGACommonState vga;
+const struct GraphicHwOps *wrapped_ops;
+void *wrapped_opaque;
+};
+
+static void virtio_vga_invalidate_display(void *opaque)
+{
+VirtIOVGA *vvga = opaque;
+
+if (!vvga-gpu.vdev.enable) {
+ vvga-vga.hw_ops-invalidate(vvga-vga);
+ return;
+}
+
+vvga-wrapped_ops-invalidate(vvga-wrapped_opaque);
+}
+
+static void virtio_vga_update_display(void *opaque)
+{
+VirtIOVGA *vvga = opaque;
+
+if (!vvga-gpu.vdev.enable) {
+ vvga-vga.hw_ops-gfx_update(vvga-vga);
+}
+vvga-wrapped_ops-gfx_update(vvga-wrapped_opaque);
+}
+
+static void virtio_vga_text_update(void *opaque, console_ch_t *chardata)
+{
+VirtIOVGA *vvga = opaque;
+
+if (!vvga-gpu.vdev.enable) {
+ if (vvga-vga.hw_ops-text_update)
+ vvga-vga.hw_ops-text_update(vvga-vga, chardata);
+}
+vvga-wrapped_ops-text_update(vvga-wrapped_opaque, chardata);
+}
+
+static void virtio_vga_notify_state(void *opaque, int idx, int x, int y,
uint32_t width, uint32_t height)
+{
+VirtIOVGA *vvga = opaque;
+
+if (!vvga-gpu.vdev.enable) {
+if (vvga-vga.hw_ops-notify_state)
+ vvga-vga.hw_ops-notify_state(vvga-vga, idx, x, y, width,
height);
+}
+vvga-wrapped_ops-notify_state(vvga-wrapped_opaque, idx, x, y, width,
height);
+}
+
+static const GraphicHwOps virtio_vga_ops = {
+.invalidate = virtio_vga_invalidate_display,
+.gfx_update = virtio_vga_update_display,
+.text_update = virtio_vga_text_update,
+.notify_state = virtio_vga_notify_state,
+};
+
+/* VGA device wrapper around PCI device around virtio GPU */
+static int virtio_vga_init(VirtIOPCIProxy *vpci_dev)
+{
+VirtIOVGA *vvga = VIRTIO_VGA(vpci_dev);
+DeviceState *vdev = DEVICE(vvga-gpu.vdev);
+VGACommonState *vga = vvga-vga;
+
+qdev_set_parent_bus(vdev, BUS(vpci_dev-bus));
+if (qdev_init(vdev) 0) {
+ return -1;
+}
+
+graphic_console_wrap(vvga-gpu.vdev.con[0], DEVICE(vpci_dev),
virtio_vga_ops, vvga, vvga-wrapped_ops, vvga-wrapped_opaque);
+vga-con = vvga-gpu.vdev.con[0];
+
+vga_common_init(vga, OBJECT(vpci_dev));
+vga_init(vga, OBJECT(vpci_dev), pci_address_space(vpci_dev-pci_dev),
pci_address_space_io(vpci_dev-pci_dev), true);
+
+pci_register_bar(vpci_dev-pci_dev, 2, PCI_BASE_ADDRESS_MEM_PREFETCH,
vga-vram);
+
+if (!vpci_dev-pci_dev.rom_bar) {
+/*
