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

[Lu, Kechen]

Hi,

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw@xxxxxxxxxxxxxxx]
> Sent: Friday, July 19, 2019 2:06 PM
> To: Lu, Kechen <kechen.lu@xxxxxxxxx>
> Cc: intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; linux- 
> kernel@xxxxxxxxxxxxxxx; Zhang, Tina <tina.zhang@xxxxxxxxx>; 
> 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; Eric Auger <eric.auger@xxxxxxxxxx>
> 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@xxxxxxxxx>
> >
> > Cap the number of irqs with fixed indexes and use capability chains 
> > to chain device specific irqs.
> >
> > Signed-off-by: Tina Zhang <tina.zhang@xxxxxxxxx>
> > Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx>
> > ---
> >  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