[Spice-devel,01/12] qxl-wddm-dod: Prepare system thread for rendering

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

Details

Message ID 1489308309-9728-2-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:44 a.m.
Create rendering thread upon device start and terminate it upon
stop. The dedicated thread is normally pending for display commands
to be sent to the host. Currently only single NULL (termination)
command is placed to the ring where the thread consumes events from.

Signed-off-by: Javier Celaya <javier.celaya@flexvdi.com>
Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 qxldod/QxlDod.cpp | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qxldod/QxlDod.h   |  16 ++++++++
 2 files changed, 130 insertions(+)

Patch hide | download patch | download mbox

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 9175300..b952bf9 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -3062,6 +3062,7 @@  QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
     m_CustomMode = 0;
     m_FreeOutputs = 0;
     m_Pending = 0;
+    m_PresentThread = NULL;
 }
 
 QxlDevice::~QxlDevice(void)
@@ -3441,6 +3442,29 @@  NTSTATUS QxlDevice::HWInit(PCM_RESOURCE_LIST pResList, DXGK_DISPLAY_INFORMATION*
     return QxlInit(pDispInfo);
 }
 
+NTSTATUS QxlDevice::StartPresentThread()
+{
+    PAGED_CODE();
+    OBJECT_ATTRIBUTES ObjectAttributes;
+    NTSTATUS Status;
+
+    KeClearEvent(&m_PresentThreadReadyEvent);
+
+    InitializeObjectAttributes(&ObjectAttributes, NULL, OBJ_KERNEL_HANDLE, NULL, NULL);
+    Status = PsCreateSystemThread(
+        &m_PresentThread,
+        THREAD_ALL_ACCESS,
+        &ObjectAttributes,
+        NULL,
+        NULL,
+        PresentThreadRoutineWrapper,
+        this);
+
+    WaitForObject(&m_PresentThreadReadyEvent, NULL);
+
+    return Status;
+}
+
 NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION* pDispInfo)
 {
     PAGED_CODE();
@@ -3467,12 +3491,17 @@  NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION* pDispInfo)
     InitDeviceMemoryResources();
     InitMonitorConfig();
     Status = AcquireDisplayInfo(*(pDispInfo));
+    if (NT_SUCCESS(Status))
+    {
+        Status = StartPresentThread();
+    }
     return Status;
 }
 
 void QxlDevice::QxlClose()
 {
     PAGED_CODE();
+    StopPresentThread();
     DestroyMemSlots();
 }
 
@@ -3609,6 +3638,12 @@  BOOL QxlDevice::CreateEvents()
     KeInitializeEvent(&m_IoCmdEvent,
                       SynchronizationEvent,
                       FALSE);
+    KeInitializeEvent(&m_PresentEvent,
+                      SynchronizationEvent,
+                      FALSE);
+    KeInitializeEvent(&m_PresentThreadReadyEvent,
+                      SynchronizationEvent,
+                      FALSE);
     KeInitializeMutex(&m_MemLock,1);
     KeInitializeMutex(&m_CmdLock,1);
     KeInitializeMutex(&m_IoLock,1);
@@ -3625,6 +3660,7 @@  BOOL QxlDevice::CreateRings()
     m_CommandRing = &(m_RamHdr->cmd_ring);
     m_CursorRing = &(m_RamHdr->cursor_ring);
     m_ReleaseRing = &(m_RamHdr->release_ring);
+    SPICE_RING_INIT(m_PresentRing);
     DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
     return TRUE;
 }
