Re: [Intel-gfx] [PATCH 1/3] drm/i915/gvt: Move mdev attribute groups into kvmgt module

[Zhenyu Wang <zhenyuw@linux.intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2781 bytes --]

On 2021.05.11 12:54:46 -0300, Jason Gunthorpe wrote:
> On Tue, May 11, 2021 at 04:33:30PM +0800, Zhenyu Wang wrote:
> > As kvmgt module contains all handling for VFIO/mdev, leaving mdev attribute
> > groups in gvt module caused dependency issue. Although it was there for possible
> > other hypervisor usage, that turns out never to be true. So this moves all mdev
> > handling into kvmgt module completely to resolve dependency issue.
> > 
> > Cc: Arnd Bergmann <arnd@kernel.org>
> > Cc: Jason Gunthorpe <jgg@ziepe.ca>
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/gvt.c       | 124 +------------------------
> >  drivers/gpu/drm/i915/gvt/gvt.h       |   3 -
> >  drivers/gpu/drm/i915/gvt/hypercall.h |   2 +-
> >  drivers/gpu/drm/i915/gvt/kvmgt.c     | 129 +++++++++++++++++++++++++--
> >  drivers/gpu/drm/i915/gvt/mpt.h       |   4 +-
> >  5 files changed, 126 insertions(+), 136 deletions(-)
> 
> There is no reason to make this into three patches, just make the one
> line change to kconfig here.
> 
> > +static struct intel_vgpu_type *
> > +intel_gvt_find_vgpu_type(struct intel_gvt *gvt, unsigned int type_group_id)
> > +{
> > +	if (WARN_ON(type_group_id >= gvt->num_types))
> > +		return NULL;
> > +	return &gvt->types[type_group_id];
> > +}
> 
> The WARN_ON can't happen, all this error handling is code should be
> deleted, just use the simple
> 
> struct intel_gvt *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
> struct intel_vgpu_type *type = &gvt->types[mtype_get_type_group_id(mtype)]
> 
> sequence like the other mdev drivers
> 
> > +static ssize_t available_instances_show(struct mdev_type *mtype,
> > +					struct mdev_type_attribute *attr,
> > +					char *buf)
> > +{
> > +	struct intel_vgpu_type *type;
> > +	unsigned int num = 0;
> > +	void *gvt = kdev_to_i915(mtype_get_parent_dev(mtype))->gvt;
> 
> Use proper types not 'void *'
> > +
> > +static int intel_gvt_init_vgpu_type_groups(struct intel_gvt *gvt)
> > +{
> > +	int i, j;
> > +	struct intel_vgpu_type *type;
> > +	struct attribute_group *group;
> > +
> > +	for (i = 0; i < gvt->num_types; i++) {
> > +		type = &gvt->types[i];
> > +
> > +		group = kzalloc(sizeof(struct attribute_group), GFP_KERNEL);
> > +		if (WARN_ON(!group))
> > +			goto unwind;
> 
> WARN_ON at allocation failure is not good

Thanks for the comments! I left those during code move, will clean them up.

> 
> This need to go into the vfio tree in some way, either directly
> through it, via rc or otherwise
> 

As this is only for i915/gvt, once drm/i915 backmerge with linus master,
it should still go through normal i915/gvt merge path.

Thanks

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx