[Qemu-devel] [PATCH v1 0/2] Virtio GPU for S390

[Qemu-devel] [PATCH v1 0/2] Virtio GPU for S390

[Farhan Ali]

These patches wire up the virtio-gpu device for CCW bus for
S390.

For the S390 architecture which does not natively support any graphics
device, virtio gpu in 2D mode could be used to emulate a simple graphics
card and use VNC as the display.

eg: qemu-system-s390x ... -device virtio-gpu-ccw,devno=fe.0.0101
    -vnc host_ip_addr:5900

Note, to actually see any display content the
guest kernel needs to support DRM layer, Virtio GPU driver,
the Virtual Terminal layer etc.

I would appreciate any feedback on these patches, specially the
first patch.

Thank you
Farhan

Farhan Ali (2):
  virtio_gpu: Handle endian conversion
  virtio-gpu-ccw: Create a virtio gpu device for the ccw bus

 hw/display/virtio-gpu.c | 53 +++++++++++++++++++++++++++++++++++++++---------
 hw/s390x/virtio-ccw.c   | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/s390x/virtio-ccw.h   | 10 +++++++++
 3 files changed, 108 insertions(+), 9 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v1 1/2] virtio_gpu: Handle endian conversion

[Farhan Ali]

Virtio GPU code currently only supports litte endian format,
and so using the Virtio GPU device on a big endian machine
does not work.

Let's fix it by supporting the correct host cpu byte order.

Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com>
---
 hw/display/virtio-gpu.c | 53 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 6aae147..36e2414 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -236,8 +236,8 @@ virtio_gpu_fill_display_info(VirtIOGPU *g,
     for (i = 0; i < g->conf.max_outputs; i++) {
         if (g->enabled_output_bitmask & (1 << i)) {
             dpy_info->pmodes[i].enabled = 1;
-            dpy_info->pmodes[i].r.width = g->req_state[i].width;
-            dpy_info->pmodes[i].r.height = g->req_state[i].height;
+            dpy_info->pmodes[i].r.width = cpu_to_le32(g->req_state[i].width);
+            dpy_info->pmodes[i].r.height = cpu_to_le32(g->req_state[i].height);
         }
     }
 }
@@ -287,6 +287,12 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
     struct virtio_gpu_resource_create_2d c2d;
 
     VIRTIO_GPU_FILL_CMD(c2d);
+
+    c2d.resource_id = le32_to_cpu(c2d.resource_id);
+    c2d.format = le32_to_cpu(c2d.format);
+    c2d.width = le32_to_cpu(c2d.width);
+    c2d.height = le32_to_cpu(c2d.height);
+
     trace_virtio_gpu_cmd_res_create_2d(c2d.resource_id, c2d.format,
                                        c2d.width, c2d.height);
 
@@ -383,6 +389,14 @@ static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
     struct virtio_gpu_transfer_to_host_2d t2d;
 
     VIRTIO_GPU_FILL_CMD(t2d);
+
+    t2d.r.x = le32_to_cpu(t2d.r.x);
+    t2d.r.y = le32_to_cpu(t2d.r.y);
+    t2d.r.width = le32_to_cpu(t2d.r.width);
+    t2d.r.height = le32_to_cpu(t2d.r.height);
+    t2d.resource_id = le32_to_cpu(t2d.resource_id);
+    t2d.offset = le64_to_cpu(t2d.offset);
+
     trace_virtio_gpu_cmd_res_xfer_toh_2d(t2d.resource_id);
 
     res = virtio_gpu_find_resource(g, t2d.resource_id);
@@ -439,6 +453,13 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
     int i;
 
     VIRTIO_GPU_FILL_CMD(rf);
+
+    rf.resource_id = le32_to_cpu(rf.resource_id);
+    rf.r.width = le32_to_cpu(rf.r.width);
+    rf.r.height = le32_to_cpu(rf.r.height);
+    rf.r.x = le32_to_cpu(rf.r.x);
+    rf.r.y = le32_to_cpu(rf.r.y);
+
     trace_virtio_gpu_cmd_res_flush(rf.resource_id,
                                    rf.r.width, rf.r.height, rf.r.x, rf.r.y);
 
@@ -511,6 +532,14 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
     struct virtio_gpu_set_scanout ss;
 
     VIRTIO_GPU_FILL_CMD(ss);
