[Spice-devel] [PATCH 3/8] Move SurfaceInfo to global data

[alexl at redhat.com]

From: Alexander Larsson <alexl at redhat.com>

When we release a surface resource, for instance when AllocMem gets
OOM, we call ReleaseOutput with the pdev in use (i.e. probably the
currently active one). However, its possible that the resource we
free is actually from another (now inactive) pdev.

Since the surface release function FreeDelSurface() uses pdev to
find the data for the surface id we may find the wrong surface data.

So, we move all the SurfaceInfos except the primary one to a global
array. This is fine since the surface ids (except 0) are never shared
between pdevs due to the surfaces_used global array.
---
 display/driver.c  |   12 ------------
 display/qxldd.h   |    3 ++-
 display/res.c     |   11 +++++++++++
 display/surface.h |   10 ++++++++--
 4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/display/driver.c b/display/driver.c
index 9abecb3..742348c 100644
--- a/display/driver.c
+++ b/display/driver.c
@@ -553,7 +553,6 @@ VOID DrvDisablePDEV(DHPDEV in_pdev)
     PDev* pdev = (PDev*)in_pdev;
 
     DEBUG_PRINT((NULL, 1, "%s: 0x%lx\n", __FUNCTION__, pdev));
-    EngFreeMem(pdev->surfaces_info);
     ResDestroy(pdev);
     DestroyPalette(pdev);
     EngDeleteSemaphore(pdev->cmd_sem);
@@ -744,12 +743,6 @@ BOOL PrepareHardware(PDev *pdev)
     pdev->log_level = dev_info.log_level;
 
     pdev->n_surfaces = dev_info.n_surfaces;
-    if (!(pdev->surfaces_info = (SurfaceInfo *)EngAllocMem(FL_ZERO_MEMORY,
-                                                           sizeof(SurfaceInfo) *
-                                                           pdev->n_surfaces, ALLOC_TAG))) {
-        DEBUG_PRINT((NULL, 0, "%s: surfaces_info alloc failed\n", __FUNCTION__));
-        return FALSE;
-    }
 
     pdev->mem_slots = EngAllocMem(FL_ZERO_MEMORY, sizeof(PMemSlot) * dev_info.num_mem_slot,
                                   ALLOC_TAG);
@@ -932,11 +925,6 @@ VOID DrvDisableSurface(DHPDEV in_pdev)
         pdev->surf = NULL;
     }
 
-    if (pdev->surfaces_info) {
-        EngFreeMem(pdev->surfaces_info);
-        pdev->surfaces_info = NULL;
-    }
-
     if (pdev->mem_slots) {
         EngFreeMem(pdev->mem_slots);
         pdev->mem_slots = NULL;
diff --git a/display/qxldd.h b/display/qxldd.h
index b896e23..7d22a0d 100644
--- a/display/qxldd.h
+++ b/display/qxldd.h
@@ -191,6 +191,7 @@ typedef struct DevRes {
     struct InternalPalette *palette_cache[PALETTE_HASH_SIZE];
     UINT32 num_palettes;
 
+    SurfaceInfo *surfaces_info;
     UINT8 *surfaces_used;
 
     HANDLE driver;
@@ -313,7 +314,7 @@ typedef struct PDev {
     UINT8 FPUSave[16 * 4 + 15];
 
     UINT32 n_surfaces;
-    SurfaceInfo *surfaces_info;
+    SurfaceInfo surface0_info;
 
     VIDEOMEMORY *pvmList;
 } PDev;
diff --git a/display/res.c b/display/res.c
index e1bb110..15e14ae 100644
--- a/display/res.c
+++ b/display/res.c
@@ -358,6 +358,10 @@ void CleanGlobalRes()
                 EngFreeMem(global_res[i].surfaces_used);
                 global_res[i].surfaces_used = NULL;
             }
+            if (global_res[i].surfaces_info) {
+                EngFreeMem(global_res[i].surfaces_info);
+                global_res[i].surfaces_info = NULL;
+            }
         }
         EngFreeMem(global_res);
         global_res = NULL;
@@ -400,6 +404,13 @@ static void InitRes(PDev *pdev)
         PANIC(pdev, "Res surfaces_used allocation failed\n");
     }
 
+    pdev->Res.surfaces_info = (SurfaceInfo *)EngAllocMem(FL_ZERO_MEMORY,
+							 sizeof(SurfaceInfo) * pdev->n_surfaces, 
+							 ALLOC_TAG);
+    if (!pdev->Res.surfaces_info) {
+        PANIC(pdev, "Res surfaces_info allocation failed\n");
+    }
+
     pdev->Res.free_outputs = 0;
     InitMspace(&pdev->Res, MSPACE_TYPE_DEVRAM, pdev->io_pages_virt, pdev->num_io_pages * PAGE_SIZE);
     InitMspace(&pdev->Res, MSPACE_TYPE_VRAM, pdev->fb, pdev->fb_size);
diff --git a/display/surface.h b/display/surface.h
index 6390d77..dfda502 100644
--- a/display/surface.h
+++ b/display/surface.h
@@ -8,12 +8,18 @@ static _inline UINT32 GetSurfaceIdFromInfo(SurfaceInfo *info)
   PDev *pdev;
 
   pdev = info->pdev;
-  return info - pdev->surfaces_info;
+  if (info == &pdev->surface0_info) {
+    return 0;
+  }
+  return info - pdev->Res.surfaces_info;
 }
 
 static _inline SurfaceInfo *GetSurfaceInfo(PDev *pdev, UINT32 id)
 {
-  return &pdev->surfaces_info[id];
+  if (id == 0) {
+    return &pdev->surface0_info;
+  }
+  return &pdev->Res.surfaces_info[id];
 }
 
 static _inline UINT32 GetSurfaceId(SURFOBJ *surf)
-- 
1.7.2.1