On 19/11/2024 13:24, Sean Christopherson wrote:
This patch avoids the overhead above in case of kernel-originated faults
Please avoid "This patch".
Ack, thanks.
by moving the `kvm_can_deliver_async_pf` check from
`kvm_arch_async_page_not_present` to `__kvm_faultin_pfn`.
Note that the existing check `kvm_can_do_async_pf` already calls
`kvm_can_deliver_async_pf` internally, however it only does that if the
`kvm_hlt_in_guest` check is true, ie userspace requested KVM not to exit
on guest halts via `KVM_CAP_X86_DISABLE_EXITS`. In that case the code
proceeds with the async fault processing with the following
justification in 1dfdb45ec510ba27e366878f97484e9c9e728902 ("KVM: x86:
clean up conditions for asynchronous page fault handling"):
"Even when asynchronous page fault is disabled, KVM does not want to pause
the host if a guest triggers a page fault; instead it will put it into
an artificial HLT state that allows running other host processes while
allowing interrupt delivery into the guest."
None of this justifies breaking host-side, non-paravirt async page faults. If a
vCPU hits a missing page, KVM can schedule out the vCPU and let something else
run on the pCPU, or enter idle and let the SMT sibling get more cycles, or maybe
even enter a low enough sleep state to let other cores turbo a wee bit.
I have no objection to disabling host async page faults, e.g. it's probably a net
negative for 1:1 vCPU:pCPU pinned setups, but such disabling needs an opt-in from
userspace.
That's a good point, I didn't think about it. The async work would
still need to execute somewhere in that case (or sleep in GUP until the
page is available). If processing the fault synchronously, the vCPU
thread can also sleep in the same way freeing the pCPU for something
else, so the amount of work to be done looks equivalent (please correct
me otherwise). What's the net gain of moving that to an async work in
the host async fault case? "while allowing interrupt delivery into the
guest." -- is this the main advantage?
