[Spice-devel] [PATCH qxl-wddm-dod v4 03/10] Use MmMapIoSpaceEx instead of MmMapIoSpace

[Frediano Ziglio]

> 
> Based on a patch by Sandy Stutsman <sstutsma at redhat.com>
> 
> Signed-off-by: Sameeh Jubran <sameeh at daynix.com>
> ---
>  qxldod/QxlDod.cpp | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp
> index d07c7b4..93fd8c6 100755
> --- a/qxldod/QxlDod.cpp
> +++ b/qxldod/QxlDod.cpp
> @@ -2009,17 +2009,17 @@ MapFrameBuffer(
>          return STATUS_INVALID_PARAMETER;
>      }
>  
> -    *VirtualAddress = MmMapIoSpace(PhysicalAddress,
> +    *VirtualAddress = MmMapIoSpaceEx(PhysicalAddress,
>                                     Length,
> -                                   MmWriteCombined);
> +                                   PAGE_WRITECOMBINE | PAGE_READWRITE);
>      if (*VirtualAddress == NULL)
>      {
>          // The underlying call to MmMapIoSpace failed. This may be because,
>          MmWriteCombined
>          // isn't supported, so try again with MmNonCached
>  
> -        *VirtualAddress = MmMapIoSpace(PhysicalAddress,
> +        *VirtualAddress = MmMapIoSpaceEx(PhysicalAddress,
>                                         Length,
> -                                       MmNonCached);
> +                                     (ULONG) (PAGE_NOCACHE |
> PAGE_READWRITE));

The conversion to ULONG here is not necessary.
I can do this change on my own, but I have some minor questions.

>          if (*VirtualAddress == NULL)
>          {
>              DbgPrint(TRACE_LEVEL_ERROR, ("MmMapIoSpace returned a NULL
>              buffer when trying to allocate %lu bytes", Length));

It looks like MmMapIoSpaceEx was introduced with Windows 10 while WDDM was
introduce in Windows 7. Are we going to support only from Windows 10?
Windows 7 supports WDDM but supports also XPDM so it's supported but
users of Windows 8 won't have a QXL driver. Are we never going to support
Windows 8?

I think the reason for this change is that the new function can specify
additional security, specifically removing the executable bit from page
mappings. Am I right? Perhaps this should be mentioned in the rationale.

OT: in the qxldod directory there is qxldod.inf and qxldod.inx file.
It looks like the project uses the .inx file to generate the final .inf
file while the .inf file in git is just an old file. Is it correct?
Should we remove the .inf file from git?

Frediano