[win-pv-devel] [PATCH] Fix NDISTest6.5 CheckConnectivity and others

[Paul Durrant <paul.durrant@xxxxxxxxxx>]

Commit e5e64b57 "Don't try to __FreePages() with local copy of MDL"
introduced subtle breakage into the receive path.

Prior to this patch, for the MDLs embedded in receiver packets, it was
always true that Mdl->MappedSystemVa == Mdl->StartVa + Mdl->ByteOffset.
However, after the commit, Mdl->StartVa was left as NULL in the assumption
that Mdl->MappedSystemVa would always be used to access the buffer. Sadly
this appears not to always be the case and in fact, wdm.h carries this
comment:

// Notice that while in the context of the subject thread, the base virtual
// address of a buffer mapped by an MDL may be referenced using the
// following:
//
//      Mdl->StartVa | Mdl->ByteOffset
//

So the the previous setting of StartVa is indeed documented, and clearly
something in the network tests is relying on it.

This patch modifies the MDL allocator code in util.h to alwasy set StartVa,
and modifies the receiver code to carry this field forward into the
embedded MDLs. It also removes some code duplication in ReceiverPacketCtor()
and __ReceiverRingPutPacket().

Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
---

NOTE: This patch needs to be back-ported into the 8.2 staging branch as
      the erroneous commit is also duplicated there.
---
 src/xenvif/receiver.c | 47 ++++++++++++++++++++++++-----------------------
 src/xenvif/util.h     |  6 +++++-
 2 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/src/xenvif/receiver.c b/src/xenvif/receiver.c
index e48b678..a6b3ad2 100644
--- a/src/xenvif/receiver.c
+++ b/src/xenvif/receiver.c
@@ -154,6 +154,28 @@ __ReceiverFree(
     __FreePoolWithTag(Buffer, XENVIF_RECEIVER_TAG);
 }
 
+static VOID FORCEINLINE
+__ReceiverPacketMdlInit(
+    IN  PXENVIF_RECEIVER_PACKET Packet
+    )
+{
+    PMDL    Mdl = Packet->SystemMdl;
+
+    ASSERT(IsZeroMemory(&Packet->Mdl, sizeof (MDL)));
+
+#pragma warning(push)
+#pragma warning(disable:28145) // modifying struct MDL
+
+    Packet->Mdl.Size = sizeof (MDL) + sizeof (PFN_NUMBER);
+    Packet->Mdl.MdlFlags = Mdl->MdlFlags;
+
+    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
+    Packet->Mdl.StartVa = Mdl->StartVa;
+    Packet->Mdl.MappedSystemVa = Mdl->MappedSystemVa;
+
+#pragma warning(pop)
+}
+
 static NTSTATUS
 ReceiverPacketCtor(
     IN  PVOID               Argument,
@@ -173,20 +195,9 @@ ReceiverPacketCtor(
     if (Mdl == NULL)
         goto fail1;
 
-    ASSERT3U(Mdl->ByteOffset, ==, 0);
-
     Packet->SystemMdl = Mdl;
 
-#pragma warning(push)
-#pragma warning(disable:28145) // modifying struct MDL
-
-    Packet->Mdl.Size = sizeof (MDL) + sizeof (PFN_NUMBER);
-    Packet->Mdl.MdlFlags = Mdl->MdlFlags;
-
-#pragma warning(pop)
-
-    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
-    Packet->Mdl.MappedSystemVa = Mdl->MappedSystemVa;
+    __ReceiverPacketMdlInit(Packet);
 
     Packet->__Pfn = MmGetMdlPfnArray(Mdl)[0];
 
@@ -258,7 +269,6 @@ __ReceiverRingPutPacket(
 {
     PXENVIF_RECEIVER            Receiver;
     PXENVIF_FRONTEND            Frontend;
-    PMDL                        Mdl = Packet->SystemMdl;
 
     Receiver = Ring->Receiver;
     Frontend = Receiver->Frontend;
@@ -277,16 +287,7 @@ __ReceiverRingPutPacket(
 
     RtlZeroMemory(&Packet->Mdl, sizeof (MDL));
 
-#pragma warning(push)
-#pragma warning(disable:28145) // modifying struct MDL
-
-    Packet->Mdl.Size = sizeof (MDL) + sizeof (PFN_NUMBER);
-    Packet->Mdl.MdlFlags = Mdl->MdlFlags;
-
-#pragma warning(pop)
-
-    ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
-    Packet->Mdl.MappedSystemVa = Mdl->MappedSystemVa;
+    __ReceiverPacketMdlInit(Packet);
 
     XENBUS_CACHE(Put,
                  &Receiver->CacheInterface,
diff --git a/src/xenvif/util.h b/src/xenvif/util.h
index 8983d4c..30322d8 100644
--- a/src/xenvif/util.h
+++ b/src/xenvif/util.h
@@ -219,7 +219,11 @@ __AllocatePages(
     if (MdlMappedSystemVa == NULL)
         goto fail3;
 
-    ASSERT3P(MdlMappedSystemVa, ==, Mdl->MappedSystemVa);
+    Mdl->StartVa = PAGE_ALIGN(MdlMappedSystemVa);
+
+    ASSERT3U(Mdl->ByteOffset, ==, 0);
+    ASSERT3P(Mdl->StartVa, ==, MdlMappedSystemVa);
+    ASSERT3P(Mdl->MappedSystemVa, ==, MdlMappedSystemVa);
 
     RtlZeroMemory(MdlMappedSystemVa, Mdl->ByteCount);
 
-- 
2.5.3


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