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

[Sameeh Jubran]

On Mon, Sep 12, 2016 at 11:32 AM, Frediano Ziglio <fziglio at redhat.com>
wrote:

> >
> > 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 we can drop this patch if we plan 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
>



-- 
Respectfully,
*Sameeh Jubran*
*Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
*Junior Software Engineer @ Daynix <http://www.daynix.com>.*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160920/82334342/attachment-0001.html>