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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> From: Sandy Stutsman <sstutsma@xxxxxxxxxx>
> 
> 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();
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]