[Spice-devel] [PATCH qxl-wddm-dod 07/26] Do not use virtual functions for code that must not be paged
Frediano Ziglio
fziglio at redhat.com
Wed Aug 17 10:01:14 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
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).
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();
More information about the Spice-devel
mailing list