[PATCH V2 1/6] drm/i915/gvt: Remove typedef intel_gvt_gtt_type_t

[Colin Xu]

Seems like V2 of this patch doesn't get updated?

Hi Zhi, Zhenyu, from architecture point of view, what kind of change is better?
- Change parameter type from enum type to int looks "cheat" klocwork static analysis,
but lose natural protection since any int could be passed into the function.
- If we keep using enum type as the function parameter, we may need extract check
so that klocwork static analysis will consider as safe.
Any comments? I prefer the latter.

On 2019-04-11 18:46, Aleksei Gimbitskii wrote:
> The klocwork static code analyzer takes the enumeration as the full range
> of intel_gvt_gtt_type_t. But the full range of intel_gvt_gtt_type_t will
> never be used in full range. For example, the GTT_TYPE_INVALID will never
> be used as an index of an array. So we will not use it as a type but only
> the enumeration.
>
> This patch fixed the critial issues #483, #551, #665 reported by
> klockwork.
>
> Signed-off-by: Aleksei Gimbitskii <aleksei.gimbitskii at intel.com>
> Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang at intel.com>
> ---
>   drivers/gpu/drm/i915/gvt/gtt.c | 12 ++++++------
>   drivers/gpu/drm/i915/gvt/gtt.h | 14 +++++++-------
>   2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 88ed2e9df253..8dcb6223b985 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -805,7 +805,7 @@ static int reclaim_one_ppgtt_mm(struct intel_gvt *gvt);
>   
>   /* Allocate shadow page table without guest page. */
>   static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt(
> -		struct intel_vgpu *vgpu, intel_gvt_gtt_type_t type)
> +		struct intel_vgpu *vgpu, int type)
>   {
>   	struct device *kdev = &vgpu->gvt->dev_priv->drm.pdev->dev;
>   	struct intel_vgpu_ppgtt_spt *spt = NULL;
> @@ -855,7 +855,7 @@ static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt(
>   
>   /* Allocate shadow page table associated with specific gfn. */
>   static struct intel_vgpu_ppgtt_spt *ppgtt_alloc_spt_gfn(
> -		struct intel_vgpu *vgpu, intel_gvt_gtt_type_t type,
> +		struct intel_vgpu *vgpu, int type,
>   		unsigned long gfn, bool guest_pde_ips)
>   {
>   	struct intel_vgpu_ppgtt_spt *spt;
> @@ -930,7 +930,7 @@ static int ppgtt_invalidate_spt_by_shadow_entry(struct intel_vgpu *vgpu,
>   {
>   	struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
>   	struct intel_vgpu_ppgtt_spt *s;
> -	intel_gvt_gtt_type_t cur_pt_type;
> +	int cur_pt_type;
>   
>   	GEM_BUG_ON(!gtt_type_is_pt(get_next_pt_type(e->type)));
>   
> @@ -1849,7 +1849,7 @@ static void vgpu_free_mm(struct intel_vgpu_mm *mm)
>    * Zero on success, negative error code in pointer if failed.
>    */
>   struct intel_vgpu_mm *intel_vgpu_create_ppgtt_mm(struct intel_vgpu *vgpu,
> -		intel_gvt_gtt_type_t root_entry_type, u64 pdps[])
> +		int root_entry_type, u64 pdps[])
>   {
>   	struct intel_gvt *gvt = vgpu->gvt;
>   	struct intel_vgpu_mm *mm;
> @@ -2303,7 +2303,7 @@ int intel_vgpu_emulate_ggtt_mmio_write(struct intel_vgpu *vgpu,
>   }
>   
>   static int alloc_scratch_pages(struct intel_vgpu *vgpu,
> -		intel_gvt_gtt_type_t type)
> +		int type)
>   {
>   	struct intel_vgpu_gtt *gtt = &vgpu->gtt;
>   	struct intel_gvt_gtt_pte_ops *ops = vgpu->gvt->gtt.pte_ops;
> @@ -2588,7 +2588,7 @@ struct intel_vgpu_mm *intel_vgpu_find_ppgtt_mm(struct intel_vgpu *vgpu,
>    * Zero on success, negative error code if failed.
>    */
>   struct intel_vgpu_mm *intel_vgpu_get_ppgtt_mm(struct intel_vgpu *vgpu,
> -		intel_gvt_gtt_type_t root_entry_type, u64 pdps[])
> +		int root_entry_type, u64 pdps[])
>   {
>   	struct intel_vgpu_mm *mm;
>   
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h
> index 32c573aea494..645ddc1bd223 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.h
> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
> @@ -95,7 +95,7 @@ struct intel_gvt_gtt {
>   	unsigned long scratch_mfn;
>   };
>   
> -typedef enum {
> +enum {
>   	GTT_TYPE_INVALID = -1,
>   
>   	GTT_TYPE_GGTT_PTE,
> @@ -124,7 +124,7 @@ typedef enum {
>   	GTT_TYPE_PPGTT_PML4_PT,
>   
>   	GTT_TYPE_MAX,
> -} intel_gvt_gtt_type_t;
> +};
>   
>   enum intel_gvt_mm_type {
>   	INTEL_GVT_MM_GGTT,
> @@ -148,7 +148,7 @@ struct intel_vgpu_mm {
>   
>   	union {
>   		struct {
> -			intel_gvt_gtt_type_t root_entry_type;
> +			int root_entry_type;
>   			/*
>   			 * The 4 PDPs in ring context. For 48bit addressing,
>   			 * only PDP0 is valid and point to PML4. For 32it
> @@ -169,7 +169,7 @@ struct intel_vgpu_mm {
>   };
>   
>   struct intel_vgpu_mm *intel_vgpu_create_ppgtt_mm(struct intel_vgpu *vgpu,
> -		intel_gvt_gtt_type_t root_entry_type, u64 pdps[]);
> +		int root_entry_type, u64 pdps[]);
>   
>   static inline void intel_vgpu_mm_get(struct intel_vgpu_mm *mm)
>   {
> @@ -233,7 +233,7 @@ struct intel_vgpu_ppgtt_spt {
>   	struct intel_vgpu *vgpu;
>   
>   	struct {
> -		intel_gvt_gtt_type_t type;
> +		int type;
>   		bool pde_ips; /* for 64KB PTEs */
>   		void *vaddr;
>   		struct page *page;
> @@ -241,7 +241,7 @@ struct intel_vgpu_ppgtt_spt {
>   	} shadow_page;
>   
>   	struct {
> -		intel_gvt_gtt_type_t type;
> +		int type;
>   		bool pde_ips; /* for 64KB PTEs */
>   		unsigned long gfn;
>   		unsigned long write_cnt;
> @@ -267,7 +267,7 @@ struct intel_vgpu_mm *intel_vgpu_find_ppgtt_mm(struct intel_vgpu *vgpu,
>   		u64 pdps[]);
>   
>   struct intel_vgpu_mm *intel_vgpu_get_ppgtt_mm(struct intel_vgpu *vgpu,
> -		intel_gvt_gtt_type_t root_entry_type, u64 pdps[]);
> +		int root_entry_type, u64 pdps[]);
>   
>   int intel_vgpu_put_ppgtt_mm(struct intel_vgpu *vgpu, u64 pdps[]);
>   

-- 
Best Regards,
Colin Xu