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

Submitted by Aleksei Gimbitskii on April 11, 2019, 10:46 a.m.

Details

Message ID 20190411104631.7627-2-aleksei.gimbitskii@intel.com
State New
Headers show
Series "Fix issues reported by klocwork" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Aleksei Gimbitskii April 11, 2019, 10:46 a.m.
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@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi Wang <zhi.a.wang@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(-)

Patch hide | download patch | download mbox

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[]);
 

Comments

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@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@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[]);
>

I think klocwork is going a bit too far actually. It always takes the full range would be used if we go enumeration type. For option 2, I guess we have to add a lot of useless check of GTT_TYPE_INVALID in every place we are going to use the enumeration as array index, which looks very ugly.

I'd like to go practical. We need the enumeration for sure and we check the GTT_TYPE_INVALID in the place we think necessary and let clockwork be happy and also not make the code ugly.

-----Original Message-----
From: Xu, Colin 

Sent: Friday, April 12, 2019 4:29 AM
To: Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: intel-gvt-dev@lists.freedesktop.org
Subject: Re: [PATCH V2 1/6] drm/i915/gvt: Remove typedef intel_gvt_gtt_type_t

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@intel.com>

> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>

> Cc: Zhi Wang <zhi.a.wang@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
Also, the other reason to remove the typedef is:
https://yarchive.net/comp/linux/typedefs.html

i915 has already removed almost all of them because of this.

Thanks,
Zhi

-----Original Message-----
From: Wang, Zhi A 

Sent: Friday, April 12, 2019 8:44 AM
To: Xu, Colin <colin.xu@intel.com>; Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: intel-gvt-dev@lists.freedesktop.org
Subject: RE: [PATCH V2 1/6] drm/i915/gvt: Remove typedef intel_gvt_gtt_type_t

I think klocwork is going a bit too far actually. It always takes the full range would be used if we go enumeration type. For option 2, I guess we have to add a lot of useless check of GTT_TYPE_INVALID in every place we are going to use the enumeration as array index, which looks very ugly.

I'd like to go practical. We need the enumeration for sure and we check the GTT_TYPE_INVALID in the place we think necessary and let clockwork be happy and also not make the code ugly.

-----Original Message-----
From: Xu, Colin

Sent: Friday, April 12, 2019 4:29 AM
To: Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: intel-gvt-dev@lists.freedesktop.org
Subject: Re: [PATCH V2 1/6] drm/i915/gvt: Remove typedef intel_gvt_gtt_type_t

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@intel.com>

> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>

> Cc: Zhi Wang <zhi.a.wang@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

Yes. But if we keep the enumeration type here, how about we let the enumeration value begins from 0? Then clockwork would be happy while we keep the enumeration type?

-----Original Message-----
From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com] 
Sent: Friday, April 12, 2019 10:19 AM
To: Wang, Zhi A <zhi.a.wang@intel.com>
Cc: Xu, Colin <colin.xu@intel.com>; Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>; intel-gvt-dev@lists.freedesktop.org
Subject: Re: [PATCH V2 1/6] drm/i915/gvt: Remove typedef intel_gvt_gtt_type_t

On 2019.04.12 06:20:26 +0000, Wang, Zhi A wrote:
> Also, the other reason to remove the typedef is:
> https://yarchive.net/comp/linux/typedefs.html
> 
> i915 has already removed almost all of them because of this.
> 

Remove typedef is ok, I think everyone agree but would be better still keep enum type.

> 
> -----Original Message-----
> From: Wang, Zhi A
> Sent: Friday, April 12, 2019 8:44 AM
> To: Xu, Colin <colin.xu@intel.com>; Gimbitskii, Aleksei 
> <aleksei.gimbitskii@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org
> Subject: RE: [PATCH V2 1/6] drm/i915/gvt: Remove typedef 
> intel_gvt_gtt_type_t
> 
> I think klocwork is going a bit too far actually. It always takes the full range would be used if we go enumeration type. For option 2, I guess we have to add a lot of useless check of GTT_TYPE_INVALID in every place we are going to use the enumeration as array index, which looks very ugly.
> 
> I'd like to go practical. We need the enumeration for sure and we check the GTT_TYPE_INVALID in the place we think necessary and let clockwork be happy and also not make the code ugly.
> 
> -----Original Message-----
> From: Xu, Colin
> Sent: Friday, April 12, 2019 4:29 AM
> To: Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; Wang, Zhi A 
> <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [PATCH V2 1/6] drm/i915/gvt: Remove typedef 
> intel_gvt_gtt_type_t
> 
> 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@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: Zhi Wang <zhi.a.wang@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
> 

--
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

Ping.

-----Original Message-----
From: Wang, Zhi A 
Sent: Friday, April 12, 2019 11:21 AM
To: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Xu, Colin <colin.xu@intel.com>; Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; intel-gvt-dev@lists.freedesktop.org
Subject: RE: [PATCH V2 1/6] drm/i915/gvt: Remove typedef intel_gvt_gtt_type_t

Yes. But if we keep the enumeration type here, how about we let the enumeration value begins from 0? Then clockwork would be happy while we keep the enumeration type?

-----Original Message-----
From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com]
Sent: Friday, April 12, 2019 10:19 AM
To: Wang, Zhi A <zhi.a.wang@intel.com>
Cc: Xu, Colin <colin.xu@intel.com>; Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>; intel-gvt-dev@lists.freedesktop.org
Subject: Re: [PATCH V2 1/6] drm/i915/gvt: Remove typedef intel_gvt_gtt_type_t

On 2019.04.12 06:20:26 +0000, Wang, Zhi A wrote:
> Also, the other reason to remove the typedef is:
> https://yarchive.net/comp/linux/typedefs.html
> 
> i915 has already removed almost all of them because of this.
> 

Remove typedef is ok, I think everyone agree but would be better still keep enum type.

> 
> -----Original Message-----
> From: Wang, Zhi A
> Sent: Friday, April 12, 2019 8:44 AM
> To: Xu, Colin <colin.xu@intel.com>; Gimbitskii, Aleksei 
> <aleksei.gimbitskii@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org
> Subject: RE: [PATCH V2 1/6] drm/i915/gvt: Remove typedef 
> intel_gvt_gtt_type_t
> 
> I think klocwork is going a bit too far actually. It always takes the full range would be used if we go enumeration type. For option 2, I guess we have to add a lot of useless check of GTT_TYPE_INVALID in every place we are going to use the enumeration as array index, which looks very ugly.
> 
> I'd like to go practical. We need the enumeration for sure and we check the GTT_TYPE_INVALID in the place we think necessary and let clockwork be happy and also not make the code ugly.
> 
> -----Original Message-----
> From: Xu, Colin
> Sent: Friday, April 12, 2019 4:29 AM
> To: Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; Wang, Zhi A 
> <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [PATCH V2 1/6] drm/i915/gvt: Remove typedef 
> intel_gvt_gtt_type_t
> 
> 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@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: Zhi Wang <zhi.a.wang@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
> 

--
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827