On 2021.04.26 10:39:24 -0300, Jason Gunthorpe wrote:
> On Mon, Apr 26, 2021 at 05:41:42PM +0800, Zhenyu Wang wrote:
> > @@ -1667,19 +1773,26 @@ static struct mdev_parent_ops intel_vgpu_ops = {
> >  
> >  static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
> >  {
> > -   struct attribute_group **kvm_vgpu_type_groups;
> > +   int ret;
> > +
> > +   ret = intel_gvt_init_vgpu_type_groups((struct intel_gvt *)gvt);
> > +   if (ret)
> > +           return ret;
> >  
> >     intel_gvt_ops = ops;
> > -   if (!intel_gvt_ops->get_gvt_attrs(&kvm_vgpu_type_groups))
> > -           return -EFAULT;
> > -   intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
> > +   intel_vgpu_ops.supported_type_groups = gvt_vgpu_type_groups;
> 
> This patch is an improvement, but this fictional dynamic behavior is
> still wrong. The supported_type_groups directly flows from the
> vgpu_types array in vgpu.c and it should not be split up like this
> 
> The code copies the rodata vgpu_types into dynamic memory gvt->types
> then copies that dynamic memory into a dynamic gvt_vgpu_type_groups,
> which makes very little sense at all.

vgpu_types is static for we want fixed vgpu mdev type, but actual supported
types depend on HW resources e.g aperture mem, fence, etc, that's dynamic for
gvt->types, so gvt_vgpu_type_groups is dynamic from gvt->types.

> 
> vgpu_types should be moved to kvmgt and everything should be static,
> like every other mdev driver. Copy the 'type' information from the
> gpu_types static when the mdev is created.
> 

I don't think that's necessary, as it's not static for gvt at all.
The logic to handle type resource change can still be in vgpu.c

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to