From patchwork Mon Sep 11 13:09:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ric Li X-Patchwork-Id: 131339 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 9C3754256F; Mon, 11 Sep 2023 15:09:47 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 39944402EF; Mon, 11 Sep 2023 15:09:47 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.115]) by mails.dpdk.org (Postfix) with ESMTP id 3D575402DB for ; Mon, 11 Sep 2023 15:09:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1694437786; x=1725973786; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=UeLcRIxoKtCQWbQh/q9rFYLk3JCu2Hj5s1cZtJnbSJk=; b=M2CvWEuubLpdas9vrmgrwdkwPU04+aiFG4M3F/ToVwNOl762qg8xqQOX gkE53VH3SrxNtdsLoikcbFpVuCGxrzWJh0Q3XW2q365JQwGgD5eleyGz1 6y/4i/oQpC1FOzI5BiKOnEPXkv2qaUxIvTusrtpbkTDBBs5DNaxeR8tTS tPektE2VUmPSxqbGozpChqRSxlm2RkMieRR0VOB7FAUQHNHDUUrHT7ivY 3090Bz1LtqdOeL2QNyM1d6W0DOuKMSvUzqqyDYaanGhJGKKIsPB645Woz kQHBJCjagaCaewVGmrT78I6ZLI0UJM2hlFihPZJHdMf//cCBqHX9TUg+S A==; X-IronPort-AV: E=McAfee;i="6600,9927,10830"; a="377989417" X-IronPort-AV: E=Sophos;i="6.02,244,1688454000"; d="scan'208";a="377989417" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Sep 2023 06:09:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10830"; a="886513547" X-IronPort-AV: E=Sophos;i="6.02,244,1688454000"; d="scan'208";a="886513547" Received: from media-ric-kahawai-icl.sh.intel.com ([10.67.119.129]) by fmsmga001.fm.intel.com with ESMTP; 11 Sep 2023 06:09:14 -0700 From: Ric Li To: ming3.li@intel.com Cc: dev@dpdk.org, dmitry.kozliuk@gmail.com Subject: [PATCH v2] windows/virt2phys: fix block MDL not updated Date: Mon, 11 Sep 2023 21:09:36 +0800 Message-Id: <20230911130936.1485584-1-ming3.li@intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230911082229.1479773-1-ming3.li@intel.com> References: <20230911082229.1479773-1-ming3.li@intel.com> 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 fix the printing of PVOID type. Signed-off-by: Ric Li --- windows/virt2phys/virt2phys.c | 7 ++-- windows/virt2phys/virt2phys_logic.c | 54 ++++++++++++++++++++++++++--- 2 files changed, 54 insertions(+), 7 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..8529a2b 100644 --- a/windows/virt2phys/virt2phys_logic.c +++ b/windows/virt2phys/virt2phys_logic.c @@ -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; @@ -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 unlock 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; }