@@ -4907,6 +4943,7 @@  UINT SpiceFromPixelFormat(D3DDDIFORMAT Format)
 
 NTSTATUS HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION& DispInfo)
 {
+    PAGED_CODE();
     NTSTATUS Status = STATUS_SUCCESS;
     if (GetId() == 0)
     {
@@ -4996,3 +5033,80 @@  QXL_NON_PAGED VOID QxlDod::VsyncTimerProcGate(_In_ _KDPC *dpc, _In_ PVOID contex
     QxlDod* pQxl = reinterpret_cast<QxlDod*>(context);
     pQxl->VsyncTimerProc();
 }
+
+void QxlDevice::StopPresentThread()
+{
+    PAGED_CODE();
+    PVOID pDispatcherObject;
+    if (m_PresentThread)
+    {
+        DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s\n", __FUNCTION__));
+        NTSTATUS Status = ObReferenceObjectByHandle(
+            m_PresentThread, 0, NULL, KernelMode, &pDispatcherObject, NULL);
+        if (NT_SUCCESS(Status))
+        {
+            PostToWorkerThread(NULL);
+            WaitForObject(pDispatcherObject, NULL);
+            ObDereferenceObject(pDispatcherObject);
+        }
+        ZwClose(m_PresentThread);
+        m_PresentThread = NULL;
+        DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
+    }
+}
+
+void QxlDevice::PresentThreadRoutine()
+{
+    PAGED_CODE();
+    int wait;
+    int notify;
+    QXLDrawable** drawables;
+
+    DbgPrint(TRACE_LEVEL_INFORMATION, ("--->%s\n", __FUNCTION__));
+
+    KeSetEvent(&m_PresentThreadReadyEvent, 0, FALSE);
+
+    while (1)
+    {
+        // Pop a drawables list from the ring
+        // No need for a mutex, only one consumer thread
+        SPICE_RING_CONS_WAIT(m_PresentRing, wait);
+        while (wait) {
+            WaitForObject(&m_PresentEvent, NULL);
+            SPICE_RING_CONS_WAIT(m_PresentRing, wait);
+        }
+        drawables = *SPICE_RING_CONS_ITEM(m_PresentRing);
+        SPICE_RING_POP(m_PresentRing, notify);
+        if (notify) {
+            KeSetEvent(&m_PresentThreadReadyEvent, 0, FALSE);
+        }
+
+        if (drawables) {
+            for (UINT i = 0; drawables[i]; ++i)
+                PushDrawable(drawables[i]);
+            delete[] reinterpret_cast<BYTE*>(drawables);
+        }
+        else {
+            DbgPrint(TRACE_LEVEL_WARNING, ("%s is being terminated\n", __FUNCTION__));
+            break;
+        }
+    }
+}
+
+void QxlDevice::PostToWorkerThread(QXLDrawable** drawables)
+{
+    PAGED_CODE();
+    // Push drawables into PresentRing and notify worker thread
+    int notify, wait;
+    SPICE_RING_PROD_WAIT(m_PresentRing, wait);
+    while (wait) {
+        WaitForObject(&m_PresentThreadReadyEvent, NULL);
+        SPICE_RING_PROD_WAIT(m_PresentRing, wait);
+    }
+    *SPICE_RING_PROD_ITEM(m_PresentRing) = drawables;
+    SPICE_RING_PUSH(m_PresentRing, notify);
+    if (notify) {
+        KeSetEvent(&m_PresentEvent, 0, FALSE);
+    }
+    DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
+}
diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
index 9cfb6de..4a62680 100755
--- a/qxldod/QxlDod.h
+++ b/qxldod/QxlDod.h
@@ -456,6 +456,10 @@  typedef struct DpcCbContext {
 #define MAX(x, y) (((x) >= (y)) ? (x) : (y))
 #define ALIGN(a, b) (((a) + ((b) - 1)) & ~((b) - 1))
 
+#include "start-packed.h"
+SPICE_RING_DECLARE(QXLPresentOnlyRing, QXLDrawable**, 1024);
+#include "end-packed.h"
+
 class QxlDevice  :
     public HwDeviceInterface
 {
@@ -559,6 +563,13 @@  private:
     NTSTATUS UpdateChildStatus(BOOLEAN connect);
     NTSTATUS SetCustomDisplay(QXLEscapeSetCustomDisplay* custom_display);
     void SetMonitorConfig(QXLHead* monitor_config);
+    NTSTATUS StartPresentThread();
+    void StopPresentThread();
+    void PresentThreadRoutine();
+    static void PresentThreadRoutineWrapper(HANDLE dev) {
+        ((QxlDevice *)dev)->PresentThreadRoutine();
+    }
+    void PostToWorkerThread(QXLDrawable** drawables);
 
     static LONG GetMaxSourceMappingHeight(RECT* DirtyRects, ULONG NumDirtyRects);
 
@@ -594,6 +605,8 @@  private:
     KEVENT m_DisplayEvent;
     KEVENT m_CursorEvent;
     KEVENT m_IoCmdEvent;
+    KEVENT m_PresentEvent;
+    KEVENT m_PresentThreadReadyEvent;
 
     PUCHAR m_LogPort;
     PUCHAR m_LogBuf;
@@ -609,6 +622,9 @@  private:
 
     QXLMonitorsConfig* m_monitor_config;
     QXLPHYSICAL* m_monitor_config_pa;
+
+    QXLPresentOnlyRing m_PresentRing[1];
+    HANDLE m_PresentThread;
 };
 
 class QxlDod {

Comments

> 
> Create rendering thread upon device start and terminate it upon
> stop. The dedicated thread is normally pending for display commands
> to be sent to the host. Currently only single NULL (termination)
> command is placed to the ring where the thread consumes events from.
> 
> Signed-off-by: Javier Celaya <javier.celaya@flexvdi.com>
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  qxldod/QxlDod.cpp | 114
>  ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qxldod/QxlDod.h   |  16 ++++++++
>  2 files changed, 130 insertions(+)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index 9175300..b952bf9 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -3062,6 +3062,7 @@ QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
>      m_CustomMode = 0;
>      m_FreeOutputs = 0;
>      m_Pending = 0;
> +    m_PresentThread = NULL;
>  }
>  
>  QxlDevice::~QxlDevice(void)
> @@ -3441,6 +3442,29 @@ NTSTATUS QxlDevice::HWInit(PCM_RESOURCE_LIST pResList,
> DXGK_DISPLAY_INFORMATION*
>      return QxlInit(pDispInfo);
>  }
>  
> +NTSTATUS QxlDevice::StartPresentThread()
> +{
> +    PAGED_CODE();
> +    OBJECT_ATTRIBUTES ObjectAttributes;
> +    NTSTATUS Status;
> +
> +    KeClearEvent(&m_PresentThreadReadyEvent);
> +
> +    InitializeObjectAttributes(&ObjectAttributes, NULL, OBJ_KERNEL_HANDLE,
> NULL, NULL);
> +    Status = PsCreateSystemThread(
> +        &m_PresentThread,
> +        THREAD_ALL_ACCESS,
> +        &ObjectAttributes,
> +        NULL,
> +        NULL,
> +        PresentThreadRoutineWrapper,
> +        this);
> +
> +    WaitForObject(&m_PresentThreadReadyEvent, NULL);
> +

Why we need to wait here? Same above, no much need to clear the event.

> +    return Status;
> +}
> +
>  NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION* pDispInfo)
>  {
>      PAGED_CODE();
> @@ -3467,12 +3491,17 @@ NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION*
> pDispInfo)
>      InitDeviceMemoryResources();
>      InitMonitorConfig();
>      Status = AcquireDisplayInfo(*(pDispInfo));
> +    if (NT_SUCCESS(Status))
> +    {
> +        Status = StartPresentThread();
> +    }
>      return Status;
>  }
>  
>  void QxlDevice::QxlClose()
>  {
>      PAGED_CODE();
> +    StopPresentThread();
>      DestroyMemSlots();
>  }
>  
> @@ -3609,6 +3638,12 @@ BOOL QxlDevice::CreateEvents()
>      KeInitializeEvent(&m_IoCmdEvent,
>                        SynchronizationEvent,
>                        FALSE);
> +    KeInitializeEvent(&m_PresentEvent,
> +                      SynchronizationEvent,
> +                      FALSE);
> +    KeInitializeEvent(&m_PresentThreadReadyEvent,
> +                      SynchronizationEvent,
> +                      FALSE);
>      KeInitializeMutex(&m_MemLock,1);
>      KeInitializeMutex(&m_CmdLock,1);
>      KeInitializeMutex(&m_IoLock,1);
> @@ -3625,6 +3660,7 @@ BOOL QxlDevice::CreateRings()
>      m_CommandRing = &(m_RamHdr->cmd_ring);
>      m_CursorRing = &(m_RamHdr->cursor_ring);
>      m_ReleaseRing = &(m_RamHdr->release_ring);
> +    SPICE_RING_INIT(m_PresentRing);
>      DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
>      return TRUE;
>  }
> @@ -4907,6 +4943,7 @@ UINT SpiceFromPixelFormat(D3DDDIFORMAT Format)
>  
>  NTSTATUS HwDeviceInterface::AcquireDisplayInfo(DXGK_DISPLAY_INFORMATION&
>  DispInfo)
>  {
> +    PAGED_CODE();
>      NTSTATUS Status = STATUS_SUCCESS;
>      if (GetId() == 0)
>      {
> @@ -4996,3 +5033,80 @@ QXL_NON_PAGED VOID QxlDod::VsyncTimerProcGate(_In_
> _KDPC *dpc, _In_ PVOID contex
>      QxlDod* pQxl = reinterpret_cast<QxlDod*>(context);
>      pQxl->VsyncTimerProc();
>  }
> +
> +void QxlDevice::StopPresentThread()
> +{
> +    PAGED_CODE();
> +    PVOID pDispatcherObject;
> +    if (m_PresentThread)
> +    {
> +        DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s\n", __FUNCTION__));
> +        NTSTATUS Status = ObReferenceObjectByHandle(
> +            m_PresentThread, 0, NULL, KernelMode, &pDispatcherObject, NULL);
> +        if (NT_SUCCESS(Status))
> +        {
> +            PostToWorkerThread(NULL);

In the unluckily event that ObReferenceObjectByHandle failed would
be better to do this call anyway, so we have a chance the thread
will be destroyed before system potentially free from memory
the thread code and data.

> +            WaitForObject(pDispatcherObject, NULL);
> +            ObDereferenceObject(pDispatcherObject);
> +        }
> +        ZwClose(m_PresentThread);
> +        m_PresentThread = NULL;
> +        DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
> +    }
> +}
> +
> +void QxlDevice::PresentThreadRoutine()
> +{
> +    PAGED_CODE();
> +    int wait;
> +    int notify;
> +    QXLDrawable** drawables;
> +
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("--->%s\n", __FUNCTION__));
> +
> +    KeSetEvent(&m_PresentThreadReadyEvent, 0, FALSE);
> +
> +    while (1)
> +    {
> +        // Pop a drawables list from the ring
> +        // No need for a mutex, only one consumer thread
> +        SPICE_RING_CONS_WAIT(m_PresentRing, wait);
> +        while (wait) {
> +            WaitForObject(&m_PresentEvent, NULL);
> +            SPICE_RING_CONS_WAIT(m_PresentRing, wait);
> +        }
> +        drawables = *SPICE_RING_CONS_ITEM(m_PresentRing);
> +        SPICE_RING_POP(m_PresentRing, notify);
> +        if (notify) {
> +            KeSetEvent(&m_PresentThreadReadyEvent, 0, FALSE);
> +        }
> +
> +        if (drawables) {
> +            for (UINT i = 0; drawables[i]; ++i)
> +                PushDrawable(drawables[i]);
> +            delete[] reinterpret_cast<BYTE*>(drawables);

why all these reinterpret_cast ?
Just

  delete[] drawables;

is enough (obviously allocation should the changed too).

> +        }
> +        else {
> +            DbgPrint(TRACE_LEVEL_WARNING, ("%s is being terminated\n",
> __FUNCTION__));
> +            break;
> +        }
> +    }
> +}
> +
> +void QxlDevice::PostToWorkerThread(QXLDrawable** drawables)
> +{
> +    PAGED_CODE();
> +    // Push drawables into PresentRing and notify worker thread
> +    int notify, wait;
> +    SPICE_RING_PROD_WAIT(m_PresentRing, wait);
> +    while (wait) {
> +        WaitForObject(&m_PresentThreadReadyEvent, NULL);
> +        SPICE_RING_PROD_WAIT(m_PresentRing, wait);
> +    }
> +    *SPICE_RING_PROD_ITEM(m_PresentRing) = drawables;
> +    SPICE_RING_PUSH(m_PresentRing, notify);
> +    if (notify) {
> +        KeSetEvent(&m_PresentEvent, 0, FALSE);
> +    }
> +    DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
> +}
> diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> index 9cfb6de..4a62680 100755
> --- a/qxldod/QxlDod.h
> +++ b/qxldod/QxlDod.h
> @@ -456,6 +456,10 @@ typedef struct DpcCbContext {
>  #define MAX(x, y) (((x) >= (y)) ? (x) : (y))
>  #define ALIGN(a, b) (((a) + ((b) - 1)) & ~((b) - 1))
>  
> +#include "start-packed.h"
> +SPICE_RING_DECLARE(QXLPresentOnlyRing, QXLDrawable**, 1024);
> +#include "end-packed.h"
> +

Not a regression, however this structure cannot be byte-packed.
This would cause potentially problems like 255+1 == 0.

>  class QxlDevice  :
>      public HwDeviceInterface
>  {
> @@ -559,6 +563,13 @@ private:
>      NTSTATUS UpdateChildStatus(BOOLEAN connect);
>      NTSTATUS SetCustomDisplay(QXLEscapeSetCustomDisplay* custom_display);
>      void SetMonitorConfig(QXLHead* monitor_config);
> +    NTSTATUS StartPresentThread();
> +    void StopPresentThread();
> +    void PresentThreadRoutine();
> +    static void PresentThreadRoutineWrapper(HANDLE dev) {
> +        ((QxlDevice *)dev)->PresentThreadRoutine();
> +    }
> +    void PostToWorkerThread(QXLDrawable** drawables);
>  
>      static LONG GetMaxSourceMappingHeight(RECT* DirtyRects, ULONG
>      NumDirtyRects);
>  
> @@ -594,6 +605,8 @@ private:
>      KEVENT m_DisplayEvent;
>      KEVENT m_CursorEvent;
>      KEVENT m_IoCmdEvent;
> +    KEVENT m_PresentEvent;
> +    KEVENT m_PresentThreadReadyEvent;
>  
>      PUCHAR m_LogPort;
>      PUCHAR m_LogBuf;
> @@ -609,6 +622,9 @@ private:
>  
>      QXLMonitorsConfig* m_monitor_config;
>      QXLPHYSICAL* m_monitor_config_pa;
> +
> +    QXLPresentOnlyRing m_PresentRing[1];
> +    HANDLE m_PresentThread;
>  };
>  
>  class QxlDod {

Frediano
On Mon, Mar 20, 2017 at 2:01 PM, Frediano Ziglio <fziglio@redhat.com> wrote:

> >
> > Create rendering thread upon device start and terminate it upon
> > stop. The dedicated thread is normally pending for display commands
> > to be sent to the host. Currently only single NULL (termination)
> > command is placed to the ring where the thread consumes events from.
> >
> > Signed-off-by: Javier Celaya <javier.celaya@flexvdi.com>
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >  qxldod/QxlDod.cpp | 114
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  qxldod/QxlDod.h   |  16 ++++++++
> >  2 files changed, 130 insertions(+)
> >
> > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> > index 9175300..b952bf9 100755
> > --- a/qxldod/QxlDod.cpp
> > +++ b/qxldod/QxlDod.cpp
> > @@ -3062,6 +3062,7 @@ QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
> >      m_CustomMode = 0;
> >      m_FreeOutputs = 0;
> >      m_Pending = 0;
> > +    m_PresentThread = NULL;
> >  }
> >
> >  QxlDevice::~QxlDevice(void)
> > @@ -3441,6 +3442,29 @@ NTSTATUS QxlDevice::HWInit(PCM_RESOURCE_LIST
> pResList,
> > DXGK_DISPLAY_INFORMATION*
> >      return QxlInit(pDispInfo);
> >  }
> >
> > +NTSTATUS QxlDevice::StartPresentThread()
> > +{
> > +    PAGED_CODE();
> > +    OBJECT_ATTRIBUTES ObjectAttributes;
> > +    NTSTATUS Status;
> > +
> > +    KeClearEvent(&m_PresentThreadReadyEvent);
> > +
> > +    InitializeObjectAttributes(&ObjectAttributes, NULL,
> OBJ_KERNEL_HANDLE,
> > NULL, NULL);
> > +    Status = PsCreateSystemThread(
> > +        &m_PresentThread,
> > +        THREAD_ALL_ACCESS,
> > +        &ObjectAttributes,
> > +        NULL,
> > +        NULL,
> > +        PresentThreadRoutineWrapper,
> > +        this);
> > +
> > +    WaitForObject(&m_PresentThreadReadyEvent, NULL);
> > +
>
> Why we need to wait here? Same above, no much need to clear the event.
>
> Wait is not mandatory - it is here to have predictable timing and ensure
the thread is ready before first 'Present' command.



> > +    return Status;
> > +}
> > +
> >  NTSTATUS QxlDevice::QxlInit(DXGK_DISPLAY_INFORMATION* pDispInfo)
> >  {
> >      PAGED_CODE();
> > @@ -3467,12 +3491,17 @@ NTSTATUS QxlDevice::QxlInit(DXGK_
> DISPLAY_INFORMATION*
> > pDispInfo)
> >      InitDeviceMemoryResources();
> >      InitMonitorConfig();
> >      Status = AcquireDisplayInfo(*(pDispInfo));
> > +    if (NT_SUCCESS(Status))
> > +    {
> > +        Status = StartPresentThread();
> > +    }
> >      return Status;
> >  }
> >
> >  void QxlDevice::QxlClose()
> >  {
> >      PAGED_CODE();
> > +    StopPresentThread();
> >      DestroyMemSlots();
> >  }
> >
> > @@ -3609,6 +3638,12 @@ BOOL QxlDevice::CreateEvents()
> >      KeInitializeEvent(&m_IoCmdEvent,
> >                        SynchronizationEvent,
> >                        FALSE);
> > +    KeInitializeEvent(&m_PresentEvent,
> > +                      SynchronizationEvent,
> > +                      FALSE);
> > +    KeInitializeEvent(&m_PresentThreadReadyEvent,
> > +                      SynchronizationEvent,
> > +                      FALSE);
> >      KeInitializeMutex(&m_MemLock,1);
> >      KeInitializeMutex(&m_CmdLock,1);
> >      KeInitializeMutex(&m_IoLock,1);
> > @@ -3625,6 +3660,7 @@ BOOL QxlDevice::CreateRings()
> >      m_CommandRing = &(m_RamHdr->cmd_ring);
> >      m_CursorRing = &(m_RamHdr->cursor_ring);
> >      m_ReleaseRing = &(m_RamHdr->release_ring);
> > +    SPICE_RING_INIT(m_PresentRing);
> >      DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
> >      return TRUE;
> >  }
> > @@ -4907,6 +4943,7 @@ UINT SpiceFromPixelFormat(D3DDDIFORMAT Format)
> >
> >  NTSTATUS HwDeviceInterface::AcquireDisplayInfo(DXGK_
> DISPLAY_INFORMATION&
> >  DispInfo)
> >  {
> > +    PAGED_CODE();
> >      NTSTATUS Status = STATUS_SUCCESS;
> >      if (GetId() == 0)
> >      {
> > @@ -4996,3 +5033,80 @@ QXL_NON_PAGED VOID QxlDod::VsyncTimerProcGate(_
> In_
> > _KDPC *dpc, _In_ PVOID contex
> >      QxlDod* pQxl = reinterpret_cast<QxlDod*>(context);
> >      pQxl->VsyncTimerProc();
> >  }
> > +
> > +void QxlDevice::StopPresentThread()
> > +{
> > +    PAGED_CODE();
> > +    PVOID pDispatcherObject;
> > +    if (m_PresentThread)
> > +    {
> > +        DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s\n", __FUNCTION__));
> > +        NTSTATUS Status = ObReferenceObjectByHandle(
> > +            m_PresentThread, 0, NULL, KernelMode, &pDispatcherObject,
> NULL);
> > +        if (NT_SUCCESS(Status))
> > +        {
> > +            PostToWorkerThread(NULL);
>
> In the unluckily event that ObReferenceObjectByHandle failed would
> be better to do this call anyway, so we have a chance the thread
> will be destroyed before system potentially free from memory
> the thread code and data.
>
> > +            WaitForObject(pDispatcherObject, NULL);
> > +            ObDereferenceObject(pDispatcherObject);
> > +        }
> > +        ZwClose(m_PresentThread);
> > +        m_PresentThread = NULL;
> > +        DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
> > +    }
> > +}
> > +
> > +void QxlDevice::PresentThreadRoutine()
> > +{
> > +    PAGED_CODE();
> > +    int wait;
> > +    int notify;
> > +    QXLDrawable** drawables;
> > +
> > +    DbgPrint(TRACE_LEVEL_INFORMATION, ("--->%s\n", __FUNCTION__));
> > +
> > +    KeSetEvent(&m_PresentThreadReadyEvent, 0, FALSE);
> > +
> > +    while (1)
> > +    {
> > +        // Pop a drawables list from the ring
> > +        // No need for a mutex, only one consumer thread
> > +        SPICE_RING_CONS_WAIT(m_PresentRing, wait);
> > +        while (wait) {
> > +            WaitForObject(&m_PresentEvent, NULL);
> > +            SPICE_RING_CONS_WAIT(m_PresentRing, wait);
> > +        }
> > +        drawables = *SPICE_RING_CONS_ITEM(m_PresentRing);
> > +        SPICE_RING_POP(m_PresentRing, notify);
> > +        if (notify) {
> > +            KeSetEvent(&m_PresentThreadReadyEvent, 0, FALSE);
> > +        }
> > +
> > +        if (drawables) {
> > +            for (UINT i = 0; drawables[i]; ++i)
> > +                PushDrawable(drawables[i]);
> > +            delete[] reinterpret_cast<BYTE*>(drawables);
>
> why all these reinterpret_cast ?
> Just
>
>   delete[] drawables;
>
> is enough (obviously allocation should the changed too).
>
> > +        }
> > +        else {
> > +            DbgPrint(TRACE_LEVEL_WARNING, ("%s is being terminated\n",
> > __FUNCTION__));
> > +            break;
> > +        }
> > +    }
> > +}
> > +
> > +void QxlDevice::PostToWorkerThread(QXLDrawable** drawables)
> > +{
> > +    PAGED_CODE();
> > +    // Push drawables into PresentRing and notify worker thread
> > +    int notify, wait;
> > +    SPICE_RING_PROD_WAIT(m_PresentRing, wait);
> > +    while (wait) {
> > +        WaitForObject(&m_PresentThreadReadyEvent, NULL);
> > +        SPICE_RING_PROD_WAIT(m_PresentRing, wait);
> > +    }
> > +    *SPICE_RING_PROD_ITEM(m_PresentRing) = drawables;
> > +    SPICE_RING_PUSH(m_PresentRing, notify);
> > +    if (notify) {
> > +        KeSetEvent(&m_PresentEvent, 0, FALSE);
> > +    }
> > +    DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
> > +}
> > diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
> > index 9cfb6de..4a62680 100755
> > --- a/qxldod/QxlDod.h
> > +++ b/qxldod/QxlDod.h
> > @@ -456,6 +456,10 @@ typedef struct DpcCbContext {
> >  #define MAX(x, y) (((x) >= (y)) ? (x) : (y))
> >  #define ALIGN(a, b) (((a) + ((b) - 1)) & ~((b) - 1))
> >
> > +#include "start-packed.h"
> > +SPICE_RING_DECLARE(QXLPresentOnlyRing, QXLDrawable**, 1024);
> > +#include "end-packed.h"
> > +
>
> Not a regression, however this structure cannot be byte-packed.
> This would cause potentially problems like 255+1 == 0.
>
> I do not see how this can cause problem.
Packing at byte boundary is not needed here, but the alternative is (ugly)
to use #define SPICE_ATTR_PACKED before declaration and #undef
SPICE_ATTR_PACKED after.


> >  class QxlDevice  :
> >      public HwDeviceInterface
> >  {
> > @@ -559,6 +563,13 @@ private:
> >      NTSTATUS UpdateChildStatus(BOOLEAN connect);
> >      NTSTATUS SetCustomDisplay(QXLEscapeSetCustomDisplay*
> custom_display);
> >      void SetMonitorConfig(QXLHead* monitor_config);
> > +    NTSTATUS StartPresentThread();
> > +    void StopPresentThread();
> > +    void PresentThreadRoutine();
> > +    static void PresentThreadRoutineWrapper(HANDLE dev) {
> > +        ((QxlDevice *)dev)->PresentThreadRoutine();
> > +    }
> > +    void PostToWorkerThread(QXLDrawable** drawables);
> >
> >      static LONG GetMaxSourceMappingHeight(RECT* DirtyRects, ULONG
> >      NumDirtyRects);
> >
> > @@ -594,6 +605,8 @@ private:
> >      KEVENT m_DisplayEvent;
> >      KEVENT m_CursorEvent;
> >      KEVENT m_IoCmdEvent;
> > +    KEVENT m_PresentEvent;
> > +    KEVENT m_PresentThreadReadyEvent;
> >
> >      PUCHAR m_LogPort;
> >      PUCHAR m_LogBuf;
> > @@ -609,6 +622,9 @@ private:
> >
> >      QXLMonitorsConfig* m_monitor_config;
> >      QXLPHYSICAL* m_monitor_config_pa;
> > +
> > +    QXLPresentOnlyRing m_PresentRing[1];
> > +    HANDLE m_PresentThread;
> >  };
> >
> >  class QxlDod {
>
> Frediano
>