On Wed, 11 Feb 2026 10:24:28 +0000
<[email protected]> wrote:
> From: Pravin M Bathija <[email protected]>
>
> This is version v7 of the patchset and it incorporates the
> recommendations made by Maxime Coquelin and Feng Cheng Wen.
> The patchset includes support for adding and removal of memory
> regions, getting max memory slots and other changes to vhost-user
> messages. These messages are sent from vhost-user front-end (qemu
> or libblkio) to a vhost-user back-end (dpdk, spdk). Support functions
> for these message functions have been implemented in the interest of
> writing optimized code. Older functions, part of vhost-user back-end
> have also been optimized using these newly defined support functions.
> This implementation has been extensively tested by doing Read/Write
> I/O from multiple instances of fio + libblkio (front-end) talking to
> spdk/dpdk (back-end) based drives. Tested with qemu front-end
> talking to dpdk + testpmd (back-end) performing add/removal of memory
> regions. Also tested post-copy live migration after doing
> add_memory_region.
>
> Version Log:
> Version v7 (Current version): Incorporate code review suggestions
> from Maxime Coquelin. Add debug messages to vhost_postcopy_register
> function.
> Version v6: Added the enablement of this feature
> as a final patch in this patch-set and other code optimizations as
> suggested by Maxime Coquelin.
> Version v5: removed the patch that increased the
> number of memory regions from 8 to 128. This will be submitted as a
> separate feature at a later point after incorporating additional
> optimizations. Also includes code optimizations as suggested by Feng
> Cheng Wen.
> Version v4: code optimizations as suggested by Feng Cheng Wen.
> Version v3: code optimizations as suggested by Maxime Coquelin
> and Thomas Monjalon.
> Version v2: code optimizations as suggested by Maxime Coquelin.
> Version v1: Initial patch set.
>
> Pravin M Bathija (5):
> vhost: add user to mailmap and define to vhost hdr
> vhost_user: header defines for add/rem mem region
> vhost_user: support function defines for back-end
> vhost_user: Function defs for add/rem mem regions
> vhost_user: enable configure memory slots
>
> .mailmap | 1 +
> lib/vhost/rte_vhost.h | 4 +
> lib/vhost/vhost_user.c | 392 ++++++++++++++++++++++++++++++++++++-----
> lib/vhost/vhost_user.h | 10 ++
> 4 files changed, 365 insertions(+), 42 deletions(-)
>
Ran this patch set through AI review (Gemini this time).
It found lots of issues...
✦ I have performed a comprehensive review of the vhost_user patchset and
identified several critical correctness issues that violate DPDK
guidelines.
Critical Correctness Bugs
1. Destructive Error Path in vhost_user_add_mem_reg:
In Patch 4/5, the error path for vhost_user_add_mem_reg (label
free_mem_table) calls free_all_mem_regions(dev) and wipes dev->mem and
dev->guest_pages. This is catastrophic: if registration of a single new
memory region fails, the entire existing memory configuration for the
running VM is destroyed, leading to immediate failure of all I/O. It should
only clean up the specifically failed region.
2. Broken Address Mapping in async_dma_map_region:
In Patch 3/5, async_dma_map_region assumes that if the first page of a
region is found at index i in dev->guest_pages, all subsequent
pages for that region are at i+1, i+2, etc. This assumption is invalid
because dev->guest_pages is a global array for the device and is
frequently sorted by address (guest_page_addrcmp). Pages from multiple
regions will be interleaved, and the contiguous index assumption will
lead to mapping incorrect memory or walking off the end of the array.
3. Stale Guest Page Entries in vhost_user_rem_mem_reg:
In Patch 4/5, vhost_user_rem_mem_reg removes a region from dev->mem but
fails to remove the corresponding entries from dev->guest_pages.
This leaves stale mappings in the translation cache, which will cause
incorrect address translations, potential memory corruption, or crashes
when the guest reuses those physical addresses.
4. Vhost-User Protocol Violation:
In Patch 2/5, VhostUserSingleMemReg is defined with a uint64_t padding
before the VhostUserMemoryRegion. The vhost-user specification for
VHOST_USER_ADD_MEM_REG and VHOST_USER_REM_MEM_REG expects the payload to be
exactly VhostUserMemoryRegion. This extra padding will cause the
back-end to misalign all fields, leading to incorrect address and size
interpretations.
5. Unsafe Assumption in async_dma_map_region Error Handling:
In Patch 3/5, async_dma_map_region logs DMA mapping/unmapping failures
but continues execution. For memory registration, a DMA mapping
failure must be treated as a fatal error for that operation to ensure the DMA
engine never attempts to access unmapped memory.
Style and Process Observations
* Low Memory Slot Limit: The implementation returns
VHOST_MEMORY_MAX_NREGIONS (8) for VHOST_USER_GET_MAX_MEM_SLOTS. While
technically
correct per the current DPDK defines, 8 slots is extremely restrictive for
dynamic memory hotplug, which this feature is intended to
support.
* Missing Locking Assertions: In dev_invalidate_vrings,
VHOST_USER_ASSERT_LOCK is called with VHOST_USER_ADD_MEM_REG. This macro
typically
expects a string description of the operation for logging, rather than an
enum value.
I recommend a significant redesign of the memory management logic to properly
handle sparse region arrays and incremental guest page updates
before this patchset can be accepted.
