| Message ID | 20190718155640.25928-3-kechen.lu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Deliver vGPU display refresh event to userspace | expand |
On Thu, 18 Jul 2019 23:56:36 +0800 Kechen Lu <kechen.lu@intel.com> wrote: > From: Tina Zhang <tina.zhang@intel.com> > > 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@intel.com> > --- > 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 > /** > * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct vfio_irq_set) > *
Hi, > -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Saturday, July 20, 2019 12:25 AM > 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> > 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@intel.com> wrote: > > > From: Tina Zhang <tina.zhang@intel.com> > > > > 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@intel.com> > > --- > > 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) > > *
On Mon, 22 Jul 2019 05:28:35 +0000 "Lu, Kechen" <kechen.lu@intel.com> wrote: > Hi, > > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > Sent: Saturday, July 20, 2019 12:25 AM > > 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> > > 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@intel.com> wrote: > > > > > From: Tina Zhang <tina.zhang@intel.com> > > > > > > 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@intel.com> > > > --- > > > 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! Couldn't you expose this as another capability within the IRQ_INFO return data? If you were to define it as a macro, I assume that means it would be hard coded, in which case this probably becomes an Intel specific IRQ, rather than what appears to be framed as a generic graphics IRQ extension. A new capability could instead allow the vendor to specify their own value, where we could define how userspace should interpret and make use of this value. Thanks, Alex
> -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Tuesday, July 23, 2019 3:41 AM > 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> > Subject: Re: [RFC PATCH v4 2/6] vfio: Introduce vGPU display irq type > > On Mon, 22 Jul 2019 05:28:35 +0000 > "Lu, Kechen" <kechen.lu@intel.com> wrote: > > > Hi, > > > > > -----Original Message----- > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > Sent: Saturday, July 20, 2019 12:25 AM > > > 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> > > > 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@intel.com> wrote: > > > > > > > From: Tina Zhang <tina.zhang@intel.com> > > > > > > > > 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@intel.com> > > > > --- > > > > 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! > > Couldn't you expose this as another capability within the IRQ_INFO return > data? If you were to define it as a macro, I assume that means it would be > hard coded, in which case this probably becomes an Intel specific IRQ, rather > than what appears to be framed as a generic graphics IRQ extension. A new > capability could instead allow the vendor to specify their own value, where > we could define how userspace should interpret and make use of this value. > Thanks, Good suggestion. Currently, vfio_irq_info is used to save one irq info. What we need here is to use it to save several events info. Maybe we could figure out a general layout of this capability so that it can be leveraged by others, not only for display irq/events. Thanks. BR, Tina > > Alex
On Tue, 23 Jul 2019 01:08:19 +0000 "Zhang, Tina" <tina.zhang@intel.com> wrote: > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > Sent: Tuesday, July 23, 2019 3:41 AM > > 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> > > Subject: Re: [RFC PATCH v4 2/6] vfio: Introduce vGPU display irq type > > > > On Mon, 22 Jul 2019 05:28:35 +0000 > > "Lu, Kechen" <kechen.lu@intel.com> wrote: > > > > > Hi, > > > > > > > -----Original Message----- > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > Sent: Saturday, July 20, 2019 12:25 AM > > > > 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> > > > > 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@intel.com> wrote: > > > > > > > > > From: Tina Zhang <tina.zhang@intel.com> > > > > > > > > > > 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@intel.com> > > > > > --- > > > > > 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! > > > > Couldn't you expose this as another capability within the IRQ_INFO return > > data? If you were to define it as a macro, I assume that means it would be > > hard coded, in which case this probably becomes an Intel specific IRQ, rather > > than what appears to be framed as a generic graphics IRQ extension. A new > > capability could instead allow the vendor to specify their own value, where > > we could define how userspace should interpret and make use of this value. > > Thanks, > Good suggestion. Currently, vfio_irq_info is used to save one irq > info. What we need here is to use it to save several events info. > Maybe we could figure out a general layout of this capability so that > it can be leveraged by others, not only for display irq/events. You could also expose a device specific IRQ with count > 1 (ie. similar to MSI/X) and avoid munging the eventfd value, which is not something we do elsewhere, at least in vfio. Thanks, Alex
> -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Tuesday, July 23, 2019 9:19 AM > To: Zhang, Tina <tina.zhang@intel.com> > Cc: Lu, Kechen <kechen.lu@intel.com>; intel-gvt-dev@lists.freedesktop.org; > kvm@vger.kernel.org; linux-kernel@vger.kernel.org; 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> > Subject: Re: [RFC PATCH v4 2/6] vfio: Introduce vGPU display irq type > > On Tue, 23 Jul 2019 01:08:19 +0000 > "Zhang, Tina" <tina.zhang@intel.com> wrote: > > > > -----Original Message----- > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > Sent: Tuesday, July 23, 2019 3:41 AM > > > 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> > > > Subject: Re: [RFC PATCH v4 2/6] vfio: Introduce vGPU display irq > > > type > > > > > > On Mon, 22 Jul 2019 05:28:35 +0000 > > > "Lu, Kechen" <kechen.lu@intel.com> wrote: > > > > > > > Hi, > > > > > > > > > -----Original Message----- > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > > > > Sent: Saturday, July 20, 2019 12:25 AM > > > > > 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> > > > > > 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@intel.com> wrote: > > > > > > > > > > > From: Tina Zhang <tina.zhang@intel.com> > > > > > > > > > > > > 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@intel.com> > > > > > > --- > > > > > > 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! > > > > > > Couldn't you expose this as another capability within the IRQ_INFO > > > return data? If you were to define it as a macro, I assume that > > > means it would be hard coded, in which case this probably becomes an > > > Intel specific IRQ, rather than what appears to be framed as a > > > generic graphics IRQ extension. A new capability could instead > > > allow the vendor to specify their own value, where we could define how > userspace should interpret and make use of this value. > > > Thanks, > > Good suggestion. Currently, vfio_irq_info is used to save one irq > > info. What we need here is to use it to save several events info. > > Maybe we could figure out a general layout of this capability so that > > it can be leveraged by others, not only for display irq/events. > > You could also expose a device specific IRQ with count > 1 (ie. similar to > MSI/X) and avoid munging the eventfd value, which is not something we do > elsewhere, at least in vfio. Thanks, Actually, we had this implementation before. At that time, we got the suggestion that count > 1 means more than one eventfd which might be not necessary. Anyway, we can consider the "count > 1" again if anyone is agree on this. Thanks BR, Tina > > Alex
Hi, > > > Couldn't you expose this as another capability within the IRQ_INFO return > > > data? If you were to define it as a macro, I assume that means it would be > > > hard coded, in which case this probably becomes an Intel specific IRQ, rather > > > than what appears to be framed as a generic graphics IRQ extension. A new > > > capability could instead allow the vendor to specify their own value, where > > > we could define how userspace should interpret and make use of this value. > > > Thanks, > > Good suggestion. Currently, vfio_irq_info is used to save one irq > > info. What we need here is to use it to save several events info. > > Maybe we could figure out a general layout of this capability so that > > it can be leveraged by others, not only for display irq/events. > > You could also expose a device specific IRQ with count > 1 (ie. similar > to MSI/X) and avoid munging the eventfd value, which is not something > we do elsewhere, at least in vfio. Thanks, Well, the basic idea is to use the eventfd value to signal the kind of changes which did happen, simliar to IRQ status register bits. So, when the guest changes the primary plane, the mdev driver notes this. Same with the cursor plane. On vblank (when the guests update is actually applied) the mdev driver wakes the eventfd and uses eventfd value to signal whenever primary plane or cursor plane or both did change. Then userspace knows which planes need an update without an extra VFIO_DEVICE_QUERY_GFX_PLANE roundtrip to the kernel. Alternatively we could have one eventfd for each change type. But given that these changes are typically applied at the same time (vblank) we would have multiple eventfds being signaled at the same time. Which doesn't look ideal to me ... cheers, Gerd
On Fri, 2 Aug 2019 15:35:31 +0200 "kraxel@redhat.com" <kraxel@redhat.com> wrote: > Hi, > > > > > Couldn't you expose this as another capability within the IRQ_INFO return > > > > data? If you were to define it as a macro, I assume that means it would be > > > > hard coded, in which case this probably becomes an Intel specific IRQ, rather > > > > than what appears to be framed as a generic graphics IRQ extension. A new > > > > capability could instead allow the vendor to specify their own value, where > > > > we could define how userspace should interpret and make use of this value. > > > > Thanks, > > > Good suggestion. Currently, vfio_irq_info is used to save one irq > > > info. What we need here is to use it to save several events info. > > > Maybe we could figure out a general layout of this capability so that > > > it can be leveraged by others, not only for display irq/events. > > > > You could also expose a device specific IRQ with count > 1 (ie. similar > > to MSI/X) and avoid munging the eventfd value, which is not something > > we do elsewhere, at least in vfio. Thanks, > > Well, the basic idea is to use the eventfd value to signal the kind of > changes which did happen, simliar to IRQ status register bits. > > So, when the guest changes the primary plane, the mdev driver notes > this. Same with the cursor plane. On vblank (when the guests update > is actually applied) the mdev driver wakes the eventfd and uses eventfd > value to signal whenever primary plane or cursor plane or both did > change. > > Then userspace knows which planes need an update without an extra > VFIO_DEVICE_QUERY_GFX_PLANE roundtrip to the kernel. > > Alternatively we could have one eventfd for each change type. But given > that these changes are typically applied at the same time (vblank) we > would have multiple eventfds being signaled at the same time. Which > doesn't look ideal to me ... Good point, looking at the bits in the eventfd value seems better than a flood of concurrent interrupts. Thanks, Alex
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) + /** * VFIO_DEVICE_SET_IRQS - _IOW(VFIO_TYPE, VFIO_BASE + 10, struct vfio_irq_set) *