On Wed, Mar 15, 2023, Anish Moorthy wrote: > Still unsure if needs conversion > -------------------------------- > * __kvm_read_guest_atomic > The EFAULT might be propagated though FNAME(sync_page)? > * kvm_write_guest_offset_cached (virt/kvm/kvm_main.c:3226) > * __kvm_write_guest_page > Called from kvm_write_guest_offset_cached: if that needs change, this does too > * kvm_write_guest_page > Two interesting paths: > - kvm_pv_clock_pairing returns a custom KVM_EFAULT error here > (arch/x86/kvm/x86.c:9578) > - kvm_write_guest_offset_cached returns this directly (so if that needs > change, this does too) > * kvm_read_guest_offset_cached > I actually do see a path to userspace, but it's through hyper-v, which we've > said is out of scope for round 1. To clarify: I didn't intend to make Hyper-V explicitly out-of-scope, rather Hyper-V happened to be out-of-scope because the existing code suppresses -EFAULT. I don't think we should make any particular feature/area out-of-scope, as that will lead to even more arbitrary behavior than we already have. What I intended, and what I still think we should do, is limit the scope of the capability to existing paths that return -EFAULT to userspace. Trying to fix all of the paths that suppress -EFAULT is going to be ridiculously difficult as so much of the behavior is arguaby ABI, and there's no authoritative documentation on what's supposed to happen. I definitely would love to fix those paths in the long term, but for the initial implementation/conversion, I think it makes sense to punt on them, otherwise it'll take months/years to merge this code. Back to the Hyper-V case, assuming you're referring to the use of kvm_hv_verify_vp_assist() in nested_svm_vmrun(), that code is a mess. KVM shouldn't inject a #GP and then exit to userspace, e.g. the guest might see a spurious #GP if userspace fixes the fault and resume the instruction. And just a few lines below, KVM skips the instruction if kvm_vcpu_map() returns -EFAULT. As above, ideally that code would be converted to gracefully report the error, but it's such a snafu that the easiest thing might be to change the "return ret;" to "return 1;" until we fix all such KVM-on-HyperV code.
