RE: [RFC PATCH v2 1/3] vfio: Use capability chains to handle device specific irq

[Zhang, Tina]

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@xxxxxxxxxxxxxxx]
> Sent: Wednesday, June 5, 2019 12:05 PM
> To: Zhang, Tina <tina.zhang@xxxxxxxxx>
> Cc: intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; kraxel@xxxxxxxxxx; zhenyuw@xxxxxxxxxxxxxxx; Lv,
> Zhiyuan <zhiyuan.lv@xxxxxxxxx>; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>; Tian,
> Kevin <kevin.tian@xxxxxxxxx>; Yuan, Hang <hang.yuan@xxxxxxxxx>;
> alex.williamson@xxxxxxxxxx
> Subject: Re: [RFC PATCH v2 1/3] vfio: Use capability chains to handle device
> specific irq
> 
> On 2019.06.04 17:55:32 +0800, Tina Zhang wrote:
> > Caps the number of irqs with fixed indexes and uses capability chains
> > to chain device specific irqs.
> >
> > VFIO vGPU leverages this mechanism to trigger primary plane and cursor
> > plane page flip event to the user space.
> >
> > Signed-off-by: Tina Zhang <tina.zhang@xxxxxxxxx>
> > ---
> >  include/uapi/linux/vfio.h | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index 02bb7ad6e986..9b5e25937c7d 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -444,11 +444,31 @@ struct vfio_irq_info {
> >  #define VFIO_IRQ_INFO_MASKABLE		(1 << 1)
> >  #define VFIO_IRQ_INFO_AUTOMASKED	(1 << 2)
> >  #define VFIO_IRQ_INFO_NORESIZE		(1 << 3)
> > +#define VFIO_IRQ_INFO_FLAG_CAPS		(1 << 4) /* Info
> supports caps */
> >  	__u32	index;		/* IRQ index */
> > +	__u32	cap_offset;	/* Offset within info struct of first cap */
> >  	__u32	count;		/* Number of IRQs within this index */
> 
> This would break ABI for get irq info. I think irq cap chain can just follow
> vfio_irq_info.
> 
> >  };
> >  #define VFIO_DEVICE_GET_IRQ_INFO	_IO(VFIO_TYPE, VFIO_BASE +
> 9)
> >
> > +/*
> > + * The irq type capability allows irqs unique to a specific device or
> > + * class of devices to be exposed.
> > + *
> > + * The structures below define version 1 of this capability.
> > + */
> > +#define VFIO_IRQ_INFO_CAP_TYPE      3
> > +
> > +struct vfio_irq_info_cap_type {
> > +	struct vfio_info_cap_header header;
> > +	__u32 type;     /* global per bus driver */
> > +	__u32 subtype;  /* type specific */
> > +};
> > +
> > +#define VFIO_IRQ_TYPE_GFX				(1)
> > +#define VFIO_IRQ_SUBTYPE_GFX_PRI_PLANE_FLIP		(1)
> > +#define VFIO_IRQ_SUBTYPE_GFX_CUR_PLANE_FLIP		(2)
> > +
> 
> Really need to split for different planes? I'd like a
> VFIO_IRQ_SUBTYPE_GFX_DISPLAY_EVENT
> so user space can probe change for all.
User space can choose to user different handlers according to the specific event. For example, user space might not want to handle every cursor event due to performance consideration. Besides, it can reduce the probe times, as we don't need to probe twice to make sure if both cursor plane and primary plane have been updated.
Thanks.

BR,
Tina

> 
> >  /**
> >   * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct
> vfio_irq_set)
> >   *
> > @@ -550,7 +570,8 @@ enum {
> >  	VFIO_PCI_MSIX_IRQ_INDEX,
> >  	VFIO_PCI_ERR_IRQ_INDEX,
> >  	VFIO_PCI_REQ_IRQ_INDEX,
> > -	VFIO_PCI_NUM_IRQS
> > +	VFIO_PCI_NUM_IRQS = 5	/* Fixed user ABI, IRQ indexes >=5
> use   */
> > +				/* device specific cap to define content */
> >  };
> >
> >  /*
> > --
> > 2.17.1
> >
> 
> --
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827