| Message ID | 15-v1-7dedf20b2b75+4f785-vfio2_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make vfio_mdev type safe | expand |
On Tue, Mar 23, 2021 at 02:55:32PM -0300, Jason Gunthorpe wrote: > Ideally all of this would be moved to kvmgt.c, but it is entangled with > the rest of the "generic" code in an odd way. Thus put in a kconfig > dependency so we don't get randconfig failures when the next patch creates > a link time dependency related to the use of MDEV_TYPE. Ideally that weird struct intel_gvt_mpt would go away entirely. But that is clearly out of scope for this patchset.. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Tue, Mar 23, 2021 at 08:26:30PM +0100, Christoph Hellwig wrote: > On Tue, Mar 23, 2021 at 02:55:32PM -0300, Jason Gunthorpe wrote: > > Ideally all of this would be moved to kvmgt.c, but it is entangled with > > the rest of the "generic" code in an odd way. Thus put in a kconfig > > dependency so we don't get randconfig failures when the next patch creates > > a link time dependency related to the use of MDEV_TYPE. > > Ideally that weird struct intel_gvt_mpt would go away entirely. But > that is clearly out of scope for this patchset.. Yes.. Maybe someone from Intel will take that on, along with that other note you had. Compared to all the others this driver is quite twisty! Jason
> From: Jason Gunthorpe > Sent: Wednesday, March 24, 2021 1:56 AM > > At some point there may have been some reason for this weird split in this > driver, but today only the VFIO side is actually implemented. > > However, it got messed up at some point and mdev code was put in gvt.c and > is pretending to be "generic" by masquerading as some generic attribute list: > > static MDEV_TYPE_ATTR_RO(description); > > But MDEV_TYPE attributes are only usable with mdev_device, nothing else. > > Ideally all of this would be moved to kvmgt.c, but it is entangled with > the rest of the "generic" code in an odd way. Thus put in a kconfig > dependency so we don't get randconfig failures when the next patch creates > a link time dependency related to the use of MDEV_TYPE. > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> > --- > drivers/gpu/drm/i915/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > index 1e1cb245fca778..483e9ff8ca1d23 100644 > --- a/drivers/gpu/drm/i915/Kconfig > +++ b/drivers/gpu/drm/i915/Kconfig > @@ -101,6 +101,7 @@ config DRM_I915_GVT > bool "Enable Intel GVT-g graphics virtualization host support" > depends on DRM_I915 > depends on 64BIT > + depends on VFIO_MDEV > default n > help > Choose this option if you want to enable Intel GVT-g graphics > -- > 2.31.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 2021.03.23 16:39:36 -0300, Jason Gunthorpe wrote: > On Tue, Mar 23, 2021 at 08:26:30PM +0100, Christoph Hellwig wrote: > > On Tue, Mar 23, 2021 at 02:55:32PM -0300, Jason Gunthorpe wrote: > > > Ideally all of this would be moved to kvmgt.c, but it is entangled with > > > the rest of the "generic" code in an odd way. Thus put in a kconfig > > > dependency so we don't get randconfig failures when the next patch creates > > > a link time dependency related to the use of MDEV_TYPE. > > > > Ideally that weird struct intel_gvt_mpt would go away entirely. But > > that is clearly out of scope for this patchset.. > > Yes.. Maybe someone from Intel will take that on, along with that > other note you had. Compared to all the others this driver is quite > twisty! > It was there for other hypervisor support, although XenGT support was never upstream, but there's also some third-party hypervisor using GVT device model. For vGPU type, it planned to be used for XenGT as well, but it turned out not to be true, yeah, I agree that should be in kvmgt.c and mdev only. Thanks to point out this. Until to clean up this, I may pick this one first. Thanks
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index 1e1cb245fca778..483e9ff8ca1d23 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -101,6 +101,7 @@ config DRM_I915_GVT bool "Enable Intel GVT-g graphics virtualization host support" depends on DRM_I915 depends on 64BIT + depends on VFIO_MDEV default n help Choose this option if you want to enable Intel GVT-g graphics
At some point there may have been some reason for this weird split in this driver, but today only the VFIO side is actually implemented. However, it got messed up at some point and mdev code was put in gvt.c and is pretending to be "generic" by masquerading as some generic attribute list: static MDEV_TYPE_ATTR_RO(description); But MDEV_TYPE attributes are only usable with mdev_device, nothing else. Ideally all of this would be moved to kvmgt.c, but it is entangled with the rest of the "generic" code in an odd way. Thus put in a kconfig dependency so we don't get randconfig failures when the next patch creates a link time dependency related to the use of MDEV_TYPE. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/gpu/drm/i915/Kconfig | 1 + 1 file changed, 1 insertion(+)