+
+    ss.scanout_id = le32_to_cpu(ss.scanout_id);
+    ss.resource_id = le32_to_cpu(ss.resource_id);
+    ss.r.width = le32_to_cpu(ss.r.width);
+    ss.r.height = le32_to_cpu(ss.r.height);
+    ss.r.x = le32_to_cpu(ss.r.x);
+    ss.r.y = le32_to_cpu(ss.r.y);
+
     trace_virtio_gpu_cmd_set_scanout(ss.scanout_id, ss.resource_id,
                                      ss.r.width, ss.r.height, ss.r.x, ss.r.y);
 
@@ -633,13 +662,15 @@ int virtio_gpu_create_mapping_iov(struct virtio_gpu_resource_attach_backing *ab,
         *addr = g_malloc0(sizeof(uint64_t) * ab->nr_entries);
     }
     for (i = 0; i < ab->nr_entries; i++) {
-        hwaddr len = ents[i].length;
-        (*iov)[i].iov_len = ents[i].length;
-        (*iov)[i].iov_base = cpu_physical_memory_map(ents[i].addr, &len, 1);
+        uint64_t a = le64_to_cpu(ents[i].addr);
+        uint32_t l = le32_to_cpu(ents[i].length);
+        hwaddr len = l;
+        (*iov)[i].iov_len = l;
+        (*iov)[i].iov_base = cpu_physical_memory_map(a, &len, 1);
         if (addr) {
-            (*addr)[i] = ents[i].addr;
+            (*addr)[i] = a;
         }
-        if (!(*iov)[i].iov_base || len != ents[i].length) {
+        if (!(*iov)[i].iov_base || len != l) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for"
                           " resource %d element %d\n",
                           __func__, ab->resource_id, i);
@@ -686,6 +717,10 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g,
     int ret;
 
     VIRTIO_GPU_FILL_CMD(ab);
+
+    ab.resource_id = le32_to_cpu(ab.resource_id);
+    ab.nr_entries = le32_to_cpu(ab.nr_entries);
+
     trace_virtio_gpu_cmd_res_back_attach(ab.resource_id);
 
     res = virtio_gpu_find_resource(g, ab.resource_id);
@@ -735,7 +770,7 @@ static void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
 {
     VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
 
-    switch (cmd->cmd_hdr.type) {
+    switch (le32_to_cpu(cmd->cmd_hdr.type)) {
     case VIRTIO_GPU_CMD_GET_DISPLAY_INFO:
         virtio_gpu_get_display_info(g, cmd);
         break;
@@ -1135,7 +1170,7 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
     }
 
     g->config_size = sizeof(struct virtio_gpu_config);
-    g->virtio_config.num_scanouts = g->conf.max_outputs;
+    g->virtio_config.num_scanouts = cpu_to_le32(g->conf.max_outputs);
     virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
                 g->config_size);
 
-- 
1.9.1

[Qemu-devel] [PATCH v1 2/2] virtio-gpu-ccw: Create a virtio gpu device for the ccw bus

[Farhan Ali]

Wire up the virtio-gpu device for the CCW bus. The virtio-gpu
is a virtio-1 device, so disable revision 0.

Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/virtio-ccw.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/s390x/virtio-ccw.h | 10 ++++++++++
 2 files changed, 64 insertions(+)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index b1976fd..3078bf0 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1007,6 +1007,20 @@ static void virtio_ccw_crypto_realize(VirtioCcwDevice *ccw_dev, Error **errp)
                              NULL);
 }
 
+static void virtio_ccw_gpu_realize(VirtioCcwDevice *ccw_dev, Error **errp)
+{
+    VirtIOGPUCcw *dev = VIRTIO_GPU_CCW(ccw_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+    Error *err = NULL;
+
+    qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
+    object_property_set_bool(OBJECT(vdev), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+}
+
 /* DeviceState to VirtioCcwDevice. Note: used on datapath,
  * be careful and test performance if you change this.
  */
@@ -1616,6 +1630,45 @@ static const TypeInfo virtio_ccw_crypto = {
     .class_init    = virtio_ccw_crypto_class_init,
 };
 
+static Property virtio_ccw_gpu_properties[] = {
+    DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
+                    VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
+                       VIRTIO_CCW_MAX_REV),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_ccw_gpu_instance_init(Object *obj)
+{
+    VirtIOGPUCcw *dev = VIRTIO_GPU_CCW(obj);
+    VirtioCcwDevice *ccw_dev = VIRTIO_CCW_DEVICE(obj);
+
+    ccw_dev->force_revision_1 = true;
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VIRTIO_GPU);
+}
+
+static void virtio_ccw_gpu_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_CLASS(klass);
+
+    k->realize = virtio_ccw_gpu_realize;
+    k->exit = virtio_ccw_exit;
+    dc->reset = virtio_ccw_reset;
+    dc->props = virtio_ccw_gpu_properties;
+    dc->hotpluggable = false;
+    set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+}
+
+static const TypeInfo virtio_ccw_gpu = {
+    .name          = TYPE_VIRTIO_GPU_CCW,
+    .parent        = TYPE_VIRTIO_CCW_DEVICE,
+    .instance_size = sizeof(VirtIOGPUCcw),
+    .instance_init = virtio_ccw_gpu_instance_init,
+    .class_init    = virtio_ccw_gpu_class_init,
+};
+
 static void virtio_ccw_busdev_realize(DeviceState *dev, Error **errp)
 {
     VirtioCcwDevice *_dev = (VirtioCcwDevice *)dev;
@@ -1815,6 +1868,7 @@ static void virtio_ccw_register(void)
     type_register_static(&vhost_vsock_ccw_info);
 #endif
     type_register_static(&virtio_ccw_crypto);
+    type_register_static(&virtio_ccw_gpu);
 }
 
 type_init(virtio_ccw_register)
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 41d4010..541fdd2 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -27,6 +27,7 @@
 #ifdef CONFIG_VHOST_VSOCK
 #include "hw/virtio/vhost-vsock.h"
 #endif /* CONFIG_VHOST_VSOCK */
