Hello,
Any update on the patch review?
Best regards,
Ming
> -Original Message-
> From: Li, Ming3
> Sent: Tuesday, September 12, 2023 7:18 PM
> To: Li, Ming3
> Cc: dev@dpdk.org; dmitry.kozl...@gmail.com; roret...@linux.microsoft.com
> Subject: [PATCH v3] windows/virt2phys: fix block MDL not updated
>
> 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. 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 larger than the hugepage size
> (2MB).
>
> To address this, a function to check block physical address 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 block is removed and a new one is created to store the correct
> mapping. To make the removal action clear, the list to store MDL blocks is
> chenged to a double linked list.
>
> Also fix the printing of PVOID type.
>
> Bugzilla ID: 1201
> Bugzilla ID: 1213
>
> Signed-off-by: Ric Li
> ---
> v3:
> * Change refresh action to block removal
> * Change block list to double linked list
>
> v2:
> * Revert wrong usage of MmGetMdlStartVa
>
> windows/virt2phys/virt2phys.c | 7 +--
> windows/virt2phys/virt2phys_logic.c | 70 ++---
> 2 files changed, 57 insertions(+), 20 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..531f08c 100644
> --- a/windows/virt2phys/virt2phys_logic.c
> +++ b/windows/virt2phys/virt2phys_logic.c
> @@ -12,13 +12,13 @@
> struct virt2phys_process {
> HANDLE id;
> LIST_ENTRY next;
> - SINGLE_LIST_ENTRY blocks;
> + LIST_ENTRY blocks;
> ULONG64 memory;
> };
>
> struct virt2phys_block {
> PMDL mdl;
> - SINGLE_LIST_ENTRY next;
> + LIST_ENTRY next;
> };
>
> static struct virt2phys_params g_params; @@ -69,24 +69,28 @@
> virt2phys_process_create(HANDLE process_id)
> struct virt2phys_process *process;
>
> process = ExAllocatePoolZero(NonPagedPool, sizeof(*process), 'pp2v');
> - if (process != NULL)
> + if (process != NULL) {
> process->id = process_id;
> + InitializeListHead(&process->blocks);
> + }
> +
> return process;
> }
>
> static void
> virt2phys_process_free(struct virt2phys_process *process, BOOLEAN unmap) {
> - PSINGLE_LIST_ENTRY node;
> + PLIST_ENTRY node, next;
> struct virt2phys_block *block;
>
> TraceInfo("ID = %p, unmap = %!bool!", process->id, unmap);
>
> - node = process->blocks.Next;
> - while (node != NULL) {
> + for (node = process->blocks.Flink; node != &process->blo
