[Intel-gfx] [bug report] drm/i915/gvt: vGPU graphics memory virtualization

[Dan Carpenter]

Hello Zhi Wang,

The patch 2707e4446688: "drm/i915/gvt: vGPU graphics memory
virtualization" from Mar 28, 2016, leads to the following static
checker warnings:

	drivers/gpu/drm/i915/gvt/gtt.c:1675 intel_vgpu_pin_mm()
	warn: 'mm->pincount.counter' not decremented on lines: 1670,1675.

	drivers/gpu/drm/i915/gvt/gtt.c:1932 intel_gvt_create_scratch_page()
	warn: signedness bug returning '(-12)'

drivers/gpu/drm/i915/gvt/gtt.c
  1647  /**
  1648   * intel_vgpu_pin_mm - increase the pin count of a vGPU mm object
  1649   * @vgpu: a vGPU
  1650   *
  1651   * This function is called when user wants to use a vGPU mm object. If this
  1652   * mm object hasn't been shadowed yet, the shadow will be populated at this
  1653   * time.
  1654   *
  1655   * Returns:
  1656   * Zero on success, negative error code if failed.
  1657   */
  1658  int intel_vgpu_pin_mm(struct intel_vgpu_mm *mm)
  1659  {
  1660          int ret;
  1661  
  1662          if (WARN_ON(mm->type != INTEL_GVT_MM_PPGTT))
  1663                  return 0;

Should this be "return -EINVAL;"?

  1664  
  1665          atomic_inc(&mm->pincount);
  1666  
  1667          if (!mm->shadowed) {
  1668                  ret = shadow_mm(mm);
  1669                  if (ret)
  1670                          return ret;

Should we decrement ->pincount before we return?  Or perhaps in
invalidate_mm()?

  1671          }
  1672  
  1673          list_del_init(&mm->lru_list);
  1674          list_add_tail(&mm->lru_list, &mm->vgpu->gvt->gtt.mm_lru_list_head);
  1675          return 0;
  1676  }

	[ snip ]

  1922  bool intel_gvt_create_scratch_page(struct intel_vgpu *vgpu)
        ^^^^
  1923  {
  1924          struct intel_vgpu_gtt *gtt = &vgpu->gtt;
  1925          void *p;
  1926          void *vaddr;
  1927          unsigned long mfn;
  1928  
  1929          gtt->scratch_page = alloc_page(GFP_KERNEL);
  1930          if (!gtt->scratch_page) {
  1931                  gvt_err("Failed to allocate scratch page.\n");
  1932                  return -ENOMEM;
                        ^^^^^^^^^^^^^^^
This means return true.

  1933          }
  1934  
  1935          /* set to zero */
  1936          p = kmap_atomic(gtt->scratch_page);
  1937          memset(p, 0, PAGE_SIZE);
  1938          kunmap_atomic(p);
  1939  
  1940          /* translate page to mfn */
  1941          vaddr = page_address(gtt->scratch_page);
  1942          mfn = intel_gvt_hypervisor_virt_to_mfn(vaddr);
  1943  
  1944          if (mfn == INTEL_GVT_INVALID_ADDR) {
  1945                  gvt_err("fail to translate vaddr:0x%llx\n", (u64)vaddr);
  1946                  __free_page(gtt->scratch_page);
  1947                  gtt->scratch_page = NULL;
  1948                  return -ENXIO;
                        ^^^^^^^^^^^^^
Same.  The caller doesn't care.  Maybe make this a void function?

  1949          }
  1950  
  1951          gtt->scratch_page_mfn = mfn;
  1952          gvt_dbg_core("vgpu%d create scratch page: mfn=0x%lx\n", vgpu->id, mfn);
  1953          return 0;
  1954  }

regards,
dan carpenter