MR!3 patch

Submitted by Jon Stumpf on April 25, 2018, 1:21 a.m.

Details

Message ID CALQX8QT45UVPVVxUVYRyQ4LsJ6-QLvpkN1cyJzNAUDLnxM6Atw@mail.gmail.com
State New
Headers show
Series "MR!3 patch" ( rev: 2 ) in Spice

Not browsing as part of any series.

Commit Message

Jon Stumpf April 25, 2018, 1:21 a.m.
Frediano,

Attached is the patch file as created by "git format-patch".  In reading
the documentation, git format-patch warns of the many problems using all
the email clients that I use.  And, frankly, I do not want to figure out
how to avoid these pitfalls.

This is becoming increasing difficult to submit such simple patches.  To
contribute,

   - I have had to subscribe to Gitlab (I am a Github user).
   - In using the WebIDE, it created a change set involving the whole file
   because it replaced the Windows EOL sequence with the UNIX EOL sequence.
   - I created Merge Requests but was directed to provide patches.
   - In providing patches, I was instructed to use "git format-patch"
   versus "git diff".

I understand that you have a development process (I have since read your
Developers page).  That understood, I don't intend to contribute beyond the
fixing of the bugs that I have discovered and attempted to rectify.  I am
Linux developer and do not have a Windows build environment.  (I had a
colleague test the compile on Windows 10.)

I have two small patches (MR!1 and MR!3) to address the issue I documented
in Issue #4.  If these patches do not work, I am asking that you choose one
of the following at this point:

   1. Use the Merge Requests already in your repo;
   2. Use the diff patches as a seed into your development process;

I would also like someone to answer the questions posed in Issue #4 and
also address the lack of documentation for the QXL parameters.

Thank you.

- jss


On Tue, Apr 24, 2018 at 8:01 AM, Jon Stumpf <jon.stumpf@gmail.com> wrote:

> Frediano,
>
> I will this evening when I return home and after I read how to use "git
> format-patch".
>
> - jss
>
>
> On Tue, Apr 24, 2018 at 7:00 AM, Frediano Ziglio <fziglio@redhat.com>
> wrote:
>
>> > Here is the patch for Merge Request 3 (MR!3). The WebIDE must have
>> changed
>> > the end-of-line sequence as I produced this with "git diff
>> > --ignore-all-space".
>>
>> > - jss
>>
>> Jon, can you provide patches in the form of git changes?
>> Instead of git diff you can use git format-patch.
>> Alternatively add some comment and email (I assume "Jon Stumpf" <
>> jon.stumpf@gmail.com>)
>> you want to see in the git commit.
>>
>> Frediano
>>
>
>
>
> --
> - jss
>
>

Patch hide | download patch | download mbox

From b32f2a3d5a0938b41188c6b183c747f63b16d0da Mon Sep 17 00:00:00 2001
From: jon-stumpf <jon.stumpf@gmail.com>
Date: Tue, 24 Apr 2018 21:10:06 -0400
Subject: [PATCH 0/3] *** SUBJECT HERE ***
To: spice-devel@lists.freedesktop.org

This is my first attempt at using "git format-patch".

Jon Stumpf (1):
  Update RegisterHWInfo() to take a second parameter, MemSize, so that
    "HardwareInformation.MemorySize" can be properly set in the
    Registry.

 qxldod/QxlDod.cpp | 14 +++++++++++---
 qxldod/QxlDod.h   |  2 +-
 2 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.7.4

From a212abdc9e003341c0f9d58ebf1b135ebb3ab8f3 Mon Sep 17 00:00:00 2001
From: Jon Stumpf <jon.stumpf@gmail.com>
Date: Sat, 14 Apr 2018 20:59:42 +0000
Subject: [PATCH 1/3] Update RegisterHWInfo() to take a second parameter,
 MemSize, so that "HardwareInformation.MemorySize" can be properly set in the
 Registry.
To: spice-devel@lists.freedesktop.org

---
 qxldod/QxlDod.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 5fa9c9a..3d50a8f 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -189,7 +189,7 @@  NTSTATUS QxlDod::StartDevice(_In_  DXGK_START_INFO*   pDxgkStartInfo,
         return Status;
     }
 
