On Tue, Aug 11, 2026 at 4:42 AM Hanna Czenczek <[email protected]> wrote: > > On 11.08.26 04:32, Connor Kite wrote: > > On Mon, Aug 3, 2026 at 6:52 AM Hanna Czenczek <[email protected]> wrote: > >>> + struct vhost_user *u = dev->opaque; > >>> + int svq_idx = file->index - dev->vq_index; > >>> + if (u->user->memory_isolation) { > >>> + VhostShadowVirtqueue *svq = g_ptr_array_index(u->shadow_vqs, > >>> + svq_idx); > >> Bounds checking via `vhost_user_get_vq_index()` would be nice. (Same > >> below.) > >> > > Adding the bounds checking outside of the memory-isolation only code path, > > since > > file->index wasn't being checked previously. > > > >>> + if (svq->hdev_kick.initialized == false) { > >>> + int r = event_notifier_init(&svq->hdev_kick, 0); > >>> + if (r) { > >>> + error_report("Failed to create kick event notifier"); > >>> + return r; > >>> + } > >>> + } > >>> + > >>> + file->fd = event_notifier_get_fd(&svq->hdev_kick); > >> What if `file->fd` was -1, i.e. `VHOST_FILE_UNBIND`? Should we put a > >> real FD here then or just continue with -1? > >> > > Good point! I think we need to propagate the -1 to the backend device, and > > run event_notifier_cleanup on the existing svq->hdev_kick. > > > > ... > > > >>> + > >>> + if (svq->hdev_call.initialized == false) { > >>> + int r = event_notifier_init(&svq->hdev_call, 0); > >> Do we need to check if the call event FD is already active? > >> > > Do you mean, do we need to call event_notifier_test_and_clear to see if it's > > been set and potentially run the call handler at this stage? > > Yes, that is what I’m wondering. `vhost_svq_set_svq_kick_fd()` has this > comment: > > > event_notifier_set_handler already checks for guest's notifications if > > they arrive at the new file descriptor in the switch, so there is no > > need to explicitly check for them. > > So it sounds like there is some mechanism that will ensure that when the > kick FD is set on a shadow virtqueue, it is checked for activity > (notification), and if it is active, this is… handled? Which I suppose > means the installed handler will be called. > > But it’s entirely possible I misunderstand something because the same > code in `vhost_svq_set_svq_kick_fd()` also just calls > `event_notifier_set(svq_kick)`, so… kinda always emulates a kick, I assume? > > In any case, there seems to be worry about pending checking kicks from > the guest in this switch, and so I wonder if the same needs to be done > for the call FD. > > Hanna >
I haven't noticed any difference from setting the notifier here, and on further thought, I think I know why. I do not believe a call notifier FD is ever sent to the back-end without getting intercepted in vhost_user_set_vring_call, and once svq->hdev_call is initialized, it should never change unless the device is reset. Since svq->hdev_call is the only notifier shared with the back-end, it should never miss notifications from the device. Connor
