From: Nuno Das Neves <[email protected]> Sent: Friday, October 17, 2025 10:55 AM > > On 10/17/2025 10:33 AM, Michael Kelley wrote: > > From: Nuno Das Neves <[email protected]> Sent: Friday, > > October 17, 2025 10:26 AM > >> > >> On 10/16/2025 6:12 PM, Michael Kelley wrote: > >>> From: Nuno Das Neves <[email protected]> Sent: Thursday, > >>> October 16, 2025 12:54 PM > >>>> > >>>> When the MSHV_ROOT_HVCALL ioctl is executing a hypercall, and gets > >>>> HV_STATUS_INSUFFICIENT_MEMORY, it deposits memory and then returns > >>>> -EAGAIN to userspace. > >>>> > >>>> However, it's much easier and efficient if the driver simply deposits > >>>> memory on demand and immediately retries the hypercall as is done with > >>>> all the other hypercall helper functions. > >>>> > >>>> But unlike those, in MSHV_ROOT_HVCALL the input is opaque to the > >>>> kernel. This is problematic for rep hypercalls, because the next part > >>>> of the input list can't be copied on each loop after depositing pages > >>>> (this was the original reason for returning -EAGAIN in this case). > >>>> > >>>> Introduce hv_do_rep_hypercall_ex(), which adds a 'rep_start' > >>>> parameter. This solves the issue, allowing the deposit loop in > >>>> MSHV_ROOT_HVCALL to restart a rep hypercall after depositing pages > >>>> partway through. > >>> > >>> >From reading the above, I'm pretty sure this code change is an > >>> optimization that lets user space avoid having to deal with the > >>> -EAGAIN result by resubmitting the ioctl with a different > >>> starting point for a rep hypercall. As such, I'd suggest the patch > >>> title should be "Improve deposit memory ...." (or something similar). > >>> The word "Fix" makes it sound like a bug fix. > >>> > >>> Or is user space code currently faulty in its handling of -EAGAIN, and > >>> this really is an indirect bug fix to make things work? If so, do you > >>> want a Fixes: tag so the change is backported? > >>> > >> > >> It's the latter case, userspace doesn't handle it correctly, so I > >> consider it a fix more than just an improvement. > > > > OK, thanks. You might want to tweak the commit message a bit > > to clarify that this really is a bug fix and why. I was leaning > > toward the wrong conclusion based on the current commit > > message. > > > > Is this clearer? > > " > mshv: Fix deposit memory in MSHV_ROOT_HVCALL > > When the MSHV_ROOT_HVCALL ioctl is executing a hypercall, and gets > HV_STATUS_INSUFFICIENT_MEMORY, it deposits memory and then returns > -EAGAIN to userspace. The expectation is that the VMM will retry. > > However, some VMM code in the wild doesn't do this and simply fails. > Rather than force the VMM to retry, change the ioctl to deposit > memory on demand and immediately retry the hypercall as is done with > all the other hypercall helper functions. > > In addition to making the ioctl easier to use, removing the need for > multiple syscalls improves performance. > > There is a complication: unlike the other hypercall helper functions, > in MSHV_ROOT_HVCALL the input is opaque to the kernel. This is > problematic for rep hypercalls, because the next part of the input > list can't be copied on each loop after depositing pages (this was > the original reason for returning -EAGAIN in this case). > > Introduce hv_do_rep_hypercall_ex(), which adds a 'rep_start' > parameter. This solves the issue, allowing the deposit loop in > MSHV_ROOT_HVCALL to restart a rep hypercall after depositing pages > partway through.
Yes, that works for me. Thanks. Michael
