Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/134811/?format=api
{ "id": 134811, "url": "http://patches.dpdk.org/api/patches/134811/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/20231204103214.2504017-1-ming3.li@intel.com/", "project": { "id": 1, "url": "http://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<20231204103214.2504017-1-ming3.li@intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/20231204103214.2504017-1-ming3.li@intel.com", "date": "2023-12-04T10:32:14", "name": "[v4] windows/virt2phys: fix block MDL not updated", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "ecbf52acfdfa0ed7f748ad22cc3b967d271f3dd1", "submitter": { "id": 3182, "url": "http://patches.dpdk.org/api/people/3182/?format=api", "name": "Ric Li", "email": "ming3.li@intel.com" }, "delegate": { "id": 1, "url": "http://patches.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/20231204103214.2504017-1-ming3.li@intel.com/mbox/", "series": [ { "id": 30443, "url": "http://patches.dpdk.org/api/series/30443/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=30443", "date": "2023-12-04T10:32:14", "name": "[v4] windows/virt2phys: fix block MDL not updated", "version": 4, "mbox": "http://patches.dpdk.org/series/30443/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/134811/comments/", "check": "warning", "checks": "http://patches.dpdk.org/api/patches/134811/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from mails.dpdk.org (mails.dpdk.org [217.70.189.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id E10F14366A;\n\tMon, 4 Dec 2023 11:32:35 +0100 (CET)", "from mails.dpdk.org (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id C3B3D40DDE;\n\tMon, 4 Dec 2023 11:32:35 +0100 (CET)", "from mgamail.intel.com (mgamail.intel.com [134.134.136.126])\n by mails.dpdk.org (Postfix) with ESMTP id 3809C40DD8\n for <dev@dpdk.org>; Mon, 4 Dec 2023 11:32:33 +0100 (CET)", "from fmsmga001.fm.intel.com ([10.253.24.23])\n by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n 04 Dec 2023 02:32:17 -0800", "from media-ric-kahawai-icl.sh.intel.com ([10.67.119.129])\n by fmsmga001.fm.intel.com with ESMTP; 04 Dec 2023 02:32:16 -0800" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple;\n d=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n t=1701685953; x=1733221953;\n h=from:to:cc:subject:date:message-id:in-reply-to:\n references:mime-version:content-transfer-encoding;\n bh=nKz7jcVqpas0LqwqopmDhf0kXYA3TpGxjclNkxbrffo=;\n b=EgxKyGbGM/fBwt0KTLFO7iMk1tvQ7RnHoJfLENAjTVPbmui0RVjSMwip\n B/QHYgeogi8OsVls+EhBci3R6X54pU6ac3edddPSRMULAJMOWygqh0kbF\n fg0yXEiDQsPJApnQ4DNU3cCnhCmjqAuuw/tJRcsyyo40NwEsS29an0q3E\n dhSQ1+uD6GJEMZA7CdmzbCY935ckPQArQTkK2WekWGjmclrMmPnVjDok+\n dDX8mAfv/QwTcHH3fYFdau1QHJ99zlVAfcMJ2qwJmKNhBciXG9TkIdv91\n +QhdTQT+ci5TcQYyP7fegqXM31cLtdFxxH8re7Gfc3zXHMI94g9iHqSOc w==;", "X-IronPort-AV": [ "E=McAfee;i=\"6600,9927,10913\"; a=\"378747423\"", "E=Sophos;i=\"6.04,249,1695711600\"; d=\"scan'208\";a=\"378747423\"", "E=McAfee;i=\"6600,9927,10913\"; a=\"914374254\"", "E=Sophos;i=\"6.04,249,1695711600\"; d=\"scan'208\";a=\"914374254\"" ], "X-ExtLoop1": "1", "From": "Ric Li <ming3.li@intel.com>", "To": "dmitry.kozliuk@gmail.com", "Cc": "dev@dpdk.org,\n\tming3.li@intel.com,\n\troretzla@linux.microsoft.com", "Subject": "[PATCH v4] windows/virt2phys: fix block MDL not updated", "Date": "Mon, 4 Dec 2023 18:32:14 +0800", "Message-Id": "<20231204103214.2504017-1-ming3.li@intel.com>", "X-Mailer": "git-send-email 2.34.1", "In-Reply-To": "<20231130071001.4c7931c8@sovereign>", "References": "<20231130071001.4c7931c8@sovereign>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org" }, "content": "The virt2phys_translate function previously scanned existing blocks,\nreturning the physical address from the stored MDL info if present.\nThis method was problematic when a virtual address pointed to a freed\nand reallocated memory segment, potentially changing the physical\naddress mapping. Yet, virt2phys_translate would consistently return\nthe originally stored physical address, which could be invalid.\n\nThis issue surfaced when allocating a memory region larger than 2MB\nusing rte_malloc. This action would allocate a new memory segment\nand use virt2phy to set the IOVA. The driver would store the MDL\nand lock the pages initially. When this region was freed, the memory\nsegment used as a whole page could be freed, invalidating the virtual\nto physical mapping. Before this fix, the driver would only return the\ninitial physical address, leading to illegal IOVA for some pages when\nallocating a new memory region larger than the hugepage size (2MB).\n\nTo address this, a function to check block physical address has been\nadded. If a block with the same base address is detected in the\ndriver's context, the MDL's physical address is compared with the real\nphysical address. If they don't match, the block is removed and a new\none is created to store the correct mapping. To make the removal action\nclear, the list to store MDL blocks is changed to a double linked list.\n\nAlso fix the printing of PVOID type.\n\nBugzilla ID: 1201\nBugzilla ID: 1213\n\nSigned-off-by: Ric Li <ming3.li@intel.com>\n---\n windows/virt2phys/virt2phys.c | 7 +--\n windows/virt2phys/virt2phys_logic.c | 70 ++++++++++++++++++++++-------\n 2 files changed, 57 insertions(+), 20 deletions(-)", "diff": "diff --git a/windows/virt2phys/virt2phys.c b/windows/virt2phys/virt2phys.c\nindex f4d5298..b64a13d 100644\n--- a/windows/virt2phys/virt2phys.c\n+++ b/windows/virt2phys/virt2phys.c\n@@ -182,7 +182,7 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request)\n {\n \tWDF_REQUEST_PARAMETERS params;\n \tULONG code;\n-\tPVOID *virt;\n+\tPVOID *pvirt, virt;\n \tPHYSICAL_ADDRESS *phys;\n \tsize_t size;\n \tNTSTATUS status;\n@@ -207,12 +207,13 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request)\n \t}\n \n \tstatus = WdfRequestRetrieveInputBuffer(\n-\t\t\trequest, sizeof(*virt), (PVOID *)&virt, &size);\n+\t\t\trequest, sizeof(*pvirt), (PVOID *)&pvirt, &size);\n \tif (!NT_SUCCESS(status)) {\n \t\tTraceWarning(\"Retrieving input buffer: %!STATUS!\", status);\n \t\tWdfRequestComplete(request, status);\n \t\treturn;\n \t}\n+\tvirt = *pvirt;\n \n \tstatus = WdfRequestRetrieveOutputBuffer(\n \t\trequest, sizeof(*phys), (PVOID *)&phys, &size);\n@@ -222,7 +223,7 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request)\n \t\treturn;\n \t}\n \n-\tstatus = virt2phys_translate(*virt, phys);\n+\tstatus = virt2phys_translate(virt, phys);\n \tif (NT_SUCCESS(status))\n \t\tWdfRequestSetInformation(request, sizeof(*phys));\n \ndiff --git a/windows/virt2phys/virt2phys_logic.c b/windows/virt2phys/virt2phys_logic.c\nindex e3ff293..f867a31 100644\n--- a/windows/virt2phys/virt2phys_logic.c\n+++ b/windows/virt2phys/virt2phys_logic.c\n@@ -12,13 +12,13 @@\n struct virt2phys_process {\n \tHANDLE id;\n \tLIST_ENTRY next;\n-\tSINGLE_LIST_ENTRY blocks;\n+\tLIST_ENTRY blocks;\n \tULONG64 memory;\n };\n \n struct virt2phys_block {\n \tPMDL mdl;\n-\tSINGLE_LIST_ENTRY next;\n+\tLIST_ENTRY next;\n };\n \n static struct virt2phys_params g_params;\n@@ -69,24 +69,28 @@ virt2phys_process_create(HANDLE process_id)\n \tstruct virt2phys_process *process;\n \n \tprocess = ExAllocatePoolZero(NonPagedPool, sizeof(*process), 'pp2v');\n-\tif (process != NULL)\n+\tif (process != NULL) {\n \t\tprocess->id = process_id;\n+\t\tInitializeListHead(&process->blocks);\n+\t}\n+\n \treturn process;\n }\n \n static void\n virt2phys_process_free(struct virt2phys_process *process, BOOLEAN unmap)\n {\n-\tPSINGLE_LIST_ENTRY node;\n+\tPLIST_ENTRY node, next;\n \tstruct virt2phys_block *block;\n \n \tTraceInfo(\"ID = %p, unmap = %!bool!\", process->id, unmap);\n \n-\tnode = process->blocks.Next;\n-\twhile (node != NULL) {\n+\tfor (node = process->blocks.Flink; node != &process->blocks; node = next) {\n+\t\tnext = node->Flink;\n \t\tblock = CONTAINING_RECORD(node, struct virt2phys_block, next);\n-\t\tnode = node->Next;\n-\t\tvirt2phys_block_free(block, unmap);\n+\t\tRemoveEntryList(&block->next);\n+\n+\t\tvirt2phys_block_free(block, TRUE);\n \t}\n \n \tExFreePool(process);\n@@ -109,10 +113,10 @@ virt2phys_process_find(HANDLE process_id)\n static struct virt2phys_block *\n virt2phys_process_find_block(struct virt2phys_process *process, PVOID virt)\n {\n-\tPSINGLE_LIST_ENTRY node;\n+\tPLIST_ENTRY node;\n \tstruct virt2phys_block *cur;\n \n-\tfor (node = process->blocks.Next; node != NULL; node = node->Next) {\n+\tfor (node = process->blocks.Flink; node != &process->blocks; node = node->Flink) {\n \t\tcur = CONTAINING_RECORD(node, struct virt2phys_block, next);\n \t\tif (cur->mdl->StartVa == virt)\n \t\t\treturn cur;\n@@ -182,7 +186,7 @@ virt2phys_process_cleanup(HANDLE process_id)\n }\n \n static struct virt2phys_block *\n-virt2phys_find_block(HANDLE process_id, void *virt,\n+virt2phys_find_block(HANDLE process_id, PVOID virt,\n \tstruct virt2phys_process **process)\n {\n \tPLIST_ENTRY node;\n@@ -244,13 +248,13 @@ virt2phys_add_block(struct virt2phys_process *process,\n \t\treturn STATUS_QUOTA_EXCEEDED;\n \t}\n \n-\tPushEntryList(&process->blocks, &block->next);\n+\tInsertHeadList(&process->blocks, &block->next);\n \tprocess->memory += size;\n \treturn STATUS_SUCCESS;\n }\n \n static NTSTATUS\n-virt2phys_query_memory(void *virt, void **base, size_t *size)\n+virt2phys_query_memory(PVOID virt, PVOID *base, size_t *size)\n {\n \tMEMORY_BASIC_INFORMATION info;\n \tSIZE_T info_size;\n@@ -321,7 +325,7 @@ virt2phys_check_memory(PMDL mdl)\n }\n \n static NTSTATUS\n-virt2phys_lock_memory(void *virt, size_t size, PMDL *mdl)\n+virt2phys_lock_memory(PVOID virt, size_t size, PMDL *mdl)\n {\n \t*mdl = IoAllocateMdl(virt, (ULONG)size, FALSE, FALSE, NULL);\n \tif (*mdl == NULL)\n@@ -346,12 +350,35 @@ virt2phys_unlock_memory(PMDL mdl)\n \tIoFreeMdl(mdl);\n }\n \n+static BOOLEAN\n+virt2phys_is_valid_block(struct virt2phys_block *block, PVOID virt)\n+{\n+\t/*\n+\t * Check if MDL in block stores the valid physical address.\n+\t * The virtual to physical memory mapping may be changed when the\n+\t * virtual memory region is freed by the user process and malloc again,\n+\t * then we need to remove the block and create a new one.\n+\t */\n+\tPHYSICAL_ADDRESS block_phys, real_phys;\n+\n+\tblock_phys = virt2phys_block_translate(block, virt);\n+\treal_phys = MmGetPhysicalAddress(virt);\n+\n+\tif (block_phys.QuadPart == real_phys.QuadPart)\n+\t\treturn TRUE;\n+\n+\tTraceWarning(\"VA = %p, invalid block physical address %llx, valid address %llx\",\n+\t\tvirt, block_phys.QuadPart, real_phys.QuadPart);\n+\n+\treturn FALSE;\n+}\n+\n NTSTATUS\n virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys)\n {\n \tPMDL mdl;\n \tHANDLE process_id;\n-\tvoid *base;\n+\tPVOID base;\n \tsize_t size;\n \tstruct virt2phys_process *process;\n \tstruct virt2phys_block *block;\n@@ -371,8 +398,17 @@ virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys)\n \n \t/* Don't lock the same memory twice. */\n \tif (block != NULL) {\n-\t\t*phys = virt2phys_block_translate(block, virt);\n-\t\treturn STATUS_SUCCESS;\n+\t\tif (virt2phys_is_valid_block(block, base)) {\n+\t\t\t*phys = virt2phys_block_translate(block, virt);\n+\t\t\treturn STATUS_SUCCESS;\n+\t\t}\n+\t\t/* Remove the invalid block. */\n+\t\tKeAcquireSpinLock(g_lock, &irql);\n+\t\tRemoveEntryList(&block->next);\n+\t\tprocess->memory -= MmGetMdlByteCount(block->mdl);\n+\t\tKeReleaseSpinLock(g_lock, irql);\n+\n+\t\tvirt2phys_block_free(block, TRUE);\n \t}\n \n \tstatus = virt2phys_lock_memory(base, size, &mdl);\n", "prefixes": [ "v4" ] }