From patchwork Mon Sep 11 08:22:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ric Li X-Patchwork-Id: 131332 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: 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]) by inbox.dpdk.org (Postfix) with ESMTP id 2C04A4256D; Mon, 11 Sep 2023 10:22:35 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id ED8E6402DD; Mon, 11 Sep 2023 10:22:34 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id 42AEE402DA for ; Mon, 11 Sep 2023 10:22:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1694420553; x=1725956553; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=13GXEwyLgjr/KBVzWL2Ml+1QfbJ35pJzl5PenNGubZI=; b=KQ5qEgaaDzaD7y73pXZyeAy4neWZnyoQPXQegrcDq/gOfGlFTo/Bz/6s vrk+7/HtB2o7LSejVGXhsPtjitCCUS4kXJrAGw4LRa7SMB7ORnuBAWmhU QiLwmqgnBxSdAnQpUybeIkQCBTtBTtpYLjZ3vC4esoHE7QmWnb+B7wPLf 1pSrwQe53ZmayZbSrY61ITHtopuhkhhSJj6XWDPhv8vdKdsp5anXNrrH4 H24tHd9TDO8zB6PV6eflC0rM0et07Mvi7uvCelQQ+sEO5CIlOPvC3GjKi Gf1xGXuD5+XY/7ZeAFrcsid1aOzfWc6fzcfm0r+KzqvnZ0nb8v2nsHVUv g==; X-IronPort-AV: E=McAfee;i="6600,9927,10829"; a="381826491" X-IronPort-AV: E=Sophos;i="6.02,243,1688454000"; d="scan'208";a="381826491" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Sep 2023 01:22:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10829"; a="866852558" X-IronPort-AV: E=Sophos;i="6.02,243,1688454000"; d="scan'208";a="866852558" Received: from media-ric-kahawai-icl.sh.intel.com ([10.67.119.129]) by orsmga004.jf.intel.com with ESMTP; 11 Sep 2023 01:22:30 -0700 From: Ric Li To: dmitry.kozliuk@gmail.com Cc: dev@dpdk.org Subject: [PATCH] windows/virt2phys: fix block MDL not updated Date: Mon, 11 Sep 2023 16:22:29 +0800 Message-Id: <20230911082229.1479773-1-ming3.li@intel.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org The virt2phys_translate function previously scanned existing blocks, returning the physical address from the stored MDL info if present. This method was problematic when a virtual address pointed to a freed and reallocated memory segment, potentially changing the physical address mapping. Yet, virt2phys_translate would consistently return the originally stored physical address, which could be invalid. This issue surfaced when allocating a memory region larger than 2MB using rte_malloc. This action would allocate a new memory segment and use virt2phy to set the iova. The driver would store the MDL and lock the pages initially. When this region was freed, the memory segment used as a whole page could be freed, invalidating the virtual to physical mapping. It then needed to be deleted from the driver's block MDL info. Before this fix, the driver would only return the initial physical address, leading to illegal iova for some pages when allocating a new memory region of the same size. To address this, a refresh function has been added. If a block with the same base address is detected in the driver's context, the MDL's physical address is compared with the real physical address. If they don't match, the MDL within the block is released and rebuilt to store the correct mapping. Also refine the access of MDL StartVa field and fix the printing of PVOID type. Signed-off-by: Ric Li --- windows/virt2phys/virt2phys.c | 7 ++-- windows/virt2phys/virt2phys_logic.c | 60 +++++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/windows/virt2phys/virt2phys.c b/windows/virt2phys/virt2phys.c index f4d5298..b64a13d 100644 --- a/windows/virt2phys/virt2phys.c +++ b/windows/virt2phys/virt2phys.c @@ -182,7 +182,7 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request) { WDF_REQUEST_PARAMETERS params; ULONG code; - PVOID *virt; + PVOID *pvirt, virt; PHYSICAL_ADDRESS *phys; size_t size; NTSTATUS status; @@ -207,12 +207,13 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request) } status = WdfRequestRetrieveInputBuffer( - request, sizeof(*virt), (PVOID *)&virt, &size); + request, sizeof(*pvirt), (PVOID *)&pvirt, &size); if (!NT_SUCCESS(status)) { TraceWarning("Retrieving input buffer: %!STATUS!", status); WdfRequestComplete(request, status); return; } + virt = *pvirt; status = WdfRequestRetrieveOutputBuffer( request, sizeof(*phys), (PVOID *)&phys, &size); @@ -222,7 +223,7 @@ virt2phys_device_EvtIoInCallerContext(WDFDEVICE device, WDFREQUEST request) return; } - status = virt2phys_translate(*virt, phys); + status = virt2phys_translate(virt, phys); if (NT_SUCCESS(status)) WdfRequestSetInformation(request, sizeof(*phys)); diff --git a/windows/virt2phys/virt2phys_logic.c b/windows/virt2phys/virt2phys_logic.c index e3ff293..5bf6734 100644 --- a/windows/virt2phys/virt2phys_logic.c +++ b/windows/virt2phys/virt2phys_logic.c @@ -40,7 +40,7 @@ virt2phys_block_create(PMDL mdl) static void virt2phys_block_free(struct virt2phys_block *block, BOOLEAN unmap) { - TraceInfo("VA = %p, unmap = %!bool!", block->mdl->StartVa, unmap); + TraceInfo("VA = %p, unmap = %!bool!", MmGetMdlBaseVa(block->mdl), unmap); if (unmap) MmUnlockPages(block->mdl); @@ -114,7 +114,7 @@ virt2phys_process_find_block(struct virt2phys_process *process, PVOID virt) for (node = process->blocks.Next; node != NULL; node = node->Next) { cur = CONTAINING_RECORD(node, struct virt2phys_block, next); - if (cur->mdl->StartVa == virt) + if (MmGetMdlBaseVa(cur->mdl) == virt) return cur; } return NULL; @@ -182,7 +182,7 @@ virt2phys_process_cleanup(HANDLE process_id) } static struct virt2phys_block * -virt2phys_find_block(HANDLE process_id, void *virt, +virt2phys_find_block(HANDLE process_id, PVOID virt, struct virt2phys_process **process) { PLIST_ENTRY node; @@ -214,7 +214,7 @@ virt2phys_add_block(struct virt2phys_process *process, struct virt2phys_process *existing; size_t size; - TraceInfo("ID = %p, VA = %p", process->id, block->mdl->StartVa); + TraceInfo("ID = %p, VA = %p", process->id, MmGetMdlBaseVa(block->mdl)); existing = virt2phys_process_find(process->id); *process_exists = existing != NULL; @@ -250,7 +250,7 @@ virt2phys_add_block(struct virt2phys_process *process, } static NTSTATUS -virt2phys_query_memory(void *virt, void **base, size_t *size) +virt2phys_query_memory(PVOID virt, PVOID *base, size_t *size) { MEMORY_BASIC_INFORMATION info; SIZE_T info_size; @@ -321,7 +321,7 @@ virt2phys_check_memory(PMDL mdl) } static NTSTATUS -virt2phys_lock_memory(void *virt, size_t size, PMDL *mdl) +virt2phys_lock_memory(PVOID virt, size_t size, PMDL *mdl) { *mdl = IoAllocateMdl(virt, (ULONG)size, FALSE, FALSE, NULL); if (*mdl == NULL) @@ -346,12 +346,53 @@ virt2phys_unlock_memory(PMDL mdl) IoFreeMdl(mdl); } +static NTSTATUS +virt2phys_block_refresh(struct virt2phys_block *block, PVOID base, size_t size) +{ + NTSTATUS status; + PMDL mdl = block->mdl; + + /* + * Check if we need to refresh MDL in block. + * The virtual to physical memory mapping may be changed when the + * virtual memory region is freed by the user process and malloc again, + * then we need to unlocked the physical memory and lock again to + * refresh the MDL information stored in block. + */ + PHYSICAL_ADDRESS block_phys, real_phys; + + block_phys = virt2phys_block_translate(block, base); + real_phys = MmGetPhysicalAddress(base); + + if (block_phys.QuadPart == real_phys.QuadPart) + /* No need to refresh block. */ + return STATUS_SUCCESS; + + virt2phys_unlock_memory(mdl); + mdl = NULL; + + status = virt2phys_lock_memory(base, size, &mdl); + if (!NT_SUCCESS(status)) + return status; + + status = virt2phys_check_memory(mdl); + if (!NT_SUCCESS(status)) { + virt2phys_unlock_memory(mdl); + return status; + } + block->mdl = mdl; + + TraceInfo("Block refreshed from %llx to %llx", block_phys.QuadPart, real_phys.QuadPart); + + return STATUS_SUCCESS; +} + NTSTATUS virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys) { PMDL mdl; HANDLE process_id; - void *base; + PVOID base; size_t size; struct virt2phys_process *process; struct virt2phys_block *block; @@ -371,6 +412,11 @@ virt2phys_translate(PVOID virt, PHYSICAL_ADDRESS *phys) /* Don't lock the same memory twice. */ if (block != NULL) { + KeAcquireSpinLock(g_lock, &irql); + status = virt2phys_block_refresh(block, base, size); + KeReleaseSpinLock(g_lock, irql); + if (!NT_SUCCESS(status)) + return status; *phys = virt2phys_block_translate(block, virt); return STATUS_SUCCESS; }