[PATCH 4/9] drm/i915/gvt: Factor out intel_vgpu_{get_or_create_ppgtt_mm, find_destroy_ppgtt_mm} interfaces

Du, Changbin changbin.du at intel.com
Mon Dec 25 10:30:26 UTC 2017


On Mon, Dec 25, 2017 at 06:36:51PM +0800, Zhi Wang wrote:
> I think in a re-factor patch, you shouldn't change a behavior without any
> notice in the comments. Also I think you should consider add your comments
> in this email to the comments of the patch so that people would know you
> changed the behavior.
> 
No, the behavior is not changed, just function name...

> Thanks,
> Zhi.
> 
> On 12/25/17 18:25, Du, Changbin wrote:
> > On Mon, Dec 25, 2017 at 06:19:08PM +0800, Zhi Wang wrote:
> > > You changed the behavior a bit. Previously, in the G2V call, the vGPU mm is
> > > "put" and now it's destroyed. Is there any reason why the behavior is
> > > changed?
> > > 
> > The request is to destroy the mm obj. So we should destroy it eventhough the
> > under implementation is same - put the reference.
> > 
> > > Thanks,
> > > Zhi.
> > > 
> > > On 12/25/17 17:11, changbin.du at intel.com wrote:
> > > > From: Changbin Du <changbin.du at intel.com>
> > > > 
> > > > Factor out these two interfaces so we can kill some duplicated code in
> > > > scheduler.c.
> > > > 
> > > > Signed-off-by: Changbin Du <changbin.du at intel.com>
> > > > ---
> > > >    drivers/gpu/drm/i915/gvt/gtt.c       | 28 ++++++++++++----------------
> > > >    drivers/gpu/drm/i915/gvt/gtt.h       |  4 ++--
> > > >    drivers/gpu/drm/i915/gvt/handlers.c  | 11 +++++++----
> > > >    drivers/gpu/drm/i915/gvt/scheduler.c | 17 +++++------------
> > > >    4 files changed, 26 insertions(+), 34 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> > > > index 9befbfa..fbb0aa2 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > > > @@ -2268,20 +2268,19 @@ struct intel_vgpu_mm *intel_vgpu_find_ppgtt_mm(struct intel_vgpu *vgpu,
> > > >    }
> > > >    /**
> > > > - * intel_vgpu_g2v_create_ppgtt_mm - create a PPGTT mm object from
> > > > - * g2v notification
> > > > + * intel_vgpu_get_or_create_ppgtt_mm - find or create a PPGTT mm object.
> > > >     * @vgpu: a vGPU
> > > >     * @root_entry_type: ppgtt root entry type
> > > >     * @pdps: guest pdps
> > > >     *
> > > > - * This function is used to create a PPGTT mm object from a guest to GVT-g
> > > > - * notification.
> > > > + * This function is used to find or create a PPGTT mm object from a guest.
> > > >     *
> > > >     * Returns:
> > > >     * Zero on success, negative error code if failed.
> > > >     */
> > > > -int intel_vgpu_g2v_create_ppgtt_mm(struct intel_vgpu *vgpu,
> > > > -		intel_gvt_gtt_type_t root_entry_type, u64 pdps[GEN8_3LVL_PDPES])
> > > > +struct intel_vgpu_mm *intel_vgpu_get_or_create_ppgtt_mm(
> > > > +		struct intel_vgpu *vgpu, intel_gvt_gtt_type_t root_entry_type,
> > > > +		u64 pdps[GEN8_3LVL_PDPES])
> > > >    {
> > > >    	struct intel_vgpu_mm *mm;
> > > > @@ -2290,27 +2289,23 @@ int intel_vgpu_g2v_create_ppgtt_mm(struct intel_vgpu *vgpu,
> > > >    		intel_vgpu_mm_get(mm);
> > > >    	} else {
> > > >    		mm = intel_vgpu_create_ppgtt_mm(vgpu, root_entry_type, pdps);
> > > > -		if (IS_ERR(mm)) {
> > > > +		if (IS_ERR(mm))
> > > >    			gvt_vgpu_err("fail to create mm\n");
> > > > -			return PTR_ERR(mm);
> > > > -		}
> > > >    	}
> > > > -	return 0;
> > > > +	return mm;
> > > >    }
> > > >    /**
> > > > - * intel_vgpu_g2v_destroy_ppgtt_mm - destroy a PPGTT mm object from
> > > > - * g2v notification
> > > > + * intel_vgpu_find_destroy_ppgtt_mm - find and destroy a PPGTT mm object.
> > > >     * @vgpu: a vGPU
> > > >     * @pdps: guest pdps
> > > >     *
> > > > - * This function is used to create a PPGTT mm object from a guest to GVT-g
> > > > - * notification.
> > > > + * This function is used to find a PPGTT mm object from a guest and destroy it.
> > > >     *
> > > >     * Returns:
> > > >     * Zero on success, negative error code if failed.
> > > >     */
> > > > -int intel_vgpu_g2v_destroy_ppgtt_mm(struct intel_vgpu *vgpu,
> > > > +int intel_vgpu_find_destroy_ppgtt_mm(struct intel_vgpu *vgpu,
> > > >    		u64 pdps[GEN8_3LVL_PDPES])
> > > >    {
> > > >    	struct intel_vgpu_mm *mm;
> > > > @@ -2320,7 +2315,8 @@ int intel_vgpu_g2v_destroy_ppgtt_mm(struct intel_vgpu *vgpu,
> > > >    		gvt_vgpu_err("fail to find ppgtt instance.\n");
> > > >    		return -EINVAL;
> > > >    	}
> > > > -	intel_vgpu_mm_put(mm);
> > > > +
> > > > +	intel_vgpu_destroy_mm(mm);
> > > >    	return 0;
> > > >    }
> > > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
> > > > index fa94e97..4533e10 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/gtt.h
> > > > +++ b/drivers/gpu/drm/i915/gvt/gtt.h
> > > > @@ -268,10 +268,10 @@ unsigned long intel_vgpu_gma_to_gpa(struct intel_vgpu_mm *mm,
> > > >    struct intel_vgpu_mm *intel_vgpu_find_ppgtt_mm(struct intel_vgpu *vgpu,
> > > >    		u64 pdps[]);
> > > > -int intel_vgpu_g2v_create_ppgtt_mm(struct intel_vgpu *vgpu,
> > > > +struct intel_vgpu_mm *intel_vgpu_get_or_create_ppgtt_mm(struct intel_vgpu *vgpu,
> > > >    		intel_gvt_gtt_type_t root_entry_type, u64 pdps[]);
> > > > -int intel_vgpu_g2v_destroy_ppgtt_mm(struct intel_vgpu *vgpu, u64 pdps[]);
> > > > +int intel_vgpu_find_destroy_ppgtt_mm(struct intel_vgpu *vgpu, u64 pdps[]);
> > > >    int intel_vgpu_emulate_ggtt_mmio_read(struct intel_vgpu *vgpu,
> > > >    	unsigned int off, void *p_data, unsigned int bytes);
> > > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> > > > index 2dff036..8c4c8c1 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/handlers.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> > > > @@ -1139,6 +1139,7 @@ static int pvinfo_mmio_read(struct intel_vgpu *vgpu, unsigned int offset,
> > > >    static int handle_g2v_notification(struct intel_vgpu *vgpu, int notification)
> > > >    {
> > > > +	struct intel_vgpu_mm *mm;
> > > >    	u64 *pdps;
> > > >    	int ret = 0;
> > > > @@ -1146,20 +1147,22 @@ static int handle_g2v_notification(struct intel_vgpu *vgpu, int notification)
> > > >    	switch (notification) {
> > > >    	case VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE:
> > > > -		ret = intel_vgpu_g2v_create_ppgtt_mm(vgpu,
> > > > +		mm = intel_vgpu_get_or_create_ppgtt_mm(vgpu,
> > > >    				GTT_TYPE_PPGTT_ROOT_L3_ENTRY,
> > > >    				pdps);
> > > > +		ret = IS_ERR(mm) ? PTR_ERR(mm) : 0;
> > > >    		break;
> > > >    	case VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY:
> > > > -		ret = intel_vgpu_g2v_destroy_ppgtt_mm(vgpu, pdps);
> > > > +		ret = intel_vgpu_find_destroy_ppgtt_mm(vgpu, pdps);
> > > >    		break;
> > > >    	case VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE:
> > > > -		ret = intel_vgpu_g2v_create_ppgtt_mm(vgpu,
> > > > +		mm = intel_vgpu_get_or_create_ppgtt_mm(vgpu,
> > > >    				GTT_TYPE_PPGTT_ROOT_L4_ENTRY,
> > > >    				pdps);
> > > > +		ret = IS_ERR(mm) ? PTR_ERR(mm) : 0;
> > > >    		break;
> > > >    	case VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY:
> > > > -		ret = intel_vgpu_g2v_destroy_ppgtt_mm(vgpu, pdps);
> > > > +		ret = intel_vgpu_find_destroy_ppgtt_mm(vgpu, pdps);
> > > >    		break;
> > > >    	case VGT_G2V_EXECLIST_CONTEXT_CREATE:
> > > >    	case VGT_G2V_EXECLIST_CONTEXT_DESTROY:
> > > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > > index 23c33e6..3440ff2 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > > @@ -1197,18 +1197,11 @@ static int prepare_mm(struct intel_vgpu_workload *workload)
> > > >    	read_guest_pdps(workload->vgpu, workload->ring_context_gpa, pdps);
> > > > -	mm = intel_vgpu_find_ppgtt_mm(workload->vgpu, pdps);
> > > > -	if (mm) {
> > > > -		intel_vgpu_mm_get(mm);
> > > > -	} else {
> > > > -
> > > > -		mm = intel_vgpu_create_ppgtt_mm(workload->vgpu, root_entry_type,
> > > > -						pdps);
> > > > -		if (IS_ERR(mm)) {
> > > > -			gvt_vgpu_err("fail to create mm object.\n");
> > > > -			return PTR_ERR(mm);
> > > > -		}
> > > > -	}
> > > > +	mm = intel_vgpu_get_or_create_ppgtt_mm(workload->vgpu, root_entry_type,
> > > > +					       pdps);
> > > > +	if (IS_ERR(mm))
> > > > +		return PTR_ERR(mm);
> > > > +
> > > >    	workload->shadow_mm = mm;
> > > >    	return 0;
> > > >    }
> > > > 
> > > _______________________________________________
> > > intel-gvt-dev mailing list
> > > intel-gvt-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> > 

-- 
Thanks,
Changbin Du


More information about the intel-gvt-dev mailing list