[Spice-devel,08/12] qxl-wddm-dod: Prepare for failure to allocate memory

Submitted by Yuri Benditovich on March 12, 2017, 8:45 a.m.

Details

Message ID 1489308309-9728-9-git-send-email-yuri.benditovich@daynix.com
State New
Headers show
Series "Set of patches for further support of VSync" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Yuri Benditovich March 12, 2017, 8:45 a.m.
Preparation for further scenarios when memory allocation failure
can happen and we'll need to use fallback when possible.
Memory allocation can fail in following cases:
- when non-forced memory allocation used and the attempt to allocate
  the memory must be as fast as possible, without waits
- when forced memory allocation used, but the driver is in stop flow
  and it is possible that video subsystem already executes switch
  to VGA mode, so QEMU will not return previously allocated objects
In case of allocation failure we may need to free previously allocated
object (when more than one allocation required for operation), this
must be protected by m_MemLock mutex.
In case of forced memory allocation the allocation routine waits
unpredictable time until the request is satisfied. Now we do not acquire
m_MemLock mutex for all this time, but release it when enter long wait
to allow another caller to enter and try to allocate memory.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 qxldod/QxlDod.cpp | 107 ++++++++++++++++++++++++++++++++++++++++++++++++------
 qxldod/QxlDod.h   |   4 +-
 2 files changed, 99 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index bb9b227..9a3803a 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -3064,6 +3064,7 @@  QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
     m_FreeOutputs = 0;
     m_Pending = 0;
     m_PresentThread = NULL;
+    m_bActive = FALSE;
 }
 
 QxlDevice::~QxlDevice(void)
@@ -3494,14 +3495,20 @@  NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION* pDispInfo)
     Status = AcquireDisplayInfo(*(pDispInfo));
     if (NT_SUCCESS(Status))
     {
+        m_bActive = TRUE;
         Status = StartPresentThread();
     }
+    if (!NT_SUCCESS(Status))
+    {
+        m_bActive = FALSE;
+    }
     return Status;
 }
 
 void QxlDevice::QxlClose()
 {
     PAGED_CODE();
+    m_bActive = FALSE;
     StopPresentThread();
     DestroyMemSlots();
 }
@@ -3982,14 +3989,18 @@  void QxlDevice::WaitForReleaseRing(void)
 {
     PAGED_CODE();
     int wait;
+    BOOLEAN locked;
 
     DbgPrint(TRACE_LEVEL_VERBOSE, ("--->%s\n", __FUNCTION__));
 
+    locked = WaitForObject(&m_MemLock, NULL);
     for (;;) {
         LARGE_INTEGER timeout;
 
         if (SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
+            ReleaseMutex(&m_MemLock, locked);
             QXL_SLEEP(10);
+            locked = WaitForObject(&m_MemLock, NULL);
             if (!SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
                 break;
             }
@@ -3997,17 +4008,20 @@  void QxlDevice::WaitForReleaseRing(void)
         }
         SPICE_RING_CONS_WAIT(m_ReleaseRing, wait);
 
-        if (!wait) {
+        if (!wait || !m_bActive) {
             break;
         }
 
+        ReleaseMutex(&m_MemLock, locked);
         timeout.QuadPart = -30 * 1000 * 10; //30ms
         WaitForObject(&m_DisplayEvent, &timeout);
+        locked = WaitForObject(&m_MemLock, NULL);
 
         if (SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
             SyncIo(QXL_IO_NOTIFY_OOM, 0);
         }
     }
+    ReleaseMutex(&m_MemLock, locked);
     DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: <---\n", __FUNCTION__));
 }
 
@@ -4074,11 +4088,18 @@  UINT64 QxlDevice::ReleaseOutput(UINT64 output_id)
     return next;
 }
 
