<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=koi8-r"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Hi, Maxime</div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> I understand your point. However we coould have a situation like this, could resize twice:</div> <blockquote style="margin-left: 0.8ex; padding-left: 1ex; border-left-width: 3px; border-left-style: solid; border-left-color: rgb(200, 200, 200);"> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) read message VHOST_USER_SET_MEM_TABLE</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) guest memory region size: 0x40000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) guest physical addr: 0x140000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) guest virtual addr: 0x140000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) host virtual addr: 0x7fde00000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) mmap addr : 0x7fde00000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) mmap size : 0x40000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) mmap align: 0x40000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) mmap off : 0x0</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) guest memory region size: 0x40000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) guest physical addr: 0x11c0000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) guest virtual addr: 0x11c0000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) host virtual addr: 0x7fddc0000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) mmap addr : 0x7fddc0000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) mmap size : 0x40000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) mmap align: 0x40000000</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> VHOST_CONFIG: (/usr/local/var/run/stdvio4) mmap off : 0x0</div> </blockquote> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> When we set memory region twice. After first iteration VIRTIO_DEV_VDPA_CONFIGURED flag will be unset here: <a href="https://github.com/DPDK/dpdk/blob/main/lib/vhost/vhost_user.c#L1425" id="OWAb0b80a98-f14d-1e16-713d-4420f53834de" class="OWAAutoLink"> https://github.com/DPDK/dpdk/blob/main/lib/vhost/vhost_user.c#L1425</a> On the second iteration, this leads to an rte_panic, as queues are accessed without a lock.</div> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> So we should check vhost message id ( VHOST_USER_SET_MEM_TABLE ).</div> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> However, extend VHOST_USER_ASSERT_LOCK macros with additional check if we work with this message VHOST_USER_SET_MEM_TABLE not handle this case, therefore translate_ring_addresses function calls q_assert_lock directly, without macros wrapper. In this function is check access_lock vhost_virtqueue and this case should be handle.</div> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px 0px 16px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> To address the described issue, we need to make the following changes:</div> <ol start="1" style="list-style-type: decimal;"> <li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); line-height: normal; margin: 0px;"> <b>Extend VHOST_USER_ASSERT_LOCK macro</b>:</li><ul style="list-style-type: circle;"> <li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); line-height: normal; margin: 0px;"> Add a check for the VHOST_USER_SET_MEM_TABLE message ID.</li><li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); line-height: normal; margin: 0px;"> Skip rte_panic if the message ID matches VHOST_USER_SET_MEM_TABLE.</li></ul> <li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); line-height: normal; margin: 0px;"> <b>Modify translate_ring_addresses function</b>:</li><ul style="list-style-type: circle;"> <li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); line-height: normal; margin: 0px;"> Extend its signature to include the id parameter (message ID).</li><li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); line-height: normal; margin: 0px;"> Add logic to skip vq_assert_lock if the message ID matches VHOST_USER_SET_MEM_TABLE.</li></ul> </ol> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <ol start="1" data-editing-info="{"applyListStyleFromLevel":false,"orderedStyleType":1}" style="margin-top: 0px; margin-bottom: 0px; list-style-type: decimal;"> <li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); margin-right: 0px; margin-left: 0px;"> <div class="elementToProof" role="presentation" style="line-height: normal; margin-top: 0px; margin-bottom: 8px;"> Extend <b>VHOST_USER_ASSERT_LOCK</b> Macro:</div> </li></ol> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 15px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <blockquote style="margin-left: 0.8ex; padding-left: 1ex; border-left-width: 3px; border-left-style: solid; border-left-color: rgb(200, 200, 200);"> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> #define VHOST_USER_ASSERT_LOCK(dev, vq, id) \</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> do { \</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> if ((id) == VHOST_USER_SET_MEM_TABLE) \</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> break; \</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> if (!(vq)->access_ok) \</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> rte_panic("Virtqueue access lock not held\n"); \</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> } while (0</div> </blockquote> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 15px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <ol start="2" data-editing-info="{"applyListStyleFromLevel":false,"orderedStyleType":1}" style="margin-top: 0px; margin-bottom: 0px; list-style-type: decimal;"> <li style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); margin-right: 0px; margin-left: 0px;"> <div class="elementToProof" role="presentation" style="line-height: normal; margin-top: 0px; margin-bottom: 8px;"> Modify <b>translate_ring_addresses</b> Function:</div> </li></ol> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <blockquote style="margin-left: 0.8ex; padding-left: 1ex; border-left-width: 3px; border-left-style: solid; border-left-color: rgb(200, 200, 200);"> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> static int</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> translate_ring_addresses(struct virtio_net *dev, struct vhost_virtqueue *vq, uint32_t id)</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> {</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> if (id != VHOST_USER_SET_MEM_TABLE)</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> vq_assert_lock(dev, vq);</div> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> // Existing logic for translating ring addresses...</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> // ...existing code...</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> return 0;</div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> }</div> </blockquote> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 14px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> This approach requires extending more functions with conditional checks to handle cases where queue locking should be ignored when the memory table is impacted. </div> <div class="elementToProof" style="line-height: normal; margin: 0px; min-height: 15px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="line-height: normal; margin: 0px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Do you have any thoughts about this? Or I should rework my patchset according to this described solution above ?</div> <div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>��:</b> Maxime Coquelin <maxime.coquelin@redhat.com><br> <b>��:</b> 12 �� 2025 �. 14:38<br> <b>��:</b> Danylo Vodopianov <dvo-plv@napatech.com>; thomas@monjalon.net <thomas@monjalon.net>; aman.deep.singh@intel.com <aman.deep.singh@intel.com>; yuying.zhang@intel.com <yuying.zhang@intel.com>; orika@nvidia.com <orika@nvidia.com>; mcoqueli@redhat.com <mcoqueli@redhat.com>; Christian Koue Muf <ckm@napatech.com>; matan@mellanox.com <matan@mellanox.com>; david.marchand@redhat.com <david.marchand@redhat.com>; Mykola Kostenok <mko-plv@napatech.com>; Serhii Iliushyk <sil-plv@napatech.com><br> <b>��:</b> stephen@networkplumber.org <stephen@networkplumber.org>; dev@dpdk.org <dev@dpdk.org>; Chenbo Xia <chenbox@nvidia.com><br> <b>��:</b> Re: [PATCH v4 1/1] vhost: handle virtqueue locking for memory hotplug</font> <div> </div> </div> <div class="BodyFragment"><font size="2"><span style="font-size:11pt;"> <div class="PlainText">Hi Danylo,<br> <br> On 6/4/25 10:32 AM, Danylo Vodopianov wrote:<br> > Hello, Maxime<br> > <br> > Thank you for your review.<br> > If I understand correctly, you propose modifying the | <br> > VHOST_USER_ASSERT_LOCK()| macro so that a |VHOST_USER_SET_MEM_TABLE| <br> > request does not trigger an assertion.<br> > However, I believe such modification would not be appropriate, as it <br> > would revert the logic introduced in commit |5e8fcc60b59d| ("vhost: <br> > enhance virtqueue access lock asserts"). With this approach, we would be <br> > performing memory hotplug without queue locking, which could lead to <br> > unintended consequences.<br> > Regarding VDPA device regression. We faced with this issue when we <br> > request the number of lcores that the default amount of memory on the <br> > socket cannot handle it.<br> > So, regression occurred during the startup part, during device <br> > configuration when it creates pkt mbuf pool.<br> > <br> > Let me know your thoughts regarding this.<br> <br> No, my point was to modify VHOST_USER_ASSERT_LOCK() to no trigger an<br> assertion in case vDPA is configured, as we don't want to lock in this<br> case.<br> <br> Regards,<br> Maxime<br> <br> > ------------------------------------------------------------------------<br> > *��:* Maxime Coquelin <maxime.coquelin@redhat.com><br> > *��:* 3 �� 2025 �. 15:30<br> > *��:* Danylo Vodopianov <dvo-plv@napatech.com>; thomas@monjalon.net <br> > <thomas@monjalon.net>; aman.deep.singh@intel.com <br> > <aman.deep.singh@intel.com>; yuying.zhang@intel.com <br> > <yuying.zhang@intel.com>; orika@nvidia.com <orika@nvidia.com>; <br> > mcoqueli@redhat.com <mcoqueli@redhat.com>; Christian Koue Muf <br> > <ckm@napatech.com>; matan@mellanox.com <matan@mellanox.com>; <br> > david.marchand@redhat.com <david.marchand@redhat.com>; Mykola Kostenok <br> > <mko-plv@napatech.com>; Serhii Iliushyk <sil-plv@napatech.com><br> > *��:* stephen@networkplumber.org <stephen@networkplumber.org>; <br> > dev@dpdk.org <dev@dpdk.org>; Chenbo Xia <chenbox@nvidia.com><br> > *��:* Re: [PATCH v4 1/1] vhost: handle virtqueue locking for memory <br> > hotplug<br> > Hello Danylo,<br> > <br> > On 6/2/25 10:50 AM, Danylo Vodopianov wrote:<br> >> For vDPA devices, virtqueues are not locked once the device has been<br> >> configured. In the<br> >> commit 5e8fcc60b59d ("vhost: enhance virtqueue access lock asserts"),<br> >> the asserts were enhanced to trigger rte_panic functionality, preventing<br> >> access to virtqueues without locking. However, this change introduced<br> >> an issue where the memory hotplug mechanism, added in the<br> >> commit 127f9c6f7b78 ("vhost: handle memory hotplug with vDPA devices"),<br> >> no longer works. Since it expects for all queues are locked.<br> >> <br> >> During the initialization of a vDPA device, the driver sets the<br> >> VIRTIO_DEV_VDPA_CONFIGURED flag, which prevents the<br> >> vhost_user_lock_all_queue_pairs function from locking the<br> >> virtqueues. This leads to the error: the VIRTIO_DEV_VDPA_CONFIGURED<br> >> flag allows the use of the hotplug mechanism, but it fails<br> >> because the virtqueues are not locked, while it expects to be locked<br> >> for VHOST_USER_SET_MEM_TABLE in the table VHOST_MESSAGE_HANDLERS.<br> >> <br> >> This patch addresses the issue by enhancing the conditional statement<br> >> to include a new condition. Specifically, when the device receives the<br> >> VHOST_USER_SET_MEM_TABLE request, the virtqueues are locked to update<br> >> the basic configurations and hotplug the guest memory.<br> >> <br> >> This fix does not impact access lock when vDPA driver is configured<br> >> for other unnecessary message handlers.<br> >> <br> >> Manual memory configuring with "--socket-mem" option allows to avoid<br> >> hotplug mechanism using.<br> > <br> > s/using/use/<br> > <br> > It needs a fixes tag, and stable@dpdk.org should be cc'ed, so that it<br> > gets backported to LTS branches.<br> > <br> >> <br> >> Signed-off-by: Danylo Vodopianov <dvo-plv@napatech.com><br> >> ---<br> >> lib/vhost/vhost_user.c | 8 +++++++-<br> >> 1 file changed, 7 insertions(+), 1 deletion(-)<br> >> <br> >> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c<br> >> index ec950acf97..16d03e1033 100644<br> >> --- a/lib/vhost/vhost_user.c<br> >> +++ b/lib/vhost/vhost_user.c<br> >> @@ -3178,7 +3178,13 @@ vhost_user_msg_handler(int vid, int fd)<br> >> * would cause a dead lock.<br> >> */<br> >> if (msg_handler != NULL && msg_handler->lock_all_qps) {<br> >> - if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {<br> >> + /* Lock all queue pairs if the device is not configured for vDPA,<br> >> + * or if it is configured for vDPA but the request is VHOST_USER_SET_MEM_TABLE.<br> >> + * This ensures proper queue locking for memory table updates and guest<br> >> + * memory hotplug.<br> >> + */<br> >> + if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED) ||<br> >> + request == VHOST_USER_SET_MEM_TABLE) {<br> > <br> > It looks like a workaround, and I'm afraid it could cause regression<br> > with some vDPA devices, or that it would not be enough and we would have<br> > to add other requests as exception.<br> > <br> > <br> > Wouldn't it better to modify VHOST_USER_ASSERT_LOCK() so that it takes<br> > into account the VIRTIO_DEV_VDPA_CONFIGURED flag?<br> > <br> > Thanks,<br> > Maxime<br> > <br> >> vhost_user_lock_all_queue_pairs(dev);<br> >> unlock_required = 1;<br> >> }<br> > <br> <br> </div> </span></font></div> </body> </html>