-    Status = RegisterHWInfo(m_pHWDevice->GetId());
+    Status = RegisterHWInfo(m_pHWDevice->GetId(), m_DeviceInfo.SystemMemorySize);
     if (!NT_SUCCESS(Status))
     {
         QXL_LOG_ASSERTION1("RegisterHWInfo failed with status 0x%X\n",
@@ -1920,7 +1920,7 @@  NTSTATUS QxlDod::WriteHWInfoStr(_In_ HANDLE DevInstRegKeyHandle, _In_ PCWSTR psz
     return Status;
 }
 
-NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id)
+NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id, _In_ LARGE_INTEGER MemSize)
 {
     PAGED_CODE();
 
@@ -1967,7 +1967,7 @@  NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id)
     // MemorySize is a ULONG, unlike the others which are all strings
     UNICODE_STRING ValueNameMemorySize;
     RtlInitUnicodeString(&ValueNameMemorySize, L"HardwareInformation.MemorySize");
-    DWORD MemorySize = 0; // BDD has no access to video memory
+    DWORD MemorySize = MemSize;
     Status = ZwSetValueKey(DevInstRegKeyHandle,
                            &ValueNameMemorySize,
                            0,
-- 
2.7.4


From 8635f1e27bdd7d1dbdcfa7eb478b66f20f9fd0c5 Mon Sep 17 00:00:00 2001
From: Jon Stumpf <jon.stumpf@gmail.com>
Date: Wed, 18 Apr 2018 02:44:33 +0000
Subject: [PATCH 2/3] Fixed reference to MemSize as LARGE_INTEGER is a
 structure and not an scalar.
To: spice-devel@lists.freedesktop.org

---
 qxldod/QxlDod.cpp | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
index 3d50a8f..e73a4f9 100755
--- a/qxldod/QxlDod.cpp
+++ b/qxldod/QxlDod.cpp
@@ -1965,9 +1965,17 @@  NTSTATUS QxlDod::RegisterHWInfo(_In_ ULONG Id, _In_ LARGE_INTEGER MemSize)
     }
 
     // MemorySize is a ULONG, unlike the others which are all strings
+    //
+    // Microsoft documents the value as counting megabytes, but observation 
+    // suggests it counts bytes. For instance, to represent 512MB, set the 
+    // value to 0x20000000, not to 0x0200.
+    //
+    // A robust implementation would check that the size in bytes, given as 
+    // 64 bits, is not too big for representation as a DWORD!
+
     UNICODE_STRING ValueNameMemorySize;
     RtlInitUnicodeString(&ValueNameMemorySize, L"HardwareInformation.MemorySize");
-    DWORD MemorySize = MemSize;
+    DWORD MemorySize = MemSize.LowPart;
     Status = ZwSetValueKey(DevInstRegKeyHandle,
                            &ValueNameMemorySize,
                            0,
-- 
2.7.4


From b32f2a3d5a0938b41188c6b183c747f63b16d0da Mon Sep 17 00:00:00 2001
From: Jon Stumpf <jon.stumpf@gmail.com>
Date: Wed, 18 Apr 2018 02:48:56 +0000
Subject: [PATCH 3/3] Fixed declaration of RegisterHWInfo() to match
 implementation in QxlDod.cpp;
To: spice-devel@lists.freedesktop.org

---
 qxldod/QxlDod.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qxldod/QxlDod.h b/qxldod/QxlDod.h
index 695b83a..dc3c9fc 100755
--- a/qxldod/QxlDod.h
+++ b/qxldod/QxlDod.h
@@ -837,7 +837,7 @@  private:
     QXL_NON_PAGED D3DDDI_VIDEO_PRESENT_SOURCE_ID FindSourceForTarget(D3DDDI_VIDEO_PRESENT_TARGET_ID TargetId, BOOLEAN DefaultToZero);
     NTSTATUS IsVidPnSourceModeFieldsValid(CONST D3DKMDT_VIDPN_SOURCE_MODE* pSourceMode) const;
     NTSTATUS IsVidPnPathFieldsValid(CONST D3DKMDT_VIDPN_PRESENT_PATH* pPath) const;
-    NTSTATUS RegisterHWInfo(_In_ ULONG Id);
+    NTSTATUS RegisterHWInfo(_In_ ULONG Id, _In_ LARGE_INTEGER MemSize);
     QXL_NON_PAGED VOID VsyncTimerProc();
     static QXL_NON_PAGED VOID VsyncTimerProcGate(_In_ _KDPC *dpc, _In_ PVOID context, _In_ PVOID arg1, _In_ PVOID arg2);
     QXL_NON_PAGED VOID IndicateVSyncInterrupt();
-- 
2.7.4