[PATCH 1/3] drm/i915/gvt: Factor out intel_vgpu_invalidate_mm()

Wang, Zhi A zhi.a.wang at intel.com
Thu Oct 26 04:55:13 UTC 2017 

But looks i915 also keeps some prefix for static function. Hmm, how should we judge that... When a prefix is needed or not....

-----Original Message-----
From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On Behalf Of Wang, Zhi A
Sent: Thursday, October 26, 2017 7:50 AM
To: Zhenyu Wang <zhenyuw at linux.intel.com>; Zhi Wang <zhi.wang.linux at gmail.com>
Cc: intel-gvt-dev at lists.freedesktop.org
Subject: RE: [PATCH 1/3] drm/i915/gvt: Factor out intel_vgpu_invalidate_mm()

Thanks! I think it's not correct to keep intel_vgpu_xxx prefix for a static function, only exposed functions should has a prefix like that.

-----Original Message-----
From: Zhenyu Wang [mailto:zhenyuw at linux.intel.com]
Sent: Thursday, October 26, 2017 7:38 AM
To: Zhi Wang <zhi.wang.linux at gmail.com>
Cc: intel-gvt-dev at lists.freedesktop.org; Wang, Zhi A <zhi.a.wang at intel.com>
Subject: Re: [PATCH 1/3] drm/i915/gvt: Factor out intel_vgpu_invalidate_mm()

