From: Alex Williamson <alex.williamson@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>,
intel-gvt-dev@lists.freedesktop.org, kvm@vger.kernel.org,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] vfio: add edid api for display (vgpu) devices.
Date: Fri, 14 Sep 2018 09:31:28 -0600 [thread overview]
Message-ID: <20180914093128.49db91a6@t450s.home> (raw)
In-Reply-To: <20180914122552.3xmepqo7azzmx7ky@sirius.home.kraxel.org>
On Fri, 14 Sep 2018 14:25:52 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Hi,
>
> > Another possible implementation would be via a vfio region, we already
> > support device specific regions via capabilities with vfio_region_info,
> > so we could have an edid region which could handle both input and
> > output using a defined structure and protocol within the region. With
> > an edid blob of up to 512 bytes now, that means the vendor driver would
> > need to buffer writes to that section of the region until some sort of
> > activation, possibly using another "register" within the field to
> > trigger the link state and only processing the edid blob on link down
> > to link up transition.
>
> Hmm, using a virtual register space makes things more complicated for no
> good reason. This is a configuration interface for qemu, not something
> we expose to the guest. So, I'm not a fan ...
Ok, I'm curious what makes it more complicated though and why it
matters that it's for QEMU vs exposed to the guest. From the user
perspective, it's just a few pwrites rather than an ioctl. If we use
the ioctl, I think we still need to improve the protocol, it's
confusing that the user can specify both an EDID and a link state.
Does the user need to toggle the link state when setting an EDID or
does that happen automatically? How are link state vs EDID specified?
Simply link_state=0 and edid_size=0 is reserved as not provided?
> New revision of the vfio.h patch attached below, how does that look
> like? If it is ok I'll go continue with that next week (more verbose
> docs, update qemu code and test, ...)
Yes, modulo the ioctl protocol questions above. Thanks,
Alex
> From 818f2ea4dd756d28908e58a32a2fdd0d197a28da Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann <kraxel@redhat.com>
> Date: Thu, 6 Sep 2018 16:17:17 +0200
> Subject: [PATCH] vfio: add edid api for display (vgpu) devices.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> include/uapi/linux/vfio.h | 48 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 1aa7b82e81..901f279033 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -200,12 +200,25 @@ struct vfio_device_info {
> #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */
> #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */
> #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */
> +#define VFIO_DEVICE_FLAGS_CAPS (1 << 5) /* cap_offset present */
> __u32 num_regions; /* Max region index + 1 */
> __u32 num_irqs; /* Max IRQ index + 1 */
> + __u32 cap_offset; /* Offset within info struct of first cap */
> };
> #define VFIO_DEVICE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 7)
>
> /*
> + * FIXME: docs ...
> + */
> +#define VFIO_DEVICE_INFO_CAP_EDID 1
> +
> +struct vfio_device_info_edid_cap {
> + struct vfio_info_cap_header header;
> + __u32 max_x; /* Max display height (zero == no limit) */
> + __u32 max_y; /* Max display height (zero == no limit) */
> +};
> +
> +/*
> * Vendor driver using Mediated device framework should provide device_api
> * attribute in supported type attribute groups. Device API string should be one
> * of the following corresponding to device flags in vfio_device_info structure.
> @@ -602,6 +615,41 @@ struct vfio_device_ioeventfd {
>
> #define VFIO_DEVICE_IOEVENTFD _IO(VFIO_TYPE, VFIO_BASE + 16)
>
> +/**
> + * VFIO_DEVICE_SET_GFX_EDID - _IOW(VFIO_TYPE, VFIO_BASE + 17,
> + * struct vfio_device_set_gfx_edid)
> + *
> + * Set display link state and edid blob (for UP state).
> + *
> + * For the edid blob spec look here:
> + * https://en.wikipedia.org/wiki/Extended_Display_Identification_Data
> + *
> + * The guest should be notified about edid changes, for example by
> + * setting the link status to down temporarely (emulate monitor
> + * hotplug).
> + *
> + * @link_state:
> + * VFIO_DEVICE_GFX_LINK_STATE_UP: Monitor is turned on.
> + * VFIO_DEVICE_GFX_LINK_STATE_DOWN: Monitor is turned off.
> + *
> + * @edid_size: Size of the edid data blob.
> + * @edid_blob: The actual edid data.
> + *
> + * Returns 0 on success, error code (such as -EINVAL) on failure.
> + */
> +struct vfio_device_set_gfx_edid {
> + __u32 argsz;
> + __u32 flags;
> + /* in */
> + __u32 link_state;
> +#define VFIO_DEVICE_GFX_LINK_STATE_UP 1
> +#define VFIO_DEVICE_GFX_LINK_STATE_DOWN 2
> + __u32 edid_size;
> + __u8 edid_blob[512];
> +};
> +
> +#define VFIO_DEVICE_SET_GFX_EDID _IO(VFIO_TYPE, VFIO_BASE + 17)
> +
> /* -------- API for Type1 VFIO IOMMU -------- */
>
> /**
next prev parent reply other threads:[~2018-09-14 15:31 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20180913054745.6353-1-kraxel@redhat.com>
2018-09-13 5:47 ` [PATCH 1/2] vfio: add edid api for display (vgpu) devices Gerd Hoffmann
2018-09-13 17:51 ` Alex Williamson
2018-09-14 12:25 ` Gerd Hoffmann
2018-09-14 15:31 ` Alex Williamson [this message]
2018-09-17 6:51 ` Zhenyu Wang
2018-09-17 8:50 ` Gerd Hoffmann
2018-09-17 9:15 ` Zhenyu Wang
2018-09-17 10:13 ` Gerd Hoffmann
2018-09-13 5:47 ` [PATCH 2/2] vfio: add edid support to mbochs sample driver Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180914093128.49db91a6@t450s.home \
--to=alex.williamson@redhat.com \
--cc=intel-gvt-dev@lists.freedesktop.org \
--cc=kraxel@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=kwankhede@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).