[Spice-devel] [PATCH qxl-wddm-dod 07/26] Do not use virtual functions for code that must not be paged

Sameeh Jubran sameeh at daynix.com
Tue Aug 9 15:28:39 UTC 2016


From: Sandy Stutsman <sstutsma at redhat.com>

There is no way to guarantee that the static vtable will be in
non-pageable memory.  This patch converts 3 virtual non-pageble functions
to functions private to each device class, the parent class will make
an explicit call (based on a new device type initialized at object
creation) to the correct function.

Fixed one misspelling: HwDeviceInterface
---
 qxldod/QxlDod.cpp | 114 ++++++++++++++++++++++++++++++++++++++++++------------
 qxldod/QxlDod.h   |  48 +++++++++++++++--------
 2 files changed, 120 insertions(+), 42 deletions(-)

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 1b3bdf1..ec960e5 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -2294,7 +2294,7 @@ VOID BltBits (
 }
 #pragma code_seg(pop) // End Non-Paged Code
 
-VgaDevice::VgaDevice(_In_ QxlDod* pQxlDod)
+VgaDevice::VgaDevice(_In_ QxlDod* pQxlDod) : HwDeviceInterface(pQxlDod)
 {
     m_pQxlDod = pQxlDod;
     m_ModeInfo = NULL;
@@ -2302,6 +2302,7 @@ VgaDevice::VgaDevice(_In_ QxlDod* pQxlDod)
     m_ModeNumbers = NULL;
     m_CurrentMode = 0;
     m_Id = 0;
+    m_type = VGA_DEVICE;
 }
 
 VgaDevice::~VgaDevice(void)
@@ -2881,18 +2882,18 @@ VOID VgaDevice::BlackOutScreen(CURRENT_BDD_MODE* pCurrentBddMod)
 #pragma code_seg()
 // BEGIN: Non-Paged Code Segment
 
-BOOLEAN VgaDevice::InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber)
+BOOLEAN VgaDevice::HWInterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber)
 {
     UNREFERENCED_PARAMETER(pDxgkInterface);
     UNREFERENCED_PARAMETER(MessageNumber);
     return FALSE;
 }
 
-VOID VgaDevice::DpcRoutine(PVOID)
+VOID VgaDevice::HWDpcRoutine(PDXGKRNL_INTERFACE pDxgkInterface)
 {
 }
 
-VOID VgaDevice::ResetDevice(VOID)
+VOID VgaDevice::HWResetDevice(VOID)
 {
 }
 #pragma  code_seg(pop) //end non-paged code
@@ -2916,7 +2917,7 @@ NTSTATUS VgaDevice::Escape(_In_ CONST DXGKARG_ESCAPE* pEscap)
     return STATUS_NOT_IMPLEMENTED;
 }
 
-QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
+QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod) : HwDeviceInterface(pQxlDod)
 {
     m_pQxlDod = pQxlDod;
     m_ModeInfo = NULL;
@@ -2926,6 +2927,7 @@ QxlDevice::QxlDevice(_In_ QxlDod* pQxlDod)
     m_CustomMode = 0;
     m_FreeOutputs = 0;
     m_Pending = 0;
+    m_type = QXL_DEVICE;
 }
 
 QxlDevice::~QxlDevice(void)
