Re: [Qemu-devel] [PATCH v2 1/3] vfio/display: add edid support.

[Gerd Hoffmann]

On Wed, Feb 20, 2019 at 02:54:35PM -0700, Alex Williamson wrote:
> On Wed, 20 Feb 2019 09:47:51 +0100
> Gerd Hoffmann <address@hidden> wrote:
> 
> > This patch adds EDID support to the vfio display (aka vgpu) code.
> > When supported by the mdev driver qemu will generate a EDID blob
> > and pass it on using the new vfio edid region.  The EDID blob will
> > be updated on UI changes (i.e. window resize), so the guest can
> > adapt.
> 
> What are the requirements to enable this resizing feature?  I grabbed
> the gvt-next-2019-02-01 branch and my ever expanding qemu:commandline
> now looks like this:
> 
>   <qemu:commandline>
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.hostdev0.x-igd-opregion=on'/>
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.hostdev0.ramfb=on'/>
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.hostdev0.driver=vfio-pci-nohotplug'/>
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.hostdev0.xres=1600'/>
>     <qemu:arg value='-set'/>
>     <qemu:arg value='device.hostdev0.yres=900'/>
>   </qemu:commandline>
> 
> Other relevant sections:
> 
>     <graphics type='spice'>
>       <listen type='none'/>
>       <gl enable='yes' rendernode='/dev/dri/by-path/pci-0000:00:02.0-render'/>
>     </graphics>

When using spice you also need the spicevmc channel and the spice agent
being installed and active in the guest.

> > +    dpy->edid_regs->link_state = VFIO_DEVICE_GFX_LINK_STATE_UP;
> > +    pwrite_field(fd, dpy->edid_info, dpy->edid_regs, link_state);
> > +    trace_vfio_display_edid_link_up();
> > +    return;
> > +
> > +err:
> > +    trace_vfio_display_edid_write_error();
> > +    return;
> 
> nit, no unwind and only one call point, could probably do without the
> goto.

Not that easily due to the goto being hidden in the pwrite_field()
macro.

> > +    trace_vfio_display_edid_available();
> > +    dpy->edid_regs = g_new0(struct vfio_region_gfx_edid, 1);
> > +    pread_field(fd, dpy->edid_info, dpy->edid_regs, edid_offset);
> > +    pread_field(fd, dpy->edid_info, dpy->edid_regs, edid_max_size);
> > +    pread_field(fd, dpy->edid_info, dpy->edid_regs, max_xres);
> > +    pread_field(fd, dpy->edid_info, dpy->edid_regs, max_yres);
> > +    dpy->edid_blob = g_malloc0(dpy->edid_regs->edid_max_size);
> > +
> > +    vfio_display_edid_update(vdev, true, 0, 0);
> > +    return;
> > +
> > +err:
> > +    fprintf(stderr, "%s: Oops, pread error\n", __func__);
> > +    g_free(dpy->edid_regs);
> > +    dpy->edid_regs = NULL;
> > +    return;
> 
> This code is unreachable.

It's not.  Again, the goto is in pread_field.

But I just noticed I missed one fprintf which should be a
trace_vfio_display_edid_write_error() ...

cheers,
  Gerd