RfC: vfio: add vgpu edid support?

[Gerd Hoffmann]

> > Subject: [PATCH] [RfC] vfio: edid interface
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>
> > ---
> >  include/uapi/linux/vfio.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> > 
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index 1aa7b82e81..9ac7dfb7c1 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -602,6 +602,48 @@ struct vfio_device_ioeventfd {
> >  
> >  #define VFIO_DEVICE_IOEVENTFD		_IO(VFIO_TYPE, VFIO_BASE + 16)
> >  
> > +/**
> > + * VFIO_DEVICE_GFX_EDID_CAPS - _IOW(VFIO_TYPE, VFIO_BASE + 17,
> > + *                                  struct vfio_device_gfx_edid_caps)
> 
> We haven't been very consistent with the _IOW vs _IOR vs _IOWR and as a
> comment it's obviously not enforced, only meant to convey the nature of
> the ioctl, but _IOW is probably the one choice that least matches.  We
> have other examples of using _IOR (ignoring that argsz is passed in)
> and _IOWR, which is technically more accurate.  Looks like we missed
> this for the other GFX ioctls as well.

Should be IORW (and IOW for SET), I'll fix.

> I'd prefer to see "GET" in the ioctl name as well, perhaps
> VFIO_DEVICE_GET_GFX_EDID_CAPS.  Is this really EDID when it's only the
> resolution though?

It is supposed to return the capabilities of the device, such as
1024x768 resolution limit of the smallest intel vgpu type, so the edid
blob generated by qemu will not contain resolutions higher than that.

> > + * VFIO_DEVICE_GFX_EDID_SET - _IOW(VFIO_TYPE, VFIO_BASE + 18,
> > + *                                 struct vfio_device_gfx_edid_set)
> 
> Perhaps flip this around, VFIO_DEVICE_SET_GFX_EDID.

I kind of like my naming, as the ioctls two edid ioctls will have the
same prefix then (VFIO_DEVICE_GFX_EDID_*).  Matter of taste though.  And
the other GFX ioctls don't match the scheme (VFIO_DEVICE_GFX_* prefix)
either ...

> > + * Set edid blob.
> > + *
> > + * Should trigger monitor hotplug emulation, to notifiy the guest os
> > + * that the edid has changed.
> > + *
> > + */
> > +struct vfio_device_gfx_edid_set {
> > +	__u32 argsz;
> > +	__u32 flags;
> > +	/* in */
> > +	__u8  edid[256];
> 
> Is the format of this blob defined somewhere?  Can we provide a
> reference here to that format?  Thanks,

Spec seems not to be public.  Wikipedia has this:

https://en.wikipedia.org/wiki/Extended_Display_Identification_Data

Googling finds you pdfs of older revisions of the spec, but I've used
the Wikipedia page to write the generator.

cheers,
  Gerd