kvm-guest-drivers-windows
kvm-guest-drivers-windows copied to clipboard
viogpudo: fix HWCursor display issues
This addresses two issues with HWCursor support:
- Sometimes stale cursor data will be presented to the host due to a race condition.
- Cursor hiding by guest was not implemented properly which sometimes results in an overlapping cursor when hovering over a text field.
After fixing these issues, it should be good to enable HWCursor by default.
Please use a full real name in sign off. Thanks.
@YanVugenfirer Done
@vrozenfe Can you please take a look?
@osy Thanks a lot for your PR. It looks fine overall. Just jive me some time to run some stress tests on it. I personalty have a different set of fixes for this problems issue, you can see below (just ignore all memory size allocation bits, they are related to the different issue). But if your fix works fine over VNC and under stress, I'm completerly fine with your patch.
diff --git a/viogpu/common/viogpu_queue.cpp b/viogpu/common/viogpu_queue.cpp
index cfe1c30d..cabce99e 100755
--- a/viogpu/common/viogpu_queue.cpp
+++ b/viogpu/common/viogpu_queue.cpp
@@ -843,19 +843,19 @@ BOOLEAN VioGpuMemSegment::Init(_In_ UINT size, _In_opt_ PPHYSICAL_ADDRESS pPAddr
ASSERT(size);
PVOID buf = NULL;
- UINT pages = BYTES_TO_PAGES(size);
- UINT sglsize = sizeof(SCATTER_GATHER_LIST) + (sizeof(SCATTER_GATHER_ELEMENT) * pages);
- size = pages * PAGE_SIZE;
if ((pPAddr == NULL) ||
pPAddr->QuadPart == 0LL) {
- m_pVAddr = new (NonPagedPoolNx) BYTE[size];
+ do {
+ m_pVAddr = new (NonPagedPoolNx) BYTE[size];
+ if (!m_pVAddr)
+ {
+ DbgPrint(TRACE_LEVEL_FATAL, ("%s insufficient resources to allocate %x bytes\n", __FUNCTION__, size));
+ VioGpuDbgBreak();
+ size /= 2;
+ }
+ } while (!m_pVAddr);
- if (!m_pVAddr)
- {
- DbgPrint(TRACE_LEVEL_FATAL, ("%s insufficient resources to allocate %x bytes\n", __FUNCTION__, size));
- return FALSE;
- }
RtlZeroMemory(m_pVAddr, size);
m_bSystemMemory = TRUE;
}
@@ -868,6 +868,10 @@ BOOLEAN VioGpuMemSegment::Init(_In_ UINT size, _In_opt_ PPHYSICAL_ADDRESS pPAddr
m_bMapped = TRUE;
}
+ UINT pages = BYTES_TO_PAGES(size);
+ UINT sglsize = sizeof(SCATTER_GATHER_LIST) + (sizeof(SCATTER_GATHER_ELEMENT) * pages);
+ size = pages * PAGE_SIZE;
+
m_pMdl = IoAllocateMdl(m_pVAddr, size, FALSE, FALSE, NULL);
if (!m_pMdl)
{
diff --git a/viogpu/viogpu.sln b/viogpu/viogpu.sln
index b4381fe3..68705d53 100755
--- a/viogpu/viogpu.sln
+++ b/viogpu/viogpu.sln
@@ -1,7 +1,7 @@
Microsoft Visual Studio Solution File, Format Version 12.00
-# Visual Studio 15
-VisualStudioVersion = 15.0.28307.168
+# Visual Studio Version 16
+VisualStudioVersion = 16.0.33927.289
MinimumVisualStudioVersion = 10.0.40219.1
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "viogpudo", "viogpudo\viogpudo.vcxproj", "{BDE07E9E-2041-4853-8B74-7CF591C255E7}"
ProjectSection(ProjectDependencies) = postProject
diff --git a/viogpu/viogpudo/trace.h b/viogpu/viogpudo/trace.h
index 76a306a2..943a10bb 100755
--- a/viogpu/viogpudo/trace.h
+++ b/viogpu/viogpudo/trace.h
@@ -1,6 +1,6 @@
#pragma once
-//#define DBG 1
+#define DBG 1
#ifndef TRACE_LEVEL_INFORMATION
#define TRACE_LEVEL_NONE 0 // Tracing is not on
diff --git a/viogpu/viogpudo/viogpudo.cpp b/viogpu/viogpudo/viogpudo.cpp
index a0a5cdc8..cd731340 100755
--- a/viogpu/viogpudo/viogpudo.cpp
+++ b/viogpu/viogpudo/viogpudo.cpp
@@ -49,6 +49,11 @@ m_pHWDevice(NULL)
DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
*((UINT*)&m_Flags) = 0;
+#if NTDDI_VERSION > NTDDI_WINBLUE
+ m_dwMemorySize = 0x1000000;
+#else
+ m_dwMemorySize = 0x800000;
+#endif
RtlZeroMemory(&m_DxgkInterface, sizeof(m_DxgkInterface));
RtlZeroMemory(&m_DeviceInfo, sizeof(m_DeviceInfo));
RtlZeroMemory(&m_CurrentMode, sizeof(m_CurrentMode));
@@ -1909,6 +1914,14 @@ NTSTATUS VioGpuDod::GetRegisterInfo(void)
SetUsePresentProgress(!!value);
}
+ value = 0;
+ VioGpuDbgBreak();
+ Status = ReadRegistryDWORD(DevInstRegKeyHandle, L"MemorySize", &value);
+ if (NT_SUCCESS(Status))
+ {
+ SetMemorySize(value);
+ }
+
ZwClose(DevInstRegKeyHandle);
DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s\n", __FUNCTION__));
return Status;
@@ -1935,6 +1948,12 @@ VioGpuAdapter::VioGpuAdapter(_In_ VioGpuDod* pVioGpuDod) // : IVioGpuAdapter(pVi
KeInitializeEvent(&m_ConfigUpdateEvent,
SynchronizationEvent,
FALSE);
+ KeInitializeEvent(&m_CursorUpdateEvent,
+ SynchronizationEvent,
+ FALSE);
+ InitializeListHead(&m_CursorList);
+ KeInitializeSpinLock(&m_CursorListLock);
+
m_bStopWorkThread = FALSE;
m_pWorkThread = NULL;
m_ResolutionEvent = NULL;
@@ -2326,12 +2345,7 @@ NTSTATUS VioGpuAdapter::HWInit(PCM_RESOURCE_LIST pResList, DXGK_DISPLAY_INFORMAT
PHYSICAL_ADDRESS fb_pa = m_PciResources.GetPciBar(0)->GetPA();
UINT fb_size = (UINT)m_PciResources.GetPciBar(0)->GetSize();
UINT req_size = pDispInfo->Pitch * pDispInfo->Height;
-//FIXME
-#if NTDDI_VERSION > NTDDI_WINBLUE
- req_size = 0x1000000;
-#else
- req_size = 0x800000;
-#endif
+ req_size = m_pVioGpuDod->GetMemorySize();
if (fb_pa.QuadPart != 0LL) {
pDispInfo->PhysicAddress = fb_pa;
}
@@ -2584,7 +2598,8 @@ VOID VioGpuAdapter::BlackOutScreen(CURRENT_MODE* pCurrentMod)
NTSTATUS VioGpuAdapter::SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* pSetPointerShape, _In_ CONST CURRENT_MODE* pModeCur)
{
PAGED_CODE();
-
+ KIRQL lockHandle;
+ NTSTATUS status = STATUS_UNSUCCESSFUL;
DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
DbgPrint(TRACE_LEVEL_INFORMATION, ("<--> %s flag = %d pitch = %d, pixels = %p, id = %d, w = %d, h = %d, x = %d, y = %d\n", __FUNCTION__,
pSetPointerShape->Flags.Value,
@@ -2598,14 +2613,15 @@ NTSTATUS VioGpuAdapter::SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* pSet
if (pSetPointerShape->Flags.Monochrome) {
VioGpuDbgBreak();
- return STATUS_UNSUCCESSFUL;
+ return status;
}
+
+ KeAcquireSpinLock(&m_CursorListLock, &lockHandle);
if (UpdateCursor(pSetPointerShape, pModeCur))
{
PGPU_UPDATE_CURSOR crsr;
PGPU_VBUFFER vbuf;
- UINT ret = 0;
crsr = (PGPU_UPDATE_CURSOR)m_CursorQueue.AllocCursor(&vbuf);
RtlZeroMemory(crsr, sizeof(*crsr));
@@ -2615,25 +2631,27 @@ NTSTATUS VioGpuAdapter::SetPointerShape(_In_ CONST DXGKARG_SETPOINTERSHAPE* pSet
crsr->pos.y = 0;
crsr->hot_x = pSetPointerShape->XHot;
crsr->hot_y = pSetPointerShape->YHot;
- ret = m_CursorQueue.QueueCursor(vbuf);
- DbgPrint(TRACE_LEVEL_INFORMATION, ("<--- %s vbuf = %p, ret = %d\n", __FUNCTION__, vbuf, ret));
- if (ret == 0) {
- return STATUS_SUCCESS;
- }
- VioGpuDbgBreak();
+ PCURSOR_REQUEST cursor_req = new (NonPagedPoolNx) CURSOR_REQUEST;
+ cursor_req->pbuf = vbuf;
+ cursor_req->type = VIRTIO_GPU_CMD_UPDATE_CURSOR;
+ InsertHeadList(&m_CursorList, &cursor_req->ListEntry);
+ KeSetEvent(&m_CursorUpdateEvent, IO_NO_INCREMENT, FALSE);
}
+ KeReleaseSpinLock(&m_CursorListLock, lockHandle);
DbgPrint(TRACE_LEVEL_ERROR, ("<--- %s Failed to create cursor\n", __FUNCTION__));
- return STATUS_UNSUCCESSFUL;
+ return status;
}
NTSTATUS VioGpuAdapter::SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION* pSetPointerPosition, _In_ CONST CURRENT_MODE* pModeCur)
{
PAGED_CODE();
+ KIRQL lockHandle;
+ NTSTATUS status = STATUS_UNSUCCESSFUL;
+ KeAcquireSpinLock(&m_CursorListLock, &lockHandle);
if (m_pCursorBuf != NULL)
{
PGPU_UPDATE_CURSOR crsr;
PGPU_VBUFFER vbuf;
- UINT ret = 0;
crsr = (PGPU_UPDATE_CURSOR)m_CursorQueue.AllocCursor(&vbuf);
RtlZeroMemory(crsr, sizeof(*crsr));
@@ -2668,14 +2686,15 @@ NTSTATUS VioGpuAdapter::SetPointerPosition(_In_ CONST DXGKARG_SETPOINTERPOSITION
crsr->pos.x = pSetPointerPosition->X;
crsr->pos.y = pSetPointerPosition->Y;
}
- ret = m_CursorQueue.QueueCursor(vbuf);
- DbgPrint(TRACE_LEVEL_VERBOSE, ("<--- %s vbuf = %p, ret = %d\n", __FUNCTION__, vbuf, ret));
- if (ret == 0) {
- return STATUS_SUCCESS;
- }
- VioGpuDbgBreak();
+ PCURSOR_REQUEST cursor_req = new (NonPagedPoolNx) CURSOR_REQUEST;
+ cursor_req->pbuf = vbuf;
+ cursor_req->type = VIRTIO_GPU_CMD_MOVE_CURSOR;
+ InsertTailList(&m_CursorList, &cursor_req->ListEntry);
+ KeSetEvent(&m_CursorUpdateEvent, IO_NO_INCREMENT, FALSE);
+ status = STATUS_SUCCESS;
}
- return STATUS_UNSUCCESSFUL;
+ KeReleaseSpinLock(&m_CursorListLock, lockHandle);
+ return status;
}
NTSTATUS VioGpuAdapter::Escape(_In_ CONST DXGKARG_ESCAPE* pEscape)
@@ -2784,7 +2803,7 @@ BOOLEAN VioGpuAdapter::GetEdids(void)
DbgPrint(TRACE_LEVEL_VERBOSE, ("---> %s\n", __FUNCTION__));
PGPU_VBUFFER vbuf = NULL;
-
+ VioGpuDbgBreak();
for (UINT32 i = 0; i < m_u32NumScanouts; i++) {
if (m_CtrlQueue.AskEdidInfo(&vbuf, i) &&
m_CtrlQueue.GetEdidInfo(vbuf, i, m_EDIDs)) {
@@ -2991,6 +3010,7 @@ NTSTATUS VioGpuAdapter::GetModeList(DXGK_DISPLAY_INFORMATION* pDispInfo)
m_ModeCount = SuitableModeCount + 1;
DbgPrint(TRACE_LEVEL_INFORMATION, ("ModeCount filtered %d\n", m_ModeCount));
+ VioGpuDbgBreak();
GetDisplayInfo();
for (ULONG idx = 0; idx < ModeCount; idx++)
@@ -3075,21 +3095,57 @@ void VioGpuAdapter::ThreadWork(_In_ PVOID Context)
void VioGpuAdapter::ThreadWorkRoutine(void)
{
KeSetPriorityThread(KeGetCurrentThread(), LOW_REALTIME_PRIORITY);
-
+ NTSTATUS status;
+ PVOID events[2];
+ KIRQL lockHandle;
+ events[0] = &m_ConfigUpdateEvent;
+ events[1] = &m_CursorUpdateEvent;
for (;;)
{
- KeWaitForSingleObject(&m_ConfigUpdateEvent,
- Executive,
- KernelMode,
- FALSE,
- NULL);
-
- if (m_bStopWorkThread) {
- PsTerminateSystemThread(STATUS_SUCCESS);
- break;
+ //KeWaitForSingleObject(&m_ConfigUpdateEvent,
+ // Executive,
+ // KernelMode,
+ // FALSE,
+ // NULL);
+ status = KeWaitForMultipleObjects(2, events, WaitAny, Executive, KernelMode,
+ FALSE, NULL, NULL);
+
+ if (status == STATUS_WAIT_0) {
+ if (m_bStopWorkThread) {
+ PsTerminateSystemThread(STATUS_SUCCESS);
+ break;
+ }
+ ConfigChanged();
+ NotifyResolutionEvent();
+ }
+ else {
+ KeAcquireSpinLock(&m_CursorListLock, &lockHandle);
+ if (!IsListEmpty(&m_CursorList))
+ {
+ PGPU_VBUFFER pbuf = NULL;
+ PLIST_ENTRY le = RemoveHeadList(&m_CursorList);
+ PCURSOR_REQUEST cursor_req = CONTAINING_RECORD(le, CURSOR_REQUEST, ListEntry);
+ if ((cursor_req->type == VIRTIO_GPU_CMD_MOVE_CURSOR) &&
+ !IsListEmpty(&m_CursorList)) {
+ pbuf = cursor_req->pbuf;
+ delete cursor_req;
+ m_CursorQueue.ReleaseBuffer(pbuf);
+ le = RemoveTailList(&m_CursorList);
+ cursor_req = CONTAINING_RECORD(le, CURSOR_REQUEST, ListEntry);
+ }
+ pbuf = cursor_req->pbuf;
+ delete cursor_req;
+ m_CursorQueue.QueueCursor(pbuf);
+ while (!IsListEmpty(&m_CursorList)) {
+ le = RemoveHeadList(&m_CursorList);
+ cursor_req = CONTAINING_RECORD(le, CURSOR_REQUEST, ListEntry);
+ pbuf = cursor_req->pbuf;
+ delete cursor_req;
+ m_CursorQueue.ReleaseBuffer(pbuf);
+ }
+ }
+ KeReleaseSpinLock(&m_CursorListLock, lockHandle);
}
- ConfigChanged();
- NotifyResolutionEvent();
}
}
diff --git a/viogpu/viogpudo/viogpudo.h b/viogpu/viogpudo/viogpudo.h
index 1fcd3ba8..d4eb573b 100755
--- a/viogpu/viogpudo/viogpudo.h
+++ b/viogpu/viogpudo/viogpudo.h
@@ -70,6 +70,13 @@ typedef struct _CURRENT_MODE
PVOID FrameBuffer;
} CURRENT_MODE;
+typedef struct _CURSOR_REQUEST {
+ LIST_ENTRY ListEntry;
+ ULONG type;
+ PGPU_VBUFFER pbuf;
+} CURSOR_REQUEST, * PCURSOR_REQUEST;
+
+
class VioGpuDod;
class VioGpuAdapter
@@ -167,6 +174,9 @@ private:
VioGpuMemSegment m_FrameSegment;
volatile ULONG m_PendingWorks;
KEVENT m_ConfigUpdateEvent;
+ KEVENT m_CursorUpdateEvent;
+ LIST_ENTRY m_CursorList;
+ KSPIN_LOCK m_CursorListLock;
PETHREAD m_pWorkThread;
BOOLEAN m_bStopWorkThread;
PKEVENT m_ResolutionEvent;
@@ -182,6 +192,7 @@ private:
DEVICE_POWER_STATE m_MonitorPowerState;
DEVICE_POWER_STATE m_AdapterPowerState;
DRIVER_STATUS_FLAG m_Flags;
+ DWORD m_dwMemorySize;
CURRENT_MODE m_CurrentMode;
@@ -246,6 +257,14 @@ public:
{
m_Flags.UsePresentProgress = enable;
}
+ void SetMemorySize(DWORD size)
+ {
+ m_dwMemorySize = max(m_dwMemorySize, (size * 1024 * 1024));
+ }
+ DWORD GetMemorySize() const
+ {
+ return m_dwMemorySize;
+ }
#pragma code_seg(pop)
NTSTATUS StartDevice(_In_ DXGK_START_INFO* pDxgkStartInfo,
diff --git a/viogpu/viogpudo/viogpudo.inx b/viogpu/viogpudo/viogpudo.inx
index 11957e55..96c4bc45 100755
--- a/viogpu/viogpudo/viogpudo.inx
+++ b/viogpu/viogpudo/viogpudo.inx
@@ -63,10 +63,11 @@ HKR,,EventMessageFile,%REG_EXPAND_SZ%,"%%SystemRoot%%\System32\IoLogMsg.dll"
HKR,,TypesSupported,%REG_DWORD%,7
[VioGpuDod_DeviceSettings]
-HKR,, HWCursor, %REG_DWORD%, 0
+HKR,, HWCursor, %REG_DWORD%, 1
HKR,, FlexResolution, %REG_DWORD%, 1
HKR,, UsePhysicalMemory, %REG_DWORD%, 1
HKR,, UsePresentProgress, %REG_DWORD%, 1
+HKR,, MemorySize, %REG_DWORD%, 32
[VioGpuDod_PCI_MSIX]
HKR, "Interrupt Management",, 0x00000010
All the best, Vadim.
@vrozenfe can I merge?
Wait a second, @kostyanf14 , is "WDDM RotateBlt Window GDI (WoW64)" a new failure?
Wait a second, @kostyanf14 , is "WDDM RotateBlt Window GDI (WoW64)" a new failure?
@YanVugenfirer, it fail sometimes not always
It crashes into KMODE_EXCEPTION_NOT_HANDLED (1E) BSOD almost instantly on my testing setup. Apart from that I saw some issues with the cursor pointer update
(qemu) info version 8.1.0v8.1.0
qemu cli /home/vrozenfe/work/upstream/qemu/build/x86_64-softmmu/qemu-system-x86_64 -cpu qemu64,hv_relaxed,hv_vapic,hv_spinlocks=0x1fff -m 2G -smp 4,maxcpus=4,cores=4,threads=1,sockets=1 -usb -device usb-tablet,id=tablet0 -drive file=/run/media/vrozenfe/elements/images/win10-32-gpu.qcow2,if=none,id=drive-ide0-0-0,cache=off,werror=stop,rerror=stop -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -netdev tap,id=hostnet0,script=/etc/qemu-ifup -device e1000,netdev=hostnet0,mac=52:83:66:77:88:68,id=net0 -boot c -uuid 5b959a71-e33f-4419-97b4-da6fe8fb7062 -rtc driftfix=slew -global kvm-pit.lost_tick_policy=discard -monitor stdio -name VIOVGA -enable-kvm -device virtio-vga,edid=on,xres=1280,yres=800,max_outputs=1 -serial tcp:127.0.0.1:4445
So complex? Can you try this?
@osy Do you want to try and fix the PR?
Sorry I've been very busy with other things.