@@ -3152,7 +3154,7 @@ NTSTATUS QxlDevice::SetPowerState(_In_ DEVICE_POWER_STATE DevicePowerState, DXGK
 NTSTATUS QxlDevice::HWInit(PCM_RESOURCE_LIST pResList, DXGK_DISPLAY_INFORMATION* pDispInfo)
 {
     DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
-    PDXGKRNL_INTERFACE pDxgkInterface = m_pQxlDod->GetDxgkInterrface();
+    PDXGKRNL_INTERFACE pDxgkInterface = m_pQxlDod->GetDxgkInterface();
     UINT pci_range = QXL_RAM_RANGE_INDEX;
     for (ULONG i = 0; i < pResList->Count; ++i)
     {
@@ -3329,7 +3331,7 @@ void QxlDevice::QxlClose()
 
 void QxlDevice::UnmapMemory(void)
 {
-    PDXGKRNL_INTERFACE pDxgkInterface = m_pQxlDod->GetDxgkInterrface();
+    PDXGKRNL_INTERFACE pDxgkInterface = m_pQxlDod->GetDxgkInterface();
     if (m_IoMapped && m_IoBase)
     {
         pDxgkInterface->DxgkCbUnmapMemory( pDxgkInterface->DeviceHandle, &m_IoBase);
@@ -4498,7 +4500,8 @@ VOID QxlDevice::PushCursor(VOID)
 #pragma code_seg(push)
 #pragma code_seg()
 // BEGIN: Non-Paged Code Segment
-BOOLEAN QxlDevice::InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber)
+
+BOOLEAN QxlDevice::HWInterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber)
 {
     UNREFERENCED_PARAMETER(MessageNumber);
     DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
@@ -4523,10 +4526,8 @@ BOOLEAN QxlDevice::InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_
     return TRUE;
 }
 
-VOID QxlDevice::DpcRoutine(PVOID ptr)
+VOID QxlDevice::HWDpcRoutine(PDXGKRNL_INTERFACE pDxgkInterface)
 {
-    PDXGKRNL_INTERFACE pDxgkInterface = (PDXGKRNL_INTERFACE)ptr;
-
     DbgPrint(TRACE_LEVEL_INFORMATION, ("---> %s\n", __FUNCTION__));
     DPC_CB_CONTEXT ctx;
     BOOLEAN dummy;
@@ -4557,13 +4558,89 @@ VOID QxlDevice::DpcRoutine(PVOID ptr)
     DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s\n", __FUNCTION__));
 }
 
-void QxlDevice::ResetDevice(void)
+void QxlDevice::HWResetDevice(void)
 {
     DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
     m_RamHdr->int_mask = ~0;
     WRITE_PORT_UCHAR(m_IoBase + QXL_IO_MEMSLOT_ADD, 0);
     DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
 }
+
+// Given bits per pixel, return the pixel format at the same bpp
+D3DDDIFORMAT PixelFormatFromBPP(UINT BPP)
+{
+    switch (BPP)
+    {
+        case  8: return D3DDDIFMT_P8;
+        case 16: return D3DDDIFMT_R5G6B5;
+        case 24: return D3DDDIFMT_R8G8B8;
+        case 32: return D3DDDIFMT_X8R8G8B8;
+        default: QXL_LOG_ASSERTION1("A bit per pixel of 0x%I64x is not supported.", BPP); return D3DDDIFMT_UNKNOWN;
+    }
+}
+
+
+BOOLEAN HwDeviceInterface::InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_ ULONG MessageNumber)
+{
+    QxlDevice * qxl_device;
+    VgaDevice * vga_device;
+
+    switch (m_type) {
+    case QXL_DEVICE:
+        qxl_device = (QxlDevice *)this;
+        return qxl_device->HWInterruptRoutine(pDxgkInterface, MessageNumber);
+    case VGA_DEVICE:
+        vga_device = (VgaDevice *)this;
+        return vga_device->HWInterruptRoutine(pDxgkInterface, MessageNumber);
+    default:
+        DbgPrint(TRACE_LEVEL_ERROR, ("%s called with invalid device type of %d\n",
+            __FUNCTION__, m_type));
+        return FALSE;
+    }
+}
+
+VOID HwDeviceInterface::DpcRoutine(PDXGKRNL_INTERFACE pDxgkInterface)
+{
+    QxlDevice * qxl_device;
+    VgaDevice * vga_device;
+
+    switch (m_type) {
+    case QXL_DEVICE:
+        qxl_device = (QxlDevice *)this;
+        qxl_device->HWDpcRoutine(pDxgkInterface);
+        return;
+    case VGA_DEVICE:
+        vga_device = (VgaDevice *)this;
+        vga_device->HWDpcRoutine(pDxgkInterface);
+        return;
+    default:
+        DbgPrint(TRACE_LEVEL_ERROR, ("%s called with invalid device type of %d\n",
+            __FUNCTION__, m_type));
+        return;
+    }
+}
+
+VOID HwDeviceInterface::ResetDevice(void)
+{
+    QxlDevice * qxl_device;
+    VgaDevice * vga_device;
+
+    switch (m_type) {
+    case QXL_DEVICE:
+        qxl_device = (QxlDevice *)this;
+        qxl_device->HWResetDevice();
+        return;
+    case VGA_DEVICE:
+        vga_device = (VgaDevice *)this;
+        vga_device->HWResetDevice();
+        return;
+    default:
+        DbgPrint(TRACE_LEVEL_ERROR, ("%s called with invalid device type of %d\n",
+            __FUNCTION__, m_type));
+        return;
+    }
+}
+
 #pragma code_seg(pop) //end non-paged code
 
 VOID QxlDevice::UpdateArea(CONST RECT* area, UINT32 surface_id)
@@ -4592,19 +4669,6 @@ VOID QxlDevice::DpcCallback(PDPC_CB_CONTEXT ctx)
 
 }
 
-// Given bits per pixel, return the pixel format at the same bpp
-D3DDDIFORMAT PixelFormatFromBPP(UINT BPP)
-{
-    switch (BPP)
-    {
-        case  8: return D3DDDIFMT_P8;
-        case 16: return D3DDDIFMT_R5G6B5;
-        case 24: return D3DDDIFMT_R8G8B8;
-        case 32: return D3DDDIFMT_X8R8G8B8;
-        default: QXL_LOG_ASSERTION1("A bit per pixel of 0x%I64x is not supported.", BPP); return D3DDDIFMT_UNKNOWN;
-    }
-}
-
 UINT SpiceFromPixelFormat(D3DDDIFORMAT Format)
 {
     switch (Format)
diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
index eb9bea7..db343e9 100755
--- a/qxldod/QxlDod.h
+++ b/qxldod/QxlDod.h
@@ -212,21 +212,34 @@ typedef struct _CURRENT_BDD_MODE
 
 class QxlDod;
 
-class HwDeviceIntrface {
+//
+// Can't use virtual for functions that  are non-pageable because there is no guarantee
+// that the vtable will not be paged out.  Will test for type in the parent call and
+// cast to call in the  correct device class.
+//
+
+typedef enum {
+    QXL_DEVICE,
+    VGA_DEVICE,
+    INVALID_DEVICE,
+}WIN_QXL_DEVICE_TYPE;
+
+class HwDeviceInterface {
 public:
-//    HwDeviceIntrface(_In_ QxlDod* pQxlDod) {;}
-    virtual ~HwDeviceIntrface() {;}
+    HwDeviceInterface(_In_ QxlDod* pQxlDod) {m_type = INVALID_DEVICE;}
+    virtual ~HwDeviceInterface() {;}
     virtual NTSTATUS QueryCurrentMode(PVIDEO_MODE RequestedMode) = 0;
     virtual NTSTATUS SetCurrentMode(ULONG Mode) = 0;
     virtual NTSTATUS GetCurrentMode(ULONG* Mode) = 0;
     virtual NTSTATUS SetPowerState(DEVICE_POWER_STATE DevicePowerState, DXGK_DISPLAY_INFORMATION* pDispInfo) = 0;
     virtual NTSTATUS HWInit(PCM_RESOURCE_LIST pResList, DXGK_DISPLAY_INFORMATION* pDispInfo) = 0;
     virtual NTSTATUS HWClose(void) = 0;
-    virtual BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber) = 0;
-    virtual VOID DpcRoutine(PVOID) = 0;
-    virtual VOID ResetDevice(void) = 0;
-
     virtual ULONG GetModeCount(void) = 0;
+
+    BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber);;
+    VOID DpcRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface);
+    VOID ResetDevice(void);;
+
     PVIDEO_MODE_INFORMATION GetModeInfo(UINT idx) {return &m_ModeInfo[idx];}
     USHORT GetModeNumber(USHORT idx) {return m_ModeNumbers[idx];}
     USHORT GetCurrentModeIndex(void) {return m_CurrentMode;}
@@ -259,10 +272,11 @@ protected:
     USHORT m_CurrentMode;
     USHORT m_CustomMode;
     ULONG  m_Id;
+    WIN_QXL_DEVICE_TYPE m_type;
 };
 
 class VgaDevice  :
