Re: [PATCH v4 05/16] KVM: Annotate -EFAULTs from kvm_vcpu_write_guest_page()

[Sean Christopherson <seanjc@google.com>]

On Thu, Jul 06, 2023, Anish Moorthy wrote:
> On Wed, Jun 14, 2023 at 12:10 PM Sean Christopherson <seanjc@google.com> wrote:
> > static int __kvm_write_guest_page(struct kvm *kvm,
> >     struct kvm_memory_slot *memslot, gfn_t gfn,
> >     const void *data, int offset, int len)
> > {
> >     int r;
> >     unsigned long addr;
> >
> >     addr = gfn_to_hva_memslot(memslot, gfn);
> >     if (kvm_is_error_hva(addr))
> >         return -EFAULT;
> > ...
> 
> Is it ok to catch these with an annotated efault? Strictly speaking
> they don't seem to arise from a failed user access (perhaps my
> definition is wrong: I'm looking at uaccess.h) so I'm not sure that
> annotating them is valid.

IMO it's ok, and even desirable, to annotate them.  This is a judgment call we
need to make as gfn=>hva translations are a KVM concept, i.e. aren't covered by
uaccess or anything else in the kernel.  Userspace would need to be aware that
an annotated -EFAULT may have failed due to the memslot lookup, but I don't see
that as being problematic, e.g. userspace will likely need to do its own memslot
lookup anyways.

In an ideal world, KVM would flag such "faults" as failing the hva lookup, and
provide the hva when it's a uaccess (or gup()) that fails, i.e. provide userspace
with precise details on the unresolved fault.  But I don't think I want to take
that on in KVM unless it proves to be absolutely necessary.  Userspace *should*
be able to derive the same information, and I'm concerned that providing precise
*and accurate* reporting in KVM would be a maintenance burden.