Re: [PATCH v7 0/5] Support add/remove memory region & get-max-slots

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 11 Feb 2026 10:24:28 +0000
<pravin.bathija@dell.com> wrote:

> From: Pravin M Bathija <pravin.bathija@dell.com>
> 
> 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.