[PATCH 1/5] drm/i915/gvt: Remove typedef intel_gvt_gtt_type_t
Colin Xu
Colin.Xu at intel.com
Mon Apr 8 06:48:06 UTC 2019
Better have a cover-letter for a patch set.
On 2019-04-08 13:54, 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.
Changing function param from intel_gvt_gtt_type_t to int doesn't actually avoid
an invalid index into the array. If the caller pass an invalid type, indexing
to scratch_pt could still overflow. Compared with int, enum can provide natural
protection by only allow possible values.
Better find another way to solve the issues.
> 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
More information about the intel-gvt-dev
mailing list