[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[win-pv-devel] [PATCH] Replace uses of MmAllocatePagesForMdlEx in __AllocatePage



Windows appears to have an edge case bug in which zeroing
memory using MmAllocatePAgesForMdlEx (which in Win 10 1803
happens even if you specify MM_DONT_ZERO_ALLOCATION) can cause
a BSOD 139 1e.

This commit users MmAllocateContinguousMemorySpecifyCache
to allocate memory instead, then builds and Mdl to wrap
it up.

__AllocatePages is left unchanged (as we don't want
to allocate multiple contiguous pages).  This issue
has not been seen outside of xenvif calls to
__AllocatePage and we expect a fix to the underlying
Windows problem in the near future

Signed-off-by: Ben.Chalmers <ben.chalmers@xxxxxxxxxx>
---
 src/xenvif/controller.c  | 12 +++++--
 src/xenvif/receiver.c    | 56 ++++++++++++++++++++++--------
 src/xenvif/transmitter.c | 24 +++++++++----
 src/xenvif/util.h        | 89 ++++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 151 insertions(+), 30 deletions(-)

diff --git a/src/xenvif/controller.c b/src/xenvif/controller.c
index 35901a2..37abb1d 100644
--- a/src/xenvif/controller.c
+++ b/src/xenvif/controller.c
@@ -469,7 +469,9 @@ ControllerConnect(
     if (Controller->Mdl == NULL)
         goto fail7;
 
-    ASSERT(Controller->Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Controller->Mdl->MdlFlags 
+            & (MDL_MAPPED_TO_SYSTEM_VA 
+                | MDL_SOURCE_IS_NONPAGED_POOL));
     Controller->Shared = Controller->Mdl->MappedSystemVa;
     ASSERT(Controller->Shared != NULL);
 
@@ -904,7 +906,9 @@ ControllerSetHashKey(
     if (Mdl == NULL)
         goto fail1;
 
-    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Mdl->MdlFlags 
+            & (MDL_MAPPED_TO_SYSTEM_VA 
+                | MDL_SOURCE_IS_NONPAGED_POOL));
     Buffer = Mdl->MappedSystemVa;
     ASSERT(Buffer != NULL);
 
@@ -1083,7 +1087,9 @@ ControllerSetHashMapping(
     if (Mdl == NULL)
         goto fail2;
 
-    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Mdl->MdlFlags 
+            & (MDL_MAPPED_TO_SYSTEM_VA 
+                | MDL_SOURCE_IS_NONPAGED_POOL));
     Buffer = Mdl->MappedSystemVa;
     ASSERT(Buffer != NULL);
 
diff --git a/src/xenvif/receiver.c b/src/xenvif/receiver.c
index a6b3ad2..ab3e416 100644
--- a/src/xenvif/receiver.c
+++ b/src/xenvif/receiver.c
@@ -169,7 +169,9 @@ __ReceiverPacketMdlInit(
     Packet->Mdl.Size = sizeof (MDL) + sizeof (PFN_NUMBER);
     Packet->Mdl.MdlFlags = Mdl->MdlFlags;
 
-    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Mdl->MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     Packet->Mdl.StartVa = Mdl->StartVa;
     Packet->Mdl.MappedSystemVa = Mdl->MappedSystemVa;
 
@@ -406,7 +408,9 @@ ReceiverRingProcessTag(
 
     PayloadLength = Packet->Length - Info->Length;
 
-    ASSERT(Packet->Mdl.MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Packet->Mdl.MdlFlags & 
+           (MDL_MAPPED_TO_SYSTEM_VA | 
+            MDL_SOURCE_IS_NONPAGED_POOL));
     BaseVa = Packet->Mdl.MappedSystemVa;
     ASSERT(BaseVa != NULL);
 
@@ -497,7 +501,9 @@ ReceiverRingProcessChecksum(
     if (Info->IpHeader.Length == 0)
         return;
 
-    ASSERT(Packet->Mdl.MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Packet->Mdl.MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     BaseVa = Packet->Mdl.MappedSystemVa;
     ASSERT(BaseVa != NULL);
 
@@ -654,7 +660,9 @@ ReceiverRingPullup(
         PUCHAR  SourceVa;
         ULONG   CopyLength;
 
-        ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+        ASSERT(Mdl->MdlFlags & 
+               (MDL_MAPPED_TO_SYSTEM_VA |
+                MDL_SOURCE_IS_NONPAGED_POOL));
         SourceVa = Mdl->MappedSystemVa;
         ASSERT(SourceVa != NULL);
 
@@ -700,7 +708,9 @@ __ReceiverRingPullupPacket(
     XENVIF_PACKET_PAYLOAD       Payload;
     ULONG                       Length;
 
-    ASSERT(Packet->Mdl.MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Packet->Mdl.MdlFlags 
+            & (MDL_MAPPED_TO_SYSTEM_VA 
+                | MDL_SOURCE_IS_NONPAGED_POOL));
     BaseVa = Packet->Mdl.MappedSystemVa;
     ASSERT(BaseVa != NULL);
 
@@ -744,7 +754,9 @@ __ReceiverRingBuildSegment(
 
     Info = &Packet->Info;
 
-    ASSERT(Packet->Mdl.MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Packet->Mdl.MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     InfoVa = Packet->Mdl.MappedSystemVa;
     ASSERT(InfoVa != NULL);
 
@@ -767,7 +779,9 @@ __ReceiverRingBuildSegment(
 
     Mdl = &Segment->Mdl;
 
-    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Mdl->MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     BaseVa = Mdl->MappedSystemVa;
     ASSERT(BaseVa != NULL);
 
@@ -849,7 +863,9 @@ __ReceiverRingBuildSegment(
 
         Mdl = Mdl->Next;
 
-        ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+        ASSERT(Mdl->MdlFlags &
+               (MDL_MAPPED_TO_SYSTEM_VA |
+                MDL_SOURCE_IS_NONPAGED_POOL));
         BaseVa = Mdl->MappedSystemVa;
         ASSERT(BaseVa != NULL);
 
@@ -939,7 +955,9 @@ ReceiverRingProcessLargePacket(
 
     Packet->Mdl.Next = NULL;
 
-    ASSERT(Packet->Mdl.MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Packet->Mdl.MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     InfoVa = Packet->Mdl.MappedSystemVa;
     ASSERT(InfoVa != NULL);
 
@@ -1134,7 +1152,9 @@ ReceiverRingProcessStandardPacket(
         if (Mdl == NULL)
             goto fail2;
 
-        ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+        ASSERT(Mdl->MdlFlags &
+               (MDL_MAPPED_TO_SYSTEM_VA |
+                MDL_SOURCE_IS_NONPAGED_POOL));
         BaseVa = Mdl->MappedSystemVa;
         ASSERT(BaseVa != NULL);
 
@@ -1240,7 +1260,9 @@ ReceiverRingProcessPacket(
     // Override offset to align
     Packet->Offset = Receiver->IpAlignOffset;
 
-    ASSERT(Packet->Mdl.MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Packet->Mdl.MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     BaseVa = Packet->Mdl.MappedSystemVa;
     ASSERT(BaseVa != NULL);
 
@@ -1422,7 +1444,9 @@ __ReceiverRingReleaseLock(
                                    XENVIF_RECEIVER_PACKET,
                                    ListEntry);
 
-        ASSERT(Packet->Mdl.MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+        ASSERT(Packet->Mdl.MdlFlags &
+               (MDL_MAPPED_TO_SYSTEM_VA |
+                MDL_SOURCE_IS_NONPAGED_POOL));
         BaseVa = Packet->Mdl.MappedSystemVa;
         ASSERT(BaseVa != NULL);
 
@@ -2034,7 +2058,9 @@ ReceiverRingPoll(
 
                 ASSERT3U(rsp->id, ==, id);
 
-                ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+                ASSERT(Mdl->MdlFlags &
+                       (MDL_MAPPED_TO_SYSTEM_VA |
+                        MDL_SOURCE_IS_NONPAGED_POOL));
                 BaseVa = Mdl->MappedSystemVa;
                 ASSERT(BaseVa != NULL);
 
@@ -2419,7 +2445,9 @@ __ReceiverRingConnect(
     if (Ring->Mdl == NULL)
         goto fail3;
 
-    ASSERT(Ring->Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Ring->Mdl->MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     Ring->Shared = Ring->Mdl->MappedSystemVa;
     ASSERT(Ring->Shared != NULL);
 
diff --git a/src/xenvif/transmitter.c b/src/xenvif/transmitter.c
index 1d46f85..bf6004a 100644
--- a/src/xenvif/transmitter.c
+++ b/src/xenvif/transmitter.c
@@ -867,7 +867,9 @@ __TransmitterRingCopyPayload(
 
         Length = __min(Payload.Length, PAGE_SIZE);
 
-        ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+        ASSERT(Mdl->MdlFlags &
+               (MDL_MAPPED_TO_SYSTEM_VA |
+                MDL_SOURCE_IS_NONPAGED_POOL));
         BaseVa = Mdl->MappedSystemVa;
         ASSERT(BaseVa != NULL);
 
@@ -1187,7 +1189,9 @@ __TransmitterRingPrepareHeader(
 
     Mdl = Buffer->Mdl;
 
-    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Mdl->MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     BaseVa = Mdl->MappedSystemVa;
     ASSERT(BaseVa != NULL);
 
@@ -1684,7 +1688,9 @@ __TransmitterRingPreparePacket(
 
             ASSERT3U(Mdl->ByteCount, <=, PAGE_SIZE - Trailer);
 
-            ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+            ASSERT(Mdl->MdlFlags &
+                   (MDL_MAPPED_TO_SYSTEM_VA |
+                    MDL_SOURCE_IS_NONPAGED_POOL));
             BaseVa = Mdl->MappedSystemVa;
             ASSERT(BaseVa != NULL);
 
@@ -1783,7 +1789,9 @@ __TransmitterRingPrepareArp(
 
     Mdl = Buffer->Mdl;
 
-    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Mdl->MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     BaseVa = Mdl->MappedSystemVa;
     ASSERT(BaseVa != NULL);
 
@@ -1924,7 +1932,9 @@ __TransmitterRingPrepareNeighbourAdvertisement(
 
     Mdl = Buffer->Mdl;
 
-    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Mdl->MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     BaseVa = Mdl->MappedSystemVa;
     ASSERT(BaseVa != NULL);
 
@@ -3638,7 +3648,9 @@ __TransmitterRingConnect(
     if (Ring->Mdl == NULL)
         goto fail3;
 
-    ASSERT(Ring->Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    ASSERT(Ring->Mdl->MdlFlags &
+           (MDL_MAPPED_TO_SYSTEM_VA |
+            MDL_SOURCE_IS_NONPAGED_POOL));
     Ring->Shared = Ring->Mdl->MappedSystemVa;
     ASSERT(Ring->Shared != NULL);
 
diff --git a/src/xenvif/util.h b/src/xenvif/util.h
index 30322d8..dfbe9ae 100644
--- a/src/xenvif/util.h
+++ b/src/xenvif/util.h
@@ -210,10 +210,10 @@ __AllocatePages(
 
     MdlMappedSystemVa = MmMapLockedPagesSpecifyCache(Mdl,
                                                      KernelMode,
-                                                                            
MmCached,
-                                                                            
NULL,
-                                                                            
FALSE,
-                                                                            
NormalPagePriority);
+                                                     MmCached,
+                                                     NULL,
+                                                     FALSE,
+                                                     NormalPagePriority);
 
     status = STATUS_UNSUCCESSFUL;
     if (MdlMappedSystemVa == NULL)
@@ -244,11 +244,69 @@ fail1:
     return NULL;
 }
 
-#define __AllocatePage()    __AllocatePages(1)
+
+static FORCEINLINE PMDL
+__AllocatePage()
+{
+    PHYSICAL_ADDRESS    LowAddress;
+    PHYSICAL_ADDRESS    HighAddress;
+    PHYSICAL_ADDRESS    Align;
+    SIZE_T              TotalBytes;
+    PMDL                Mdl;
+    PUCHAR              MdlMappedSystemVa;
+    NTSTATUS            status;
+
+    ASSERT3U(KeGetCurrentIrql(), <=, DISPATCH_LEVEL);
+
+    LowAddress.QuadPart  = 0ull;
+    HighAddress.QuadPart = ~0ull;
+    Align.QuadPart       = PAGE_SIZE;
+    TotalBytes           = (SIZE_T)PAGE_SIZE;
+
+    MdlMappedSystemVa = MmAllocateContiguousMemorySpecifyCache(
+                            TotalBytes,
+                            LowAddress,
+                            HighAddress,
+                            Align,
+                            MmCached);
+
+    status = STATUS_NO_MEMORY;
+    if (MdlMappedSystemVa == NULL)
+        goto fail1;
+
+
+    Mdl = IoAllocateMdl(MdlMappedSystemVa,
+                        (ULONG)TotalBytes,
+                        FALSE,
+                        FALSE,
+                        NULL);
+
+    if (Mdl == NULL)
+        goto fail2;
+
+    MmBuildMdlForNonPagedPool(Mdl);
+
+    ASSERT3U(Mdl->ByteOffset, ==, 0);
+    ASSERT3P(Mdl->StartVa, ==, MdlMappedSystemVa);
+    ASSERT3P(Mdl->MappedSystemVa, ==, MdlMappedSystemVa);
+
+    RtlZeroMemory(MdlMappedSystemVa, Mdl->ByteCount);
+
+    return Mdl;
+
+fail2:
+    Error("fail2\n");
+
+    MmFreeContiguousMemory(MdlMappedSystemVa);
+fail1:
+    Error("fail1 (%08x)\n", status);
+
+    return NULL;
+}
 
 static FORCEINLINE VOID
 __FreePages(
-    IN PMDL    Mdl
+    IN  PMDL    Mdl
     )
 {
     PUCHAR     MdlMappedSystemVa;
@@ -262,7 +320,24 @@ __FreePages(
     ExFreePool(Mdl);
 }
 
-#define __FreePage(_Mdl)    __FreePages(_Mdl)
+static FORCEINLINE VOID
+__FreePage(
+    IN  PMDL    Mdl
+    )
+{
+    PUCHAR  MdlMappedSystemVa;
+
+    ASSERT(Mdl->MdlFlags &
+            (MDL_MAPPED_TO_SYSTEM_VA |
+             MDL_SOURCE_IS_NONPAGED_POOL));
+
+    MdlMappedSystemVa = Mdl->MappedSystemVa;
+    
+    IoFreeMdl(Mdl);
+    
+    MmFreeContiguousMemory(MdlMappedSystemVa);
+
+}
 
 static FORCEINLINE PCHAR
 __strtok_r(
-- 
2.10.1.windows.1


_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.