+#include "hw/virtio/virtio-gpu.h"
 
 #include "hw/s390x/s390_flic.h"
 #include "hw/s390x/css.h"
@@ -223,4 +224,13 @@ typedef struct VHostVSockCCWState {
 
 #endif /* CONFIG_VHOST_VSOCK */
 
+#define TYPE_VIRTIO_GPU_CCW "virtio-gpu-ccw"
+#define VIRTIO_GPU_CCW(obj) \
+        OBJECT_CHECK(VirtIOGPUCcw, (obj), TYPE_VIRTIO_GPU_CCW)
+
+typedef struct VirtIOGPUCcw {
+    VirtioCcwDevice parent_obj;
+    VirtIOGPU vdev;
+} VirtIOGPUCcw;
+
 #endif
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 1/2] virtio_gpu: Handle endian conversion

[Gerd Hoffmann]

  Hi,

> @@ -287,6 +287,12 @@ static void
> virtio_gpu_resource_create_2d(VirtIOGPU *g,
>      struct virtio_gpu_resource_create_2d c2d;
>  
>      VIRTIO_GPU_FILL_CMD(c2d);
> +
> +    c2d.resource_id = le32_to_cpu(c2d.resource_id);
> +    c2d.format = le32_to_cpu(c2d.format);
> +    c2d.width = le32_to_cpu(c2d.width);
> +    c2d.height = le32_to_cpu(c2d.height);
> +

Please move this to a helper function, maybe by updating the
VIRTIO_GPU_FILL_CMD macro.

The header fields should be byteswapped too.  As most structs have
32bit fields only (with the exception of hdr.fence_id) you should be
able to create a generic byteswap function which only needs the struct
size as argument and handles all structs without addresses/offsets
(which are 64bit fields).

The conversion looks incomplete, at least virtio_gpu_ctrl_response will
need adaptions too.  It probably works by luck because the guest driver
uses fences only in virgl (3d) mode.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v1 2/2] virtio-gpu-ccw: Create a virtio gpu device for the ccw bus

[David Hildenbrand]

On 12.09.2017 16:26, Farhan Ali wrote:
> Wire up the virtio-gpu device for the CCW bus. The virtio-gpu
> is a virtio-1 device, so disable revision 0.
> 
> Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/s390x/virtio-ccw.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/s390x/virtio-ccw.h | 10 ++++++++++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index b1976fd..3078bf0 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -1007,6 +1007,20 @@ static void virtio_ccw_crypto_realize(VirtioCcwDevice *ccw_dev, Error **errp)
>                               NULL);
>  }
>  
> +static void virtio_ccw_gpu_realize(VirtioCcwDevice *ccw_dev, Error **errp)
> +{
> +    VirtIOGPUCcw *dev = VIRTIO_GPU_CCW(ccw_dev);
> +    DeviceState *vdev = DEVICE(&dev->vdev);
> +    Error *err = NULL;
> +
> +    qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
> +    object_property_set_bool(OBJECT(vdev), true, "realized", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }

You can call error_propagate() unconditionally, it can deal with !err.

> +}
> +
>  /* DeviceState to VirtioCcwDevice. Note: used on datapath,
>   * be careful and test performance if you change this.
>   */
> @@ -1616,6 +1630,45 @@ static const TypeInfo virtio_ccw_crypto = {
>      .class_init    = virtio_ccw_crypto_class_init,
>  };
>  
> +static Property virtio_ccw_gpu_properties[] = {
> +    DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
> +                    VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
> +    DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
> +                       VIRTIO_CCW_MAX_REV),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void virtio_ccw_gpu_instance_init(Object *obj)
> +{
> +    VirtIOGPUCcw *dev = VIRTIO_GPU_CCW(obj);
> +    VirtioCcwDevice *ccw_dev = VIRTIO_CCW_DEVICE(obj);
> +
> +    ccw_dev->force_revision_1 = true;
> +    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> +                                TYPE_VIRTIO_GPU);
> +}
> +
> +static void virtio_ccw_gpu_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_CLASS(klass);
> +
> +    k->realize = virtio_ccw_gpu_realize;
> +    k->exit = virtio_ccw_exit;
> +    dc->reset = virtio_ccw_reset;
> +    dc->props = virtio_ccw_gpu_properties;
> +    dc->hotpluggable = false;

