On Thu, Aug 18, 2016 at 6:48 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> 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 <sameeh@xxxxxxxxxx>
> ---
> 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;
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
> 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--
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel