RE: [RFC PATCH v4 2/6] vfio: Introduce vGPU display irq type

[Lu, Kechen]

Hi, 

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx]
> Sent: Saturday, July 20, 2019 12:25 AM
> 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>
> Subject: Re: [RFC PATCH v4 2/6] vfio: Introduce vGPU display irq type
> 
> On Thu, 18 Jul 2019 23:56:36 +0800
> Kechen Lu <kechen.lu@xxxxxxxxx> wrote:
> 
> > From: Tina Zhang <tina.zhang@xxxxxxxxx>
> >
> > Introduce vGPU specific irq type VFIO_IRQ_TYPE_GFX, and
> > VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ as the subtype for vGPU display
> >
> > Signed-off-by: Tina Zhang <tina.zhang@xxxxxxxxx>
> > ---
> >  include/uapi/linux/vfio.h | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index be6adab4f759..df28b17a6e2e 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -469,6 +469,9 @@ struct vfio_irq_info_cap_type {
> >  	__u32 subtype;  /* type specific */
> >  };
> >
> > +#define VFIO_IRQ_TYPE_GFX				(1)
> > +#define VFIO_IRQ_SUBTYPE_GFX_DISPLAY_IRQ		(1)
> > +
> 
> Please include a description defining exactly what this IRQ is intended to signal.
> For instance, if another vGPU vendor wanted to implement this in their driver
> and didn't have the QEMU code for reference to what it does with the IRQ, what
> would they need to know?  Thanks,
> 
> Alex
> 

Yes, that makes more sense. I'll add the description for it at next version patch.

BTW, may I have one more question? In the current design ideas, we partitioned 
the vGPU display eventfd counted 8-byte value into at most 8 events to deliver 
multiple display events, so we need different increasement counter value to 
differentiate the events. As this is the exposed thing the QEMU has to know, we
plan adds a macro here VFIO_IRQ_SUBTYPE_GFX_DISPLAY_EVENTFD_BASE_SHIFT to
make sure the partitions shift in 1 byte, does it make sense putting here? Looking  
forward to your and Gerd's comments. Thanks!


Best Regards,
Kechen

> >  /**
> >   * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct
> vfio_irq_set)
> >   *