<div dir="ltr">It seems that after all this patch is unnecessary at all. By default all of the driver code is nonpaged unless stated otherwise using pragma directives.</div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Aug 18, 2016 at 8:25 PM, Frediano Ziglio <<a href="mailto:fziglio@redhat.com" target="_blank">fziglio@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > > > > From: Sandy Stutsman <<a href="mailto:sstutsma@redhat.com">sstutsma@redhat.com</a>> > > > > 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 > > Ok for the spelling. > This patch is moving some code around instead of marking function > to specific sections with #pragma code_seg. > vtable are generated in the constant section (by default .rodata) if this > is pageable I would rather define a section (#pragma section) and generate > all constants in this new section (#pragma const_seg). > Had a look at some driver. Looks like all main sections (.rdata, .data, etc) are not pageable. A better check where the vtable are allocated should be done but this looks like a false positive. Where can I get the last repository or the binary (not sure the binary is however enough, perhaps object files would be better) ? <div><div class="h5"> Frediano > > > --- > > 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(); </div></div>> _______________________________________________ > Spice-devel mailing list > <a href="mailto:Spice-devel@lists.freedesktop.org">Spice-devel@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/spice-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/spice-devel</a> > </blockquote></div> <div> </div>-- <div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr">Respectfully, <div style="font-size:12.8px;color:rgb(136,136,136)">Sameeh Jubran</div><div style="font-size:12.8px;color:rgb(136,136,136)"><a href="https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a" title="View public profile" name="UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_UNIQUE_ID_SafeHtmlFilter_14e2c1de96f8c195_UNIQUE_ID_SafeHtmlFilter_SafeHtmlFilter_SafeHtmlFilter_webProfileURL" style="color:rgb(17,85,204);margin:0px;padding:0px;border-width:0px;outline:none;vertical-align:baseline;text-decoration:none" target="_blank">Linkedin</a> </div><div style="font-size:12.8px;color:rgb(136,136,136)">Junior Software Engineer @ <a href="http://www.daynix.com" target="_blank">Daynix</a>.</div></div></div></div></div></div></div> </div>