+void  QxlDevice::ReleaseOutputUnderLock(UINT64 output_id)
+{
+    WaitForObject(&m_MemLock, NULL);
+    ReleaseOutput(output_id);
+    ReleaseMutex(&m_MemLock, TRUE);
+}
+
 void *QxlDevice::AllocMem(UINT32 mspace_type, size_t size, BOOL force)
 {
     PAGED_CODE();
     PVOID ptr;
-    BOOLEAN locked = FALSE;
+    BOOLEAN locked = FALSE, bInfiniteWait = !!force;
 
     ASSERT(m_MSInfo[mspace_type]._mspace);
     DbgPrint(TRACE_LEVEL_VERBOSE, ("--->%s: %p(%d) size %u\n", __FUNCTION__,
@@ -4089,7 +4110,19 @@  void *QxlDevice::AllocMem(UINT32 mspace_type, size_t size, BOOL force)
      mspace_malloc_stats(m_MSInfo[mspace_type]._mspace);
 #endif
 
-    locked = WaitForObject(&m_MemLock, NULL);
+     if (bInfiniteWait)
+        locked = WaitForObject(&m_MemLock, NULL);
+     else
+     {
+         LARGE_INTEGER doNotWait;
+         doNotWait.QuadPart = 0;
+         locked = WaitForObject(&m_MemLock, &doNotWait);
+         if (!locked)
+         {
+             ptr = NULL;
+             goto skip_allocation;
+         }
+     }
 
     while (1) {
         /* Release lots of queued resources, before allocating, as we
@@ -4107,17 +4140,21 @@  void *QxlDevice::AllocMem(UINT32 mspace_type, size_t size, BOOL force)
             continue;
         }
 
-        if (force) {
+        if (force && m_bActive) {
             /* Ask spice to free some stuff */
+            ReleaseMutex(&m_MemLock, locked);
             WaitForReleaseRing();
+            locked = WaitForObject(&m_MemLock, NULL);
         } else {
             /* Fail */
             break;
         }
     }
+
+skip_allocation:
     ReleaseMutex(&m_MemLock, locked);
 
-    ASSERT((!ptr && !force) || (ptr >= m_MSInfo[mspace_type].mspace_start &&
+    ASSERT((!ptr && (!force || !m_bActive)) || (ptr >= m_MSInfo[mspace_type].mspace_start &&
                                       ptr < m_MSInfo[mspace_type].mspace_end));
     DbgPrint(TRACE_LEVEL_VERBOSE, ("<---%s: ptr 0x%x\n", __FUNCTION__, ptr));
     return ptr;
@@ -4150,6 +4187,10 @@  QXLDrawable *QxlDevice::GetDrawable()
     QXLOutput *output;
 
     output = (QXLOutput *)AllocMem(MSPACE_TYPE_VRAM, sizeof(QXLOutput) + sizeof(QXLDrawable), TRUE);
+    if (!output)
+    {
+        return NULL;
+    }
     output->num_res = 0;
     RESOURCE_TYPE(output, RESOURCE_TYPE_DRAWABLE);
     ((QXLDrawable *)output->data)->release_info.id = (UINT64)output;
@@ -4165,6 +4206,9 @@  QXLCursorCmd *QxlDevice::CursorCmd()
 
     DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
     output = (QXLOutput *)AllocMem(MSPACE_TYPE_VRAM, sizeof(QXLOutput) + sizeof(QXLCursorCmd), TRUE);
+    if (!output) {
+        return NULL;
+    }
     output->num_res = 0;
     RESOURCE_TYPE(output, RESOURCE_TYPE_CURSOR);
     cursor_cmd = (QXLCursorCmd *)output->data;
@@ -4314,6 +4358,10 @@  QXLDrawable *QxlDevice::Drawable(UINT8 type, CONST RECT *area, CONST RECT *clip,
     DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
 
     drawable = GetDrawable();
+    if (!drawable)
+    {
+        return NULL;
+    }
     drawable->surface_id = surface_id;
     drawable->type = type;
     drawable->effect = QXL_EFFECT_OPAQUE;
@@ -4326,7 +4374,7 @@  QXLDrawable *QxlDevice::Drawable(UINT8 type, CONST RECT *area, CONST RECT *clip,
 
     if (!SetClip(clip, drawable)) {
         DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: set clip failed\n", __FUNCTION__));
-        ReleaseOutput(drawable->release_info.id);
+        ReleaseOutputUnderLock(drawable->release_info.id);
         drawable = NULL;
     }
     DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
@@ -4457,6 +4505,12 @@  QXLDrawable *QxlDevice::BltBits (
     alloc_size = BITMAP_ALLOC_BASE + BITS_BUF_MAX - BITS_BUF_MAX % line_size;
     alloc_size = MIN(BITMAP_ALLOC_BASE + height * line_size, alloc_size);
     image_res = (Resource*)AllocMem(MSPACE_TYPE_VRAM, alloc_size, TRUE);
+    if (!image_res)
+    {
+        ReleaseOutputUnderLock(drawable->release_info.id);
+        DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate bitmap for drawable\n"));
+        return NULL;
+    }
 
     image_res->refs = 1;
     image_res->free = FreeBitmapImageEx;
@@ -4487,8 +4541,13 @@  QXLDrawable *QxlDevice::BltBits (
     alloc_size = height * line_size;
 
     for (; src != src_end; src -= pSrc->Pitch, alloc_size -= line_size) {
-        PutBytesAlign(&chunk, &dest, &dest_end, src,
-                      line_size, alloc_size, line_size);
+        if (!PutBytesAlign(&chunk, &dest, &dest_end, src,
+            line_size, alloc_size, line_size))
+        {
+            ReleaseOutputUnderLock(drawable->release_info.id);
+            DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate additional bitmap for drawable\n"));
+            return NULL;
+        }
     }
 
     internal->image.bitmap.palette = 0;
@@ -4509,7 +4568,7 @@  QXLDrawable *QxlDevice::BltBits (
     return drawable;
 }
 
-VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
+BOOLEAN QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
                             UINT8 **end_ptr, UINT8 *src, int size,
                             size_t alloc_size, uint32_t alignment)
 {
@@ -4527,6 +4586,10 @@  VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
             aligned_size -=  aligned_size % alignment;
 
             void *ptr = AllocMem(MSPACE_TYPE_VRAM, size + sizeof(QXLDataChunk), TRUE);
+            if (!ptr)
+            {
+                return FALSE;
+            }
             chunk->next_chunk = PA(ptr, m_SurfaceMemSlot);
             ((QXLDataChunk *)ptr)->prev_chunk = PA(chunk, m_SurfaceMemSlot);
             chunk = (QXLDataChunk *)ptr;
@@ -4547,6 +4610,7 @@  VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
     *now_ptr = now;
     *end_ptr = end;
     DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
+    return TRUE;
 }
 
 VOID QxlDevice::BlackOutScreen(CURRENT_BDD_MODE* pCurrentBddMod)
@@ -4610,6 +4674,11 @@  NTSTATUS  QxlDevice::SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* pSetPoi
     int num_images = 1;
 
     cursor_cmd = CursorCmd();
+    if (!cursor_cmd) {
+        DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor command\n", __FUNCTION__));
+        return STATUS_INSUFFICIENT_RESOURCES;
+    }
+
     cursor_cmd->type = QXL_CURSOR_SET;
 
     cursor_cmd->u.set.visible = TRUE;
@@ -4617,6 +4686,12 @@  NTSTATUS  QxlDevice::SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* pSetPoi
     cursor_cmd->u.set.position.y = 0;
 
     res = (Resource *)AllocMem(MSPACE_TYPE_VRAM, CURSOR_ALLOC_SIZE, TRUE);
+    if (!res) {
+        DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor data\n", __FUNCTION__));
+        ReleaseOutputUnderLock(cursor_cmd->release_info.id);
+        return STATUS_INSUFFICIENT_RESOURCES;
+    }
+
     res->refs = 1;
     res->free = FreeCursorEx;
     res->ptr = this;
@@ -4653,8 +4728,13 @@  NTSTATUS  QxlDevice::SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* pSetPoi
     end = (UINT8 *)res + CURSOR_ALLOC_SIZE;
     src_end = src + (pSetPointerShape->Pitch * pSetPointerShape->Height * num_images);
     for (; src != src_end; src += pSetPointerShape->Pitch) {
-        PutBytesAlign(&chunk, &now, &end, src, line_size,
-                 PAGE_SIZE, 1);
+        if (!PutBytesAlign(&chunk, &now, &end, src, line_size,
+                 PAGE_SIZE, 1))
+        {
+            DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor bitmap\n", __FUNCTION__));
+            ReleaseOutputUnderLock(cursor_cmd->release_info.id);
+            return STATUS_INSUFFICIENT_RESOURCES;
+        }
     }
     CursorCmdAddRes(cursor_cmd, res);
     RELEASE_RES(res);
@@ -4675,6 +4755,11 @@  NTSTATUS QxlDevice::SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION* pS
                                  pSetPointerPosition->X,
                                  pSetPointerPosition->Y));
     QXLCursorCmd *cursor_cmd = CursorCmd();
+    if (!cursor_cmd) {
+        DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor command\n", __FUNCTION__));
+        return STATUS_INSUFFICIENT_RESOURCES;
+    }
+
     if (pSetPointerPosition->X < 0 || !pSetPointerPosition->Flags.Visible) {
         cursor_cmd->type = QXL_CURSOR_HIDE;
     } else {
diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
index 7bb5de5..3ee16ae 100755
--- a/qxldod/QxlDod.h
+++ b/qxldod/QxlDod.h
@@ -585,6 +585,7 @@  private:
     void FlushReleaseRing();
     void FreeMem(UINT32 mspace_type, void *ptr);
     UINT64 ReleaseOutput(UINT64 output_id);
+    void ReleaseOutputUnderLock(UINT64 output_id);
     void WaitForReleaseRing(void);
     void EmptyReleaseRing(void);
     BOOL SetClip(const RECT *clip, QXLDrawable *drawable);
@@ -601,7 +602,7 @@  private:
     void PushCmd(void);
     void WaitForCursorRing(void);
     void PushCursor(void);
-    void PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
+    BOOLEAN PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
                             UINT8 **end_ptr, UINT8 *src, int size,
                             size_t alloc_size, uint32_t alignment);
     void AsyncIo(UCHAR  Port, UCHAR Value);
@@ -671,6 +672,7 @@  private:
 
     QXLPresentOnlyRing m_PresentRing[1];
     HANDLE m_PresentThread;
+    BOOLEAN m_bActive;
 };
 
 class QxlDod {

Comments

I'm reading through some of these patches and the commit logs are not
very clear to me. Part of this may be that I'm unfamiliar with many
details of windows drivers. But I think part of it is that the comments
may be a bit unclear.

On Sun, 2017-03-12 at 10:45 +0200, Yuri Benditovich wrote:
> Preparation for further scenarios when memory allocation failure
> can happen and we'll need to use fallback when possible.
> Memory allocation can fail in following cases:
> - when non-forced memory allocation used and the attempt to allocate
>   the memory must be as fast as possible, without waits
> - when forced memory allocation used, but the driver is in stop flow

What does it mean for a driver to be "in stop flow"? Is this a common
name for a state that a driver can be in? If it's a common term that
would be understood by any competent windows driver developer, then
fine. But otherwise, perhaps it needs to be clarified.

>   and it is possible that video subsystem already executes switch
>   to VGA mode, so QEMU will not return previously allocated objects

Perhaps describe why it will not release these objects?

> In case of allocation failure we may need to free previously
> allocated
> object (when more than one allocation required for operation), this
> must be protected by m_MemLock mutex.
> In case of forced memory allocation the allocation routine waits
> unpredictable time until the request is satisfied. Now we do not
> acquire
> m_MemLock mutex for all this time, but release it when enter long
> wait
> to allow another caller to enter and try to allocate memory.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  qxldod/QxlDod.cpp | 107
> ++++++++++++++++++++++++++++++++++++++++++++++++------
>  qxldod/QxlDod.h   |   4 +-
>  2 files changed, 99 insertions(+), 12 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index bb9b227..9a3803a 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -3064,6 +3064,7 @@ QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
>      m_FreeOutputs = 0;
>      m_Pending = 0;
>      m_PresentThread = NULL;
> +    m_bActive = FALSE;
>  }
>  
>  QxlDevice::~QxlDevice(void)
> @@ -3494,14 +3495,20 @@ NTSTATUS
> QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION* pDispInfo)
>      Status = AcquireDisplayInfo(*(pDispInfo));
>      if (NT_SUCCESS(Status))
>      {
> +        m_bActive = TRUE;
>          Status = StartPresentThread();
>      }
> +    if (!NT_SUCCESS(Status))
> +    {
> +        m_bActive = FALSE;
> +    }
>      return Status;
>  }
>  
>  void QxlDevice::QxlClose()
>  {
>      PAGED_CODE();
> +    m_bActive = FALSE;
>      StopPresentThread();
>      DestroyMemSlots();
>  }
> @@ -3982,14 +3989,18 @@ void QxlDevice::WaitForReleaseRing(void)
>  {
>      PAGED_CODE();
>      int wait;
> +    BOOLEAN locked;
>  
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("--->%s\n", __FUNCTION__));
>  
> +    locked = WaitForObject(&m_MemLock, NULL);
>      for (;;) {
>          LARGE_INTEGER timeout;
>  
>          if (SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
> +            ReleaseMutex(&m_MemLock, locked);
>              QXL_SLEEP(10);
> +            locked = WaitForObject(&m_MemLock, NULL);
>              if (!SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
>                  break;
>              }
> @@ -3997,17 +4008,20 @@ void QxlDevice::WaitForReleaseRing(void)
>          }
>          SPICE_RING_CONS_WAIT(m_ReleaseRing, wait);
>  
> -        if (!wait) {
> +        if (!wait || !m_bActive) {
>              break;
>          }
>  
> +        ReleaseMutex(&m_MemLock, locked);
>          timeout.QuadPart = -30 * 1000 * 10; //30ms
>          WaitForObject(&m_DisplayEvent, &timeout);
> +        locked = WaitForObject(&m_MemLock, NULL);
>  
>          if (SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
>              SyncIo(QXL_IO_NOTIFY_OOM, 0);
>          }
>      }
> +    ReleaseMutex(&m_MemLock, locked);
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: <---\n", __FUNCTION__));
>  }
>  
> @@ -4074,11 +4088,18 @@ UINT64 QxlDevice::ReleaseOutput(UINT64
> output_id)
>      return next;
>  }
>  
> +void  QxlDevice::ReleaseOutputUnderLock(UINT64 output_id)
> +{
> +    WaitForObject(&m_MemLock, NULL);
> +    ReleaseOutput(output_id);
> +    ReleaseMutex(&m_MemLock, TRUE);
> +}
> +
>  void *QxlDevice::AllocMem(UINT32 mspace_type, size_t size, BOOL
> force)
>  {
>      PAGED_CODE();
>      PVOID ptr;
> -    BOOLEAN locked = FALSE;
> +    BOOLEAN locked = FALSE, bInfiniteWait = !!force;
>  
>      ASSERT(m_MSInfo[mspace_type]._mspace);
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("--->%s: %p(%d) size %u\n",
> __FUNCTION__,
> @@ -4089,7 +4110,19 @@ void *QxlDevice::AllocMem(UINT32 mspace_type,
> size_t size, BOOL force)
>       mspace_malloc_stats(m_MSInfo[mspace_type]._mspace);
>  #endif
>  
> -    locked = WaitForObject(&m_MemLock, NULL);
> +     if (bInfiniteWait)
> +        locked = WaitForObject(&m_MemLock, NULL);
> +     else
> +     {
> +         LARGE_INTEGER doNotWait;
> +         doNotWait.QuadPart = 0;
> +         locked = WaitForObject(&m_MemLock, &doNotWait);
> +         if (!locked)
> +         {
> +             ptr = NULL;
> +             goto skip_allocation;
> +         }
> +     }
>  
>      while (1) {
>          /* Release lots of queued resources, before allocating, as
> we
> @@ -4107,17 +4140,21 @@ void *QxlDevice::AllocMem(UINT32 mspace_type,
> size_t size, BOOL force)
>              continue;
>          }
>  
> -        if (force) {
> +        if (force && m_bActive) {
>              /* Ask spice to free some stuff */
> +            ReleaseMutex(&m_MemLock, locked);
>              WaitForReleaseRing();
> +            locked = WaitForObject(&m_MemLock, NULL);
>          } else {
>              /* Fail */
>              break;
>          }
>      }
> +
> +skip_allocation:
>      ReleaseMutex(&m_MemLock, locked);
>  
> -    ASSERT((!ptr && !force) || (ptr >=
> m_MSInfo[mspace_type].mspace_start &&
> +    ASSERT((!ptr && (!force || !m_bActive)) || (ptr >=
> m_MSInfo[mspace_type].mspace_start &&
>                                        ptr <
> m_MSInfo[mspace_type].mspace_end));
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("<---%s: ptr 0x%x\n",
> __FUNCTION__, ptr));
>      return ptr;
> @@ -4150,6 +4187,10 @@ QXLDrawable *QxlDevice::GetDrawable()
>      QXLOutput *output;
>  
>      output = (QXLOutput *)AllocMem(MSPACE_TYPE_VRAM,
> sizeof(QXLOutput) + sizeof(QXLDrawable), TRUE);
> +    if (!output)
> +    {
> +        return NULL;
> +    }
>      output->num_res = 0;
>      RESOURCE_TYPE(output, RESOURCE_TYPE_DRAWABLE);
>      ((QXLDrawable *)output->data)->release_info.id = (UINT64)output;
> @@ -4165,6 +4206,9 @@ QXLCursorCmd *QxlDevice::CursorCmd()
>  
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
>      output = (QXLOutput *)AllocMem(MSPACE_TYPE_VRAM,
> sizeof(QXLOutput) + sizeof(QXLCursorCmd), TRUE);
> +    if (!output) {
> +        return NULL;
> +    }
>      output->num_res = 0;
>      RESOURCE_TYPE(output, RESOURCE_TYPE_CURSOR);
>      cursor_cmd = (QXLCursorCmd *)output->data;
> @@ -4314,6 +4358,10 @@ QXLDrawable *QxlDevice::Drawable(UINT8 type,
> CONST RECT *area, CONST RECT *clip,
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
>  
>      drawable = GetDrawable();
> +    if (!drawable)
> +    {
> +        return NULL;
> +    }
>      drawable->surface_id = surface_id;
>      drawable->type = type;
>      drawable->effect = QXL_EFFECT_OPAQUE;
> @@ -4326,7 +4374,7 @@ QXLDrawable *QxlDevice::Drawable(UINT8 type,
> CONST RECT *area, CONST RECT *clip,
>  
>      if (!SetClip(clip, drawable)) {
>          DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: set clip failed\n",
> __FUNCTION__));
> -        ReleaseOutput(drawable->release_info.id);
> +        ReleaseOutputUnderLock(drawable->release_info.id);
>          drawable = NULL;
>      }
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> @@ -4457,6 +4505,12 @@ QXLDrawable *QxlDevice::BltBits (
>      alloc_size = BITMAP_ALLOC_BASE + BITS_BUF_MAX - BITS_BUF_MAX %
> line_size;
>      alloc_size = MIN(BITMAP_ALLOC_BASE + height * line_size,
> alloc_size);
>      image_res = (Resource*)AllocMem(MSPACE_TYPE_VRAM, alloc_size,
> TRUE);
> +    if (!image_res)
> +    {
> +        ReleaseOutputUnderLock(drawable->release_info.id);
> +        DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate bitmap for
> drawable\n"));
> +        return NULL;
> +    }
>  
>      image_res->refs = 1;
>      image_res->free = FreeBitmapImageEx;
> @@ -4487,8 +4541,13 @@ QXLDrawable *QxlDevice::BltBits (
>      alloc_size = height * line_size;
>  
>      for (; src != src_end; src -= pSrc->Pitch, alloc_size -=
> line_size) {
> -        PutBytesAlign(&chunk, &dest, &dest_end, src,
> -                      line_size, alloc_size, line_size);
> +        if (!PutBytesAlign(&chunk, &dest, &dest_end, src,
> +            line_size, alloc_size, line_size))
> +        {
> +            ReleaseOutputUnderLock(drawable->release_info.id);
> +            DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate additional
> bitmap for drawable\n"));
> +            return NULL;
> +        }
>      }
>  
>      internal->image.bitmap.palette = 0;
> @@ -4509,7 +4568,7 @@ QXLDrawable *QxlDevice::BltBits (
>      return drawable;
>  }
>  
> -VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8
> **now_ptr,
> +BOOLEAN QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8
> **now_ptr,
>                              UINT8 **end_ptr, UINT8 *src, int size,
>                              size_t alloc_size, uint32_t alignment)
>  {
> @@ -4527,6 +4586,10 @@ VOID QxlDevice::PutBytesAlign(QXLDataChunk
> **chunk_ptr, UINT8 **now_ptr,
>              aligned_size -=  aligned_size % alignment;
>  
>              void *ptr = AllocMem(MSPACE_TYPE_VRAM, size +
> sizeof(QXLDataChunk), TRUE);
> +            if (!ptr)
> +            {
> +                return FALSE;
> +            }
>              chunk->next_chunk = PA(ptr, m_SurfaceMemSlot);
>              ((QXLDataChunk *)ptr)->prev_chunk = PA(chunk,
> m_SurfaceMemSlot);
>              chunk = (QXLDataChunk *)ptr;
> @@ -4547,6 +4610,7 @@ VOID QxlDevice::PutBytesAlign(QXLDataChunk
> **chunk_ptr, UINT8 **now_ptr,
>      *now_ptr = now;
>      *end_ptr = end;
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> +    return TRUE;
>  }
>  
>  VOID QxlDevice::BlackOutScreen(CURRENT_BDD_MODE* pCurrentBddMod)
> @@ -4610,6 +4674,11 @@ NTSTATUS  QxlDevice::SetPointerShape(_In_
> CONST DXGKARG_SETPOINTERSHAPE* pSetPoi
>      int num_images = 1;
>  
>      cursor_cmd = CursorCmd();
> +    if (!cursor_cmd) {
> +        DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor
> command\n", __FUNCTION__));
> +        return STATUS_INSUFFICIENT_RESOURCES;
> +    }
> +
>      cursor_cmd->type = QXL_CURSOR_SET;
>  
>      cursor_cmd->u.set.visible = TRUE;
> @@ -4617,6 +4686,12 @@ NTSTATUS  QxlDevice::SetPointerShape(_In_
> CONST DXGKARG_SETPOINTERSHAPE* pSetPoi
>      cursor_cmd->u.set.position.y = 0;
>  
>      res = (Resource *)AllocMem(MSPACE_TYPE_VRAM, CURSOR_ALLOC_SIZE,
> TRUE);
> +    if (!res) {
> +        DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor
> data\n", __FUNCTION__));
> +        ReleaseOutputUnderLock(cursor_cmd->release_info.id);
> +        return STATUS_INSUFFICIENT_RESOURCES;
> +    }
> +
>      res->refs = 1;
>      res->free = FreeCursorEx;
>      res->ptr = this;
> @@ -4653,8 +4728,13 @@ NTSTATUS  QxlDevice::SetPointerShape(_In_
> CONST DXGKARG_SETPOINTERSHAPE* pSetPoi
>      end = (UINT8 *)res + CURSOR_ALLOC_SIZE;
>      src_end = src + (pSetPointerShape->Pitch * pSetPointerShape-
> >Height * num_images);
>      for (; src != src_end; src += pSetPointerShape->Pitch) {
> -        PutBytesAlign(&chunk, &now, &end, src, line_size,
> -                 PAGE_SIZE, 1);
> +        if (!PutBytesAlign(&chunk, &now, &end, src, line_size,
> +                 PAGE_SIZE, 1))
> +        {
> +            DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate
> cursor bitmap\n", __FUNCTION__));
> +            ReleaseOutputUnderLock(cursor_cmd->release_info.id);
> +            return STATUS_INSUFFICIENT_RESOURCES;
> +        }
>      }
>      CursorCmdAddRes(cursor_cmd, res);
>      RELEASE_RES(res);
> @@ -4675,6 +4755,11 @@ NTSTATUS QxlDevice::SetPointerPosition(_In_
> CONST DXGKARG_SETPOINTERPOSITION* pS
>                                   pSetPointerPosition->X,
>                                   pSetPointerPosition->Y));
>      QXLCursorCmd *cursor_cmd = CursorCmd();
> +    if (!cursor_cmd) {
> +        DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor
> command\n", __FUNCTION__));
> +        return STATUS_INSUFFICIENT_RESOURCES;
> +    }
> +
>      if (pSetPointerPosition->X < 0 || !pSetPointerPosition-
> >Flags.Visible) {
>          cursor_cmd->type = QXL_CURSOR_HIDE;
>      } else {
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index 7bb5de5..3ee16ae 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -585,6 +585,7 @@ private:
>      void FlushReleaseRing();
>      void FreeMem(UINT32 mspace_type, void *ptr);
>      UINT64 ReleaseOutput(UINT64 output_id);
> +    void ReleaseOutputUnderLock(UINT64 output_id);
>      void WaitForReleaseRing(void);
>      void EmptyReleaseRing(void);
>      BOOL SetClip(const RECT *clip, QXLDrawable *drawable);
> @@ -601,7 +602,7 @@ private:
>      void PushCmd(void);
>      void WaitForCursorRing(void);
>      void PushCursor(void);
> -    void PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> +    BOOLEAN PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
>                              UINT8 **end_ptr, UINT8 *src, int size,
>                              size_t alloc_size, uint32_t alignment);
>      void AsyncIo(UCHAR  Port, UCHAR Value);
> @@ -671,6 +672,7 @@ private:
>  
>      QXLPresentOnlyRing m_PresentRing[1];
>      HANDLE m_PresentThread;
> +    BOOLEAN m_bActive;
>  };
>  
>  class QxlDod {
> 
> Preparation for further scenarios when memory allocation failure
> can happen and we'll need to use fallback when possible.
> Memory allocation can fail in following cases:
> - when non-forced memory allocation used and the attempt to allocate
>   the memory must be as fast as possible, without waits
> - when forced memory allocation used, but the driver is in stop flow
>   and it is possible that video subsystem already executes switch
>   to VGA mode, so QEMU will not return previously allocated objects
> In case of allocation failure we may need to free previously allocated
> object (when more than one allocation required for operation), this
> must be protected by m_MemLock mutex.
> In case of forced memory allocation the allocation routine waits
> unpredictable time until the request is satisfied. Now we do not acquire
> m_MemLock mutex for all this time, but release it when enter long wait
> to allow another caller to enter and try to allocate memory.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  qxldod/QxlDod.cpp | 107
>  ++++++++++++++++++++++++++++++++++++++++++++++++------
>  qxldod/QxlDod.h   |   4 +-
>  2 files changed, 99 insertions(+), 12 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index bb9b227..9a3803a 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -3064,6 +3064,7 @@ QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
>      m_FreeOutputs = 0;
>      m_Pending = 0;
>      m_PresentThread = NULL;
> +    m_bActive = FALSE;

What does it mean that the device is not active?

>  }
>  
>  QxlDevice::~QxlDevice(void)
> @@ -3494,14 +3495,20 @@ NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION*
> pDispInfo)
>      Status = AcquireDisplayInfo(*(pDispInfo));
>      if (NT_SUCCESS(Status))
>      {
> +        m_bActive = TRUE;
>          Status = StartPresentThread();
>      }
> +    if (!NT_SUCCESS(Status))
> +    {
> +        m_bActive = FALSE;

So this means that if the thread cannot be created the device
will work in a an inactive state?

> +    }
>      return Status;
>  }
>  
>  void QxlDevice::QxlClose()
>  {
>      PAGED_CODE();
> +    m_bActive = FALSE;
>      StopPresentThread();
>      DestroyMemSlots();
>  }
> @@ -3982,14 +3989,18 @@ void QxlDevice::WaitForReleaseRing(void)
>  {
>      PAGED_CODE();
>      int wait;
> +    BOOLEAN locked;
>  
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("--->%s\n", __FUNCTION__));
>  
> +    locked = WaitForObject(&m_MemLock, NULL);
>      for (;;) {
>          LARGE_INTEGER timeout;
>  
>          if (SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
> +            ReleaseMutex(&m_MemLock, locked);
>              QXL_SLEEP(10);
> +            locked = WaitForObject(&m_MemLock, NULL);
>              if (!SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
>                  break;
>              }
> @@ -3997,17 +4008,20 @@ void QxlDevice::WaitForReleaseRing(void)
>          }
>          SPICE_RING_CONS_WAIT(m_ReleaseRing, wait);
>  
> -        if (!wait) {
> +        if (!wait || !m_bActive) {
>              break;
>          }
>  
> +        ReleaseMutex(&m_MemLock, locked);
>          timeout.QuadPart = -30 * 1000 * 10; //30ms
>          WaitForObject(&m_DisplayEvent, &timeout);
> +        locked = WaitForObject(&m_MemLock, NULL);
>  
>          if (SPICE_RING_IS_EMPTY(m_ReleaseRing)) {
>              SyncIo(QXL_IO_NOTIFY_OOM, 0);
>          }
>      }
> +    ReleaseMutex(&m_MemLock, locked);
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: <---\n", __FUNCTION__));
>  }
>  
> @@ -4074,11 +4088,18 @@ UINT64 QxlDevice::ReleaseOutput(UINT64 output_id)
>      return next;
>  }
>  
> +void  QxlDevice::ReleaseOutputUnderLock(UINT64 output_id)
> +{
> +    WaitForObject(&m_MemLock, NULL);
> +    ReleaseOutput(output_id);
> +    ReleaseMutex(&m_MemLock, TRUE);
> +}
> +

Is not really clear why we need additional locking here.
We are allocating resources, failing to allocate some and cleaning up.
The FreeMem calls already use a MemLock to make mspace serialized.
Why we need additional serialization?

>  void *QxlDevice::AllocMem(UINT32 mspace_type, size_t size, BOOL force)
>  {
>      PAGED_CODE();
>      PVOID ptr;
> -    BOOLEAN locked = FALSE;
> +    BOOLEAN locked = FALSE, bInfiniteWait = !!force;
>  
>      ASSERT(m_MSInfo[mspace_type]._mspace);
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("--->%s: %p(%d) size %u\n", __FUNCTION__,
> @@ -4089,7 +4110,19 @@ void *QxlDevice::AllocMem(UINT32 mspace_type, size_t
> size, BOOL force)
>       mspace_malloc_stats(m_MSInfo[mspace_type]._mspace);
>  #endif
>  
> -    locked = WaitForObject(&m_MemLock, NULL);
> +     if (bInfiniteWait)

Why don't you use force here instead?
I don't see the reason for another variable.
Also note that this code is aligned with 5 spaces instead of 4.

> +        locked = WaitForObject(&m_MemLock, NULL);
> +     else
> +     {
> +         LARGE_INTEGER doNotWait;
> +         doNotWait.QuadPart = 0;
> +         locked = WaitForObject(&m_MemLock, &doNotWait);
> +         if (!locked)
> +         {
> +             ptr = NULL;
> +             goto skip_allocation;

I would prefer a return NULL here without the goto.

> +         }
> +     }
>  
>      while (1) {
>          /* Release lots of queued resources, before allocating, as we
> @@ -4107,17 +4140,21 @@ void *QxlDevice::AllocMem(UINT32 mspace_type, size_t
> size, BOOL force)
>              continue;
>          }
>  
> -        if (force) {
> +        if (force && m_bActive) {
>              /* Ask spice to free some stuff */
> +            ReleaseMutex(&m_MemLock, locked);
>              WaitForReleaseRing();
> +            locked = WaitForObject(&m_MemLock, NULL);
>          } else {
>              /* Fail */
>              break;
>          }
>      }
> +
> +skip_allocation:
>      ReleaseMutex(&m_MemLock, locked);
>  
> -    ASSERT((!ptr && !force) || (ptr >= m_MSInfo[mspace_type].mspace_start &&
> +    ASSERT((!ptr && (!force || !m_bActive)) || (ptr >=
> m_MSInfo[mspace_type].mspace_start &&
>                                        ptr <
>                                        m_MSInfo[mspace_type].mspace_end));
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("<---%s: ptr 0x%x\n", __FUNCTION__,
>      ptr));
>      return ptr;
> @@ -4150,6 +4187,10 @@ QXLDrawable *QxlDevice::GetDrawable()
>      QXLOutput *output;
>  
>      output = (QXLOutput *)AllocMem(MSPACE_TYPE_VRAM, sizeof(QXLOutput) +
>      sizeof(QXLDrawable), TRUE);
> +    if (!output)
> +    {
> +        return NULL;
> +    }
>      output->num_res = 0;
>      RESOURCE_TYPE(output, RESOURCE_TYPE_DRAWABLE);
>      ((QXLDrawable *)output->data)->release_info.id = (UINT64)output;
> @@ -4165,6 +4206,9 @@ QXLCursorCmd *QxlDevice::CursorCmd()
>  
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
>      output = (QXLOutput *)AllocMem(MSPACE_TYPE_VRAM, sizeof(QXLOutput) +
>      sizeof(QXLCursorCmd), TRUE);
> +    if (!output) {
> +        return NULL;
> +    }

We really should define a style!
Is it

   if (cond) code;

or

   if (cond) {
       code;
   }

or

   if (cond)
       code;

or

   if (cond)
   {
       code;
   }

only this patch contains at least 3 styles of if indentation!

>      output->num_res = 0;
>      RESOURCE_TYPE(output, RESOURCE_TYPE_CURSOR);
>      cursor_cmd = (QXLCursorCmd *)output->data;
> @@ -4314,6 +4358,10 @@ QXLDrawable *QxlDevice::Drawable(UINT8 type, CONST
> RECT *area, CONST RECT *clip,
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
>  
>      drawable = GetDrawable();
> +    if (!drawable)
> +    {
> +        return NULL;
> +    }
>      drawable->surface_id = surface_id;
>      drawable->type = type;
>      drawable->effect = QXL_EFFECT_OPAQUE;
> @@ -4326,7 +4374,7 @@ QXLDrawable *QxlDevice::Drawable(UINT8 type, CONST RECT
> *area, CONST RECT *clip,
>  
>      if (!SetClip(clip, drawable)) {
>          DbgPrint(TRACE_LEVEL_VERBOSE, ("%s: set clip failed\n",
>          __FUNCTION__));
> -        ReleaseOutput(drawable->release_info.id);
> +        ReleaseOutputUnderLock(drawable->release_info.id);
>          drawable = NULL;
>      }
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> @@ -4457,6 +4505,12 @@ QXLDrawable *QxlDevice::BltBits (
>      alloc_size = BITMAP_ALLOC_BASE + BITS_BUF_MAX - BITS_BUF_MAX %
>      line_size;
>      alloc_size = MIN(BITMAP_ALLOC_BASE + height * line_size, alloc_size);
>      image_res = (Resource*)AllocMem(MSPACE_TYPE_VRAM, alloc_size, TRUE);
> +    if (!image_res)
> +    {
> +        ReleaseOutputUnderLock(drawable->release_info.id);
> +        DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate bitmap for
> drawable\n"));
> +        return NULL;
> +    }
>  
>      image_res->refs = 1;
>      image_res->free = FreeBitmapImageEx;
> @@ -4487,8 +4541,13 @@ QXLDrawable *QxlDevice::BltBits (
>      alloc_size = height * line_size;
>  
>      for (; src != src_end; src -= pSrc->Pitch, alloc_size -= line_size) {
> -        PutBytesAlign(&chunk, &dest, &dest_end, src,
> -                      line_size, alloc_size, line_size);
> +        if (!PutBytesAlign(&chunk, &dest, &dest_end, src,
> +            line_size, alloc_size, line_size))
> +        {
> +            ReleaseOutputUnderLock(drawable->release_info.id);
> +            DbgPrint(TRACE_LEVEL_ERROR, ("Cannot allocate additional bitmap
> for drawable\n"));
> +            return NULL;
> +        }
>      }
>  
>      internal->image.bitmap.palette = 0;
> @@ -4509,7 +4568,7 @@ QXLDrawable *QxlDevice::BltBits (
>      return drawable;
>  }
>  
> -VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> +BOOLEAN QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
>                              UINT8 **end_ptr, UINT8 *src, int size,
>                              size_t alloc_size, uint32_t alignment)
>  {
> @@ -4527,6 +4586,10 @@ VOID QxlDevice::PutBytesAlign(QXLDataChunk
> **chunk_ptr, UINT8 **now_ptr,
>              aligned_size -=  aligned_size % alignment;
>  
>              void *ptr = AllocMem(MSPACE_TYPE_VRAM, size +
>              sizeof(QXLDataChunk), TRUE);
> +            if (!ptr)
> +            {
> +                return FALSE;
> +            }
>              chunk->next_chunk = PA(ptr, m_SurfaceMemSlot);
>              ((QXLDataChunk *)ptr)->prev_chunk = PA(chunk, m_SurfaceMemSlot);
>              chunk = (QXLDataChunk *)ptr;
> @@ -4547,6 +4610,7 @@ VOID QxlDevice::PutBytesAlign(QXLDataChunk **chunk_ptr,
> UINT8 **now_ptr,
>      *now_ptr = now;
>      *end_ptr = end;
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> +    return TRUE;
>  }
>  
>  VOID QxlDevice::BlackOutScreen(CURRENT_BDD_MODE* pCurrentBddMod)
> @@ -4610,6 +4674,11 @@ NTSTATUS  QxlDevice::SetPointerShape(_In_ CONST
> DXGKARG_SETPOINTERSHAPE* pSetPoi
>      int num_images = 1;
>  
>      cursor_cmd = CursorCmd();
> +    if (!cursor_cmd) {
> +        DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor
> command\n", __FUNCTION__));
> +        return STATUS_INSUFFICIENT_RESOURCES;
> +    }
> +
>      cursor_cmd->type = QXL_CURSOR_SET;
>  
>      cursor_cmd->u.set.visible = TRUE;
> @@ -4617,6 +4686,12 @@ NTSTATUS  QxlDevice::SetPointerShape(_In_ CONST
> DXGKARG_SETPOINTERSHAPE* pSetPoi
>      cursor_cmd->u.set.position.y = 0;
>  
>      res = (Resource *)AllocMem(MSPACE_TYPE_VRAM, CURSOR_ALLOC_SIZE, TRUE);
> +    if (!res) {
> +        DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor data\n",
> __FUNCTION__));
> +        ReleaseOutputUnderLock(cursor_cmd->release_info.id);
> +        return STATUS_INSUFFICIENT_RESOURCES;
> +    }
> +
>      res->refs = 1;
>      res->free = FreeCursorEx;
>      res->ptr = this;
> @@ -4653,8 +4728,13 @@ NTSTATUS  QxlDevice::SetPointerShape(_In_ CONST
> DXGKARG_SETPOINTERSHAPE* pSetPoi
>      end = (UINT8 *)res + CURSOR_ALLOC_SIZE;
>      src_end = src + (pSetPointerShape->Pitch * pSetPointerShape->Height *
>      num_images);
>      for (; src != src_end; src += pSetPointerShape->Pitch) {
> -        PutBytesAlign(&chunk, &now, &end, src, line_size,
> -                 PAGE_SIZE, 1);
> +        if (!PutBytesAlign(&chunk, &now, &end, src, line_size,
> +                 PAGE_SIZE, 1))
> +        {
> +            DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor
> bitmap\n", __FUNCTION__));
> +            ReleaseOutputUnderLock(cursor_cmd->release_info.id);
> +            return STATUS_INSUFFICIENT_RESOURCES;
> +        }
>      }
>      CursorCmdAddRes(cursor_cmd, res);
>      RELEASE_RES(res);
> @@ -4675,6 +4755,11 @@ NTSTATUS QxlDevice::SetPointerPosition(_In_ CONST
> DXGKARG_SETPOINTERPOSITION* pS
>                                   pSetPointerPosition->X,
>                                   pSetPointerPosition->Y));
>      QXLCursorCmd *cursor_cmd = CursorCmd();
> +    if (!cursor_cmd) {
> +        DbgPrint(TRACE_LEVEL_ERROR, ("%s: Failed to allocate cursor
> command\n", __FUNCTION__));
> +        return STATUS_INSUFFICIENT_RESOURCES;
> +    }
> +
>      if (pSetPointerPosition->X < 0 || !pSetPointerPosition->Flags.Visible) {
>          cursor_cmd->type = QXL_CURSOR_HIDE;
>      } else {
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index 7bb5de5..3ee16ae 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -585,6 +585,7 @@ private:
>      void FlushReleaseRing();
>      void FreeMem(UINT32 mspace_type, void *ptr);
>      UINT64 ReleaseOutput(UINT64 output_id);
> +    void ReleaseOutputUnderLock(UINT64 output_id);
>      void WaitForReleaseRing(void);
>      void EmptyReleaseRing(void);
>      BOOL SetClip(const RECT *clip, QXLDrawable *drawable);
> @@ -601,7 +602,7 @@ private:
>      void PushCmd(void);
>      void WaitForCursorRing(void);
>      void PushCursor(void);
> -    void PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
> +    BOOLEAN PutBytesAlign(QXLDataChunk **chunk_ptr, UINT8 **now_ptr,
>                              UINT8 **end_ptr, UINT8 *src, int size,
>                              size_t alloc_size, uint32_t alignment);
>      void AsyncIo(UCHAR  Port, UCHAR Value);
> @@ -671,6 +672,7 @@ private:
>  
>      QXLPresentOnlyRing m_PresentRing[1];
>      HANDLE m_PresentThread;
> +    BOOLEAN m_bActive;
>  };
>  
>  class QxlDod {

Frediano