On Tue, 24 Sep 2019 14:41:39 +0800
Tina Zhang wrote:
> Introduce vGPU specific irq type VFIO_IRQ_TYPE_GFX, and
> VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ as the subtype for vGPU display.
>
> Introduce vfio_irq_info_cap_display_plane_events capability to notify
> user space with the vGPU's plane update events
>
> v3:
> - Add more description to VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ and
> VFIO_IRQ_INFO_CAP_DISPLAY. (Alex & Gerd)
>
> v2:
> - Add VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ description. (Alex & Kechen)
> - Introduce vfio_irq_info_cap_display_plane_events. (Gerd & Alex)
>
> Signed-off-by: Tina Zhang
> ---
> include/uapi/linux/vfio.h | 38 ++
> 1 file changed, 38 insertions(+)
>
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index aa6850f1daef..2946a028b0c3 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -476,6 +476,44 @@ struct vfio_irq_info_cap_type {
> __u32 subtype; /* type specific */
> };
>
> +/* vGPU IRQ TYPE */
> +#define VFIO_IRQ_TYPE_GFX(1)
> +
> +/* sub-types for VFIO_IRQ_TYPE_GFX */
> +/*
> + * vGPU device display refresh interrupt request. This irq asking for
> + * a user space display refresh, covers all display updates events,
> + * i.e. user space can stop the display update timer and fully depend
> + * on getting the notification if an update is needed.
> + */
> +#define VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ (1)
> +
> +/*
> + * Display capability of reporting display refresh interrupt events.
Perhaps, "Capability for interpreting GFX_DISPLAY_IRQ eventfd value"
> + * This gives user space the capability to identify different display
> + * updates events of the display refresh interrupt request.
> + *
> + * When notified by VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ, user space can
> + * use the eventfd counter value to identify which plane has been
> + * updated.
> + *
> + * Note that there might be some cases like counter_value >
> + * (cur_event_val + pri_event_val), if notifications haven't been
> + * handled on time in user mode. These cases can be handled as all
> + * plane updated case or signle plane updated case depending on the
> + * value.
Seems like in the GVT-g implementation such a value is not possible.
In fact, when this capability is provided, doesn't userspace interpret
the eventfd value more as a bitmask of events rather than a counter?
If so, (cur_event_val + pri_event_val) may be mathematically accurate,
but maybe obfuscates the logical interpretation... or maybe that's just
me.
> + *
> + * cur_event_val: eventfd counter value for cursor plane change event.
> + * pri_event_val: eventfd counter value for primary plane change event.
I think there should be a note that this capability is optional and
lacking this feature, userspace should refresh all display events on
notification.
> + */
> +#define VFIO_IRQ_INFO_CAP_DISPLAY2
> +
> +struct vfio_irq_info_cap_display_plane_events {
> + struct vfio_info_cap_header header;
> + __u64 cur_event_val;
> + __u64 pri_event_val;
AIUI, the GVT-g implementation sets a single bit and userspace expects
one or both of those bits to be set on notification. Should we
simplify this a bit and just define these as cur_event_bit,
pri_event_bit and use a __u8 for each to define the bit position?
Thanks,
Alex
