> On Thu, Aug 18, 2016 at 6:48 PM, Frediano Ziglio < [email protected] > > wrote:
> > > > > > > The Direct3D 9 API operates on either the Windows XP display driver > > > > model (XPDM) or the Windows Vista display driver model (WDDM), depending > > > > on the operating system installed. > > > > > > > > This patch implements the WDDM interface while using the CCD API to do > > > > so. Moreover it introduces multiple monitors support and arbitrary > > > > resolution for Windows 10 while preserving backward compatiblity with > > > > previous versions of Windows. > > > > > > > > Based on a patch by Sandy Stutsman <sstutsma at redhat.com > > > > > > > > > Signed-off-by: Dmitry Fleytman <dfleytma at redhat.com > > > > > Signed-off-by: Sameeh Jubran < [email protected] > > > > > --- > > > > vdagent/display_configuration.cpp | 346 > > > > +++++++++++++++++++++++++++++++++++++- > > > > vdagent/display_configuration.h | 47 ++++++ > > > > 2 files changed, 392 insertions(+), 1 deletion(-) > > > > > > > Missing changes however looking at the code I understood that > > > now the ABI is as was before and you are using some malloc/free. > > > So the protocol change is not needed. > > > I prefer the version before you decided to introduce the protocol > > > change. > > Can you please provide me the patch number that you preferred? > Beside the malloc/free, what's wrong with this version? I personally like v7 version, I would just add this patch diff --git a/vdagent/display_configuration.cpp b/vdagent/display_configuration.cpp index 1504a60..9cc39ae 100644 --- a/vdagent/display_configuration.cpp +++ b/vdagent/display_configuration.cpp @@ -17,6 +17,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. #include "display_configuration.h" #include <winternl.h> +#include <spice/macros.h> /* The following definitions and structures are taken from the wine project repository and can be found @@ -631,7 +632,7 @@ D3D_HANDLE WDDMInterface::handle_from_GDI_name(LPCTSTR adapter_name) NTSTATUS status; ZeroMemory(&gdi_display_name, sizeof(gdi_display_name)); - wcsncpy(gdi_display_name.DeviceName, adapter_name, sizeof(TCHAR)* CCHDEVICENAME); + wcsncpy(gdi_display_name.DeviceName, adapter_name, SPICE_N_ELEMENTS(gdi_display_name.DeviceName)); if (NT_SUCCESS(status = _pfnOpen_adapter_gdi_name(&gdi_display_name))) { return gdi_display_name.hAdapter; which fixes a potential buffer overflow (wcsncpy require number of characters, not bytes) Other versions rely on QXL_ESCAPE which was removed or v10 which is the latest. I don't like malloc/free as they does not fit with the current style, are not exception safe and are heavy for small static structure allocations. Frediano > > Frediano > > > > diff --git a/vdagent/display_configuration.cpp > > > > b/vdagent/display_configuration.cpp > > > > index 01fdbb0..bab4c95 100644 > > > > --- a/vdagent/display_configuration.cpp > > > > +++ b/vdagent/display_configuration.cpp > > > > @@ -153,6 +153,56 @@ struct DISPLAYCONFIG_PATH_INFO { > > > > UINT32 flags; > > > > }; > > > > > > > > +/* The following definitions and structures are taken > > > > + * from here > > > > https://github.com/notr1ch/DWMCapture/blob/master/DWMCaptureSource.cpp */ > > > > + > > > > +enum D3DKMT_ESCAPETYPE { > > > > + D3DKMT_ESCAPE_DRIVERPRIVATE = 0 > > > > +}; > > > > + > > > > +struct D3DDDI_ESCAPEFLAGS { > > > > + union { > > > > + struct { > > > > + UINT Reserved : 31; > > > > + }; > > > > + UINT Value; > > > > + }; > > > > +}; > > > > + > > > > +struct D3DKMT_ESCAPE { > > > > + D3D_HANDLE hAdapter; > > > > + D3D_HANDLE hDevice; > > > > + D3DKMT_ESCAPETYPE Type; > > > > + D3DDDI_ESCAPEFLAGS Flags; > > > > + VOID* pPrivateDriverData; > > > > + UINT PrivateDriverDataSize; > > > > + D3D_HANDLE hContext; > > > > +}; > > > > + > > > > +struct D3DKMT_OPENADAPTERFROMHDC { > > > > + HDC hDc; > > > > + D3D_HANDLE hAdapter; > > > > + LUID AdapterLuid; > > > > + UINT VidPnSourceId; > > > > +}; > > > > + > > > > +struct D3DKMT_CLOSEADAPTER { > > > > + D3D_HANDLE hAdapter; > > > > +}; > > > > + > > > > +struct D3DKMT_OPENADAPTERFROMDEVICENAME { > > > > + const WCHAR *pDeviceName; > > > > + D3D_HANDLE hAdapter; > > > > + LUID AdapterLuid; > > > > +}; > > > > + > > > > +struct D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME { > > > > + WCHAR DeviceName[32]; > > > > + D3D_HANDLE hAdapter; > > > > + LUID AdapterLuid; > > > > + UINT VidPnSourceId; > > > > +}; > > > > + > > > > struct QXLMonitorEscape { > > > > QXLMonitorEscape(DEVMODE* dev_mode) > > > > { > > > > @@ -178,7 +228,14 @@ struct QxlCustomEscapeObj : public > > > > QXLEscapeSetCustomDisplay { > > > > DisplayConfig* DisplayConfig::create_config() > > > > { > > > > DisplayConfig* new_interface; > > > > - new_interface = new XPDMInterface(); > > > > + /* Try to open a WDDM adapter. > > > > + If that failed, assume we have an XPDM driver */ > > > > + try { > > > > + new_interface = new WDDMInterface(); > > > > + } > > > > + catch (std::exception& e) { > > > > + new_interface = new XPDMInterface(); > > > > + } > > > > return new_interface; > > > > } > > > > > > > > @@ -328,6 +385,293 @@ bool XPDMInterface::find_best_mode(LPCTSTR Device, > > > > DEVMODE* dev_mode) > > > > return NT_SUCCESS(status); > > > > } > > > > > > > > +WDDMInterface::WDDMInterface() > > > > + : _pfnOpen_adapter_hdc(NULL) > > > > + , _pfnClose_adapter(NULL) > > > > + , _pfnEscape(NULL) > > > > + , _pfnOpen_adapter_device_name(NULL) > > > > + , _pfnOpen_adapter_gdi_name(NULL) > > > > +{ > > > > + LONG error(0); > > > > + //Can we find the D3D calls we need? > > > > + if (!init_d3d_api()) { > > > > + throw std::exception(); > > > > + } > > > > + > > > > + //Initialize CCD path stuff > > > > + if (!_ccd.query_display_config()) { > > > > + throw std::exception(); > > > > + } > > > > + > > > > + if (!_ccd.set_display_config(error)) { > > > > + throw std::exception(); > > > > + } > > > > +} > > > > + > > > > +bool WDDMInterface::is_attached(DISPLAY_DEVICE* dev_info) > > > > +{ > > > > + return _ccd.is_attached(dev_info->DeviceName); > > > > +} > > > > + > > > > +bool WDDMInterface::set_monitor_state(LPCTSTR device_name, DEVMODE* > > > > dev_mode, MONITOR_STATE state) > > > > +{ > > > > + return _ccd.set_path_state(device_name, state); > > > > +} > > > > + > > > > +bool WDDMInterface::custom_display_escape(LPCTSTR device_name, DEVMODE* > > > > dev_mode) > > > > +{ > > > > + DISPLAYCONFIG_MODE_INFO* mode = _ccd.get_active_mode(device_name, > > > > false); > > > > + if (!mode) { > > > > + return false; > > > > + } > > > > + > > > > + //Don't bother if we are already set to the new resolution > > > > + if (mode->sourceMode.width == dev_mode->dmPelsWidth && > > > > + mode->sourceMode.height == dev_mode->dmPelsHeight) { > > > > + return true; > > > > + } > > > > + > > > > + vd_printf("%s: updating %S resolution\n", __FUNCTION__, device_name); > > > > + > > > > + size_t data_size = sizeof(QXLEscape) + > > > > sizeof(QXLEscapeSetCustomDisplay); > > > > + QXLEscape * qxl_escape = (QXLEscape *) malloc(data_size); > > > > + qxl_escape->ioctl = QXL_ESCAPE_SET_CUSTOM_DISPLAY; > > > > + QXLEscapeSetCustomDisplay * wddm_escape = (QXLEscapeSetCustomDisplay > > > > *)qxl_escape->data; > > > > + wddm_escape->bpp = dev_mode->dmBitsPerPel; > > > > + wddm_escape->xres = dev_mode->dmPelsWidth; > > > > + wddm_escape->yres = dev_mode->dmPelsHeight; > > > > + bool escape_result = escape(device_name, qxl_escape, data_size); > > > > + free(qxl_escape); > > > > + if (escape_result) { > > > > + return _ccd.update_mode_size(device_name, dev_mode); > > > > + } > > > > + > > > > + vd_printf("%s: (%dx%d)", __FUNCTION__, mode->sourceMode.width, > > > > mode->sourceMode.height); > > > > + return false; > > > > +} > > > > + > > > > +bool WDDMInterface::update_monitor_config(LPCTSTR device_name, > > > DisplayMode* > > > > display_mode, > > > > + DEVMODE* dev_mode) > > > > +{ > > > > + if (!display_mode || !display_mode->get_attached()) { > > > > + return false; > > > > + } > > > > + DISPLAYCONFIG_MODE_INFO* mode = _ccd.get_active_mode(device_name, > > > > false); > > > > + if (!mode || !_send_monitors_config) > > > > + return false; > > > > + > > > > + size_t data_size = sizeof(QXLEscape) + sizeof(QXLHead); > > > > + QXLEscape * qxl_escape = (QXLEscape *) malloc(data_size); > > > > + qxl_escape->ioctl = QXL_ESCAPE_MONITOR_CONFIG; > > > > + QXLHead * wddm_escape = (QXLHead *) qxl_escape->data; > > > > + wddm_escape->id = wddm_escape->surface_id = 0; > > > > + wddm_escape->x = display_mode->get_pos_x(); > > > > + wddm_escape->y = display_mode->get_pos_y(); > > > > + wddm_escape->width = display_mode->get_width(); > > > > + wddm_escape->height = display_mode->get_height(); > > > > + bool escape_result = escape(device_name, qxl_escape, data_size); > > > > + free(qxl_escape); > > > > + if (escape_result) { > > > > + //Update the path position > > > > + return _ccd.update_mode_position(device_name, dev_mode); > > > > + } > > > > + > > > > + vd_printf("%s: %S failed", __FUNCTION__, device_name); > > > > + return false; > > > > + > > > > +} > > > > + > > > > +LONG WDDMInterface::update_display_settings() > > > > +{ > > > > + LONG error(0); > > > > + //If we removed the primary monitor since the last call, we need to > > > > + //reorder the other monitors, making the leftmost one the primary > > > > + _ccd.verify_primary_position(); > > > > + _ccd.set_display_config(error); > > > > + return error; > > > > +} > > > > + > > > > +void WDDMInterface::update_config_path() > > > > +{ > > > > + _ccd.query_display_config(); > > > > +} > > > > + > > > > +bool WDDMInterface::update_dev_mode_position(LPCTSTR device_name, > > > DEVMODE* > > > > dev_mode, > > > > + LONG x, LONG y) > > > > +{ > > > > + dev_mode->dmPosition.x = x; > > > > + dev_mode->dmPosition.y = y; > > > > + return _ccd.update_mode_position(device_name, dev_mode); > > > > +} > > > > + > > > > +bool WDDMInterface::init_d3d_api() > > > > +{ > > > > + HMODULE hModule = GetModuleHandle(L"gdi32.dll"); > > > > + > > > > + //Look for the gdi32 functions we need to perform driver escapes > > > > + if (!hModule) { > > > > + vd_printf("%s something wildly wrong as we can't open gdi32.dll", > > > > __FUNCTION__); > > > > + return false; > > > > + } > > > > + > > > > + do { > > > > + _pfnClose_adapter = (PFND3DKMT_CLOSEADAPTER) > > > > + GetProcAddress(hModule, "D3DKMTCloseAdapter"); > > > > + if (!_pfnClose_adapter) { > > > > + break; > > > > + } > > > > + > > > > + _pfnEscape = (PFND3DKMT_ESCAPE) GetProcAddress(hModule, > > > > "D3DKMTEscape"); > > > > + if (!_pfnEscape) { > > > > + break; > > > > + } > > > > + > > > > + _pfnOpen_adapter_hdc = (PFND3DKMT_OPENADAPTERFROMHDC) > > > > + GetProcAddress(hModule, "D3DKMTOpenAdapterFromHdc"); > > > > + if (!_pfnOpen_adapter_hdc) { > > > > + break; > > > > + } > > > > + > > > > + _pfnOpen_adapter_device_name = (PFND3DKMT_OPENADAPTERFROMDEVICENAME) > > > > + GetProcAddress(hModule, "D3DKMTOpenAdapterFromDeviceName"); > > > > + if (!_pfnOpen_adapter_device_name) { > > > > + break; > > > > + } > > > > + > > > > + _pfnOpen_adapter_gdi_name = > > > > (PFND3DKMT_OPENADAPTERFROMGDIDISPLAYNAME) > > > > + GetProcAddress(hModule, "D3DKMTOpenAdapterFromGdiDisplayName"); > > > > + if (!_pfnOpen_adapter_gdi_name) { > > > > + break; > > > > + } > > > > + > > > > + } > > > > + while(0); > > > > + > > > > + //Did we get them ? > > > > + if (!_pfnClose_adapter || !_pfnOpen_adapter_hdc || !_pfnEscape) { > > > > + return false; > > > > + } > > > > + return true; > > > > +} > > > > + > > > > +D3D_HANDLE WDDMInterface::adapter_handle(LPCTSTR device_name) > > > > +{ > > > > + D3D_HANDLE hAdapter(0); > > > > + > > > > + //For some reason, this call will occasionally fail. > > > > + if ((hAdapter = handle_from_DC(device_name))) { > > > > + return hAdapter; > > > > + } > > > > + //So try other available methods. > > > > + if (_pfnOpen_adapter_device_name && (hAdapter = > > > > handle_from_device_name(device_name))) { > > > > + return hAdapter; > > > > + } > > > > + //One last chance to open this guy > > > > + if (_pfnOpen_adapter_gdi_name) { > > > > + hAdapter = handle_from_GDI_name(device_name); > > > > + } > > > > + > > > > + if (!hAdapter) { > > > > + vd_printf("%s: failed to open adapter %S", __FUNCTION__, > > > > device_name); > > > > + } > > > > + > > > > + return hAdapter; > > > > +} > > > > + > > > > +D3D_HANDLE WDDMInterface::handle_from_DC(LPCTSTR adapter_name) > > > > +{ > > > > + NTSTATUS status; > > > > + D3DKMT_OPENADAPTERFROMHDC open_data; > > > > + HDC hDc(CreateDC(adapter_name, NULL, NULL, NULL)); > > > > + > > > > + if (!hDc) { > > > > + vd_printf("%s: %S CreateDC failed with %lu", __FUNCTION__, > > > > adapter_name, GetLastError()); > > > > + return 0; > > > > + } > > > > + > > > > + ZeroMemory(&open_data, sizeof(D3DKMT_OPENADAPTERFROMHDC)); > > > > + open_data.hDc = hDc; > > > > + > > > > + if (!NT_SUCCESS(status = _pfnOpen_adapter_hdc(&open_data))) { > > > > + vd_printf("%s: %S open adapter from hdc failed with %lu", > > > > __FUNCTION__, adapter_name, > > > > + status); > > > > + open_data.hAdapter = 0; > > > > + } > > > > + > > > > + DeleteDC(hDc); > > > > + return open_data.hAdapter; > > > > +} > > > > + > > > > +D3D_HANDLE WDDMInterface::handle_from_device_name(LPCTSTR adapter_name) > > > > +{ > > > > + D3DKMT_OPENADAPTERFROMDEVICENAME display_name_data; > > > > + NTSTATUS status; > > > > + > > > > + ZeroMemory(&display_name_data, sizeof(display_name_data)); > > > > + display_name_data.pDeviceName = adapter_name; > > > > + > > > > + if (NT_SUCCESS(status = > > > > _pfnOpen_adapter_device_name(&display_name_data))) { > > > > + return display_name_data.hAdapter; > > > > + } > > > > + > > > > + vd_printf("%s %S failed with 0x%lx", __FUNCTION__, adapter_name, > > > > status); > > > > + return 0; > > > > +} > > > > + > > > > +D3D_HANDLE WDDMInterface::handle_from_GDI_name(LPCTSTR adapter_name) > > > > +{ > > > > + D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME gdi_display_name; > > > > + NTSTATUS status; > > > > + > > > > + ZeroMemory(&gdi_display_name, sizeof(gdi_display_name)); > > > > + wcsncpy(gdi_display_name.DeviceName, adapter_name, sizeof(TCHAR)* > > > > CCHDEVICENAME); > > > > + > > > > + if (NT_SUCCESS(status = _pfnOpen_adapter_gdi_name(&gdi_display_name))) > > > { > > > > + return gdi_display_name.hAdapter; > > > > + } > > > > + > > > > + vd_printf("%s: %S aurrrgghh nothing works..error is 0x%lx", > > > > __FUNCTION__, adapter_name, > > > > + status); > > > > + return 0; > > > > +} > > > > + > > > > +void WDDMInterface::close_adapter(D3D_HANDLE handle) > > > > +{ > > > > + D3DKMT_CLOSEADAPTER closeData; > > > > + if (handle) { > > > > + closeData.hAdapter = handle; > > > > + _pfnClose_adapter(&closeData); > > > > + } > > > > +} > > > > + > > > > +bool WDDMInterface::escape(LPCTSTR device_name, void* data, UINT > > > size_data) > > > > +{ > > > > + D3DKMT_ESCAPE escapeData; > > > > + NTSTATUS status; > > > > + D3D_HANDLE hAdapter(0); > > > > + > > > > + if (!(hAdapter = adapter_handle(device_name))) > > > > + return false; > > > > + > > > > + escapeData.hAdapter = hAdapter; > > > > + escapeData.hDevice = 0; > > > > + escapeData.hContext = 0; > > > > + escapeData.Type = D3DKMT_ESCAPE_DRIVERPRIVATE; > > > > + escapeData.Flags.Value = 0; > > > > + escapeData.pPrivateDriverData = data; > > > > + escapeData.PrivateDriverDataSize = size_data; > > > > + > > > > + status = _pfnEscape(&escapeData); > > > > + > > > > + if (!NT_SUCCESS(status)) { > > > > + vd_printf("%s: this should never happen. Status is 0x%lx", > > > > __FUNCTION__, status); > > > > + } > > > > + > > > > + //Close the handle to this device > > > > + close_adapter(hAdapter); > > > > + return NT_SUCCESS(status); > > > > +} > > > > + > > > > CCD::CCD() > > > > :_numPathElements(0) > > > > ,_numModeElements(0) > > > > diff --git a/vdagent/display_configuration.h > > > > b/vdagent/display_configuration.h > > > > index 388ac9d..7abf526 100644 > > > > --- a/vdagent/display_configuration.h > > > > +++ b/vdagent/display_configuration.h > > > > @@ -125,4 +125,51 @@ private: > > > > bool find_best_mode(LPCTSTR Device, DEVMODE* dev_mode); > > > > }; > > > > > > > > +//DisplayConfig implementation for guest with WDDM graphics drivers > > > > +typedef UINT D3D_HANDLE; > > > > + > > > > +struct D3DKMT_ESCAPE; > > > > +struct D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME; > > > > +struct D3DKMT_OPENADAPTERFROMDEVICENAME; > > > > +struct D3DKMT_CLOSEADAPTER; > > > > +struct D3DKMT_OPENADAPTERFROMHDC; > > > > + > > > > +typedef NTSTATUS(APIENTRY* PFND3DKMT_ESCAPE)(CONST D3DKMT_ESCAPE*); > > > > +typedef NTSTATUS(APIENTRY* > > > > PFND3DKMT_OPENADAPTERFROMGDIDISPLAYNAME)(D3DKMT_OPENADAPTERFROMGDIDISPLAYNAME*); > > > > +typedef NTSTATUS(APIENTRY* > > > > PFND3DKMT_OPENADAPTERFROMDEVICENAME)(D3DKMT_OPENADAPTERFROMDEVICENAME*); > > > > +typedef NTSTATUS(APIENTRY* > > > PFND3DKMT_CLOSEADAPTER)(D3DKMT_CLOSEADAPTER*); > > > > +typedef NTSTATUS(APIENTRY* > > > > PFND3DKMT_OPENADAPTERFROMHDC)(D3DKMT_OPENADAPTERFROMHDC*); > > > > + > > > > +class WDDMInterface : public DisplayConfig { > > > > +public: > > > > + WDDMInterface(); > > > > + bool is_attached(DISPLAY_DEVICE* dev_info); > > > > + bool set_monitor_state(LPCTSTR device_name, DEVMODE* dev_mode, > > > > MONITOR_STATE state); > > > > + LONG update_display_settings(); > > > > + bool custom_display_escape(LPCTSTR device_name, DEVMODE* dev_mode); > > > > + bool update_monitor_config(LPCTSTR device_name, DisplayMode* mode, > > > > DEVMODE* dev_mode); > > > > + bool update_dev_mode_position(LPCTSTR device_name, DEVMODE * dev_mode, > > > > LONG x, LONG y); > > > > + void update_config_path(); > > > > + > > > > +private: > > > > + bool init_d3d_api(); > > > > + D3D_HANDLE adapter_handle(LPCTSTR device_name); > > > > + D3D_HANDLE handle_from_DC(LPCTSTR adapter_name); > > > > + D3D_HANDLE handle_from_device_name(LPCTSTR adapter_name); > > > > + D3D_HANDLE handle_from_GDI_name(LPCTSTR adapter_name); > > > > + > > > > + void close_adapter(D3D_HANDLE handle); > > > > + bool escape(LPCTSTR device_name, void* data, UINT sizeData); > > > > + > > > > + //GDI Function pointers > > > > + PFND3DKMT_OPENADAPTERFROMHDC _pfnOpen_adapter_hdc; > > > > + PFND3DKMT_CLOSEADAPTER _pfnClose_adapter; > > > > + PFND3DKMT_ESCAPE _pfnEscape; > > > > + PFND3DKMT_OPENADAPTERFROMDEVICENAME _pfnOpen_adapter_device_name; > > > > + PFND3DKMT_OPENADAPTERFROMGDIDISPLAYNAME _pfnOpen_adapter_gdi_name; > > > > + > > > > + //object handles the CCD API > > > > + CCD _ccd; > > > > +}; > > > > + > > > > #endif > > > > \ No newline at end of file > > -- > Respectfully, > Sameeh Jubran > Linkedin > Junior Software Engineer @ Daynix .
_______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