Wonder if hotplug could work?


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v1 2/2] virtio-gpu-ccw: Create a virtio gpu device for the ccw bus

[Halil Pasic]

On 09/12/2017 04:26 PM, Farhan Ali wrote:
> Wire up the virtio-gpu device for the CCW bus. The virtio-gpu
> is a virtio-1 device, so disable revision 0.
> 
> Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.com>

[..]

Re: [Qemu-devel] [PATCH v1 2/2] virtio-gpu-ccw: Create a virtio gpu device for the ccw bus

[Gerd Hoffmann]

On Wed, 2017-09-13 at 14:09 +0200, David Hildenbrand wrote:
> > +    dc->props = virtio_ccw_gpu_properties;
> > +    dc->hotpluggable = false;
> 
> Wonder if hotplug could work?

No, ui/* doesn't support hotplug.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v1 1/2] virtio_gpu: Handle endian conversion

[Farhan Ali]

On 09/13/2017 04:13 AM, Gerd Hoffmann wrote:
> Please move this to a helper function, maybe by updating the
> VIRTIO_GPU_FILL_CMD macro.
>
> The header fields should be byteswapped too.  As most structs have
> 32bit fields only (with the exception of hdr.fence_id) you should be
> able to create a generic byteswap function which only needs the struct
> size as argument and handles all structs without addresses/offsets
> (which are 64bit fields).

I am not sure if I understand what you mean here. Since struct 
virtio_gpu_ctrl_hdr is part of every struct, so any such function
would also need to handle the case of hdr.fence_id, right?

>
> The conversion looks incomplete, at least virtio_gpu_ctrl_response will
> need adaptions too.  It probably works by luck because the guest driver
> uses fences only in virgl (3d) mode.
>

Oh right, I need to handle the conversion there as well. Thanks for 
catching that.

Also I believe this conversion patch isn't comprehensive, it's mostly 
the changes I made to get a display working on S390. So I appreciate
you reviewing the changes.

> cheers,
>   Gerd

Thanks
Farhan

Re: [Qemu-devel] [PATCH v1 2/2] virtio-gpu-ccw: Create a virtio gpu device for the ccw bus

[Thomas Huth]

On 13.09.2017 14:09, David Hildenbrand wrote:
> On 12.09.2017 16:26, Farhan Ali wrote:
>> Wire up the virtio-gpu device for the CCW bus. The virtio-gpu
>> is a virtio-1 device, so disable revision 0.
>>
>> Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com>
>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  hw/s390x/virtio-ccw.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/s390x/virtio-ccw.h | 10 ++++++++++
>>  2 files changed, 64 insertions(+)
>>
>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>> index b1976fd..3078bf0 100644
>> --- a/hw/s390x/virtio-ccw.c
>> +++ b/hw/s390x/virtio-ccw.c
>> @@ -1007,6 +1007,20 @@ static void virtio_ccw_crypto_realize(VirtioCcwDevice *ccw_dev, Error **errp)
>>                               NULL);
>>  }
>>  
>> +static void virtio_ccw_gpu_realize(VirtioCcwDevice *ccw_dev, Error **errp)
>> +{
>> +    VirtIOGPUCcw *dev = VIRTIO_GPU_CCW(ccw_dev);
>> +    DeviceState *vdev = DEVICE(&dev->vdev);
>> +    Error *err = NULL;
>> +
>> +    qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
>> +    object_property_set_bool(OBJECT(vdev), true, "realized", &err);
>> +    if (err) {
>> +        error_propagate(errp, err);
>> +        return;
>> +    }
> 
> You can call error_propagate() unconditionally, it can deal with !err.

I think you even do not need error_propagate here - just pass errp
directly to object_property_set_bool. You exit the function afterwards
anyway.

 Thomas

Re: [Qemu-devel] [PATCH v1 0/2] Virtio GPU for S390

[Thomas Huth]

On 12.09.2017 16:26, Farhan Ali wrote:
> These patches wire up the virtio-gpu device for CCW bus for
> S390.
> 
> For the S390 architecture which does not natively support any graphics
> device, virtio gpu in 2D mode could be used to emulate a simple graphics
> card and use VNC as the display.
> 
> eg: qemu-system-s390x ... -device virtio-gpu-ccw,devno=fe.0.0101
>     -vnc host_ip_addr:5900
> 
> Note, to actually see any display content the
> guest kernel needs to support DRM layer, Virtio GPU driver,
> the Virtual Terminal layer etc.

Do you have a list of CONFIG options that need to be enabled there?
Are there also any patches to the guest kernel driver required? Or did
that work out of the box once you've enabled the right CONFIG options?

> I would appreciate any feedback on these patches, specially the
> first patch.

Patches look good to me, but I'm not at all familiar with the virtio-gpu
code, so that likely does not count...

Anyway, thanks a lot for tackling this! It's pretty cool to finally have
a graphics card on s390x, too :-)

 Thomas

Re: [Qemu-devel] [PATCH v1 0/2] Virtio GPU for S390

[Farhan Ali]

On 09/13/2017 03:00 PM, Thomas Huth wrote:
> On 12.09.2017 16:26, Farhan Ali wrote:
>> These patches wire up the virtio-gpu device for CCW bus for S390.
>>
>> For the S390 architecture which does not natively support any
>> graphics device, virtio gpu in 2D mode could be used to emulate a
>> simple graphics card and use VNC as the display.
>>
>> eg: qemu-system-s390x ... -device virtio-gpu-ccw,devno=fe.0.0101
>> -vnc host_ip_addr:5900
>>
>> Note, to actually see any display content the guest kernel needs to
>> support DRM layer, Virtio GPU driver, the Virtual Terminal layer
>> etc.
>
> Do you have a list of CONFIG options that need to be enabled there?
> Are there also any patches to the guest kernel driver required? Or
> did that work out of the box once you've enabled the right CONFIG
> options?
>
It required some kernel hacking. You need to enable the VT layer for
S390 to get any kind of graphics displayed.

I experimented on the guest side to enable the VT layer and run a
framebuffer console and also the Xfce desktop :)

Anyway the CONFIG options I used are:

The DRM configs to enable the DRM layer and virtio-gpu. I went with the 
default options for DRM layer.

CONFIG_DRM

CONFIG_DRM_VIRTIO_GPU

We also need to enable configs for the VT layer

CONFIG_VT

CONFIG_DUMMY_CONSOLE

And to display a framebuffer console for the guest

CONFIG_FRAMEBUFFER_CONSOLE



>> I would appreciate any feedback on these patches, specially the
>> first patch.
>
> Patches look good to me, but I'm not at all familiar with the
> virtio-gpu code, so that likely does not count...
>
> Anyway, thanks a lot for tackling this! It's pretty cool to finally
> have a graphics card on s390x, too :-)
>
> Thomas
>

Re: [Qemu-devel] [PATCH v1 1/2] virtio_gpu: Handle endian conversion

[Gerd Hoffmann]

On Wed, 2017-09-13 at 11:53 -0400, Farhan Ali wrote:
> 
> On 09/13/2017 04:13 AM, Gerd Hoffmann wrote:
> > Please move this to a helper function, maybe by updating the
> > VIRTIO_GPU_FILL_CMD macro.
> > 
> > The header fields should be byteswapped too.  As most structs have
> > 32bit fields only (with the exception of hdr.fence_id) you should
> > be
> > able to create a generic byteswap function which only needs the
> > struct
> > size as argument and handles all structs without addresses/offsets
> > (which are 64bit fields).
> 
> I am not sure if I understand what you mean here. Since struct 
> virtio_gpu_ctrl_hdr is part of every struct, so any such function
> would also need to handle the case of hdr.fence_id, right?

Yes, but that is common in all structs.  You can have one function to
handle the header, one generic function (calls the header function for
the header and just does 32bit byteswaps for everything else), one
specific function for each operation which has a 64bit address
somewhere in the struct (which again can use the header function for
the header).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v1 0/2] Virtio GPU for S390

[Gerd Hoffmann]

  Hi,

> Do you have a list of CONFIG options that need to be enabled there?
> Are there also any patches to the guest kernel driver required?

guest kernel driver should be fine, it works for ppc64 (big endian)
guests.

cheers,
  Gerd