-    public HwDeviceIntrface
+    public HwDeviceInterface
 {
 public:
     VgaDevice(_In_ QxlDod* pQxlDod);
@@ -287,9 +301,9 @@ public:
                                  _In_ D3DKMDT_VIDPN_PRESENT_PATH_ROTATION Rotation,
                                  _In_ const CURRENT_BDD_MODE* pModeCur);
     VOID BlackOutScreen(CURRENT_BDD_MODE* pCurrentBddMod);
-    BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber);
-    VOID DpcRoutine(PVOID);
-    VOID ResetDevice(VOID);
+    BOOLEAN HWInterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber);
+    VOID HWDpcRoutine(PDXGKRNL_INTERFACE pDxgkInterface);
+    VOID HWResetDevice(VOID);
     NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* pSetPointerShape);
     NTSTATUS SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION* pSetPointerPosition);
     NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap);
@@ -435,7 +449,7 @@ typedef struct DpcCbContext {
 #define ALIGN(a, b) (((a) + ((b) - 1)) & ~((b) - 1))
 
 class QxlDevice  :
-    public HwDeviceIntrface
+    public HwDeviceInterface
 {
 public:
     QxlDevice(_In_ QxlDod* pQxlDod);
@@ -460,9 +474,9 @@ public:
                     _In_ D3DKMDT_VIDPN_PRESENT_PATH_ROTATION Rotation,
                     _In_ const CURRENT_BDD_MODE* pModeCur);
     VOID BlackOutScreen(CURRENT_BDD_MODE* pCurrentBddMod);
-    BOOLEAN InterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber);
-    VOID DpcRoutine(PVOID);
-    VOID ResetDevice(VOID);
+    BOOLEAN HWInterruptRoutine(_In_ PDXGKRNL_INTERFACE pDxgkInterface, _In_  ULONG MessageNumber);
+    VOID HWDpcRoutine(PDXGKRNL_INTERFACE pDxgkInterface);
+    VOID HWResetDevice(VOID);
     NTSTATUS SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* pSetPointerShape);
     NTSTATUS SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION* pSetPointerPosition);
     NTSTATUS Escape(_In_ CONST DXGKARG_ESCAPE* pEscap);
@@ -592,7 +606,7 @@ private:
 
     D3DDDI_VIDEO_PRESENT_SOURCE_ID m_SystemDisplaySourceId;
     DXGKARG_SETPOINTERSHAPE m_PointerShape;
-    HwDeviceIntrface* m_pHWDevice;
+    HwDeviceInterface* m_pHWDevice;
 public:
     QxlDod(_In_ DEVICE_OBJECT* pPhysicalDeviceObject);
     ~QxlDod(void);
@@ -685,7 +699,7 @@ public:
                                  _In_                                     UINT  SourceStride,
                                  _In_                                     INT   PositionX,
                                  _In_                                     INT   PositionY);
-    PDXGKRNL_INTERFACE GetDxgkInterrface(void) { return &m_DxgkInterface;}
+    PDXGKRNL_INTERFACE GetDxgkInterface(void) { return &m_DxgkInterface;}
 private:
     VOID CleanUp(VOID);
     NTSTATUS CheckHardware();
-- 
2.7.0.windows.1



More information about the Spice-devel mailing list