ENGR00309915 [#1087] enhanced video memory mutex

this patch can fix NULL pointer issue in GPU kernel driver with the following log

[<7f240438>] (gckEVENT_AddList+0x0/0x810 [galcore]) from [<7f239ebc>] (gckCOMMAND_Commit+0xf28/0x118c [galcore])
[<7f238f94>] (gckCOMMAND_Commit+0x0/0x118c [galcore]) from [<7f2362dc>] (gckKERNEL_Dispatch+0x120c/0x24e4 [galcore])
[<7f2350d0>] (gckKERNEL_Dispatch+0x0/0x24e4 [galcore]) from [<7f222280>] (drv_ioctl+0x390/0x540 [galcore])
[<7f221ef0>] (drv_ioctl+0x0/0x540 [galcore]) from [<800facd0>] (vfs_ioctl+0x30/0x44)

The false code is at 0x217bc where the 0-pointer happens (r3 = 0)

gcuVIDMEM_NODE_PTR node = (gcuVIDMEM_NODE_PTR)(gcmUINT64_TO_PTR(Record->info.u.FreeVideoMemory.node));
   217b8:                e5953028             ldr           r3, [r5, torvalds#40]         ; 0x28

                if (node->VidMem.memory->object.type == gcvOBJ_VIDMEM)
   217bc:                e5932000             ldr           r2, [r3]
   217c0:                e5922000             ldr           r2, [r2]
   217c4:                e152000a             cmp       r2, sl
                {
                     gcmkVERIFY_OK(gckKERNEL_RemoveProcessDB(Event->kernel,

Date: Apr 23, 2014
Signed-off-by: Xianzhong <b07117@freescale.com>
Acked-by: Jason Liu

drivers/mxc/gpu-viv/hal/kernel/gc_hal_kernel.c

@@ -240,6 +240,8 @@ gckKERNEL_Construct(
     kernel->dvfs         = gcvNULL;
 #endif
 
+    kernel->vidmemMutex  = gcvNULL;
+
     /* Initialize the gckKERNEL object. */
     kernel->object.type = gcvOBJ_KERNEL;
     kernel->os          = Os;
@@ -386,6 +388,8 @@ gckKERNEL_Construct(
     gcmkONERROR(
         gckOS_CreateMutex(Os, (gctPOINTER)&kernel->virtualBufferLock));
 
+    /* Construct a video memory mutex. */
+    gcmkONERROR(gckOS_CreateMutex(Os, &kernel->vidmemMutex));
 
     /* Return pointer to the gckKERNEL object. */
     *Kernel = kernel;
@@ -612,6 +616,7 @@ gckKERNEL_Destroy(
     gcmkVERIFY_OK(gckOS_DestroySyncTimeline(Kernel->os, Kernel->timeline));
 #endif
 
+    gcmkVERIFY_OK(gckOS_DeleteMutex(Kernel->os, Kernel->vidmemMutex));
 
     /* Mark the gckKERNEL object as unknown. */
     Kernel->object.type = gcvOBJ_UNKNOWN;

drivers/mxc/gpu-viv/hal/kernel/gc_hal_kernel.h

@@ -547,6 +547,7 @@ struct _gckKERNEL
     /* Level of dump information after stuck. */
     gctUINT                     stuckDump;
 
+    gctPOINTER                  vidmemMutex;
 };
 
 struct _FrequencyHistory
@@ -896,9 +897,6 @@ typedef union _gcuVIDMEM_NODE
         /* Actual physical address */
         gctUINT32               addresses[gcdMAX_GPU_COUNT];
 
-        /* Mutex. */
-        gctPOINTER              mutex;
-
         /* Locked counter. */
         gctINT32                lockeds[gcdMAX_GPU_COUNT];
 
@@ -938,9 +936,6 @@ struct _gckVIDMEM
 
     /* Allocation threshold. */
     gctSIZE_T                   threshold;
-
-    /* The heap mutex. */
-    gctPOINTER                  mutex;
 };
 
 typedef struct _gcsVIDMEM_NODE * gckVIDMEM_NODE;

drivers/mxc/gpu-viv/hal/kernel/gc_hal_kernel_command_vg.c

@@ -1133,14 +1133,8 @@ _AllocateLinear(
             node->Virtual.lockKernels[i]    = gcvNULL;
         }
 
-        node->Virtual.mutex         = gcvNULL;
-
         node->Virtual.processID = 0;
 
-        /* Create the mutex. */
-        gcmkERR_BREAK(
-            gckOS_CreateMutex(Command->os, &node->Virtual.mutex));
-
         node->Virtual.bytes    = ((Size + Alignment -1)/ Alignment)*Alignment;;
 
             gcmkERR_BREAK(gckOS_AllocateNonPagedMemory(
@@ -1166,12 +1160,6 @@ _AllocateLinear(
     /* Roll back. */
     if (node != gcvNULL)
     {
-        if (node->Virtual.mutex != gcvNULL)
-        {
-            /* Destroy the mutex. */
-            gcmkCHECK_STATUS(gckOS_DeleteMutex(Command->os, node->Virtual.mutex));
-        }
-
         /* Free the structure. */
         gcmkCHECK_STATUS(gcmkOS_SAFE_FREE(Command->os, node));
     }

drivers/mxc/gpu-viv/hal/kernel/gc_hal_kernel_video_memory.c

@@ -253,14 +253,8 @@ gckVIDMEM_ConstructVirtual(
         node->Virtual.lockKernels[i]    = gcvNULL;
     }
 
-    node->Virtual.mutex         = gcvNULL;
-
     gcmkONERROR(gckOS_GetProcessID(&node->Virtual.processID));
 
-    /* Create the mutex. */
-    gcmkONERROR(
-        gckOS_CreateMutex(os, &node->Virtual.mutex));
-
 #if LINUX_CMA_FSL
     if(node->Virtual.contiguous && (!node->Virtual.cacheable))
     {
@@ -306,12 +300,6 @@ gckVIDMEM_ConstructVirtual(
     /* Roll back. */
     if (node != gcvNULL)
     {
-        if (node->Virtual.mutex != gcvNULL)
-        {
-            /* Destroy the mutex. */
-            gcmkVERIFY_OK(gckOS_DeleteMutex(os, node->Virtual.mutex));
-        }
-
         /* Free the structure. */
         gcmkVERIFY_OK(gcmkOS_SAFE_FREE(os, node));
     }
@@ -352,9 +340,6 @@ gckVIDMEM_DestroyVirtual(
     os = Node->Virtual.kernel->os;
     gcmkVERIFY_OBJECT(os, gcvOBJ_OS);
 
-    /* Delete the mutex. */
-    gcmkVERIFY_OK(gckOS_DeleteMutex(os, Node->Virtual.mutex));
-
     /* Delete the gcuVIDMEM_NODE union. */
     gcmkVERIFY_OK(gcmkOS_SAFE_FREE(os, Node));
 
@@ -438,7 +423,6 @@ gckVIDMEM_Construct(
     memory->bytes       = heapBytes;
     memory->freeBytes   = heapBytes;
     memory->threshold   = Threshold;
-    memory->mutex       = gcvNULL;
 
     BaseAddress = 0;
 
@@ -561,9 +545,6 @@ gckVIDMEM_Construct(
                   "[GALCORE] TILE_STATUS:   bank %d",
                   memory->mapping[gcvSURF_TILE_STATUS]);
 
-    /* Allocate the mutex. */
-    gcmkONERROR(gckOS_CreateMutex(Os, &memory->mutex));
-
     /* Return pointer to the gckVIDMEM object. */
     *Memory = memory;
 
@@ -575,12 +556,6 @@ gckVIDMEM_Construct(
     /* Roll back. */
     if (memory != gcvNULL)
     {
-        if (memory->mutex != gcvNULL)
-        {
-            /* Delete the mutex. */
-            gcmkVERIFY_OK(gckOS_DeleteMutex(Os, memory->mutex));
-        }
-
         for (i = 0; i < banks; ++i)
         {
             /* Free the heap. */
@@ -647,9 +622,6 @@ gckVIDMEM_Destroy(
         }
     }
 
-    /* Free the mutex. */
-    gcmkVERIFY_OK(gckOS_DeleteMutex(Memory->os, Memory->mutex));
-
     /* Mark the object as unknown. */
     Memory->object.type = gcvOBJ_UNKNOWN;
 
@@ -914,7 +886,7 @@ gckVIDMEM_AllocateLinear(
     gcmkVERIFY_ARGUMENT(Type < gcvSURF_NUM_TYPES);
 
     /* Acquire the mutex. */
-    gcmkONERROR(gckOS_AcquireMutex(Memory->os, Memory->mutex, gcvINFINITE));
+    gcmkONERROR(gckOS_AcquireMutex(Memory->os, Kernel->vidmemMutex, gcvINFINITE));
 
     acquired = gcvTRUE;
 
@@ -1030,7 +1002,7 @@ gckVIDMEM_AllocateLinear(
 #endif
 
     /* Release the mutex. */
-    gcmkVERIFY_OK(gckOS_ReleaseMutex(Memory->os, Memory->mutex));
+    gcmkVERIFY_OK(gckOS_ReleaseMutex(Memory->os, Kernel->vidmemMutex));
 
     /* Return the pointer to the node. */
     *Node = node;
@@ -1047,7 +1019,7 @@ gckVIDMEM_AllocateLinear(
     if (acquired)
     {
      /* Release the mutex. */
-        gcmkVERIFY_OK(gckOS_ReleaseMutex(Memory->os, Memory->mutex));
+        gcmkVERIFY_OK(gckOS_ReleaseMutex(Memory->os, Kernel->vidmemMutex));
     }
 
     /* Return the status. */
@@ -1087,6 +1059,11 @@ gckVIDMEM_Free(
 
     gcmkHEADER_ARG("Node=0x%x", Node);
 
+    /* Acquire the mutex. */
+    gcmkONERROR(
+        gckOS_AcquireMutex(Kernel->os, Kernel->vidmemMutex, gcvINFINITE));
+    mutexAcquired = gcvTRUE;
+
     /* Verify the arguments. */
     if ((Node == gcvNULL)
     ||  (Node->VidMem.memory == gcvNULL)
@@ -1103,12 +1080,6 @@ gckVIDMEM_Free(
         /* Extract pointer to gckVIDMEM object owning the node. */
         memory = Node->VidMem.memory;
 
-        /* Acquire the mutex. */
-        gcmkONERROR(
-            gckOS_AcquireMutex(memory->os, memory->mutex, gcvINFINITE));
-
-        mutexAcquired = gcvTRUE;
-
 #ifdef __QNXNTO__
         /* Unmap the video memory. */
         if (Node->VidMem.logical != gcvNULL)
@@ -1193,7 +1164,7 @@ gckVIDMEM_Free(
         }
 
         /* Release the mutex. */
-        gcmkVERIFY_OK(gckOS_ReleaseMutex(memory->os, memory->mutex));
+        gcmkVERIFY_OK(gckOS_ReleaseMutex(Kernel->os, Kernel->vidmemMutex));
 
         gcmkTRACE_ZONE(gcvLEVEL_INFO, gcvZONE_VIDMEM,
                        "Node 0x%x is freed.",
@@ -1247,16 +1218,19 @@ gckVIDMEM_Free(
     /* Destroy the gcuVIDMEM_NODE union. */
     gcmkVERIFY_OK(gckVIDMEM_DestroyVirtual(Node));
 
+    gcmkVERIFY_OK(gckOS_ReleaseMutex(Kernel->os, Kernel->vidmemMutex));
+
     /* Success. */
     gcmkFOOTER_NO();
     return gcvSTATUS_OK;
 
 OnError:
+
     if (mutexAcquired)
     {
         /* Release the mutex. */
         gcmkVERIFY_OK(gckOS_ReleaseMutex(
-            memory->os, memory->mutex
+            Kernel->os,Kernel->vidmemMutex
             ));
     }
 
@@ -1492,6 +1466,10 @@ gckVIDMEM_Lock(
     /* Verify the arguments. */
     gcmkVERIFY_ARGUMENT(Address != gcvNULL);
 
+    /* Grab the mutex. */
+    gcmkONERROR(gckOS_AcquireMutex(Kernel->os, Kernel->vidmemMutex, gcvINFINITE));
+    acquired = gcvTRUE;
+
     if ((Node == gcvNULL)
     ||  (Node->VidMem.memory == gcvNULL)
     )
@@ -1553,10 +1531,6 @@ gckVIDMEM_Lock(
         os = Node->Virtual.kernel->os;
         gcmkVERIFY_OBJECT(os, gcvOBJ_OS);
 
-        /* Grab the mutex. */
-        gcmkONERROR(gckOS_AcquireMutex(os, Node->Virtual.mutex, gcvINFINITE));
-        acquired = gcvTRUE;
-
 #if gcdPAGED_MEMORY_CACHEABLE
         /* Force video memory cacheable. */
         Cacheable = gcvTRUE;
@@ -1655,10 +1629,11 @@ gckVIDMEM_Lock(
         *Address = Node->Virtual.addresses[Kernel->core];
 #endif
 
-        /* Release the mutex. */
-        gcmkVERIFY_OK(gckOS_ReleaseMutex(os, Node->Virtual.mutex));
     }
 
+    /* Release the mutex. */
+    gcmkVERIFY_OK(gckOS_ReleaseMutex(Kernel->os, Kernel->vidmemMutex));
+
     /* Success. */
     gcmkFOOTER_ARG("*Address=%08x", *Address);
     return gcvSTATUS_OK;
@@ -1704,7 +1679,7 @@ gckVIDMEM_Lock(
     if (acquired)
     {
         /* Release the mutex. */
-        gcmkVERIFY_OK(gckOS_ReleaseMutex(os, Node->Virtual.mutex));
+        gcmkVERIFY_OK(gckOS_ReleaseMutex(Kernel->os, Kernel->vidmemMutex));
     }
 
     /* Return the status. */
@@ -1755,6 +1730,10 @@ gckVIDMEM_Unlock(
     gcmkHEADER_ARG("Node=0x%x Type=%d *Asynchroneous=%d",
                    Node, Type, gcmOPT_VALUE(Asynchroneous));
 
+    /* Grab the mutex. */
+    gcmkONERROR(gckOS_AcquireMutex(Kernel->os, Kernel->vidmemMutex, gcvINFINITE));
+    acquired = gcvTRUE;
+
     /* Verify the arguments. */
     if ((Node == gcvNULL)
     ||  (Node->VidMem.memory == gcvNULL)
@@ -1800,12 +1779,6 @@ gckVIDMEM_Unlock(
         os = Kernel->os;
         gcmkVERIFY_OBJECT(os, gcvOBJ_OS);
 
-        /* Grab the mutex. */
-        gcmkONERROR(
-            gckOS_AcquireMutex(os, Node->Virtual.mutex, gcvINFINITE));
-
-        acquired = gcvTRUE;
-
         if (Asynchroneous == gcvNULL)
         {
 #if !gcdPROCESS_ADDRESS_SPACE
@@ -1868,13 +1841,12 @@ gckVIDMEM_Unlock(
             /* Schedule the surface to be unlocked. */
             *Asynchroneous = gcvTRUE;
         }
-
-        /* Release the mutex. */
-        gcmkVERIFY_OK(gckOS_ReleaseMutex(os, Node->Virtual.mutex));
-
-        acquired = gcvFALSE;
     }
 
+    /* Release the mutex. */
+    gcmkVERIFY_OK(gckOS_ReleaseMutex(Kernel->os, Kernel->vidmemMutex));
+    acquired = gcvFALSE;
+
     /* Success. */
     gcmkFOOTER_ARG("*Asynchroneous=%d", gcmOPT_VALUE(Asynchroneous));
     return gcvSTATUS_OK;
@@ -1883,7 +1855,7 @@ gckVIDMEM_Unlock(
     if (acquired)
     {
         /* Release the mutex. */
-        gcmkVERIFY_OK(gckOS_ReleaseMutex(os, Node->Virtual.mutex));
+        gcmkVERIFY_OK(gckOS_ReleaseMutex(Kernel->os, Kernel->vidmemMutex));
     }
 
     /* Return the status. */