On 2017.10.23 15:23:05 +0800, Zhi Wang wrote:
> Factor out intel_vgpu_invalidate_mm() for callers which wants to 
> invalidate vGPU mm.
> 
> Signed-off-by: Zhi Wang <zhi.a.wang at intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/gtt.c | 110 +++++++++++++++++++++++------------------
>  drivers/gpu/drm/i915/gvt/gtt.h |   4 +-
>  2 files changed, 64 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c 
> b/drivers/gpu/drm/i915/gvt/gtt.c index 17e87df..46741ac 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1753,6 +1753,62 @@ int intel_vgpu_pin_mm(struct intel_vgpu_mm *mm)
>  	return 0;
>  }
>  
> +static void invalidate_ggtt(struct intel_vgpu *vgpu) {

I'd still like to keep intel_vgpu_xxx for function name as it's consistent for reading and aim for vgpu instance.

> +	struct intel_gvt *gvt = vgpu->gvt;
> +	struct drm_i915_private *dev_priv = gvt->dev_priv;
> +	struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
> +	u32 index;
> +	u32 offset;
> +	u32 num_entries;
> +	struct intel_gvt_gtt_entry e;
> +
> +	memset(&e, 0, sizeof(struct intel_gvt_gtt_entry));
> +	e.type = GTT_TYPE_GGTT_PTE;
> +	ops->set_pfn(&e, gvt->gtt.scratch_mfn);
> +	e.val64 |= _PAGE_PRESENT;
> +
> +	index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
> +	num_entries = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
> +	for (offset = 0; offset < num_entries; offset++)
> +		ops->set_entry(NULL, &e, index + offset, false, 0, vgpu);
> +
> +	index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
> +	num_entries = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
> +	for (offset = 0; offset < num_entries; offset++)
> +		ops->set_entry(NULL, &e, index + offset, false, 0, vgpu);
> +
> +	gtt_invalidate(dev_priv);
> +}
> +
> +/*
> + * intel_vgpu_invalidate_mm - Invalidate vGPU mm by type
> + * @vgpu: a vGPU
> + * @type: mm type
> + *
> + * This function is used to invalidate mm of specific type
> + *
> + */
> +void intel_vgpu_invalidate_mm(struct intel_vgpu *vgpu, int type) {
> +	struct list_head *pos, *n;
> +	struct intel_vgpu_mm *mm;
> +
> +	if (type == INTEL_GVT_MM_PPGTT) {

typo?

> +		invalidate_ggtt(vgpu);
> +		return;
> +	}
> +
> +	list_for_each_safe(pos, n, &vgpu->gtt.mm_list_head) {
> +		mm = container_of(pos, struct intel_vgpu_mm, list);
> +		if (mm->type == INTEL_GVT_MM_PPGTT) {
> +			list_del_init(&mm->lru_list);
> +			if (mm->has_shadow_page_table && mm->shadowed)
> +				invalidate_mm(mm);
> +		}
> +	}
> +}
> +
>  static int reclaim_one_mm(struct intel_gvt *gvt)  {
>  	struct intel_vgpu_mm *mm;
> @@ -2112,7 +2168,7 @@ int intel_vgpu_init_gtt(struct intel_vgpu *vgpu)
>  	INIT_LIST_HEAD(&gtt->oos_page_list_head);
>  	INIT_LIST_HEAD(&gtt->post_shadow_list_head);
>  
> -	intel_vgpu_reset_ggtt(vgpu);
> +	invalidate_ggtt(vgpu);
>  
>  	ggtt_mm = intel_vgpu_create_mm(vgpu, INTEL_GVT_MM_GGTT,
>  			NULL, 1, 0);
> @@ -2125,7 +2181,7 @@ int intel_vgpu_init_gtt(struct intel_vgpu *vgpu)
>  	return ppgtt_create_scratch(vgpu);
>  }
>  
> -static void intel_vgpu_free_mm(struct intel_vgpu *vgpu, int type)
> +static void free_mm(struct intel_vgpu *vgpu, int type)
>  {
>  	struct list_head *pos, *n;
>  	struct intel_vgpu_mm *mm;
> @@ -2156,8 +2212,8 @@ void intel_vgpu_clean_gtt(struct intel_vgpu *vgpu)
>  	ppgtt_free_all_shadow_page(vgpu);
>  	ppgtt_destroy_scratch(vgpu);
>  
> -	intel_vgpu_free_mm(vgpu, INTEL_GVT_MM_PPGTT);
> -	intel_vgpu_free_mm(vgpu, INTEL_GVT_MM_GGTT);
> +	free_mm(vgpu, INTEL_GVT_MM_PPGTT);
> +	free_mm(vgpu, INTEL_GVT_MM_GGTT);
>  }
>  
>  static void clean_spt_oos(struct intel_gvt *gvt) @@ -2397,42 +2453,6 
> @@ void intel_gvt_clean_gtt(struct intel_gvt *gvt)  }
>  
>  /**
> - * intel_vgpu_reset_ggtt - reset the GGTT entry
> - * @vgpu: a vGPU
> - *
> - * This function is called at the vGPU create stage
> - * to reset all the GGTT entries.
> - *
> - */
> -void intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu) -{
> -	struct intel_gvt *gvt = vgpu->gvt;
> -	struct drm_i915_private *dev_priv = gvt->dev_priv;
> -	struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
> -	u32 index;
> -	u32 offset;
> -	u32 num_entries;
> -	struct intel_gvt_gtt_entry e;
> -
> -	memset(&e, 0, sizeof(struct intel_gvt_gtt_entry));
> -	e.type = GTT_TYPE_GGTT_PTE;
> -	ops->set_pfn(&e, gvt->gtt.scratch_mfn);
> -	e.val64 |= _PAGE_PRESENT;
> -
> -	index = vgpu_aperture_gmadr_base(vgpu) >> PAGE_SHIFT;
> -	num_entries = vgpu_aperture_sz(vgpu) >> PAGE_SHIFT;
> -	for (offset = 0; offset < num_entries; offset++)
> -		ops->set_entry(NULL, &e, index + offset, false, 0, vgpu);
> -
> -	index = vgpu_hidden_gmadr_base(vgpu) >> PAGE_SHIFT;
> -	num_entries = vgpu_hidden_sz(vgpu) >> PAGE_SHIFT;
> -	for (offset = 0; offset < num_entries; offset++)
> -		ops->set_entry(NULL, &e, index + offset, false, 0, vgpu);
> -
> -	gtt_invalidate(dev_priv);
> -}
> -
> -/**
>   * intel_vgpu_reset_gtt - reset the all GTT related status
>   * @vgpu: a vGPU
>   *
> @@ -2443,12 +2463,6 @@ void intel_vgpu_reset_ggtt(struct intel_vgpu
> *vgpu)  void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu)  {
>  	ppgtt_free_all_shadow_page(vgpu);
> -
> -	/* Shadow pages are only created when there is no page
> -	 * table tracking data, so remove page tracking data after
> -	 * removing the shadow pages.
> -	 */
> -	intel_vgpu_free_mm(vgpu, INTEL_GVT_MM_PPGTT);
> -
> -	intel_vgpu_reset_ggtt(vgpu);
> +	free_mm(vgpu, INTEL_GVT_MM_PPGTT);
> +	invalidate_ggtt(vgpu);
>  }
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h 
> b/drivers/gpu/drm/i915/gvt/gtt.h index 9a8506e..c46350e 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.h
> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
> @@ -185,6 +185,7 @@ extern struct intel_vgpu_mm *intel_vgpu_create_mm(struct intel_vgpu *vgpu,
>  		int mm_type, void *virtual_page_table, int page_table_level,
>  		u32 pde_base_index);
>  extern void intel_vgpu_destroy_mm(struct kref *mm_ref);
> +extern void intel_vgpu_invalidate_mm(struct intel_vgpu *vgpu, int 
> +type);
>  
>  struct intel_vgpu_guest_page;
>  
> @@ -210,11 +211,10 @@ struct intel_vgpu_gtt {
>  
>  extern int intel_vgpu_init_gtt(struct intel_vgpu *vgpu);  extern void 
> intel_vgpu_clean_gtt(struct intel_vgpu *vgpu); -void 
> intel_vgpu_reset_ggtt(struct intel_vgpu *vgpu);
>  
>  extern int intel_gvt_init_gtt(struct intel_gvt *gvt); -void 
> intel_vgpu_reset_gtt(struct intel_vgpu *vgpu);  extern void 
> intel_gvt_clean_gtt(struct intel_gvt *gvt);
> +void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu);
>  
>  extern struct intel_vgpu_mm *intel_gvt_find_ppgtt_mm(struct intel_vgpu *vgpu,
>  		int page_table_level, void *root_entry);
> --
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

--
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 _______________________________________________
intel-gvt-dev mailing list
intel-gvt-dev at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

More information about the intel-gvt-dev mailing list