diff mbox series

[RFC,v4,1/6] vfio: Define device specific irq type capability

Message ID 20190718155640.25928-2-kechen.lu@intel.com (mailing list archive)
State New, archived
Headers show
Series Deliver vGPU display refresh event to userspace | expand

Commit Message

Lu, Kechen July 18, 2019, 3:56 p.m. UTC 
From: Tina Zhang <tina.zhang@intel.com>

Cap the number of irqs with fixed indexes and use capability chains
to chain device specific irqs.

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/uapi/linux/vfio.h | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Comments

Zhenyu Wang July 19, 2019, 6:05 a.m. UTC | #1 
On 2019.07.18 23:56:35 +0800, Kechen Lu wrote:
> From: Tina Zhang <tina.zhang@intel.com>
> 
> Cap the number of irqs with fixed indexes and use capability chains
> to chain device specific irqs.
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  include/uapi/linux/vfio.h | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 8f10748dac79..be6adab4f759 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -448,11 +448,27 @@ 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	count;		/* Number of IRQs within this index */
> +	__u32	cap_offset;	/* Offset within info struct of first cap */

This still breaks ABI as argsz would be updated with this new field, so it would
cause compat issue. I think my last suggestion was to assume cap list starts after
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 */
> +};
> +
>  /**
>   * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct vfio_irq_set)
>   *
> @@ -554,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
>
Lu, Kechen July 19, 2019, 9:02 a.m. UTC | #2 
Hi,

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> Sent: Friday, July 19, 2019 2:06 PM
> To: Lu, Kechen <kechen.lu@intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org; kvm@vger.kernel.org; linux- 
> kernel@vger.kernel.org; Zhang, Tina <tina.zhang@intel.com>; 
> kraxel@redhat.com; zhenyuw@linux.intel.com; Lv, Zhiyuan 
> <zhiyuan.lv@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, 
> Kevin <kevin.tian@intel.com>; Yuan, Hang <hang.yuan@intel.com>; 
> alex.williamson@redhat.com; Eric Auger <eric.auger@redhat.com>
> Subject: Re: [RFC PATCH v4 1/6] vfio: Define device specific irq type 
> capability
> 
> On 2019.07.18 23:56:35 +0800, Kechen Lu wrote:
> > From: Tina Zhang <tina.zhang@intel.com>
> >
> > Cap the number of irqs with fixed indexes and use capability chains 
> > to chain device specific irqs.
> >
> > Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> > ---
> >  include/uapi/linux/vfio.h | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h 
> > index 8f10748dac79..be6adab4f759 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -448,11 +448,27 @@ 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	count;		/* Number of IRQs within this index */
> > +	__u32	cap_offset;	/* Offset within info struct of first cap */
> 
> This still breaks ABI as argsz would be updated with this new field, 
> so it would cause compat issue. I think my last suggestion was to 
> assume cap list starts after vfio_irq_info.
>
 
In the common practice, the general logic is first use the "count" as the "minsz" boundary to perform copy from user, and then perform following logic, so that the incompatibility issue would not happen. BTW, this patch has been double checked by Eric Auger before included in his patch-set. 

Best Regards,
Kechen

> >  };
> >  #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 */ };
> > +
> >  /**
> >   * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct
> vfio_irq_set)
> >   *
> > @@ -554,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
Zhenyu Wang July 19, 2019, 9:59 a.m. UTC | #3 
On 2019.07.19 09:02:33 +0000, Lu, Kechen wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
> > Sent: Friday, July 19, 2019 2:06 PM
> > To: Lu, Kechen <kechen.lu@intel.com>
> > Cc: intel-gvt-dev@lists.freedesktop.org; kvm@vger.kernel.org; linux- 
> > kernel@vger.kernel.org; Zhang, Tina <tina.zhang@intel.com>; 
> > kraxel@redhat.com; zhenyuw@linux.intel.com; Lv, Zhiyuan 
> > <zhiyuan.lv@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, 
> > Kevin <kevin.tian@intel.com>; Yuan, Hang <hang.yuan@intel.com>; 
> > alex.williamson@redhat.com; Eric Auger <eric.auger@redhat.com>
> > Subject: Re: [RFC PATCH v4 1/6] vfio: Define device specific irq type 
> > capability
> > 
> > On 2019.07.18 23:56:35 +0800, Kechen Lu wrote:
> > > From: Tina Zhang <tina.zhang@intel.com>
> > >
> > > Cap the number of irqs with fixed indexes and use capability chains 
> > > to chain device specific irqs.
> > >
> > > Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> > > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> > > ---
> > >  include/uapi/linux/vfio.h | 19 ++++++++++++++++++-
> > >  1 file changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h 
> > > index 8f10748dac79..be6adab4f759 100644
> > > --- a/include/uapi/linux/vfio.h
> > > +++ b/include/uapi/linux/vfio.h
> > > @@ -448,11 +448,27 @@ 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	count;		/* Number of IRQs within this index */
> > > +	__u32	cap_offset;	/* Offset within info struct of first cap */
> > 
> > This still breaks ABI as argsz would be updated with this new field, 
> > so it would cause compat issue. I think my last suggestion was to 
> > assume cap list starts after vfio_irq_info.
> >
>  
> In the common practice, the general logic is first use the "count" as the "minsz" boundary to perform copy from user, and then perform following logic, so that the incompatibility issue would not happen. BTW, this patch has been double checked by Eric Auger before included in his patch-set. 
> 

yeah, sorry I was thinking vfio might fail in that case but it seems
current code assume argsz should be larger than minsz for count here,
so that's fine.

> 
> > >  };
> > >  #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 */ };
> > > +
> > >  /**
> > >   * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct
> > vfio_irq_set)
> > >   *
> > > @@ -554,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
diff mbox series

Patch

diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 8f10748dac79..be6adab4f759 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -448,11 +448,27 @@  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	count;		/* Number of IRQs within this index */
+	__u32	cap_offset;	/* Offset within info struct of first cap */
 };
 #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 */
+};
+
 /**
  * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct vfio_irq_set)
  *
@@ -554,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 */
 };
 
 /*