<html><body><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Aug 18, 2016 at 6:48 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">> > 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 <a href="http://redhat.com" rel="noreferrer" target="_blank">redhat.com</a>> > > Signed-off-by: Dmitry Fleytman <dfleytma at <a href="http://redhat.com" rel="noreferrer" target="_blank">redhat.com</a>> > Signed-off-by: Sameeh Jubran <<a href="mailto:sameeh@daynix.com" target="_blank">sameeh@daynix.com</a>> > --- > 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. </blockquote><div> </div><div>Can you please provide me the patch number that you preferred?</div><div>Beside the malloc/free, what's wrong with this version?</div></div></div></div></blockquote><div> </div><div><div>I personally like v7 version, I would just add this patch</div><div> </div><div> </div><div>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; </div><div>which fixes a potential buffer overflow (wcsncpy require number of characters, not bytes)</div><div> </div><div> </div><div>Other versions rely on QXL_ESCAPE which was removed or v10 which is the latest. </div><div>I don't like malloc/free as they does not fit with the current style, are not exception safe and</div><div>are heavy for small static structure allocations. </div><div> </div>Frediano</div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Frediano <div class="HOEnZb"><div class="h5"> > 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 > <a href="https://github.com/notr1ch/DWMCapture/blob/master/DWMCaptureSource.cpp" rel="noreferrer" target="_blank">https://github.com/notr1ch/DWMCapture/blob/master/DWMCaptureSource.cpp</a> */ > + > +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 </div></div></blockquote></div> <div> </div>-- <div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><span style="color: #0b5394; font-family: times new roman,serif; font-size: large;" data-mce-style="color: #0b5394; font-family: times new roman,serif; font-size: large;" color="#0b5394" face="times new roman, serif" size="4">Respectfully, <div style="font-size:12.8px;color:rgb(136,136,136)"><span style="color: #0b5394; font-family: times new roman,serif; font-size: large;" data-mce-style="color: #0b5394; font-family: times new roman,serif; font-size: large;" color="#0b5394" face="times new roman, serif" size="4">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)"><span style="color: #073763; font-family: times new roman,serif; font-size: large;" data-mce-style="color: #073763; font-family: times new roman,serif; font-size: large;" color="#073763" face="times new roman, serif" size="4">Junior Software Engineer @ <a href="http://www.daynix.com" target="_blank">Daynix</a>.</div></div></div></div></div></div></div></div></div></blockquote><div> </div></div></body></html>